From 25f6864602717debf15f456b367b28ae6b2deb06 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 25 Oct 2021 22:43:30 +0200 Subject: [PATCH] Migrate reactor-netty CONNECT instrumentation to Instrumenter API (#4493) --- .../NettyClientInstrumenterFactory.java | 9 +- .../v4_0/client/NettyClientSingletons.java | 6 +- .../v4_1/client/NettyClientSingletons.java | 6 +- .../javaagent/build.gradle.kts | 1 + .../reactornetty/v1_0/ConnectionWrapper.java | 18 +-- .../v1_0/ReactorNettySingletons.java | 32 ++++++ .../reactornetty/v1_0/ReactorNettyTracer.java | 104 ------------------ .../TransportConnectorInstrumentation.java | 25 +++-- .../ReactorNettyConnectionSpanTest.groovy | 1 + 9 files changed, 68 insertions(+), 134 deletions(-) create mode 100644 instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettySingletons.java delete mode 100644 instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyTracer.java diff --git a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java index 5ad163b02a..203176a2fa 100644 --- a/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java +++ b/instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/client/NettyClientInstrumenterFactory.java @@ -8,7 +8,6 @@ package io.opentelemetry.javaagent.instrumentation.netty.common.client; import io.netty.channel.Channel; import io.netty.handler.codec.http.HttpResponse; import io.opentelemetry.api.GlobalOpenTelemetry; -import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.PeerServiceAttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; @@ -19,13 +18,13 @@ import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndCha public final class NettyClientInstrumenterFactory { - private static final boolean alwaysCreateConnectSpan = - Config.get().getBoolean("otel.instrumentation.netty.always-create-connect-span", false); - private final String instrumentationName; + private final boolean alwaysCreateConnectSpan; - public NettyClientInstrumenterFactory(String instrumentationName) { + public NettyClientInstrumenterFactory( + String instrumentationName, boolean alwaysCreateConnectSpan) { this.instrumentationName = instrumentationName; + this.alwaysCreateConnectSpan = alwaysCreateConnectSpan; } public Instrumenter createHttpInstrumenter() { diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/NettyClientSingletons.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/NettyClientSingletons.java index 64c241ae08..902219976b 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/NettyClientSingletons.java +++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/NettyClientSingletons.java @@ -6,6 +6,7 @@ package io.opentelemetry.javaagent.instrumentation.netty.v4_0.client; import io.netty.handler.codec.http.HttpResponse; +import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel; import io.opentelemetry.javaagent.instrumentation.netty.common.client.NettyClientInstrumenterFactory; @@ -13,12 +14,15 @@ import io.opentelemetry.javaagent.instrumentation.netty.common.client.NettyConne public final class NettyClientSingletons { + private static final boolean alwaysCreateConnectSpan = + Config.get().getBoolean("otel.instrumentation.netty.always-create-connect-span", false); + private static final Instrumenter INSTRUMENTER; private static final NettyConnectInstrumenter CONNECT_INSTRUMENTER; static { NettyClientInstrumenterFactory factory = - new NettyClientInstrumenterFactory("io.opentelemetry.netty-4.0"); + new NettyClientInstrumenterFactory("io.opentelemetry.netty-4.0", alwaysCreateConnectSpan); INSTRUMENTER = factory.createHttpInstrumenter(); CONNECT_INSTRUMENTER = factory.createConnectInstrumenter(); } diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/NettyClientSingletons.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/NettyClientSingletons.java index e32e01ea66..4eb68bcefd 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/NettyClientSingletons.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/client/NettyClientSingletons.java @@ -7,6 +7,7 @@ package io.opentelemetry.javaagent.instrumentation.netty.v4_1.client; import io.netty.handler.codec.http.HttpResponse; import io.netty.util.AttributeKey; +import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel; import io.opentelemetry.javaagent.instrumentation.netty.common.client.NettyClientInstrumenterFactory; @@ -19,12 +20,15 @@ public final class NettyClientSingletons { static final AttributeKey HTTP_RESPONSE = AttributeKey.valueOf(NettyClientSingletons.class, "http-client-response"); + private static final boolean alwaysCreateConnectSpan = + Config.get().getBoolean("otel.instrumentation.netty.always-create-connect-span", false); + private static final Instrumenter INSTRUMENTER; private static final NettyConnectInstrumenter CONNECT_INSTRUMENTER; static { NettyClientInstrumenterFactory factory = - new NettyClientInstrumenterFactory("io.opentelemetry.netty-4.1"); + new NettyClientInstrumenterFactory("io.opentelemetry.netty-4.1", alwaysCreateConnectSpan); INSTRUMENTER = factory.createHttpInstrumenter(); CONNECT_INSTRUMENTER = factory.createConnectInstrumenter(); } diff --git a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/build.gradle.kts b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/build.gradle.kts index a4b9fc91ea..f85f9744e0 100644 --- a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/build.gradle.kts +++ b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/build.gradle.kts @@ -18,6 +18,7 @@ muzzle { dependencies { implementation(project(":instrumentation:netty:netty-4.1:library")) + implementation(project(":instrumentation:netty:netty-4-common:javaagent")) library("io.projectreactor.netty:reactor-netty-http:1.0.0") testInstrumentation(project(":instrumentation:reactor-netty:reactor-netty-0.9:javaagent")) diff --git a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ConnectionWrapper.java b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ConnectionWrapper.java index cc72a25405..eb85d8560a 100644 --- a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ConnectionWrapper.java +++ b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ConnectionWrapper.java @@ -5,26 +5,18 @@ package io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0; -import static io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0.ReactorNettyTracer.tracer; +import static io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0.ReactorNettySingletons.connectInstrumenter; import io.netty.channel.Channel; import io.opentelemetry.context.Context; -import java.net.SocketAddress; +import io.opentelemetry.javaagent.instrumentation.netty.common.client.NettyConnectRequest; import reactor.core.publisher.Mono; public class ConnectionWrapper { public static Mono wrap( - Context context, Context parentContext, SocketAddress remoteAddress, Mono mono) { - return mono.doOnError( - throwable -> { - tracer().endConnectionSpan(context, parentContext, remoteAddress, null, throwable); - }) - .doOnSuccess( - channel -> { - if (context != null) { - tracer().endConnectionSpan(context, parentContext, remoteAddress, channel, null); - } - }); + Context context, NettyConnectRequest request, Mono mono) { + return mono.doOnError(error -> connectInstrumenter().end(context, request, null, error)) + .doOnSuccess(channel -> connectInstrumenter().end(context, request, channel, null)); } } diff --git a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettySingletons.java b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettySingletons.java new file mode 100644 index 0000000000..4909b77268 --- /dev/null +++ b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettySingletons.java @@ -0,0 +1,32 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0; + +import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.javaagent.instrumentation.netty.common.client.NettyClientInstrumenterFactory; +import io.opentelemetry.javaagent.instrumentation.netty.common.client.NettyConnectInstrumenter; + +public final class ReactorNettySingletons { + + private static final boolean alwaysCreateConnectSpan = + Config.get() + .getBoolean("otel.instrumentation.reactor-netty.always-create-connect-span", false); + + private static final NettyConnectInstrumenter CONNECT_INSTRUMENTER; + + static { + NettyClientInstrumenterFactory instrumenterFactory = + new NettyClientInstrumenterFactory( + "io.opentelemetry.reactor-netty-1.0", alwaysCreateConnectSpan); + CONNECT_INSTRUMENTER = instrumenterFactory.createConnectInstrumenter(); + } + + public static NettyConnectInstrumenter connectInstrumenter() { + return CONNECT_INSTRUMENTER; + } + + private ReactorNettySingletons() {} +} diff --git a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyTracer.java b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyTracer.java deleted file mode 100644 index 78e74556bb..0000000000 --- a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyTracer.java +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0; - -import static io.opentelemetry.api.trace.SpanKind.CLIENT; -import static io.opentelemetry.api.trace.SpanKind.INTERNAL; -import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP; -import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_UDP; - -import io.netty.channel.Channel; -import io.netty.channel.socket.DatagramChannel; -import io.opentelemetry.api.GlobalOpenTelemetry; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanBuilder; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.config.Config; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.net.NetPeerAttributes; -import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; -import java.net.InetSocketAddress; -import java.net.SocketAddress; - -public class ReactorNettyTracer extends BaseTracer { - private static final ReactorNettyTracer TRACER = new ReactorNettyTracer(); - - private static final boolean alwaysCreateConnectSpan = - Config.get() - .getBoolean("otel.instrumentation.reactor-netty.always-create-connect-span", false); - - protected ReactorNettyTracer() { - super(GlobalOpenTelemetry.get()); - } - - public static ReactorNettyTracer tracer() { - return TRACER; - } - - public Context startConnectionSpan(Context parentContext, SocketAddress remoteAddress) { - if (!alwaysCreateConnectSpan) { - return null; - } - - SpanBuilder spanBuilder = spanBuilder(parentContext, "CONNECT", INTERNAL); - NetPeerAttributes.INSTANCE.setNetPeer(spanBuilder, (InetSocketAddress) remoteAddress); - - return parentContext.with(spanBuilder.startSpan()); - } - - public void endConnectionSpan( - Context context, - Context parentContext, - SocketAddress remoteAddress, - Channel channel, - Throwable throwable) { - if (alwaysCreateConnectSpan) { - if (context != null) { - // if context is present we started span in startConnectionSpan - endConnectionSpan(context, channel, throwable); - } - } else if (throwable != null && shouldStartSpan(parentContext, CLIENT)) { - // if we didn't start span in startConnectionSpan create a span only when the request fails - // and when not inside a client span - connectionFailure(parentContext, remoteAddress, channel, throwable); - } - } - - private void endConnectionSpan(Context context, Channel channel, Throwable throwable) { - if (channel != null) { - Span span = Span.fromContext(context); - span.setAttribute( - SemanticAttributes.NET_TRANSPORT, channel instanceof DatagramChannel ? IP_UDP : IP_TCP); - NetPeerAttributes.INSTANCE.setNetPeer(span, (InetSocketAddress) channel.remoteAddress()); - } - if (throwable != null) { - endExceptionally(context, throwable); - } else { - end(context); - } - } - - private void connectionFailure( - Context parentContext, SocketAddress remoteAddress, Channel channel, Throwable throwable) { - SpanBuilder spanBuilder = spanBuilder(parentContext, "CONNECT", CLIENT); - if (channel != null) { - spanBuilder.setAttribute( - SemanticAttributes.NET_TRANSPORT, channel instanceof DatagramChannel ? IP_UDP : IP_TCP); - NetPeerAttributes.INSTANCE.setNetPeer( - spanBuilder, (InetSocketAddress) channel.remoteAddress()); - } else if (remoteAddress != null) { - NetPeerAttributes.INSTANCE.setNetPeer(spanBuilder, (InetSocketAddress) remoteAddress); - } - - Context context = withClientSpan(parentContext, spanBuilder.startSpan()); - endExceptionally(context, throwable); - } - - @Override - protected String getInstrumentationName() { - return "io.opentelemetry.reactor-netty-1.0"; - } -} diff --git a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/TransportConnectorInstrumentation.java b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/TransportConnectorInstrumentation.java index 139812c80c..072e85db2e 100644 --- a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/TransportConnectorInstrumentation.java +++ b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/TransportConnectorInstrumentation.java @@ -5,7 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0; -import static io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0.ReactorNettyTracer.tracer; +import static io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0.ReactorNettySingletons.connectInstrumenter; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -15,6 +15,7 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; +import io.opentelemetry.javaagent.instrumentation.netty.common.client.NettyConnectRequest; import java.net.SocketAddress; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -40,31 +41,35 @@ public class TransportConnectorInstrumentation implements TypeInstrumentation { public static void startConnect( @Advice.Argument(1) SocketAddress remoteAddress, @Advice.Local("otelContext") Context context, - @Advice.Local("otelParentContext") Context parentContext, + @Advice.Local("otelRequest") NettyConnectRequest request, @Advice.Local("otelScope") Scope scope) { - parentContext = Java8BytecodeBridge.currentContext(); - context = tracer().startConnectionSpan(parentContext, remoteAddress); - if (context != null) { - scope = context.makeCurrent(); + + Context parentContext = Java8BytecodeBridge.currentContext(); + request = NettyConnectRequest.create(remoteAddress); + if (!connectInstrumenter().shouldStart(parentContext, request)) { + return; } + + context = connectInstrumenter().start(parentContext, request); + scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void endConnect( @Advice.Thrown Throwable throwable, - @Advice.Argument(1) SocketAddress remoteAddress, @Advice.Return(readOnly = false) Mono mono, @Advice.Local("otelContext") Context context, - @Advice.Local("otelParentContext") Context parentContext, + @Advice.Local("otelRequest") NettyConnectRequest request, @Advice.Local("otelScope") Scope scope) { + if (scope != null) { scope.close(); } if (throwable != null) { - tracer().endConnectionSpan(context, parentContext, remoteAddress, null, throwable); + connectInstrumenter().end(context, request, null, throwable); } else { - mono = ConnectionWrapper.wrap(context, parentContext, remoteAddress, mono); + mono = ConnectionWrapper.wrap(context, request, mono); } } } diff --git a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyConnectionSpanTest.groovy b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyConnectionSpanTest.groovy index 1328307b50..4e1edba4e6 100644 --- a/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyConnectionSpanTest.groovy +++ b/instrumentation/reactor-netty/reactor-netty-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyConnectionSpanTest.groovy @@ -115,6 +115,7 @@ class ReactorNettyConnectionSpanTest extends InstrumentationSpecification implem status ERROR errorEvent(connectException.class, connectException.message) attributes { + "${SemanticAttributes.NET_TRANSPORT.key}" IP_TCP "${SemanticAttributes.NET_PEER_NAME.key}" "localhost" "${SemanticAttributes.NET_PEER_PORT.key}" PortUtils.UNUSABLE_PORT "${SemanticAttributes.NET_PEER_IP.key}" { it == null || it == "127.0.0.1" }