From 55f987613af269d4251e7b18b6039859c04a2c60 Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Tue, 12 Oct 2021 13:45:18 +0300 Subject: [PATCH] Remove deprecated ServerSpanNaming.updateSource method (#4327) * Remove deprecated ServerSpanNaming.updateSource method --- .../api/servlet/ServerSpanNaming.java | 18 ++------- .../bootstrap/jaxrs/JaxrsContextPath.java | 4 +- .../v2_0/CxfJaxRsInvokerInstrumentation.java | 2 +- .../{CxfTracingUtil.java => CxfSpanName.java} | 36 ++++++++++-------- ...sourceMethodDispatcherInstrumentation.java | 2 +- ...eyTracingUtil.java => JerseySpanName.java} | 37 ++++++++----------- ...ResourceLocatorInvokerInstrumentation.java | 2 +- ...yResourceMethodInvokerInstrumentation.java | 2 +- .../jaxrs/v2_0/ResteasySpanName.java | 36 ++++++++++++++++++ .../jaxrs/v2_0/ResteasyTracingUtil.java | 34 ----------------- 10 files changed, 82 insertions(+), 91 deletions(-) rename instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/{CxfTracingUtil.java => CxfSpanName.java} (64%) rename instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/{JerseyTracingUtil.java => JerseySpanName.java} (50%) create mode 100644 instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasySpanName.java delete mode 100644 instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyTracingUtil.java diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNaming.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNaming.java index 7ec387b705..2f7f205067 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNaming.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNaming.java @@ -102,19 +102,6 @@ public final class ServerSpanNaming { } } - // TODO (trask) migrate the one usage (ServletHttpServerTracer) to ServerSpanNaming.init() once we - // migrate to new Instrumenters (see - // https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/2814#discussion_r617351334 - // for the challenge with doing this now in the current Tracer structure, at least without some - // bigger changes, which we want to avoid in the Tracers as they are already deprecated) - @Deprecated - public static void updateSource(Context context, Source source) { - ServerSpanNaming serverSpanNaming = context.get(CONTEXT_KEY); - if (serverSpanNaming != null && source.order > serverSpanNaming.updatedBySource.order) { - serverSpanNaming.updatedBySource = source; - } - } - private boolean isBetterName(String name) { return name.length() > nameLength; } @@ -125,7 +112,10 @@ public final class ServerSpanNaming { // filter that is called FILTER(2, /* useFirst= */ false), SERVLET(3), - CONTROLLER(4); + CONTROLLER(4), + // Some frameworks, e.g. JaxRS, allow for nested controller/paths and we want to select the + // longest one + NESTED_CONTROLLER(5, false); private final int order; private final boolean useFirst; diff --git a/instrumentation/jaxrs/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/jaxrs/JaxrsContextPath.java b/instrumentation/jaxrs/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/jaxrs/JaxrsContextPath.java index 547bc81c95..7050b23491 100644 --- a/instrumentation/jaxrs/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/jaxrs/JaxrsContextPath.java +++ b/instrumentation/jaxrs/bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/jaxrs/JaxrsContextPath.java @@ -13,8 +13,8 @@ import org.checkerframework.checker.nullness.qual.Nullable; * Helper container for storing context path for jax-rs requests. Jax-rs context path is the path * where jax-rs servlet is mapped or the value of ApplicationPath annotation. Span name is built by * combining servlet context path from {@code - * io.opentelemetry.instrumentation.api.servlet.ServletContextPath} jax-rs context path and the Path - * annotation from called method or class. + * io.opentelemetry.instrumentation.api.servlet.ServletContextPath}, jax-rs context path and the + * Path annotation from called method or class. */ public final class JaxrsContextPath { private static final ContextKey CONTEXT_KEY = diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfJaxRsInvokerInstrumentation.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfJaxRsInvokerInstrumentation.java index 6dd7137395..70363d4e14 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfJaxRsInvokerInstrumentation.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfJaxRsInvokerInstrumentation.java @@ -40,7 +40,7 @@ public class CxfJaxRsInvokerInstrumentation implements TypeInstrumentation { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.Argument(0) Exchange exchange, @Advice.Local("otelScope") Scope scope) { - Context context = CxfTracingUtil.updateServerSpanName(exchange); + Context context = CxfSpanName.INSTANCE.updateServerSpanName(exchange); if (context != null) { scope = context.makeCurrent(); } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfTracingUtil.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfSpanName.java similarity index 64% rename from instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfTracingUtil.java rename to instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfSpanName.java index b69a9dd616..961f3f56c1 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfTracingUtil.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-cxf-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/CxfSpanName.java @@ -7,41 +7,38 @@ package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0; import static io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0.JaxrsPathUtil.normalizePath; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNameSupplier; import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import io.opentelemetry.javaagent.bootstrap.jaxrs.JaxrsContextPath; import org.apache.cxf.jaxrs.model.ClassResourceInfo; import org.apache.cxf.jaxrs.model.OperationResourceInfo; import org.apache.cxf.jaxrs.model.URITemplate; import org.apache.cxf.message.Exchange; -public final class CxfTracingUtil { +public final class CxfSpanName implements ServerSpanNameSupplier { - private CxfTracingUtil() {} + public static final CxfSpanName INSTANCE = new CxfSpanName(); - public static Context updateServerSpanName(Exchange exchange) { + public Context updateServerSpanName(Exchange exchange) { Context context = Context.current(); - Span serverSpan = ServerSpan.fromContextOrNull(context); - if (serverSpan == null) { - return null; - } + String jaxrsName = calculateJaxrsName(context, exchange); + ServerSpanNaming.updateServerSpanName( + context, ServerSpanNaming.Source.NESTED_CONTROLLER, this, jaxrsName); + + return JaxrsContextPath.init(context, jaxrsName); + } + + private static String calculateJaxrsName(Context context, Exchange exchange) { OperationResourceInfo ori = exchange.get(OperationResourceInfo.class); ClassResourceInfo cri = ori.getClassResourceInfo(); String name = getName(cri.getURITemplate(), ori.getURITemplate()); if (name.isEmpty()) { return null; } - - serverSpan.updateName( - ServletContextPath.prepend(context, JaxrsContextPath.prepend(context, name))); - // mark span name as updated from controller to avoid JaxRsAnnotationsTracer updating it - ServerSpanNaming.updateSource(context, ServerSpanNaming.Source.CONTROLLER); - - return JaxrsContextPath.init(context, JaxrsContextPath.prepend(context, name)); + return JaxrsContextPath.prepend(context, name); } private static String getName(URITemplate classTemplate, URITemplate operationTemplate) { @@ -58,4 +55,11 @@ public final class CxfTracingUtil { return normalizePath(uriTemplate.getValue()); } + + @Override + public String get(Context context, String jaxrsName) { + return ServletContextPath.prepend(context, jaxrsName); + } + + private CxfSpanName() {} } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyResourceMethodDispatcherInstrumentation.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyResourceMethodDispatcherInstrumentation.java index d59f41f7bb..eb30ae28fe 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyResourceMethodDispatcherInstrumentation.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyResourceMethodDispatcherInstrumentation.java @@ -41,7 +41,7 @@ public class JerseyResourceMethodDispatcherInstrumentation implements TypeInstru @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter(@Advice.Argument(1) Request request) { - JerseyTracingUtil.updateServerSpanName(request); + JerseySpanName.INSTANCE.updateServerSpanName(request); } } } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyTracingUtil.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseySpanName.java similarity index 50% rename from instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyTracingUtil.java rename to instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseySpanName.java index 093a237a8f..759a6e93f5 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseyTracingUtil.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JerseySpanName.java @@ -7,41 +7,36 @@ package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0; import static io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0.JaxrsPathUtil.normalizePath; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNameSupplier; import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import io.opentelemetry.javaagent.bootstrap.jaxrs.JaxrsContextPath; -import java.util.Optional; import javax.ws.rs.core.Request; import javax.ws.rs.core.UriInfo; +import org.checkerframework.checker.nullness.qual.Nullable; import org.glassfish.jersey.server.ContainerRequest; import org.glassfish.jersey.server.ExtendedUriInfo; -public class JerseyTracingUtil { +public class JerseySpanName implements ServerSpanNameSupplier { - public static void updateServerSpanName(Request request) { + public static final JerseySpanName INSTANCE = new JerseySpanName(); + + public void updateServerSpanName(Request request) { Context context = Context.current(); - Span serverSpan = ServerSpan.fromContextOrNull(context); - if (serverSpan == null) { - return; - } + ServerSpanNaming.updateServerSpanName( + context, ServerSpanNaming.Source.NESTED_CONTROLLER, this, request); + } + @Override + public @Nullable String get(Context context, Request request) { ContainerRequest containerRequest = (ContainerRequest) request; UriInfo uriInfo = containerRequest.getUriInfo(); ExtendedUriInfo extendedUriInfo = (ExtendedUriInfo) uriInfo; - Optional name = - extendedUriInfo.getMatchedTemplates().stream() - .map((uriTemplate) -> normalizePath(uriTemplate.getTemplate())) - .reduce((a, b) -> b + a); - if (!name.isPresent()) { - return; - } - - serverSpan.updateName( - ServletContextPath.prepend(context, JaxrsContextPath.prepend(context, name.get()))); - // mark span name as updated from controller to avoid JaxRsAnnotationsTracer updating it - ServerSpanNaming.updateSource(context, ServerSpanNaming.Source.CONTROLLER); + return extendedUriInfo.getMatchedTemplates().stream() + .map((uriTemplate) -> normalizePath(uriTemplate.getTemplate())) + .reduce((a, b) -> b + a) + .map(s -> ServletContextPath.prepend(context, JaxrsContextPath.prepend(context, s))) + .orElse(null); } } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyResourceLocatorInvokerInstrumentation.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyResourceLocatorInvokerInstrumentation.java index b6e573c490..6a4241fb24 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyResourceLocatorInvokerInstrumentation.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyResourceLocatorInvokerInstrumentation.java @@ -49,7 +49,7 @@ public class ResteasyResourceLocatorInvokerInstrumentation implements TypeInstru String name = VirtualField.find(ResourceLocatorInvoker.class, String.class).get(resourceInvoker); - ResteasyTracingUtil.updateServerSpanName(currentContext, name); + ResteasySpanName.INSTANCE.updateServerSpanName(currentContext, name); // subresource locator returns a resources class that may have @Path annotations // append current path to jax-rs context path so that it would be present in the final path diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyResourceMethodInvokerInstrumentation.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyResourceMethodInvokerInstrumentation.java index 8c2feb1833..65300aedc8 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyResourceMethodInvokerInstrumentation.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyResourceMethodInvokerInstrumentation.java @@ -41,7 +41,7 @@ public class ResteasyResourceMethodInvokerInstrumentation implements TypeInstrum String name = VirtualField.find(ResourceMethodInvoker.class, String.class).get(resourceInvoker); - ResteasyTracingUtil.updateServerSpanName(Java8BytecodeBridge.currentContext(), name); + ResteasySpanName.INSTANCE.updateServerSpanName(Java8BytecodeBridge.currentContext(), name); } } } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasySpanName.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasySpanName.java new file mode 100644 index 0000000000..724ce9d81a --- /dev/null +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasySpanName.java @@ -0,0 +1,36 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0; + +import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.NESTED_CONTROLLER; + +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNameSupplier; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; +import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; +import io.opentelemetry.javaagent.bootstrap.jaxrs.JaxrsContextPath; +import org.checkerframework.checker.nullness.qual.Nullable; + +public final class ResteasySpanName implements ServerSpanNameSupplier { + + public static final ResteasySpanName INSTANCE = new ResteasySpanName(); + + public void updateServerSpanName(Context context, String name) { + if (name != null) { + ServerSpanNaming.updateServerSpanName(context, NESTED_CONTROLLER, this, name); + } + } + + @Override + public @Nullable String get(Context context, String name) { + if (name.isEmpty()) { + return null; + } + return ServletContextPath.prepend(context, JaxrsContextPath.prepend(context, name)); + } + + private ResteasySpanName() {} +} diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyTracingUtil.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyTracingUtil.java deleted file mode 100644 index 8ade79c0a0..0000000000 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-resteasy-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/ResteasyTracingUtil.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0; - -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; -import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; -import io.opentelemetry.javaagent.bootstrap.jaxrs.JaxrsContextPath; - -public final class ResteasyTracingUtil { - - private ResteasyTracingUtil() {} - - public static void updateServerSpanName(Context context, String name) { - if (name == null || name.isEmpty()) { - return; - } - - Span serverSpan = ServerSpan.fromContextOrNull(context); - if (serverSpan == null) { - return; - } - - serverSpan.updateName( - ServletContextPath.prepend(context, JaxrsContextPath.prepend(context, name))); - // mark span name as updated from controller to avoid JaxRsAnnotationsTracer updating it - ServerSpanNaming.updateSource(context, ServerSpanNaming.Source.CONTROLLER); - } -}