From 2c4acd7c813def893358ce18d9ef083809f49bd3 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Thu, 21 Apr 2022 11:29:10 +0200 Subject: [PATCH] Introduce LocalRootSpan (replacing ServerSpan) (#5896) * Introduce LocalRootSpan (replacing ServerSpan) * fix tests * bridge old ServerSpan class --- .../instrumenter/http/HttpRouteHolder.java | 4 +- .../api/server/ServerSpan.java | 13 ++- .../http/HttpRouteHolderTest.java | 29 +++---- .../api/instrumenter/Instrumenter.java | 4 + .../api/instrumenter/LocalRootSpan.java | 57 +++++++++++++ .../instrumenter/SpanSuppressionStrategy.java | 10 +-- .../api/instrumenter/InstrumenterTest.java | 2 + .../api/instrumenter/LocalRootSpanTest.java | 85 +++++++++++++++++++ .../SpanSuppressionStrategyTest.java | 15 +--- .../jaxrs/v2_0/RequestContextHelper.java | 4 +- .../axis2/Axis2ServerSpanNaming.java | 4 +- .../cxf/CxfServerSpanNaming.java | 4 +- .../metro/MetroServerSpanNaming.java | 4 +- .../jsf/JsfServerSpanNaming.java | 4 +- .../javaagent/build.gradle.kts | 32 +++++++ .../context/AgentContextStorage.java | 37 +++++++- .../src/test/groovy/ContextBridgeTest.groovy | 8 +- .../LegacyServerSpanContextBridgeTest.java | 38 +++++++++ .../play/v2_4/Play24Singletons.java | 4 +- .../play/v2_6/Play26Singletons.java | 4 +- .../servlet/BaseServletHelper.java | 4 +- .../vertx/RoutingContextHandlerWrapper.java | 4 +- ...DefaultExceptionMapperInstrumentation.java | 4 +- ...RequestHandlerExecutorInstrumentation.java | 10 +-- 24 files changed, 299 insertions(+), 85 deletions(-) create mode 100644 instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/LocalRootSpan.java create mode 100644 instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/LocalRootSpanTest.java create mode 100644 instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/testOldInstrumentationApi/java/LegacyServerSpanContextBridgeTest.java diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java index 9b25dcddf4c2..70eee7f5e4f9 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolder.java @@ -10,7 +10,7 @@ import io.opentelemetry.context.ContextKey; import io.opentelemetry.instrumentation.api.instrumenter.ContextCustomizer; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.server.ServerSpan; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import javax.annotation.Nullable; @@ -101,7 +101,7 @@ public static void updateHttpRoute( HttpRouteBiGetter httpRouteGetter, T arg1, U arg2) { - Span serverSpan = ServerSpan.fromContextOrNull(context); + Span serverSpan = LocalRootSpan.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-semconv/src/main/java/io/opentelemetry/instrumentation/api/server/ServerSpan.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/server/ServerSpan.java index 7b3841fd8656..67c541853bfa 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/server/ServerSpan.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/server/ServerSpan.java @@ -8,13 +8,16 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.internal.SpanKey; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import javax.annotation.Nullable; /** * This class encapsulates the context key for storing the current {@link SpanKind#SERVER} span in * the {@link Context}. + * + * @deprecated Use {@link LocalRootSpan} instead. */ +@Deprecated public final class ServerSpan { /** @@ -23,13 +26,7 @@ public final class ServerSpan { */ @Nullable public static Span fromContextOrNull(Context context) { - // try the HTTP semconv span first - Span span = SpanKey.HTTP_SERVER.fromContextOrNull(context); - // if it's absent then try the SERVER one - perhaps suppression by span kind is enabled - if (span == null) { - span = SpanKey.KIND_SERVER.fromContextOrNull(context); - } - return span; + return LocalRootSpan.fromContextOrNull(context); } private ServerSpan() {} diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolderTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolderTest.java index 94227252faae..1f8fc4944ffe 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolderTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpRouteHolderTest.java @@ -8,31 +8,24 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanContext; -import io.opentelemetry.api.trace.TraceFlags; -import io.opentelemetry.api.trace.TraceState; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.internal.SpanKey; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; class HttpRouteHolderTest { + @RegisterExtension final OpenTelemetryExtension testing = OpenTelemetryExtension.create(); + @Test void shouldSetRouteEvenIfSpanIsNotSampled() { - Span span = - Span.wrap( - SpanContext.create( - "00000000000000000000000000000042", - "0000000000000012", - TraceFlags.getDefault(), - TraceState.getDefault())); - - Context context = Context.root(); - context = context.with(span); - context = SpanKey.HTTP_SERVER.storeInContext(context, span); - context = HttpRouteHolder.get().start(context, null, Attributes.empty()); + Instrumenter instrumenter = + Instrumenter.builder(testing.getOpenTelemetry(), "test", s -> s) + .addContextCustomizer(HttpRouteHolder.get()) + .newInstrumenter(); + + Context context = instrumenter.start(Context.root(), "test"); assertNull(HttpRouteHolder.getRoute(context)); 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 57d7bf5dced5..89e25aff3663 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 @@ -162,6 +162,10 @@ public Context start(Context parentContext, REQUEST request) { } } + if (LocalRootSpan.isLocalRoot(parentContext)) { + context = LocalRootSpan.store(context, span); + } + return spanSuppressor.storeInContext(context, spanKind, span); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/LocalRootSpan.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/LocalRootSpan.java new file mode 100644 index 000000000000..3afafdb35f74 --- /dev/null +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/LocalRootSpan.java @@ -0,0 +1,57 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.ContextKey; +import javax.annotation.Nullable; + +/** + * A local root span is a span that either does not have a parent span (it is the root span of a + * trace), or its parent span is a remote span (context was propagated from another application). + */ +public final class LocalRootSpan { + + private static final ContextKey KEY = + ContextKey.named("opentelemetry-traces-local-root-span"); + + /** + * Returns the local root span from the current context or {@linkplain Span#getInvalid() invalid + * span} if there is no local root span. + */ + public static Span current() { + return fromContext(Context.current()); + } + + /** + * Returns the local root span from the given context or {@linkplain Span#getInvalid() invalid + * span} if there is no local root span in the context. + */ + public static Span fromContext(Context context) { + Span span = fromContextOrNull(context); + return span == null ? Span.getInvalid() : span; + } + + /** + * Returns the local root span from the given context or {@code null} if there is no local root + * span in the context. + */ + @Nullable + public static Span fromContextOrNull(Context context) { + return context.get(KEY); + } + + static boolean isLocalRoot(Context parentContext) { + SpanContext spanContext = Span.fromContext(parentContext).getSpanContext(); + return !spanContext.isValid() || spanContext.isRemote(); + } + + static Context store(Context context, Span span) { + return context.with(KEY, span); + } +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategy.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategy.java index ff4c37a7ebf4..5b48edb40694 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategy.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategy.java @@ -11,7 +11,6 @@ import io.opentelemetry.instrumentation.api.config.Config; import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.BySpanKey; import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.DelegateBySpanKind; -import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.JustStoreServer; import io.opentelemetry.instrumentation.api.instrumenter.SpanSuppressor.Noop; import io.opentelemetry.instrumentation.api.internal.SpanKey; import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider; @@ -21,16 +20,11 @@ import java.util.Set; enum SpanSuppressionStrategy { - /** - * Do not suppress spans at all. - * - *

This strategy will mark spans of {@link SpanKind#SERVER SERVER} kind in the context, so that - * they can be accessed using {@code ServerSpan}, but will not suppress server spans. - */ + /** Do not suppress spans at all. */ NONE { @Override SpanSuppressor create(Set spanKeys) { - return JustStoreServer.INSTANCE; + return Noop.INSTANCE; } }, /** diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java index f7f3ada0ca2f..985b7fdaa3fc 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java @@ -250,6 +250,7 @@ void server_parent() { Context context = instrumenter.start(Context.root(), request); assertThat(Span.fromContext(context).getSpanContext().isValid()).isTrue(); + assertThat(LocalRootSpan.fromContext(context)).isSameAs(Span.fromContext(context)); instrumenter.end(context, request, RESPONSE, null); @@ -358,6 +359,7 @@ void client_parent() { assertThat(spanContext.isValid()).isTrue(); assertThat(request).containsKey("traceparent"); + assertThat(LocalRootSpan.fromContextOrNull(context)).isNull(); instrumenter.end(context, request, RESPONSE, new IllegalStateException("test")); diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/LocalRootSpanTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/LocalRootSpanTest.java new file mode 100644 index 000000000000..2c51e510b182 --- /dev/null +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/LocalRootSpanTest.java @@ -0,0 +1,85 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.api.trace.TraceFlags; +import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import org.junit.jupiter.api.Test; + +class LocalRootSpanTest { + + @Test + void spanWithoutParentIsLocalRoot() { + Context parentContext = Context.root(); + assertTrue(LocalRootSpan.isLocalRoot(parentContext)); + } + + @Test + void spanWithInvalidParentIsLocalRoot() { + Context parentContext = Context.root().with(Span.getInvalid()); + assertTrue(LocalRootSpan.isLocalRoot(parentContext)); + } + + @Test + void spanWithRemoteParentIsLocalRoot() { + Context parentContext = + Context.root() + .with( + Span.wrap( + SpanContext.createFromRemoteParent( + "00000000000000000000000000000001", + "0000000000000002", + TraceFlags.getSampled(), + TraceState.getDefault()))); + assertTrue(LocalRootSpan.isLocalRoot(parentContext)); + } + + @Test + void spanWithValidLocalParentIsNotLocalRoot() { + Context parentContext = + Context.root() + .with( + Span.wrap( + SpanContext.create( + "00000000000000000000000000000001", + "0000000000000002", + TraceFlags.getSampled(), + TraceState.getDefault()))); + assertFalse(LocalRootSpan.isLocalRoot(parentContext)); + } + + @Test + void shouldGetLocalRootSpan() { + Span span = Span.getInvalid(); + Context context = LocalRootSpan.store(Context.root(), span); + + try (Scope ignored = context.makeCurrent()) { + assertSame(span, LocalRootSpan.current()); + } + assertSame(span, LocalRootSpan.fromContext(context)); + assertSame(span, LocalRootSpan.fromContextOrNull(context)); + } + + @Test + void shouldNotGetLocalRootSpanIfThereIsNone() { + Context context = Context.root(); + + try (Scope ignored = context.makeCurrent()) { + assertFalse(LocalRootSpan.current().getSpanContext().isValid()); + } + assertFalse(LocalRootSpan.fromContext(context).getSpanContext().isValid()); + assertNull(LocalRootSpan.fromContextOrNull(context)); + } +} diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategyTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategyTest.java index 2e2bbfe60f93..c96aaf508713 100644 --- a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategyTest.java +++ b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/SpanSuppressionStrategyTest.java @@ -69,20 +69,9 @@ void none_shouldNotSuppressAnything(SpanKind spanKind, SpanKey spanKey) { assertFalse(suppressor.shouldSuppress(context, spanKind)); } - @Test - void none_shouldStoreServerSpanInContext() { - SpanSuppressor suppressor = SpanSuppressionStrategy.NONE.create(emptySet()); - Context context = Context.root(); - - Context newContext = suppressor.storeInContext(context, SpanKind.SERVER, span); - - assertNotSame(newContext, context); - assertSame(span, SpanKey.KIND_SERVER.fromContextOrNull(newContext)); - } - @ParameterizedTest - @EnumSource(value = SpanKind.class, mode = EnumSource.Mode.EXCLUDE, names = "SERVER") - void none_shouldNotStoreNonServerSpansInContext(SpanKind spanKind) { + @EnumSource(value = SpanKind.class) + void none_shouldNotStoreSpansInContext(SpanKind spanKind) { SpanSuppressor suppressor = SpanSuppressionStrategy.NONE.create(emptySet()); Context context = Context.root(); diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/RequestContextHelper.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/RequestContextHelper.java index e8021a3c9f35..c803b56a0662 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/RequestContextHelper.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/RequestContextHelper.java @@ -9,9 +9,9 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource; -import io.opentelemetry.instrumentation.api.server.ServerSpan; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import javax.ws.rs.container.ContainerRequestContext; @@ -25,7 +25,7 @@ public static Context createOrUpdateAbortSpan( requestContext.setProperty(JaxrsSingletons.ABORT_HANDLED, true); Context parentContext = Java8BytecodeBridge.currentContext(); - Span serverSpan = ServerSpan.fromContextOrNull(parentContext); + Span serverSpan = LocalRootSpan.fromContextOrNull(parentContext); Span currentSpan = Java8BytecodeBridge.spanFromContext(parentContext); HttpRouteHolder.updateHttpRoute( diff --git a/instrumentation/jaxws/jaxws-2.0-axis2-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/axis2/Axis2ServerSpanNaming.java b/instrumentation/jaxws/jaxws-2.0-axis2-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/axis2/Axis2ServerSpanNaming.java index 077946965087..428413643adc 100644 --- a/instrumentation/jaxws/jaxws-2.0-axis2-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/axis2/Axis2ServerSpanNaming.java +++ b/instrumentation/jaxws/jaxws-2.0-axis2-1.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/axis2/Axis2ServerSpanNaming.java @@ -7,7 +7,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.server.ServerSpan; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath; import javax.servlet.http.HttpServletRequest; import org.apache.axis2.jaxws.core.MessageContext; @@ -15,7 +15,7 @@ public final class Axis2ServerSpanNaming { public static void updateServerSpan(Context context, Axis2Request axis2Request) { - Span serverSpan = ServerSpan.fromContextOrNull(context); + Span serverSpan = LocalRootSpan.fromContextOrNull(context); if (serverSpan == null) { return; } diff --git a/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cxf/CxfServerSpanNaming.java b/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cxf/CxfServerSpanNaming.java index e9a4e7a410ff..96c5a74f16d2 100644 --- a/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cxf/CxfServerSpanNaming.java +++ b/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cxf/CxfServerSpanNaming.java @@ -7,7 +7,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.server.ServerSpan; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath; import javax.servlet.http.HttpServletRequest; @@ -19,7 +19,7 @@ public static void updateServerSpanName(Context context, CxfRequest cxfRequest) return; } - Span serverSpan = ServerSpan.fromContextOrNull(context); + Span serverSpan = LocalRootSpan.fromContextOrNull(context); if (serverSpan == null) { return; } diff --git a/instrumentation/jaxws/jaxws-2.0-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroServerSpanNaming.java b/instrumentation/jaxws/jaxws-2.0-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroServerSpanNaming.java index 5dc4d7fa9fdc..b0967efc71c5 100644 --- a/instrumentation/jaxws/jaxws-2.0-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroServerSpanNaming.java +++ b/instrumentation/jaxws/jaxws-2.0-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/MetroServerSpanNaming.java @@ -8,7 +8,7 @@ import com.sun.xml.ws.api.message.Packet; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.server.ServerSpan; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath; import javax.servlet.http.HttpServletRequest; import javax.xml.ws.handler.MessageContext; @@ -21,7 +21,7 @@ public static void updateServerSpanName(Context context, MetroRequest metroReque return; } - Span serverSpan = ServerSpan.fromContextOrNull(context); + Span serverSpan = LocalRootSpan.fromContextOrNull(context); if (serverSpan == null) { return; } diff --git a/instrumentation/jsf/jsf-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/JsfServerSpanNaming.java b/instrumentation/jsf/jsf-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/JsfServerSpanNaming.java index a4f01eb0fccc..e45268cddb6c 100644 --- a/instrumentation/jsf/jsf-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/JsfServerSpanNaming.java +++ b/instrumentation/jsf/jsf-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsf/JsfServerSpanNaming.java @@ -7,7 +7,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.server.ServerSpan; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath; import javax.faces.component.UIViewRoot; import javax.faces.context.FacesContext; @@ -16,7 +16,7 @@ public final class JsfServerSpanNaming { public static void updateViewName(Context context, FacesContext facesContext) { // just update the server span name, without touching the http.route - Span serverSpan = ServerSpan.fromContextOrNull(context); + Span serverSpan = LocalRootSpan.fromContextOrNull(context); if (serverSpan == null) { return; } diff --git a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/build.gradle.kts b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/build.gradle.kts index 824e420244b5..024401a55415 100644 --- a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/build.gradle.kts +++ b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/build.gradle.kts @@ -46,3 +46,35 @@ dependencies { testImplementation(project(":instrumentation:opentelemetry-api:opentelemetry-api-1.0:testing")) testInstrumentation(project(":instrumentation:opentelemetry-api:opentelemetry-api-1.0:testing")) } + +testing { + suites { + val testOldInstrumentationApi by registering(JvmTestSuite::class) { + dependencies { + implementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv") + implementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api") + implementation(project(":instrumentation:opentelemetry-api:opentelemetry-api-1.0:testing")) + } + } + } +} + +configurations.configureEach { + if (name.contains("testOldInstrumentationApi")) { + resolutionStrategy { + dependencySubstitution { + // version 1.13.0 contains the old ServerSpan implementation that uses SERVER_KEY context key + substitute(module("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv")) + .using(module("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:1.13.0-alpha")) + substitute(module("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api")) + .using(module("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api:1.13.0-alpha")) + } + } + } +} + +tasks { + check { + dependsOn(testing.suites) + } +} diff --git a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/AgentContextStorage.java b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/AgentContextStorage.java index 5411d04f72ad..b814f316bcf3 100644 --- a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/AgentContextStorage.java +++ b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/context/AgentContextStorage.java @@ -157,6 +157,20 @@ public static Context newContextWrapper( "io.opentelemetry.api.baggage.BaggageContextKey", BaggageBridging::toApplication, BaggageBridging::toAgent), + new ContextKeyBridge( + "application.io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan", + "io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan", + Bridging::toApplication, + Bridging::toAgentOrNull), + // old SERVER_KEY bridge - needed to make legacy ServerSpan work, for users who're using old + // instrumentation-api version with the newest agent version + new ContextKeyBridge( + "application.io.opentelemetry.instrumentation.api.internal.SpanKey", + "io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan", + "SERVER_KEY", + "KEY", + Bridging::toApplication, + Bridging::toAgentOrNull), // span kind keys bridgeSpanKey("KIND_SERVER_KEY"), bridgeSpanKey("KIND_CLIENT_KEY"), @@ -306,20 +320,37 @@ static class ContextKeyBridge { this(applicationKeyHolderClassName, agentKeyHolderClassName, "KEY", toApplication, toAgent); } - @SuppressWarnings("unchecked") ContextKeyBridge( String applicationKeyHolderClassName, String agentKeyHolderClassName, String fieldName, Function toApplication, Function toAgent) { + this( + applicationKeyHolderClassName, + agentKeyHolderClassName, + fieldName, + fieldName, + toApplication, + toAgent); + } + + @SuppressWarnings("unchecked") + ContextKeyBridge( + String applicationKeyHolderClassName, + String agentKeyHolderClassName, + String applicationFieldName, + String agentFieldName, + Function toApplication, + Function toAgent) { this.toApplication = toApplication; this.toAgent = toAgent; ContextKey applicationContextKey; try { Class applicationKeyHolderClass = Class.forName(applicationKeyHolderClassName); - Field applicationContextKeyField = applicationKeyHolderClass.getDeclaredField(fieldName); + Field applicationContextKeyField = + applicationKeyHolderClass.getDeclaredField(applicationFieldName); applicationContextKeyField.setAccessible(true); applicationContextKey = (ContextKey) applicationContextKeyField.get(null); } catch (Throwable t) { @@ -330,7 +361,7 @@ static class ContextKeyBridge { io.opentelemetry.context.ContextKey agentContextKey; try { Class agentKeyHolderClass = Class.forName(agentKeyHolderClassName); - Field agentContextKeyField = agentKeyHolderClass.getDeclaredField(fieldName); + Field agentContextKeyField = agentKeyHolderClass.getDeclaredField(agentFieldName); agentContextKeyField.setAccessible(true); agentContextKey = (io.opentelemetry.context.ContextKey) agentContextKeyField.get(null); diff --git a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/groovy/ContextBridgeTest.groovy b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/groovy/ContextBridgeTest.groovy index 9e5d4ac0a00d..16405f96b430 100644 --- a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/groovy/ContextBridgeTest.groovy +++ b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/test/groovy/ContextBridgeTest.groovy @@ -9,8 +9,8 @@ import io.opentelemetry.api.trace.Span import io.opentelemetry.context.Context import io.opentelemetry.context.ContextKey import io.opentelemetry.extension.annotations.WithSpan +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan import io.opentelemetry.instrumentation.api.internal.SpanKey -import io.opentelemetry.instrumentation.api.server.ServerSpan import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification import java.util.concurrent.CountDownLatch @@ -148,13 +148,13 @@ class ContextBridgeTest extends AgentInstrumentationSpecification { Context.current() == Context.root() } - def "test server span bridge"() { + def "test local root span bridge"() { expect: AgentSpanTesting.runWithServerSpan("server") { assert Span.current() != null - assert ServerSpan.fromContextOrNull(Context.current()) != null + assert LocalRootSpan.fromContextOrNull(Context.current()) != null runWithSpan("internal") { - assert ServerSpan.fromContextOrNull(Context.current()) != null + assert LocalRootSpan.fromContextOrNull(Context.current()) != null } } } diff --git a/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/testOldInstrumentationApi/java/LegacyServerSpanContextBridgeTest.java b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/testOldInstrumentationApi/java/LegacyServerSpanContextBridgeTest.java new file mode 100644 index 000000000000..ecdf942e2e10 --- /dev/null +++ b/instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/testOldInstrumentationApi/java/LegacyServerSpanContextBridgeTest.java @@ -0,0 +1,38 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.server.ServerSpan; +import org.junit.jupiter.api.Test; + +class LegacyServerSpanContextBridgeTest { + + // cannot use AgentInstrumentationExtension because it'd try to initialize Instrumenters with new + // SpanKeys + static final Tracer tracer = GlobalOpenTelemetry.get().getTracer("test"); + + @Test + void shouldBridgeLegacyServerSpanClass() { + AgentSpanTesting.runWithServerSpan( + "server", + () -> { + assertNotNull(Span.current()); + assertNotNull(ServerSpan.fromContextOrNull(Context.current())); + + Span internalSpan = tracer.spanBuilder("internal").startSpan(); + try (Scope ignored = internalSpan.makeCurrent()) { + assertNotNull(ServerSpan.fromContextOrNull(Context.current())); + } finally { + internalSpan.end(); + } + }); + } +} diff --git a/instrumentation/play/play-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_4/Play24Singletons.java b/instrumentation/play/play-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_4/Play24Singletons.java index 0d4e49101869..1be47dff55db 100644 --- a/instrumentation/play/play-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_4/Play24Singletons.java +++ b/instrumentation/play/play-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_4/Play24Singletons.java @@ -9,7 +9,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.server.ServerSpan; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import play.api.mvc.Request; import scala.Option; @@ -33,7 +33,7 @@ public static void updateSpanNames(Context context, Request request) { Span.fromContext(context).updateName(route); // set the span name on the upstream akka/netty span - Span serverSpan = ServerSpan.fromContextOrNull(context); + Span serverSpan = LocalRootSpan.fromContextOrNull(context); if (serverSpan != null) { serverSpan.updateName(route); } diff --git a/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/Play26Singletons.java b/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/Play26Singletons.java index cbdcc5168b69..fb61f77c56f8 100644 --- a/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/Play26Singletons.java +++ b/instrumentation/play/play-2.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_6/Play26Singletons.java @@ -9,7 +9,7 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.server.ServerSpan; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import javax.annotation.Nullable; @@ -60,7 +60,7 @@ public static void updateSpanNames(Context context, Request request) { Span.fromContext(context).updateName(route); // set the span name on the upstream akka/netty span - Span serverSpan = ServerSpan.fromContextOrNull(context); + Span serverSpan = LocalRootSpan.fromContextOrNull(context); if (serverSpan != null) { serverSpan.updateName(route); } diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/BaseServletHelper.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/BaseServletHelper.java index 6e240c025559..2b1f0d87794c 100644 --- a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/BaseServletHelper.java +++ b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/BaseServletHelper.java @@ -12,8 +12,8 @@ import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder; -import io.opentelemetry.instrumentation.api.server.ServerSpan; import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge; import io.opentelemetry.javaagent.bootstrap.servlet.MappingResolver; import io.opentelemetry.javaagent.bootstrap.servlet.ServletContextPath; @@ -103,7 +103,7 @@ public void captureServletAttributes(Context context, REQUEST request) { if (parameterExtractor == null || !AppServerBridge.captureServletAttributes(context)) { return; } - Span serverSpan = ServerSpan.fromContextOrNull(context); + Span serverSpan = LocalRootSpan.fromContextOrNull(context); if (serverSpan == null) { return; } diff --git a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java index 69da38b335ea..cd4175286810 100644 --- a/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java +++ b/instrumentation/vertx/vertx-web-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java @@ -7,9 +7,9 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource; -import io.opentelemetry.instrumentation.api.server.ServerSpan; import io.vertx.core.Handler; import io.vertx.ext.web.RoutingContext; import java.lang.reflect.InvocationTargetException; @@ -35,7 +35,7 @@ public void handle(RoutingContext context) { try { handler.handle(context); } catch (Throwable throwable) { - Span serverSpan = ServerSpan.fromContextOrNull(otelContext); + Span serverSpan = LocalRootSpan.fromContextOrNull(otelContext); if (serverSpan != null) { serverSpan.recordException(unwrapThrowable(throwable)); } diff --git a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/DefaultExceptionMapperInstrumentation.java b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/DefaultExceptionMapperInstrumentation.java index ef6aa2f98064..c16372304b1d 100644 --- a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/DefaultExceptionMapperInstrumentation.java +++ b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/DefaultExceptionMapperInstrumentation.java @@ -9,7 +9,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import io.opentelemetry.api.trace.Span; -import io.opentelemetry.instrumentation.api.server.ServerSpan; +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; @@ -38,7 +38,7 @@ public static class ExceptionAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onExit(@Advice.Argument(0) Exception exception) { - Span serverSpan = ServerSpan.fromContextOrNull(Java8BytecodeBridge.currentContext()); + Span serverSpan = LocalRootSpan.fromContextOrNull(Java8BytecodeBridge.currentContext()); if (serverSpan != null) { // unwrap exception Throwable throwable = exception; diff --git a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/RequestHandlerExecutorInstrumentation.java b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/RequestHandlerExecutorInstrumentation.java index dfd61d7e67cb..6203694a0e43 100644 --- a/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/RequestHandlerExecutorInstrumentation.java +++ b/instrumentation/wicket-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/wicket/RequestHandlerExecutorInstrumentation.java @@ -9,10 +9,7 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteHolder; -import io.opentelemetry.instrumentation.api.server.ServerSpan; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; @@ -41,14 +38,9 @@ public static class ExecuteAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onExit(@Advice.Argument(0) IRequestHandler handler) { - Context context = Java8BytecodeBridge.currentContext(); - Span serverSpan = ServerSpan.fromContextOrNull(context); - if (serverSpan == null) { - return; - } if (handler instanceof IPageClassRequestHandler) { HttpRouteHolder.updateHttpRoute( - context, + Java8BytecodeBridge.currentContext(), CONTROLLER, WicketServerSpanNaming.SERVER_SPAN_NAME, (IPageClassRequestHandler) handler);