From 2141354a5a85bc3cb7bbd17dcc8622f9990d1847 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 19 Jan 2024 13:21:06 +0200 Subject: [PATCH 1/5] Set route only on the SERVER span --- .../instrumentation/api/semconv/http/HttpServerRoute.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java index a252b6b22af1..80724b06b4c6 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java @@ -10,8 +10,8 @@ import io.opentelemetry.instrumentation.api.instrumenter.ContextCustomizer; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; -import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.instrumentation.api.internal.HttpRouteState; +import io.opentelemetry.instrumentation.api.internal.SpanKey; import javax.annotation.Nullable; /** @@ -97,7 +97,7 @@ public static void update( HttpServerRouteBiGetter httpRouteGetter, T arg1, U arg2) { - Span serverSpan = LocalRootSpan.fromContextOrNull(context); + Span serverSpan = SpanKey.HTTP_SERVER.fromContextOrNull(context); // even if the server span is not sampled, we have to continue - we need to compute the // http.route properly so that it can be captured by the server metrics if (serverSpan == null) { From 483c3c17652e9c23bd279f466503a02712b3ccb5 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 19 Jan 2024 15:21:04 +0200 Subject: [PATCH 2/5] use separate key to track the server span --- .../api/instrumenter/Instrumenter.java | 4 +++ .../api/internal/LocalRootServerSpan.java | 36 +++++++++++++++++++ .../api/semconv/http/HttpServerRoute.java | 4 +-- .../api/semconv/http/HttpServerRouteTest.java | 24 ++++++++++++- 4 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/LocalRootServerSpan.java diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java index 97f523bea61f..7e84b39083eb 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java @@ -13,6 +13,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.internal.InstrumenterAccess; import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; +import io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan; import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics; import java.time.Instant; import java.util.ArrayList; @@ -203,6 +204,9 @@ private Context doStart(Context parentContext, REQUEST request, @Nullable Instan if (localRoot) { context = LocalRootSpan.store(context, span); + if (spanKind == SpanKind.SERVER) { + context = LocalRootServerSpan.store(context, span); + } } return spanSuppressor.storeInContext(context, spanKind, span); diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/LocalRootServerSpan.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/LocalRootServerSpan.java new file mode 100644 index 000000000000..540b35dc7107 --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/LocalRootServerSpan.java @@ -0,0 +1,36 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.internal; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; +import javax.annotation.Nullable; + +/** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ +public final class LocalRootServerSpan { + + private static final ContextKey KEY = + ContextKey.named("opentelemetry-traces-local-root-server-span"); + + /** + * Returns the local root server span from the given context or {@code null} if there is no local + * root server span in the context. + */ + @Nullable + public static Span fromContextOrNull(Context context) { + return context.get(KEY); + } + + public static Context store(Context context, Span span) { + return context.with(KEY, span); + } + + private LocalRootServerSpan() {} +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java index 80724b06b4c6..63c6d50f7fe4 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java @@ -11,7 +11,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; import io.opentelemetry.instrumentation.api.internal.HttpRouteState; -import io.opentelemetry.instrumentation.api.internal.SpanKey; +import io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan; import javax.annotation.Nullable; /** @@ -97,7 +97,7 @@ public static void update( HttpServerRouteBiGetter httpRouteGetter, T arg1, U arg2) { - Span serverSpan = SpanKey.HTTP_SERVER.fromContextOrNull(context); + Span serverSpan = LocalRootServerSpan.fromContextOrNull(context); // even if the server span is not sampled, we have to continue - we need to compute the // http.route properly so that it can be captured by the server metrics if (serverSpan == null) { diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRouteTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRouteTest.java index e2d36862f958..c57261c83ab7 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRouteTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRouteTest.java @@ -12,6 +12,7 @@ import static org.mockito.Mockito.when; import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension; @@ -39,7 +40,7 @@ void setUp() { HttpServerRoute.builder(getter) .setKnownMethods(new HashSet<>(singletonList("GET"))) .build()) - .buildInstrumenter(); + .buildInstrumenter(s -> SpanKind.SERVER); } @Test @@ -61,6 +62,27 @@ void noLocalRootSpan() { span -> assertThat(span).hasName("parent"), span -> assertThat(span).hasName("test")); } + @Test + void nonServerRootSpan() { + Instrumenter testInstrumenter = + Instrumenter.builder(testing.getOpenTelemetry(), "test", s -> s) + .addContextCustomizer( + HttpServerRoute.builder(getter) + .setKnownMethods(new HashSet<>(singletonList("GET"))) + .build()) + .buildInstrumenter(s -> SpanKind.INTERNAL); + + Context context = testInstrumenter.start(Context.root(), "test"); + assertNull(HttpServerRoute.get(context)); + + HttpServerRoute.update(context, HttpServerRouteSource.SERVER, "/get/:id"); + + testInstrumenter.end(context, "test", null, null); + + assertNull(HttpServerRoute.get(context)); + assertThat(testing.getSpans()).satisfiesExactly(span -> assertThat(span).hasName("test")); + } + @Test void shouldSetRoute() { when(getter.getHttpRequestMethod("test")).thenReturn("GET"); From a2a42fceef8701db653c760412afe9d8b392d86c Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 19 Jan 2024 16:04:30 +0200 Subject: [PATCH 3/5] add bridging --- .../context/InstrumentationApiContextBridging.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java index 2c1e36124395..e9f2ac18f057 100644 --- a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java +++ b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java @@ -31,6 +31,17 @@ final class InstrumentationApiContextBridging { // no instrumentation-api on classpath } + try { + bridges.add( + new ContextKeyBridge( + "application.io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan", + "io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan", + Bridging::toApplication, + Bridging::toAgentOrNull)); + } catch (Throwable e) { + // no instrumentation-api on classpath + } + try { // old SERVER_KEY bridge - needed to make legacy ServerSpan work, for users who're using old // instrumentation-api version with the newest agent version From cd96083682e400cd1fbc28f1924d635e75c81404 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 25 Jan 2024 16:55:23 +0200 Subject: [PATCH 4/5] keep server span in http route state --- .../api/instrumenter/Instrumenter.java | 4 +- .../api/internal/HttpRouteState.java | 26 ++++++- .../api/internal/LocalRootServerSpan.java | 36 --------- .../api/semconv/http/HttpServerRoute.java | 12 +-- .../InstrumentationApiContextBridging.java | 78 ++++++++++++++----- .../HttpRouteStateInstrumentation.java | 20 +++++ 6 files changed, 109 insertions(+), 67 deletions(-) delete mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/LocalRootServerSpan.java diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java index 7e84b39083eb..0d4ed4eedda4 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java @@ -11,9 +11,9 @@ import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.internal.HttpRouteState; import io.opentelemetry.instrumentation.api.internal.InstrumenterAccess; import io.opentelemetry.instrumentation.api.internal.InstrumenterUtil; -import io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan; import io.opentelemetry.instrumentation.api.internal.SupportabilityMetrics; import java.time.Instant; import java.util.ArrayList; @@ -205,7 +205,7 @@ private Context doStart(Context parentContext, REQUEST request, @Nullable Instan if (localRoot) { context = LocalRootSpan.store(context, span); if (spanKind == SpanKind.SERVER) { - context = LocalRootServerSpan.store(context, span); + HttpRouteState.updateSpan(context, span); } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/HttpRouteState.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/HttpRouteState.java index 0eda2755da92..afd023467c16 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/HttpRouteState.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/HttpRouteState.java @@ -5,6 +5,7 @@ package io.opentelemetry.instrumentation.api.internal; +import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.context.ContextKey; import io.opentelemetry.context.ImplicitContextKeyed; @@ -24,20 +25,36 @@ public static HttpRouteState fromContextOrNull(Context context) { return context.get(KEY); } + public static void updateSpan(Context context, Span span) { + HttpRouteState state = fromContextOrNull(context); + if (state != null) { + state.span = span; + } + } + + // this method is used reflectively from InstrumentationApiContextBridging public static HttpRouteState create( @Nullable String method, @Nullable String route, int updatedBySourceOrder) { - return new HttpRouteState(method, route, updatedBySourceOrder); + return create(method, route, updatedBySourceOrder, null); + } + + // this method is used reflectively from InstrumentationApiContextBridging + public static HttpRouteState create( + @Nullable String method, @Nullable String route, int updatedBySourceOrder, Span span) { + return new HttpRouteState(method, route, updatedBySourceOrder, span); } @Nullable private final String method; @Nullable private volatile String route; private volatile int updatedBySourceOrder; + @Nullable private volatile Span span; private HttpRouteState( - @Nullable String method, @Nullable String route, int updatedBySourceOrder) { + @Nullable String method, @Nullable String route, int updatedBySourceOrder, Span span) { this.method = method; this.updatedBySourceOrder = updatedBySourceOrder; this.route = route; + this.span = span; } @Override @@ -59,6 +76,11 @@ public String getRoute() { return route; } + @Nullable + public Span getSpan() { + return span; + } + public void update( @SuppressWarnings("unused") Context context, // context is used by the javaagent bridge instrumentation diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/LocalRootServerSpan.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/LocalRootServerSpan.java deleted file mode 100644 index 540b35dc7107..000000000000 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/LocalRootServerSpan.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.api.internal; - -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.context.Context; -import io.opentelemetry.context.ContextKey; -import javax.annotation.Nullable; - -/** - * This class is internal and is hence not for public use. Its APIs are unstable and can change at - * any time. - */ -public final class LocalRootServerSpan { - - private static final ContextKey KEY = - ContextKey.named("opentelemetry-traces-local-root-server-span"); - - /** - * Returns the local root server span from the given context or {@code null} if there is no local - * root server span in the context. - */ - @Nullable - public static Span fromContextOrNull(Context context) { - return context.get(KEY); - } - - public static Context store(Context context, Span span) { - return context.with(KEY, span); - } - - private LocalRootServerSpan() {} -} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java index 63c6d50f7fe4..69172ca72fb9 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerRoute.java @@ -11,7 +11,6 @@ import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; import io.opentelemetry.instrumentation.api.internal.HttpRouteState; -import io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan; import javax.annotation.Nullable; /** @@ -97,16 +96,17 @@ public static void update( HttpServerRouteBiGetter httpRouteGetter, T arg1, U arg2) { - Span serverSpan = LocalRootServerSpan.fromContextOrNull(context); + HttpRouteState httpRouteState = HttpRouteState.fromContextOrNull(context); + if (httpRouteState == null) { + return; + } + Span serverSpan = httpRouteState.getSpan(); // even if the server span is not sampled, we have to continue - we need to compute the // http.route properly so that it can be captured by the server metrics if (serverSpan == null) { return; } - HttpRouteState httpRouteState = HttpRouteState.fromContextOrNull(context); - if (httpRouteState == null) { - return; - } + // special case for servlet filters, even when we have a route from previous filter see whether // the new route is better and if so use it instead boolean onlyIfBetterRoute = diff --git a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java index e9f2ac18f057..c0b9eca4df38 100644 --- a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java +++ b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java @@ -31,17 +31,6 @@ final class InstrumentationApiContextBridging { // no instrumentation-api on classpath } - try { - bridges.add( - new ContextKeyBridge( - "application.io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan", - "io.opentelemetry.instrumentation.api.internal.LocalRootServerSpan", - Bridging::toApplication, - Bridging::toAgentOrNull)); - } catch (Throwable e) { - // no instrumentation-api on classpath - } - try { // old SERVER_KEY bridge - needed to make legacy ServerSpan work, for users who're using old // instrumentation-api version with the newest agent version @@ -70,12 +59,14 @@ final class InstrumentationApiContextBridging { private static final MethodHandle AGENT_GET_METHOD; private static final MethodHandle AGENT_GET_ROUTE; private static final MethodHandle AGENT_GET_UPDATED_BY_SOURCE_ORDER; + private static final MethodHandle AGENT_GET_SPAN; private static final Class APPLICATION_HTTP_ROUTE_STATE; private static final MethodHandle APPLICATION_CREATE; private static final MethodHandle APPLICATION_GET_METHOD; private static final MethodHandle APPLICATION_GET_ROUTE; private static final MethodHandle APPLICATION_GET_UPDATED_BY_SOURCE_ORDER; + private static final MethodHandle APPLICATION_GET_SPAN; static { MethodHandles.Lookup lookup = MethodHandles.lookup(); @@ -85,11 +76,13 @@ final class InstrumentationApiContextBridging { MethodHandle agentGetMethod = null; MethodHandle agentGetRoute = null; MethodHandle agentGetUpdatedBySourceOrder = null; + MethodHandle agentGetSpan = null; Class applicationHttpRouteState = null; MethodHandle applicationCreate = null; MethodHandle applicationGetMethod = null; MethodHandle applicationGetRoute = null; MethodHandle applicationGetUpdatedBySourceOrder = null; + MethodHandle applicationGetSpan = null; try { agentHttpRouteState = @@ -98,7 +91,12 @@ final class InstrumentationApiContextBridging { lookup.findStatic( agentHttpRouteState, "create", - MethodType.methodType(agentHttpRouteState, String.class, String.class, int.class)); + MethodType.methodType( + agentHttpRouteState, + String.class, + String.class, + int.class, + io.opentelemetry.api.trace.Span.class)); agentGetMethod = lookup.findVirtual(agentHttpRouteState, "getMethod", MethodType.methodType(String.class)); agentGetRoute = @@ -106,15 +104,34 @@ final class InstrumentationApiContextBridging { agentGetUpdatedBySourceOrder = lookup.findVirtual( agentHttpRouteState, "getUpdatedBySourceOrder", MethodType.methodType(int.class)); + agentGetSpan = + lookup.findVirtual( + agentHttpRouteState, + "getSpan", + MethodType.methodType(io.opentelemetry.api.trace.Span.class)); applicationHttpRouteState = Class.forName("application.io.opentelemetry.instrumentation.api.internal.HttpRouteState"); - applicationCreate = - lookup.findStatic( - applicationHttpRouteState, - "create", - MethodType.methodType( - applicationHttpRouteState, String.class, String.class, int.class)); + try { + applicationCreate = + lookup.findStatic( + applicationHttpRouteState, + "create", + MethodType.methodType( + applicationHttpRouteState, + String.class, + String.class, + int.class, + io.opentelemetry.api.trace.Span.class)); + } catch (NoSuchMethodException exception) { + // older instrumentation-api has only the variant that does not take span + applicationCreate = + lookup.findStatic( + applicationHttpRouteState, + "create", + MethodType.methodType( + applicationHttpRouteState, String.class, String.class, int.class)); + } applicationGetMethod = lookup.findVirtual( applicationHttpRouteState, "getMethod", MethodType.methodType(String.class)); @@ -126,6 +143,13 @@ final class InstrumentationApiContextBridging { applicationHttpRouteState, "getUpdatedBySourceOrder", MethodType.methodType(int.class)); + try { + applicationGetSpan = + lookup.findVirtual( + applicationHttpRouteState, "getSpan", MethodType.methodType(Span.class)); + } catch (NoSuchMethodException ignored) { + // not present in older instrumentation-api + } } catch (Throwable ignored) { // instrumentation-api may be absent on the classpath, or it might be an older version } @@ -135,11 +159,13 @@ final class InstrumentationApiContextBridging { AGENT_GET_METHOD = agentGetMethod; AGENT_GET_ROUTE = agentGetRoute; AGENT_GET_UPDATED_BY_SOURCE_ORDER = agentGetUpdatedBySourceOrder; + AGENT_GET_SPAN = agentGetSpan; APPLICATION_HTTP_ROUTE_STATE = applicationHttpRouteState; APPLICATION_CREATE = applicationCreate; APPLICATION_GET_METHOD = applicationGetMethod; APPLICATION_GET_ROUTE = applicationGetRoute; APPLICATION_GET_UPDATED_BY_SOURCE_ORDER = applicationGetUpdatedBySourceOrder; + APPLICATION_GET_SPAN = applicationGetSpan; } @Nullable @@ -162,12 +188,16 @@ final class InstrumentationApiContextBridging { APPLICATION_CREATE, AGENT_GET_METHOD, AGENT_GET_ROUTE, - AGENT_GET_UPDATED_BY_SOURCE_ORDER), + AGENT_GET_UPDATED_BY_SOURCE_ORDER, + AGENT_GET_SPAN, + o -> o != null ? Bridging.toApplication((io.opentelemetry.api.trace.Span) o) : null), httpRouteStateConvert( AGENT_CREATE, APPLICATION_GET_METHOD, APPLICATION_GET_ROUTE, - APPLICATION_GET_UPDATED_BY_SOURCE_ORDER)); + APPLICATION_GET_UPDATED_BY_SOURCE_ORDER, + APPLICATION_GET_SPAN, + o -> o != null ? Bridging.toAgentOrNull((Span) o) : null)); } catch (Throwable ignored) { return null; } @@ -177,12 +207,18 @@ private static Function httpRouteStateConvert( MethodHandle create, MethodHandle getMethod, MethodHandle getRoute, - MethodHandle getUpdatedBySourceOrder) { + MethodHandle getUpdatedBySourceOrder, + MethodHandle getSpan, + Function convertSpan) { return httpRouteHolder -> { try { String method = (String) getMethod.invoke(httpRouteHolder); String route = (String) getRoute.invoke(httpRouteHolder); int updatedBySourceOrder = (int) getUpdatedBySourceOrder.invoke(httpRouteHolder); + if (create.type().parameterCount() == 4 && getSpan != null) { + Object span = convertSpan.apply(getSpan.invoke(httpRouteHolder)); + return create.invoke(method, route, updatedBySourceOrder, span); + } return create.invoke(method, route, updatedBySourceOrder); } catch (Throwable e) { return null; diff --git a/instrumentation/opentelemetry-instrumentation-api/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/instrumentationapi/HttpRouteStateInstrumentation.java b/instrumentation/opentelemetry-instrumentation-api/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/instrumentationapi/HttpRouteStateInstrumentation.java index e1438dc085ff..52f93afba7c5 100644 --- a/instrumentation/opentelemetry-instrumentation-api/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/instrumentationapi/HttpRouteStateInstrumentation.java +++ b/instrumentation/opentelemetry-instrumentation-api/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/instrumentationapi/HttpRouteStateInstrumentation.java @@ -8,10 +8,12 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import application.io.opentelemetry.api.trace.Span; import application.io.opentelemetry.context.Context; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.opentelemetryapi.context.AgentContextStorage; +import io.opentelemetry.javaagent.instrumentation.opentelemetryapi.trace.Bridging; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -31,6 +33,11 @@ public void transform(TypeTransformer transformer) { .and(takesArgument(1, int.class)) .and(takesArgument(2, String.class)), this.getClass().getName() + "$UpdateAdvice"); + transformer.applyAdviceToMethod( + named("updateSpan") + .and(takesArgument(0, named("application.io.opentelemetry.context.Context"))) + .and(takesArgument(1, named("application.io.opentelemetry.api.trace.Span"))), + this.getClass().getName() + "$UpdateSpanAdvice"); } @SuppressWarnings("unused") @@ -54,4 +61,17 @@ public static void onEnter( agentRouteState.update(agentContext, updatedBySourceOrder, route); } } + + @SuppressWarnings("unused") + public static class UpdateSpanAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.Argument(0) Context applicationContext, @Advice.Argument(1) Span applicationSpan) { + + io.opentelemetry.context.Context agentContext = + AgentContextStorage.getAgentContext(applicationContext); + io.opentelemetry.instrumentation.api.internal.HttpRouteState.updateSpan( + agentContext, Bridging.toAgentOrNull(applicationSpan)); + } + } } From 167417ecd60f6a9be2f04e7a30548209f56323b2 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 25 Jan 2024 23:30:40 +0200 Subject: [PATCH 5/5] use the correct span type --- .../context/InstrumentationApiContextBridging.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java index c0b9eca4df38..eed3c4d281df 100644 --- a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java +++ b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/InstrumentationApiContextBridging.java @@ -118,11 +118,7 @@ final class InstrumentationApiContextBridging { applicationHttpRouteState, "create", MethodType.methodType( - applicationHttpRouteState, - String.class, - String.class, - int.class, - io.opentelemetry.api.trace.Span.class)); + applicationHttpRouteState, String.class, String.class, int.class, Span.class)); } catch (NoSuchMethodException exception) { // older instrumentation-api has only the variant that does not take span applicationCreate =