diff --git a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxRsAnnotationsTracer.java b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/HandlerData.java similarity index 62% rename from instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxRsAnnotationsTracer.java rename to instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/HandlerData.java index 1008ff88ad6e..be24ac667895 100644 --- a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxRsAnnotationsTracer.java +++ b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/HandlerData.java @@ -5,16 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.jaxrs.v1_0; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanBuilder; -import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.ServerSpan; -import io.opentelemetry.instrumentation.api.tracer.SpanNames; import io.opentelemetry.javaagent.bootstrap.jaxrs.ClassHierarchyIterable; -import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.Map; @@ -22,15 +13,9 @@ import javax.ws.rs.HttpMethod; import javax.ws.rs.Path; -public class JaxRsAnnotationsTracer extends BaseTracer { +public class HandlerData { - private static final JaxRsAnnotationsTracer TRACER = new JaxRsAnnotationsTracer(); - - public static JaxRsAnnotationsTracer tracer() { - return TRACER; - } - - private final ClassValue> spanNames = + private static final ClassValue> serverSpanNames = new ClassValue>() { @Override protected Map computeValue(Class type) { @@ -38,37 +23,20 @@ protected Map computeValue(Class type) { } }; - public Context startSpan(Class target, Method method) { - String pathBasedSpanName = getPathSpanName(target, method); - Context parentContext = Context.current(); - Span serverSpan = ServerSpan.fromContextOrNull(parentContext); - - // When jax-rs is the root, we want to name using the path, otherwise use the class/method. - String spanName; - if (serverSpan == null) { - spanName = pathBasedSpanName; - } else { - spanName = SpanNames.fromMethod(target, method); - updateServerSpanName(parentContext, serverSpan, pathBasedSpanName); - } + private final Class target; + private final Method method; - SpanBuilder spanBuilder = spanBuilder(parentContext, spanName, SpanKind.INTERNAL); - setCodeAttributes(spanBuilder, target, method); - Span span = spanBuilder.startSpan(); - return parentContext.with(span); + public HandlerData(Class target, Method method) { + this.target = target; + this.method = method; } - private static void setCodeAttributes(SpanBuilder spanBuilder, Class target, Method method) { - spanBuilder.setAttribute(SemanticAttributes.CODE_NAMESPACE, target.getName()); - if (method != null) { - spanBuilder.setAttribute(SemanticAttributes.CODE_FUNCTION, method.getName()); - } + public Class codeClass() { + return target; } - private static void updateServerSpanName(Context context, Span span, String spanName) { - if (!spanName.isEmpty()) { - span.updateName(ServletContextPath.prepend(context, spanName)); - } + public String methodName() { + return method.getName(); } /** @@ -77,8 +45,8 @@ private static void updateServerSpanName(Context context, Span span, String span * * @return The result can be an empty string but will never be {@code null}. */ - private String getPathSpanName(Class target, Method method) { - Map classMap = spanNames.get(target); + String getServerSpanName() { + Map classMap = serverSpanNames.get(target); String spanName = classMap.get(method); if (spanName == null) { String httpMethod = null; @@ -168,11 +136,12 @@ private static String buildSpanName(Path classPath, Path methodPath) { StringBuilder spanNameBuilder = new StringBuilder(); boolean skipSlash = false; if (classPath != null) { - if (!classPath.value().startsWith("/")) { + String classPathValue = classPath.value(); + if (!classPathValue.startsWith("/")) { spanNameBuilder.append("/"); } - spanNameBuilder.append(classPath.value()); - skipSlash = classPath.value().endsWith("/"); + spanNameBuilder.append(classPathValue); + skipSlash = classPathValue.endsWith("/") || classPathValue.isEmpty(); } if (methodPath != null) { @@ -189,9 +158,4 @@ private static String buildSpanName(Path classPath, Path methodPath) { return spanNameBuilder.toString().trim(); } - - @Override - protected String getInstrumentationName() { - return "io.opentelemetry.jaxrs-1.0"; - } } diff --git a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxRsAnnotationsInstrumentation.java b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsAnnotationsInstrumentation.java similarity index 75% rename from instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxRsAnnotationsInstrumentation.java rename to instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsAnnotationsInstrumentation.java index ff741c9b14e1..50bb3fd28fa7 100644 --- a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxRsAnnotationsInstrumentation.java +++ b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsAnnotationsInstrumentation.java @@ -8,7 +8,7 @@ import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasSuperMethod; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasSuperType; -import static io.opentelemetry.javaagent.instrumentation.jaxrs.v1_0.JaxRsAnnotationsTracer.tracer; +import static io.opentelemetry.javaagent.instrumentation.jaxrs.v1_0.JaxrsSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.declaresMethod; import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -17,16 +17,18 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import java.lang.reflect.Method; import javax.ws.rs.Path; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -public class JaxRsAnnotationsInstrumentation implements TypeInstrumentation { +public class JaxrsAnnotationsInstrumentation implements TypeInstrumentation { @Override public ElementMatcher classLoaderOptimization() { @@ -55,7 +57,7 @@ public void transform(TypeTransformer transformer) { "javax.ws.rs.OPTIONS", "javax.ws.rs.POST", "javax.ws.rs.PUT")))), - JaxRsAnnotationsInstrumentation.class.getName() + "$JaxRsAnnotationsAdvice"); + JaxrsAnnotationsInstrumentation.class.getName() + "$JaxRsAnnotationsAdvice"); } @SuppressWarnings("unused") @@ -66,13 +68,27 @@ public static void nameSpan( @Advice.This Object target, @Advice.Origin Method method, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHandlerData") HandlerData handlerData, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { callDepth = CallDepth.forClass(Path.class); if (callDepth.getAndIncrement() > 0) { return; } - context = tracer().startSpan(target.getClass(), method); + + Context parentContext = Java8BytecodeBridge.currentContext(); + handlerData = new HandlerData(target.getClass(), method); + + ServerSpanNaming.updateServerSpanName( + parentContext, + ServerSpanNaming.Source.CONTROLLER, + JaxrsServerSpanNaming.getServerSpanNameSupplier(parentContext, handlerData)); + + if (!instrumenter().shouldStart(parentContext, handlerData)) { + return; + } + + context = instrumenter().start(parentContext, handlerData); scope = context.makeCurrent(); } @@ -80,18 +96,18 @@ public static void nameSpan( public static void stopSpan( @Advice.Thrown Throwable throwable, @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelHandlerData") HandlerData handlerData, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { if (callDepth.decrementAndGet() > 0) { return; } - scope.close(); - if (throwable == null) { - tracer().end(context); - } else { - tracer().endExceptionally(context, throwable); + if (scope == null) { + return; } + scope.close(); + instrumenter().end(context, handlerData, null, throwable); } } } diff --git a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsCodeAttributesExtractor.java b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsCodeAttributesExtractor.java new file mode 100644 index 000000000000..654b4435759c --- /dev/null +++ b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsCodeAttributesExtractor.java @@ -0,0 +1,31 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrs.v1_0; + +import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesExtractor; +import org.checkerframework.checker.nullness.qual.Nullable; + +public class JaxrsCodeAttributesExtractor extends CodeAttributesExtractor { + @Override + protected @Nullable Class codeClass(HandlerData handlerData) { + return handlerData.codeClass(); + } + + @Override + protected @Nullable String methodName(HandlerData handlerData) { + return handlerData.methodName(); + } + + @Override + protected @Nullable String filePath(HandlerData handlerData) { + return null; + } + + @Override + protected @Nullable Long lineNumber(HandlerData handlerData) { + return null; + } +} diff --git a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxRsInstrumentationModule.java b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsInstrumentationModule.java similarity index 86% rename from instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxRsInstrumentationModule.java rename to instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsInstrumentationModule.java index 7bd80844b6ab..2cdfe46bbe0f 100644 --- a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxRsInstrumentationModule.java +++ b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsInstrumentationModule.java @@ -16,8 +16,8 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(InstrumentationModule.class) -public class JaxRsInstrumentationModule extends InstrumentationModule { - public JaxRsInstrumentationModule() { +public class JaxrsInstrumentationModule extends InstrumentationModule { + public JaxrsInstrumentationModule() { super("jaxrs", "jaxrs-1.0"); } @@ -29,6 +29,6 @@ public ElementMatcher.Junction classLoaderMatcher() { @Override public List typeInstrumentations() { - return singletonList(new JaxRsAnnotationsInstrumentation()); + return singletonList(new JaxrsAnnotationsInstrumentation()); } } diff --git a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsServerSpanNaming.java b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsServerSpanNaming.java new file mode 100644 index 000000000000..1c7092c2fb6c --- /dev/null +++ b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsServerSpanNaming.java @@ -0,0 +1,32 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrs.v1_0; + +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; +import io.opentelemetry.javaagent.bootstrap.jaxrs.JaxrsContextPath; +import java.util.function.Supplier; + +public class JaxrsServerSpanNaming { + + public static Supplier getServerSpanNameSupplier( + Context context, HandlerData handlerData) { + return () -> { + String pathBasedSpanName = handlerData.getServerSpanName(); + // If path based name is empty skip prepending context path so that path based name would + // remain as an empty string for which we skip updating span name. Path base span name is + // empty when method and class don't have a jax-rs path annotation, this can happen when + // creating an "abort" span, see RequestContextHelper. + if (!pathBasedSpanName.isEmpty()) { + pathBasedSpanName = JaxrsContextPath.prepend(context, pathBasedSpanName); + pathBasedSpanName = ServletContextPath.prepend(context, pathBasedSpanName); + } + return pathBasedSpanName; + }; + } + + private JaxrsServerSpanNaming() {} +} diff --git a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsSingletons.java b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsSingletons.java new file mode 100644 index 000000000000..9bb33780debb --- /dev/null +++ b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsSingletons.java @@ -0,0 +1,43 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jaxrs.v1_0; + +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.code.CodeSpanNameExtractor; + +public final class JaxrsSingletons { + + public static final String ABORT_FILTER_CLASS = + "io.opentelemetry.javaagent.instrumentation.jaxrs2.filter.abort.class"; + public static final String ABORT_HANDLED = + "io.opentelemetry.javaagent.instrumentation.jaxrs2.filter.abort.handled"; + + private static final String INSTRUMENTATION_NAME = "io.opentelemetry.jaxrs-1.0-common"; + + private static final Instrumenter INSTRUMENTER; + + static { + CodeAttributesExtractor codeAttributesExtractor = + new JaxrsCodeAttributesExtractor(); + SpanNameExtractor spanNameExtractor = + CodeSpanNameExtractor.create(codeAttributesExtractor); + + INSTRUMENTER = + Instrumenter.newBuilder( + GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, spanNameExtractor) + .addAttributesExtractor(codeAttributesExtractor) + .newInstrumenter(); + } + + public static Instrumenter instrumenter() { + return INSTRUMENTER; + } + + private JaxrsSingletons() {} +} diff --git a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy index 6b9a5ea0356a..1fd951da2941 100644 --- a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy +++ b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy @@ -20,30 +20,6 @@ import spock.lang.Unroll class JaxRsAnnotations1InstrumentationTest extends AgentInstrumentationSpecification { - def "instrumentation can be used as root span and resource is set to METHOD PATH"() { - setup: - def jax = new Jax() { - @POST - @Path("/a") - void call() { - } - } - jax.call() - - expect: - assertTraces(1) { - trace(0, 1) { - span(0) { - name "/a" - attributes { - "${SemanticAttributes.CODE_NAMESPACE.key}" jax.getClass().getName() - "${SemanticAttributes.CODE_FUNCTION.key}" "call" - } - } - } - } - } - @Unroll def "span named '#paramName' from annotations on class '#className' when is not root span"() { setup: