From 3fc21bf535790324ed4cd18904d1f5e566a1c516 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 8 Dec 2020 20:31:52 +0100 Subject: [PATCH 1/7] Capture HTTP2 headers in gRPC client/server Signed-off-by: Pavol Loffay --- instrumentation/grpc-1.5/build.gradle.kts | 6 +- ...ientHTTP2HeadersInstrumentationModule.java | 128 ++++++++++++++++++ .../grpc/v1_5/GrpcSemanticAttributes.java | 37 +++++ .../v1_5/client/GrpcClientInterceptor.java | 7 + .../grpc/v1_5/GrpcInstrumentationTest.java | 74 ++++++++++ 5 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java create mode 100644 instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java diff --git a/instrumentation/grpc-1.5/build.gradle.kts b/instrumentation/grpc-1.5/build.gradle.kts index a17b95dde..cd7e4c6b4 100644 --- a/instrumentation/grpc-1.5/build.gradle.kts +++ b/instrumentation/grpc-1.5/build.gradle.kts @@ -16,6 +16,7 @@ muzzle { versions = "[1.5.0, 1.33.0)" // see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/1453 // for body capture via com.google.protobuf.util.JsonFormat extraDependency("io.grpc:grpc-protobuf:1.5.0") + extraDependency("io.grpc:grpc-netty:1.5.0") } } @@ -57,11 +58,12 @@ val versions: Map by extra dependencies { api("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-grpc-1.5:${versions["opentelemetry_java_agent"]}") - api("io.opentelemetry.instrumentation:opentelemetry-grpc-1.5:0.11.0") + api("io.opentelemetry.instrumentation:opentelemetry-grpc-1.5:${versions["opentelemetry_java_agent"]}") compileOnly("io.grpc:grpc-core:1.5.0") compileOnly("io.grpc:grpc-protobuf:1.5.0") compileOnly("io.grpc:grpc-stub:1.5.0") + compileOnly("io.grpc:grpc-netty:1.5.0") implementation("javax.annotation:javax.annotation-api:1.3.2") @@ -70,4 +72,6 @@ dependencies { testImplementation("io.grpc:grpc-protobuf:1.5.0") testImplementation("io.grpc:grpc-stub:1.5.0") testImplementation("io.grpc:grpc-netty:1.5.0") + // TODO remove if not needed + testImplementation("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-netty-4.1:${versions["opentelemetry_java_agent"]}") } diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java new file mode 100644 index 000000000..dfacddea6 --- /dev/null +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java @@ -0,0 +1,128 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.instrumentation.hypertrace.grpc.v1_5; + +import static net.bytebuddy.matcher.ElementMatchers.failSafe; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import io.grpc.Metadata; +import io.netty.handler.codec.http2.Http2Headers; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.tooling.InstrumentationModule; +import io.opentelemetry.javaagent.tooling.TypeInstrumentation; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.hypertrace.agent.core.HypertraceSemanticAttributes; + +@AutoService(InstrumentationModule.class) +public class ClientHTTP2HeadersInstrumentationModule extends InstrumentationModule { + + public ClientHTTP2HeadersInstrumentationModule() { + super(GrpcInstrumentationName.PRIMARY, GrpcInstrumentationName.OTHER); + } + + @Override + public List typeInstrumentations() { + return Arrays.asList(new NettyUtilsInstrumentation()); + } + + class NettyUtilsInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return failSafe(named("io.grpc.netty.Utils")); + } + + @Override + public Map, String> transformers() { + Map, String> transformers = new HashMap<>(); + transformers.put( + isMethod().and(named("convertClientHeaders")).and(takesArguments(6)), + Utils_convertClientHeaders_Advice.class.getName()); + transformers.put( + isMethod().and(named("convertHeaders")).and(takesArguments(1)), + GrpcUtils_convertHeaders_Advice.class.getName()); + return transformers; + } + } + + static class Utils_convertClientHeaders_Advice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void exit( + @Advice.Argument(1) Object scheme, + @Advice.Argument(2) Object defaultPath, + @Advice.Argument(3) Object authority, + @Advice.Argument(4) Object method) { + + Span currentSpan = Java8BytecodeBridge.currentSpan(); + System.out.println("\n\n Utils.convertHeaders exit"); + System.out.println(currentSpan); + if (scheme != null) { + currentSpan.setAttribute( + HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.SCHEME), + scheme.toString()); + } + if (defaultPath != null) { + currentSpan.setAttribute( + HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.PATH), + defaultPath.toString()); + } + if (authority != null) { + currentSpan.setAttribute( + HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.AUTHORITY), + authority.toString()); + } + if (method != null) { + currentSpan.setAttribute( + HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.METHOD), + method.toString()); + } + } + } + + static class GrpcUtils_convertHeaders_Advice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void exit( + @Advice.Argument(0) Http2Headers http2Headers, + @Advice.Return(readOnly = false) Metadata metadata) { + + if (http2Headers.authority() != null) { + metadata.put( + GrpcSemanticAttributes.AUTHORITY_METADATA_KEY, http2Headers.authority().toString()); + } + if (http2Headers.path() != null) { + metadata.put(GrpcSemanticAttributes.PATH_METADATA_KEY, http2Headers.path().toString()); + } + if (http2Headers.method() != null) { + metadata.put(GrpcSemanticAttributes.METHOD_METADATA_KEY, http2Headers.method().toString()); + } + if (http2Headers.scheme() != null) { + metadata.put(GrpcSemanticAttributes.SCHEME_METADATA_KEY, http2Headers.scheme().toString()); + } + } + } +} diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java new file mode 100644 index 000000000..f1c9f5ec0 --- /dev/null +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java @@ -0,0 +1,37 @@ +/* + * Copyright The Hypertrace Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.opentelemetry.instrumentation.hypertrace.grpc.v1_5; + +import io.grpc.Metadata; + +public class GrpcSemanticAttributes { + private GrpcSemanticAttributes() {} + + public static final String SCHEME = ":scheme"; + public static final String PATH = ":path"; + public static final String AUTHORITY = ":authority"; + public static final String METHOD = ":method"; + + public static final Metadata.Key SCHEME_METADATA_KEY = + Metadata.Key.of(SCHEME, Metadata.ASCII_STRING_MARSHALLER); + public static final Metadata.Key PATH_METADATA_KEY = + Metadata.Key.of(PATH, Metadata.ASCII_STRING_MARSHALLER); + public static final Metadata.Key AUTHORITY_METADATA_KEY = + Metadata.Key.of(AUTHORITY, Metadata.ASCII_STRING_MARSHALLER); + public static final Metadata.Key METHOD_METADATA_KEY = + Metadata.Key.of(METHOD, Metadata.ASCII_STRING_MARSHALLER); +} diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/client/GrpcClientInterceptor.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/client/GrpcClientInterceptor.java index cc39edad3..99c5b684a 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/client/GrpcClientInterceptor.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/client/GrpcClientInterceptor.java @@ -25,6 +25,7 @@ import io.grpc.ForwardingClientCallListener; import io.grpc.Metadata; import io.grpc.MethodDescriptor; +import io.grpc.Status; import io.opentelemetry.api.trace.Span; import io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcInstrumentationName; import io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcSpanDecorator; @@ -84,6 +85,12 @@ static final class TracingClientCallListener this.span = span; } + @Override + public void onClose(Status status, Metadata trailers) { + super.onClose(status, trailers); + System.out.println("\n\nclosing tracing listener"); + } + @Override public void onMessage(RespT message) { delegate().onMessage(message); diff --git a/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java b/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java index 65f1b5a21..55c7c135a 100644 --- a/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java +++ b/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java @@ -48,8 +48,55 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.MethodOrderer.OrderAnnotation; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; +/** + * The current span in Utils.convertClientHeaders is DefaultSpan for the first request. The first + * request uses io.grpc.internal.DelayedClientTransport and it is called from + * io.grpc.stub.ClientCalls.blockingUnaryCall. Subsequent requests use + * io.grpc.stub.ClientCalls.futureUnaryCall - see (1). The DelayedClientTransport calls network in a + * separate branch after ClientCallImpl or gRPC interceptors. To propagate span to + * Utils.convertClientHeaders it would have to be started in + * io.grpc.stub.ClientCalls.blockingUnaryCall. + * + *

Span is not recording (Default) java.lang.Exception: Stack trace at + * java.lang.Thread.dumpStack(Thread.java:1336) at + * io.grpc.netty.Utils.convertClientHeaders(Utils.java:107) at + * io.grpc.netty.NettyClientStream$Sink.writeHeaders(NettyClientStream.java:124) at + * io.grpc.internal.AbstractClientStream.start(AbstractClientStream.java:132) at + * io.grpc.internal.DelayedStream$4.run(DelayedStream.java:197) at + * io.grpc.internal.DelayedStream.drainPendingCalls(DelayedStream.java:132) at + * io.grpc.internal.DelayedStream.setStream(DelayedStream.java:101) at + * io.grpc.internal.DelayedClientTransport$PendingStream.createRealStream(DelayedClientTransport.java:351) + * at + * io.grpc.internal.DelayedClientTransport$PendingStream.access$200(DelayedClientTransport.java:334) + * at io.grpc.internal.DelayedClientTransport$5.run(DelayedClientTransport.java:293) at + * io.grpc.stub.ClientCalls$ThreadlessExecutor.waitAndDrain(ClientCalls.java:575) (1) at + * + *

io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:120) at + * org.hypertrace.example.GreeterGrpc$GreeterBlockingStub.sayHello(GreeterGrpc.java:172) at + * io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcInstrumentationTest.serverRequestBlocking(GrpcInstrumentationTest.java:150) + * + *

Span is recording java.lang.Exception: Stack trace at + * java.lang.Thread.dumpStack(Thread.java:1336) at + * io.grpc.netty.Utils.convertClientHeaders(Utils.java:107) at + * io.grpc.netty.NettyClientStream$Sink.writeHeaders(NettyClientStream.java:124) at + * io.grpc.internal.AbstractClientStream.start(AbstractClientStream.java:132) at + * io.grpc.internal.ClientCallImpl.start(ClientCallImpl.java:225) at + * io.grpc.ForwardingClientCall.start(ForwardingClientCall.java:32) at + * io.opentelemetry.instrumentation.grpc.v1_5.client.TracingClientInterceptor$TracingClientCall.start(TracingClientInterceptor.java:102) + * at io.grpc.stub.ClientCalls.startCall(ClientCalls.java:261) at + * io.grpc.stub.ClientCalls.asyncUnaryRequestCall(ClientCalls.java:237) at + * io.grpc.stub.ClientCalls.futureUnaryCall(ClientCalls.java:171) at + * + *

io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:117) at + * org.hypertrace.example.GreeterGrpc$GreeterBlockingStub.sayHello(GreeterGrpc.java:172) at + * io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcInstrumentationTest.disabledInstrumentation_dynamicConfig(GrpcInstrumentationTest.java:182) + */ +@TestMethodOrder(OrderAnnotation.class) public class GrpcInstrumentationTest extends AbstractInstrumenterTest { private static final Helloworld.Request REQUEST = @@ -113,6 +160,7 @@ public void afterEach() { } @Test + @Order(2) public void blockingStub() throws IOException, TimeoutException, InterruptedException { Metadata headers = new Metadata(); headers.put(CLIENT_STRING_METADATA_KEY, "clientheader"); @@ -135,9 +183,13 @@ public void blockingStub() throws IOException, TimeoutException, InterruptedExce assertBodiesAndHeaders(clientSpan, requestJson, responseJson); SpanData serverSpan = spans.get(1); assertBodiesAndHeaders(serverSpan, requestJson, responseJson); + + assertHttp2HeadersForSayHelloMethod(serverSpan); + assertHttp2HeadersForSayHelloMethod(clientSpan); } @Test + @Order(1) public void serverRequestBlocking() throws TimeoutException, InterruptedException { Metadata blockHeaders = new Metadata(); blockHeaders.put(Metadata.Key.of("mockblock", Metadata.ASCII_STRING_MARSHALLER), "true"); @@ -167,9 +219,11 @@ public void serverRequestBlocking() throws TimeoutException, InterruptedExceptio serverSpan .getAttributes() .get(HypertraceSemanticAttributes.rpcRequestMetadata("mockblock"))); + assertHttp2HeadersForSayHelloMethod(serverSpan); } @Test + @Order(3) public void disabledInstrumentation_dynamicConfig() throws TimeoutException, InterruptedException { URL configUrl = getClass().getClassLoader().getResource("ht-config-all-disabled.yaml"); @@ -215,4 +269,24 @@ private void assertBodiesAndHeaders(SpanData span, String requestJson, String re HypertraceSemanticAttributes.rpcResponseMetadata( SERVER_STRING_METADATA_KEY.name()))); } + + private void assertHttp2HeadersForSayHelloMethod(SpanData span) { + Assertions.assertEquals( + "http", + span.getAttributes() + .get(HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.SCHEME))); + Assertions.assertEquals( + "POST", + span.getAttributes() + .get(HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.METHOD))); + Assertions.assertEquals( + String.format("localhost:%d", SERVER.getPort()), + span.getAttributes() + .get( + HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.AUTHORITY))); + Assertions.assertEquals( + "/org.hypertrace.example.Greeter/SayHello", + span.getAttributes() + .get(HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.PATH))); + } } From aa99746098c6be3be547e1bb712e71c2811c2ac7 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 8 Dec 2020 20:40:23 +0100 Subject: [PATCH 2/7] Fixes Signed-off-by: Pavol Loffay --- instrumentation/grpc-1.5/build.gradle.kts | 2 - ...ientHTTP2HeadersInstrumentationModule.java | 2 - .../v1_5/client/GrpcClientInterceptor.java | 7 --- .../grpc/v1_5/GrpcInstrumentationTest.java | 43 +------------------ 4 files changed, 2 insertions(+), 52 deletions(-) diff --git a/instrumentation/grpc-1.5/build.gradle.kts b/instrumentation/grpc-1.5/build.gradle.kts index cd7e4c6b4..ada5006c6 100644 --- a/instrumentation/grpc-1.5/build.gradle.kts +++ b/instrumentation/grpc-1.5/build.gradle.kts @@ -72,6 +72,4 @@ dependencies { testImplementation("io.grpc:grpc-protobuf:1.5.0") testImplementation("io.grpc:grpc-stub:1.5.0") testImplementation("io.grpc:grpc-netty:1.5.0") - // TODO remove if not needed - testImplementation("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-netty-4.1:${versions["opentelemetry_java_agent"]}") } diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java index dfacddea6..ac800cf7d 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java @@ -79,8 +79,6 @@ public static void exit( @Advice.Argument(4) Object method) { Span currentSpan = Java8BytecodeBridge.currentSpan(); - System.out.println("\n\n Utils.convertHeaders exit"); - System.out.println(currentSpan); if (scheme != null) { currentSpan.setAttribute( HypertraceSemanticAttributes.rpcRequestMetadata(GrpcSemanticAttributes.SCHEME), diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/client/GrpcClientInterceptor.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/client/GrpcClientInterceptor.java index 99c5b684a..cc39edad3 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/client/GrpcClientInterceptor.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/client/GrpcClientInterceptor.java @@ -25,7 +25,6 @@ import io.grpc.ForwardingClientCallListener; import io.grpc.Metadata; import io.grpc.MethodDescriptor; -import io.grpc.Status; import io.opentelemetry.api.trace.Span; import io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcInstrumentationName; import io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcSpanDecorator; @@ -85,12 +84,6 @@ static final class TracingClientCallListener this.span = span; } - @Override - public void onClose(Status status, Metadata trailers) { - super.onClose(status, trailers); - System.out.println("\n\nclosing tracing listener"); - } - @Override public void onMessage(RespT message) { delegate().onMessage(message); diff --git a/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java b/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java index 55c7c135a..864ab15bf 100644 --- a/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java +++ b/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java @@ -54,47 +54,8 @@ import org.junit.jupiter.api.TestMethodOrder; /** - * The current span in Utils.convertClientHeaders is DefaultSpan for the first request. The first - * request uses io.grpc.internal.DelayedClientTransport and it is called from - * io.grpc.stub.ClientCalls.blockingUnaryCall. Subsequent requests use - * io.grpc.stub.ClientCalls.futureUnaryCall - see (1). The DelayedClientTransport calls network in a - * separate branch after ClientCallImpl or gRPC interceptors. To propagate span to - * Utils.convertClientHeaders it would have to be started in - * io.grpc.stub.ClientCalls.blockingUnaryCall. - * - *

Span is not recording (Default) java.lang.Exception: Stack trace at - * java.lang.Thread.dumpStack(Thread.java:1336) at - * io.grpc.netty.Utils.convertClientHeaders(Utils.java:107) at - * io.grpc.netty.NettyClientStream$Sink.writeHeaders(NettyClientStream.java:124) at - * io.grpc.internal.AbstractClientStream.start(AbstractClientStream.java:132) at - * io.grpc.internal.DelayedStream$4.run(DelayedStream.java:197) at - * io.grpc.internal.DelayedStream.drainPendingCalls(DelayedStream.java:132) at - * io.grpc.internal.DelayedStream.setStream(DelayedStream.java:101) at - * io.grpc.internal.DelayedClientTransport$PendingStream.createRealStream(DelayedClientTransport.java:351) - * at - * io.grpc.internal.DelayedClientTransport$PendingStream.access$200(DelayedClientTransport.java:334) - * at io.grpc.internal.DelayedClientTransport$5.run(DelayedClientTransport.java:293) at - * io.grpc.stub.ClientCalls$ThreadlessExecutor.waitAndDrain(ClientCalls.java:575) (1) at - * - *

io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:120) at - * org.hypertrace.example.GreeterGrpc$GreeterBlockingStub.sayHello(GreeterGrpc.java:172) at - * io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcInstrumentationTest.serverRequestBlocking(GrpcInstrumentationTest.java:150) - * - *

Span is recording java.lang.Exception: Stack trace at - * java.lang.Thread.dumpStack(Thread.java:1336) at - * io.grpc.netty.Utils.convertClientHeaders(Utils.java:107) at - * io.grpc.netty.NettyClientStream$Sink.writeHeaders(NettyClientStream.java:124) at - * io.grpc.internal.AbstractClientStream.start(AbstractClientStream.java:132) at - * io.grpc.internal.ClientCallImpl.start(ClientCallImpl.java:225) at - * io.grpc.ForwardingClientCall.start(ForwardingClientCall.java:32) at - * io.opentelemetry.instrumentation.grpc.v1_5.client.TracingClientInterceptor$TracingClientCall.start(TracingClientInterceptor.java:102) - * at io.grpc.stub.ClientCalls.startCall(ClientCalls.java:261) at - * io.grpc.stub.ClientCalls.asyncUnaryRequestCall(ClientCalls.java:237) at - * io.grpc.stub.ClientCalls.futureUnaryCall(ClientCalls.java:171) at - * - *

io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:117) at - * org.hypertrace.example.GreeterGrpc$GreeterBlockingStub.sayHello(GreeterGrpc.java:172) at - * io.opentelemetry.instrumentation.hypertrace.grpc.v1_5.GrpcInstrumentationTest.disabledInstrumentation_dynamicConfig(GrpcInstrumentationTest.java:182) + * TODO the HTTP2 headers for client does not work for the first request - therefore the ordering + * https://github.com/hypertrace/javaagent/issues/109#issuecomment-740918018 */ @TestMethodOrder(OrderAnnotation.class) public class GrpcInstrumentationTest extends AbstractInstrumenterTest { From 960b66075af14b499c19bf53eb75b143f1e6f016 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 8 Dec 2020 20:44:02 +0100 Subject: [PATCH 3/7] Fixes 2 Signed-off-by: Pavol Loffay --- ....java => Http2HeadersInstrumentationModule.java} | 13 ++++++++++--- .../grpc/v1_5/GrpcInstrumentationTest.java | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) rename instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/{ClientHTTP2HeadersInstrumentationModule.java => Http2HeadersInstrumentationModule.java} (89%) diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/Http2HeadersInstrumentationModule.java similarity index 89% rename from instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java rename to instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/Http2HeadersInstrumentationModule.java index ac800cf7d..b65f42b69 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/ClientHTTP2HeadersInstrumentationModule.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/Http2HeadersInstrumentationModule.java @@ -39,9 +39,9 @@ import org.hypertrace.agent.core.HypertraceSemanticAttributes; @AutoService(InstrumentationModule.class) -public class ClientHTTP2HeadersInstrumentationModule extends InstrumentationModule { +public class Http2HeadersInstrumentationModule extends InstrumentationModule { - public ClientHTTP2HeadersInstrumentationModule() { + public Http2HeadersInstrumentationModule() { super(GrpcInstrumentationName.PRIMARY, GrpcInstrumentationName.OTHER); } @@ -50,8 +50,15 @@ public List typeInstrumentations() { return Arrays.asList(new NettyUtilsInstrumentation()); } + /** + * The server side HTTP2 headers are added in tracing gRPC interceptor. The headers are added to + * metadata in {@link GrpcUtils_convertHeaders_Advice}. + * + *

The client side HTTP2 headers are added directly to span in {@link + * Utils_convertClientHeaders_Advice}. TODO However it does not work for the first request + * https://github.com/hypertrace/javaagent/issues/109#issuecomment-740918018. + */ class NettyUtilsInstrumentation implements TypeInstrumentation { - @Override public ElementMatcher typeMatcher() { return failSafe(named("io.grpc.netty.Utils")); diff --git a/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java b/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java index 864ab15bf..a4fa297b9 100644 --- a/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java +++ b/instrumentation/grpc-1.5/src/test/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcInstrumentationTest.java @@ -54,8 +54,8 @@ import org.junit.jupiter.api.TestMethodOrder; /** - * TODO the HTTP2 headers for client does not work for the first request - therefore the ordering - * https://github.com/hypertrace/javaagent/issues/109#issuecomment-740918018 + * TODO the HTTP2 headers for client does not work for the first request - therefore the explicit + * ordering https://github.com/hypertrace/javaagent/issues/109#issuecomment-740918018 */ @TestMethodOrder(OrderAnnotation.class) public class GrpcInstrumentationTest extends AbstractInstrumenterTest { From 35f895183a35eba820206dc3d18792f76e8a1974 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 11:01:55 +0100 Subject: [PATCH 4/7] Use suffix for HT keys Signed-off-by: Pavol Loffay --- .../grpc/v1_5/GrpcSemanticAttributes.java | 23 +++++++++++++++---- .../grpc/v1_5/GrpcSpanDecorator.java | 2 ++ ...ttyHttp2HeadersInstrumentationModule.java} | 14 +++++++---- 3 files changed, 31 insertions(+), 8 deletions(-) rename instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/{Http2HeadersInstrumentationModule.java => NettyHttp2HeadersInstrumentationModule.java} (89%) diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java index f1c9f5ec0..4858b9d12 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java @@ -26,12 +26,27 @@ private GrpcSemanticAttributes() {} public static final String AUTHORITY = ":authority"; public static final String METHOD = ":method"; + /** + * These metadata headers are added in Http2Headers instrumentation. We use different names than + * original HTTP2 header names to avoid any collisions with app code. + * + *

