From 91ff56b458b960891ce19a20d699ceea63caf730 Mon Sep 17 00:00:00 2001 From: "gary.wgy" Date: Wed, 8 May 2024 17:40:34 +0800 Subject: [PATCH 1/5] fix grpc set metadata multi-value --- .../opentelemetry/instrumentation/grpc/v1_6/MetadataSetter.java | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetter.java b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetter.java index 99a50d9a151c..515d29cc733f 100644 --- a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetter.java +++ b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetter.java @@ -13,6 +13,7 @@ enum MetadataSetter implements TextMapSetter { @Override public void set(Metadata carrier, String key, String value) { + carrier.removeAll(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER)); carrier.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); } } From 4e54e1e9880a0ed8a25d6a60992987ef2651e4e3 Mon Sep 17 00:00:00 2001 From: "gary.wgy" Date: Fri, 24 May 2024 17:02:07 +0800 Subject: [PATCH 2/5] add test for grpc metadata setter --- .../grpc/v1_6/MetadataSetter.java | 5 +-- .../grpc/v1_6/MetadataSetterTest.java | 27 ++++++++++++++++ .../grpc/v1_6/propagator/GrpcPropagator.java | 32 +++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java create mode 100644 instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java diff --git a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetter.java b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetter.java index 515d29cc733f..e79c4bb86907 100644 --- a/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetter.java +++ b/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetter.java @@ -13,7 +13,8 @@ enum MetadataSetter implements TextMapSetter { @Override public void set(Metadata carrier, String key, String value) { - carrier.removeAll(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER)); - carrier.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value); + Metadata.Key metadataKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER); + carrier.removeAll(metadataKey); + carrier.put(metadataKey, value); } } diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java new file mode 100644 index 000000000000..4d2d28789e37 --- /dev/null +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java @@ -0,0 +1,27 @@ +package io.opentelemetry.instrumentation.grpc.v1_6; + +import io.opentelemetry.context.Context; +import io.grpc.Metadata; +import io.opentelemetry.instrumentation.grpc.v1_6.propagator.GrpcPropagator; +import org.junit.jupiter.api.Test; + +import static org.junit.Assert.assertTrue; + +class MetadataSetterTest { + + @Test + void checkThatIterableSizeEqualsOne() { + GrpcPropagator tested = new GrpcPropagator(); + Metadata metadata = new Metadata(); + tested.inject(Context.current(), metadata, MetadataSetter.INSTANCE); + + int size = 0; + for (String s : metadata.getAll( + Metadata.Key.of(GrpcPropagator.FIELD, Metadata.ASCII_STRING_MARSHALLER))) { + if (s != null) { + size++; + } + } + assertTrue(size == 1); + } +} \ No newline at end of file diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java new file mode 100644 index 000000000000..341c7248dff5 --- /dev/null +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java @@ -0,0 +1,32 @@ +package io.opentelemetry.instrumentation.grpc.v1_6.propagator; + +import io.opentelemetry.context.Context; +import io.opentelemetry.context.propagation.TextMapGetter; +import io.opentelemetry.context.propagation.TextMapPropagator; +import io.opentelemetry.context.propagation.TextMapSetter; + +import java.util.Collections; +import java.util.List; + +public class GrpcPropagator implements TextMapPropagator { + + public static final String FIELD = "X-grpc-field"; + private static final String METADATA = "value"; + + @Override + public List fields() { + return Collections.singletonList(FIELD); + } + + @Override + public void inject(Context context, C carrier, TextMapSetter setter) { + for (int i = 0; i < 2; i++) { + setter.set(carrier, FIELD, METADATA); + } + } + + @Override + public Context extract(Context context, C carrier, TextMapGetter getter) { + return context; + } +} From 035dad3f402595155c64795305607993f16a2d79 Mon Sep 17 00:00:00 2001 From: "gary.wgy" Date: Fri, 24 May 2024 17:49:19 +0800 Subject: [PATCH 3/5] apply spotless --- .../grpc/v1_6/MetadataSetterTest.java | 39 +++++++++++-------- .../grpc/v1_6/propagator/GrpcPropagator.java | 36 +++++++++-------- 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java index 4d2d28789e37..7923d0666a4f 100644 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java @@ -1,27 +1,32 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.instrumentation.grpc.v1_6; -import io.opentelemetry.context.Context; +import static org.junit.Assert.assertTrue; + import io.grpc.Metadata; +import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.grpc.v1_6.propagator.GrpcPropagator; import org.junit.jupiter.api.Test; -import static org.junit.Assert.assertTrue; - class MetadataSetterTest { - @Test - void checkThatIterableSizeEqualsOne() { - GrpcPropagator tested = new GrpcPropagator(); - Metadata metadata = new Metadata(); - tested.inject(Context.current(), metadata, MetadataSetter.INSTANCE); + @Test + void checkThatIterableSizeEqualsOne() { + GrpcPropagator tested = new GrpcPropagator(); + Metadata metadata = new Metadata(); + tested.inject(Context.current(), metadata, MetadataSetter.INSTANCE); - int size = 0; - for (String s : metadata.getAll( - Metadata.Key.of(GrpcPropagator.FIELD, Metadata.ASCII_STRING_MARSHALLER))) { - if (s != null) { - size++; - } - } - assertTrue(size == 1); + int size = 0; + for (String s : + metadata.getAll(Metadata.Key.of(GrpcPropagator.FIELD, Metadata.ASCII_STRING_MARSHALLER))) { + if (s != null) { + size++; + } } -} \ No newline at end of file + assertTrue(size == 1); + } +} diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java index 341c7248dff5..7e0f8e37dc89 100644 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java @@ -1,32 +1,36 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.instrumentation.grpc.v1_6.propagator; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.context.propagation.TextMapSetter; - import java.util.Collections; import java.util.List; public class GrpcPropagator implements TextMapPropagator { - public static final String FIELD = "X-grpc-field"; - private static final String METADATA = "value"; + public static final String FIELD = "X-grpc-field"; + private static final String METADATA = "value"; - @Override - public List fields() { - return Collections.singletonList(FIELD); - } + @Override + public List fields() { + return Collections.singletonList(FIELD); + } - @Override - public void inject(Context context, C carrier, TextMapSetter setter) { - for (int i = 0; i < 2; i++) { - setter.set(carrier, FIELD, METADATA); - } + @Override + public void inject(Context context, C carrier, TextMapSetter setter) { + for (int i = 0; i < 2; i++) { + setter.set(carrier, FIELD, METADATA); } + } - @Override - public Context extract(Context context, C carrier, TextMapGetter getter) { - return context; - } + @Override + public Context extract(Context context, C carrier, TextMapGetter getter) { + return context; + } } From 9bb816f8908f635a25aa8fafc5cbe233e7f17a10 Mon Sep 17 00:00:00 2001 From: "gary.wgy" Date: Wed, 29 May 2024 15:55:01 +0800 Subject: [PATCH 4/5] resolve comments --- .../grpc/v1_6/MetadataSetterTest.java | 26 +++++++------- .../grpc/v1_6/propagator/GrpcPropagator.java | 34 +++++++++++-------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java index 7923d0666a4f..a36105f58bbf 100644 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java @@ -14,19 +14,19 @@ class MetadataSetterTest { - @Test - void checkThatIterableSizeEqualsOne() { - GrpcPropagator tested = new GrpcPropagator(); - Metadata metadata = new Metadata(); - tested.inject(Context.current(), metadata, MetadataSetter.INSTANCE); + @Test + void checkThatIterableSizeEqualsOne() { + GrpcPropagator testGrpcPropagator = new GrpcPropagator(); + Metadata metadata = new Metadata(); + testGrpcPropagator.inject(Context.current(), metadata, MetadataSetter.INSTANCE); - int size = 0; - for (String s : - metadata.getAll(Metadata.Key.of(GrpcPropagator.FIELD, Metadata.ASCII_STRING_MARSHALLER))) { - if (s != null) { - size++; - } + int size = 0; + for (String s : + metadata.getAll(Metadata.Key.of(testGrpcPropagator.fields().get(0), Metadata.ASCII_STRING_MARSHALLER))) { + if (s != null) { + size++; + } + } + assertTrue(size == 1); } - assertTrue(size == 1); - } } diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java index 7e0f8e37dc89..da6e2a2de0f1 100644 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java @@ -9,28 +9,32 @@ import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.context.propagation.TextMapSetter; + +import java.util.Arrays; import java.util.Collections; import java.util.List; public class GrpcPropagator implements TextMapPropagator { - public static final String FIELD = "X-grpc-field"; - private static final String METADATA = "value"; + static final String FIELD = "X-grpc-field"; + private static final String METADATA = "value"; + private static final List FIELDS = + Collections.unmodifiableList(Arrays.asList(FIELD)); - @Override - public List fields() { - return Collections.singletonList(FIELD); - } + @Override + public List fields() { + return FIELDS; + } - @Override - public void inject(Context context, C carrier, TextMapSetter setter) { - for (int i = 0; i < 2; i++) { - setter.set(carrier, FIELD, METADATA); + @Override + public void inject(Context context, C carrier, TextMapSetter setter) { + for (int i = 0; i < 2; i++) { + setter.set(carrier, FIELD, METADATA); + } } - } - @Override - public Context extract(Context context, C carrier, TextMapGetter getter) { - return context; - } + @Override + public Context extract(Context context, C carrier, TextMapGetter getter) { + return context; + } } From 9ceb0fbe4f1f80aaa5e784c411d773d39a79ba0d Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 29 May 2024 14:27:14 +0300 Subject: [PATCH 5/5] Improve grpc metadata overwrite test --- .../grpc/v1_6/MetadataSetterTest.java | 67 ++++++++++++++----- .../grpc/v1_6/propagator/GrpcPropagator.java | 40 ----------- 2 files changed, 51 insertions(+), 56 deletions(-) delete mode 100644 instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java index a36105f58bbf..f138f4db0a4e 100644 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java +++ b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/MetadataSetterTest.java @@ -5,28 +5,63 @@ package io.opentelemetry.instrumentation.grpc.v1_6; -import static org.junit.Assert.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; import io.grpc.Metadata; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.SpanId; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceId; +import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.grpc.v1_6.propagator.GrpcPropagator; +import io.opentelemetry.context.propagation.TextMapGetter; +import io.opentelemetry.context.propagation.TextMapPropagator; +import javax.annotation.Nullable; import org.junit.jupiter.api.Test; class MetadataSetterTest { - @Test - void checkThatIterableSizeEqualsOne() { - GrpcPropagator testGrpcPropagator = new GrpcPropagator(); - Metadata metadata = new Metadata(); - testGrpcPropagator.inject(Context.current(), metadata, MetadataSetter.INSTANCE); - - int size = 0; - for (String s : - metadata.getAll(Metadata.Key.of(testGrpcPropagator.fields().get(0), Metadata.ASCII_STRING_MARSHALLER))) { - if (s != null) { - size++; - } - } - assertTrue(size == 1); + @Test + void overwriteTracingHeader() { + Metadata metadata = new Metadata(); + TextMapPropagator propagator = W3CTraceContextPropagator.getInstance(); + for (int i = 1; i <= 2; i++) { + Context context = + Context.root() + .with( + Span.wrap( + SpanContext.create( + TraceId.fromLongs(0, i), + SpanId.fromLong(i), + TraceFlags.getDefault(), + TraceState.getDefault()))); + propagator.inject(context, metadata, MetadataSetter.INSTANCE); } + + assertThat(metadata.getAll(Metadata.Key.of("traceparent", Metadata.ASCII_STRING_MARSHALLER))) + .hasSize(1); + + Context context = + propagator.extract( + Context.root(), + metadata, + new TextMapGetter() { + @Override + public Iterable keys(Metadata metadata) { + return metadata.keys(); + } + + @Nullable + @Override + public String get(@Nullable Metadata metadata, String key) { + return metadata.get(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER)); + } + }); + + SpanContext spanContext = Span.fromContext(context).getSpanContext(); + assertThat(spanContext.getTraceId()).isEqualTo(TraceId.fromLongs(0, 2)); + assertThat(spanContext.getSpanId()).isEqualTo(SpanId.fromLong(2)); + } } diff --git a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java b/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java deleted file mode 100644 index da6e2a2de0f1..000000000000 --- a/instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/propagator/GrpcPropagator.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.grpc.v1_6.propagator; - -import io.opentelemetry.context.Context; -import io.opentelemetry.context.propagation.TextMapGetter; -import io.opentelemetry.context.propagation.TextMapPropagator; -import io.opentelemetry.context.propagation.TextMapSetter; - -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - -public class GrpcPropagator implements TextMapPropagator { - - static final String FIELD = "X-grpc-field"; - private static final String METADATA = "value"; - private static final List FIELDS = - Collections.unmodifiableList(Arrays.asList(FIELD)); - - @Override - public List fields() { - return FIELDS; - } - - @Override - public void inject(Context context, C carrier, TextMapSetter setter) { - for (int i = 0; i < 2; i++) { - setter.set(carrier, FIELD, METADATA); - } - } - - @Override - public Context extract(Context context, C carrier, TextMapGetter getter) { - return context; - } -}