We cannot use prefix because e.g. ht.:path is not a valid key. + */ + private static final String SUFFIX = "ht"; + public static final Metadata.Key SCHEME_METADATA_KEY = - Metadata.Key.of(SCHEME, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key.of(SCHEME + SUFFIX, Metadata.ASCII_STRING_MARSHALLER); public static final Metadata.Key PATH_METADATA_KEY = - Metadata.Key.of(PATH, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key.of(PATH + SUFFIX, Metadata.ASCII_STRING_MARSHALLER); public static final Metadata.Key AUTHORITY_METADATA_KEY = - Metadata.Key.of(AUTHORITY, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key.of(AUTHORITY + SUFFIX, Metadata.ASCII_STRING_MARSHALLER); public static final Metadata.Key METHOD_METADATA_KEY = - Metadata.Key.of(METHOD, Metadata.ASCII_STRING_MARSHALLER); + Metadata.Key.of(METHOD + SUFFIX, Metadata.ASCII_STRING_MARSHALLER); + + public static String removeHypertracePrefix(String key) { + if (key.endsWith(SUFFIX)) { + return key.replace(SUFFIX, ""); + } + return key; + } } diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSpanDecorator.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSpanDecorator.java index d8e4a07b8..1a664de6d 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSpanDecorator.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSpanDecorator.java @@ -58,6 +58,7 @@ public static void addMetadataAttributes( Key stringKey = Key.of(key, Metadata.ASCII_STRING_MARSHALLER); Iterable stringValues = metadata.getAll(stringKey); for (String stringValue : stringValues) { + key = GrpcSemanticAttributes.removeHypertracePrefix(key); span.setAttribute(keySupplier.apply(key), stringValue); } } @@ -79,6 +80,7 @@ public static Map metadataToMap(Metadata metadata) { Key stringKey = Key.of(key, Metadata.ASCII_STRING_MARSHALLER); Iterable stringValues = metadata.getAll(stringKey); for (String stringValue : stringValues) { + key = GrpcSemanticAttributes.removeHypertracePrefix(key); mapHeaders.put(key, stringValue); } } diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/Http2HeadersInstrumentationModule.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java similarity index 89% rename from instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/Http2HeadersInstrumentationModule.java rename to instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java index b65f42b69..2b4f00ec0 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/Http2HeadersInstrumentationModule.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java @@ -39,9 +39,9 @@ import org.hypertrace.agent.core.HypertraceSemanticAttributes; @AutoService(InstrumentationModule.class) -public class Http2HeadersInstrumentationModule extends InstrumentationModule { +public class NettyHttp2HeadersInstrumentationModule extends InstrumentationModule { - public Http2HeadersInstrumentationModule() { + public NettyHttp2HeadersInstrumentationModule() { super(GrpcInstrumentationName.PRIMARY, GrpcInstrumentationName.OTHER); } @@ -109,11 +109,17 @@ public static void exit( } } + /** + * There are multiple implementations of {@link Http2Headers}. Only some of them support getting + * authority, path etc. For instance {@code GrpcHttp2ResponseHeaders} throws unsupported exception + * when accessing authority etc. This header is used client response. + * + * @see {@link io.grpc.netty.GrpcHttp2HeadersUtils} + */ static class GrpcUtils_convertHeaders_Advice { @Advice.OnMethodExit(suppress = Throwable.class) public static void exit( - @Advice.Argument(0) Http2Headers http2Headers, - @Advice.Return(readOnly = false) Metadata metadata) { + @Advice.Argument(0) Http2Headers http2Headers, @Advice.Return Metadata metadata) { if (http2Headers.authority() != null) { metadata.put( From ac79bcfc6748f35feced63f01966d8266080c652 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 11:04:49 +0100 Subject: [PATCH 5/7] Use . Signed-off-by: Pavol Loffay --- .../hypertrace/grpc/v1_5/GrpcSemanticAttributes.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java index 4858b9d12..c6e8cc066 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/GrpcSemanticAttributes.java @@ -32,7 +32,7 @@ private GrpcSemanticAttributes() {} * *

We cannot use prefix because e.g. ht.:path is not a valid key. */ - private static final String SUFFIX = "ht"; + private static final String SUFFIX = ".ht"; public static final Metadata.Key SCHEME_METADATA_KEY = Metadata.Key.of(SCHEME + SUFFIX, Metadata.ASCII_STRING_MARSHALLER); From 6530cb894925497536bdf628a42e8ac4d17493c0 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 11:13:37 +0100 Subject: [PATCH 6/7] Add unique name Signed-off-by: Pavol Loffay --- .../grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java index 2b4f00ec0..434b515fc 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java @@ -42,7 +42,9 @@ public class NettyHttp2HeadersInstrumentationModule extends InstrumentationModule { public NettyHttp2HeadersInstrumentationModule() { - super(GrpcInstrumentationName.PRIMARY, GrpcInstrumentationName.OTHER); + super( + GrpcInstrumentationName.PRIMARY, + Arrays.asList("grpc-netty-ht", GrpcInstrumentationName.OTHER).toArray(new String[0])); } @Override From 3b187b40a73d35c2bdec71bbd26367b9d8ad799f Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 9 Dec 2020 11:33:50 +0100 Subject: [PATCH 7/7] Fix Signed-off-by: Pavol Loffay --- .../NettyHttp2HeadersInstrumentationModule.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java index 434b515fc..962271eb7 100644 --- a/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java +++ b/instrumentation/grpc-1.5/src/main/java/io/opentelemetry/instrumentation/hypertrace/grpc/v1_5/NettyHttp2HeadersInstrumentationModule.java @@ -28,6 +28,7 @@ import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import io.opentelemetry.javaagent.tooling.InstrumentationModule; import io.opentelemetry.javaagent.tooling.TypeInstrumentation; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -41,10 +42,16 @@ @AutoService(InstrumentationModule.class) public class NettyHttp2HeadersInstrumentationModule extends InstrumentationModule { + private static final List INSTRUMENTATION_NAMES = new ArrayList<>(); + + static { + INSTRUMENTATION_NAMES.add(GrpcInstrumentationName.PRIMARY); + INSTRUMENTATION_NAMES.addAll(Arrays.asList(GrpcInstrumentationName.OTHER)); + INSTRUMENTATION_NAMES.add("grpc-netty-ht"); + } + public NettyHttp2HeadersInstrumentationModule() { - super( - GrpcInstrumentationName.PRIMARY, - Arrays.asList("grpc-netty-ht", GrpcInstrumentationName.OTHER).toArray(new String[0])); + super(INSTRUMENTATION_NAMES); } @Override