From 1ca4211cf6021ab00606ef92d440924b59e06b34 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 14 Apr 2021 17:04:51 +0300 Subject: [PATCH 1/4] JAX-RS: add code.namespace and code.function attributes --- .../jaxrs/v2_0/JaxRsAnnotationsTracer.java | 16 ++++++++++++++-- .../JaxRsAnnotationsInstrumentationTest.groovy | 10 ++++++++-- .../src/main/groovy/JaxRsFilterTest.groovy | 16 ++++++++++++---- .../src/main/groovy/JaxRsHttpServerTest.groovy | 2 ++ 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java index 447585f028ea..a00e5fbf0f94 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java @@ -13,6 +13,7 @@ import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import io.opentelemetry.instrumentation.api.tracer.ServerSpan; import io.opentelemetry.javaagent.instrumentation.api.ClassHierarchyIterable; +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.Map; @@ -51,12 +52,16 @@ public Context startSpan(Context parentContext, Class target, Method method) Span span = spanBuilder(parentContext, "jax-rs.request", INTERNAL).startSpan(); updateSpanNames( parentContext, span, ServerSpan.fromContextOrNull(parentContext), target, method); + setCodeAttributes(span, target, method); return parentContext.with(span); } public void updateSpanNames( Context context, Span span, Span serverSpan, Class target, Method method) { - String pathBasedSpanName = ServletContextPath.prepend(context, getPathSpanName(target, method)); + String pathBasedSpanName = getPathSpanName(target, method); + if (pathBasedSpanName != null) { + pathBasedSpanName = ServletContextPath.prepend(context, pathBasedSpanName); + } if (serverSpan == null) { updateSpanName(span, pathBasedSpanName); } else { @@ -66,11 +71,18 @@ public void updateSpanNames( } private void updateSpanName(Span span, String spanName) { - if (!spanName.isEmpty()) { + if (spanName != null && !spanName.isEmpty()) { span.updateName(spanName); } } + private void setCodeAttributes(Span span, Class target, Method method) { + span.setAttribute(SemanticAttributes.CODE_NAMESPACE, target.getName()); + if (method != null) { + span.setAttribute(SemanticAttributes.CODE_FUNCTION, method.getName()); + } + } + /** * Returns the span name given a JaxRS annotated method. Results are cached so this method can be * called multiple times without significantly impacting performance. diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsAnnotationsInstrumentationTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsAnnotationsInstrumentationTest.groovy index 5b90e1eb2b2c..94be17057859 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsAnnotationsInstrumentationTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsAnnotationsInstrumentationTest.groovy @@ -8,6 +8,7 @@ import static io.opentelemetry.instrumentation.test.utils.ClassUtils.getClassNam import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderServerTrace import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import javax.ws.rs.DELETE import javax.ws.rs.GET import javax.ws.rs.HEAD @@ -21,12 +22,13 @@ abstract class JaxRsAnnotationsInstrumentationTest extends AgentInstrumentationS def "instrumentation can be used as root span and resource is set to METHOD PATH"() { setup: - new Jax() { + def jax = new Jax() { @POST @Path("/a") void call() { } - }.call() + } + jax.call() expect: assertTraces(1) { @@ -34,6 +36,8 @@ abstract class JaxRsAnnotationsInstrumentationTest extends AgentInstrumentationS span(0) { name "/a" attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" jax.getClass().getName() + "${SemanticAttributes.CODE_FUNCTION.key}" "call" } } } @@ -61,6 +65,8 @@ abstract class JaxRsAnnotationsInstrumentationTest extends AgentInstrumentationS name "${className}.call" childOf span(0) attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" obj.getClass().getName() + "${SemanticAttributes.CODE_FUNCTION.key}" "call" } } } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy index ef301493c7e6..c95e7e672003 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsFilterTest.groovy @@ -8,6 +8,7 @@ import static io.opentelemetry.api.trace.SpanKind.SERVER import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderServerTrace import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import javax.ws.rs.container.ContainerRequestContext import javax.ws.rs.container.ContainerRequestFilter import javax.ws.rs.container.PreMatching @@ -83,8 +84,15 @@ abstract class JaxRsFilterTest extends AgentInstrumentationSpecification { span(1) { childOf span(0) name controllerName - if (!runsOnServer()) { + if (abortPrematch) { + attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" "JaxRsFilterTest\$PrematchRequestFilter" + "${SemanticAttributes.CODE_FUNCTION.key}" "filter" + } + } else { attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" ~/Resource[$]Test*/ + "${SemanticAttributes.CODE_FUNCTION.key}" "hello" } } } @@ -135,9 +143,9 @@ abstract class JaxRsFilterTest extends AgentInstrumentationSpecification { childOf span(0) name controller1Name kind INTERNAL - if (!runsOnServer()) { - attributes { - } + attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" ~/Resource[$]Test*/ + "${SemanticAttributes.CODE_FUNCTION.key}" "nested" } } } diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy index 97fbce29db12..b6bb32b0232e 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy @@ -222,6 +222,8 @@ abstract class JaxRsHttpServerTest extends HttpServerTest implements Agent } childOf((SpanData) parent) attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" "JaxRsTestResource" + "${SemanticAttributes.CODE_FUNCTION.key}" methodName if (isCancelled) { "jaxrs.canceled" true } From 4b257804600ca8124b2f5e45886f44d19541b57c Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 14 Apr 2021 20:52:21 +0300 Subject: [PATCH 2/4] review fix --- .../jaxrs/v2_0/JaxRsAnnotationsTracer.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java index a00e5fbf0f94..0bfd7bc0daef 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java @@ -8,6 +8,7 @@ import static io.opentelemetry.api.trace.SpanKind.INTERNAL; import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.servlet.ServletContextPath; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; @@ -49,17 +50,22 @@ public Context startSpan(Context parentContext, Class target, Method method) // We create span and immediately update its name // We do that in order to reuse logic inside updateSpanNames method, which is used externally as // well. - Span span = spanBuilder(parentContext, "jax-rs.request", INTERNAL).startSpan(); + SpanBuilder spanBuilder = spanBuilder(parentContext, "jax-rs.request", INTERNAL); + setCodeAttributes(spanBuilder, target, method); + Span span = spanBuilder.startSpan(); updateSpanNames( parentContext, span, ServerSpan.fromContextOrNull(parentContext), target, method); - setCodeAttributes(span, target, method); return parentContext.with(span); } public void updateSpanNames( Context context, Span span, Span serverSpan, Class target, Method method) { String pathBasedSpanName = getPathSpanName(target, method); - if (pathBasedSpanName != null) { + // If path based name is empty skip appending 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 = ServletContextPath.prepend(context, pathBasedSpanName); } if (serverSpan == null) { @@ -71,15 +77,15 @@ public void updateSpanNames( } private void updateSpanName(Span span, String spanName) { - if (spanName != null && !spanName.isEmpty()) { + if (!spanName.isEmpty()) { span.updateName(spanName); } } - private void setCodeAttributes(Span span, Class target, Method method) { - span.setAttribute(SemanticAttributes.CODE_NAMESPACE, target.getName()); + private void setCodeAttributes(SpanBuilder spanBuilder, Class target, Method method) { + spanBuilder.setAttribute(SemanticAttributes.CODE_NAMESPACE, target.getName()); if (method != null) { - span.setAttribute(SemanticAttributes.CODE_FUNCTION, method.getName()); + spanBuilder.setAttribute(SemanticAttributes.CODE_FUNCTION, method.getName()); } } From 924640fc7e1e77affb836ed36232dbb74d14bf40 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 14 Apr 2021 20:53:05 +0300 Subject: [PATCH 3/4] add code.namespace and code.function to jax-rs1 --- .../jaxrs/v1_0/JaxRsAnnotationsTracer.java | 14 +++++++++++++- .../JaxRsAnnotations1InstrumentationTest.groovy | 10 ++++++++-- .../javaagent/src/test/groovy/JerseyTest.groovy | 5 +++++ 3 files changed, 26 insertions(+), 3 deletions(-) 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/JaxRsAnnotationsTracer.java index 4f6940b03ed0..620416e8be6e 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/JaxRsAnnotationsTracer.java @@ -6,12 +6,14 @@ 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.javaagent.instrumentation.api.ClassHierarchyIterable; +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.Map; @@ -49,7 +51,17 @@ public Context startSpan(Class target, Method method) { updateServerSpanName(parentContext, serverSpan, pathBasedSpanName); } - return startSpan(parentContext, spanName, SpanKind.INTERNAL); + SpanBuilder spanBuilder = spanBuilder(parentContext, spanName, SpanKind.INTERNAL); + setCodeAttributes(spanBuilder, target, method); + Span span = spanBuilder.startSpan(); + return parentContext.with(span); + } + + private 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()); + } } private void updateServerSpanName(Context context, Span span, String spanName) { 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 87517ce65212..f5804a78cc2b 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 @@ -8,6 +8,7 @@ import static io.opentelemetry.instrumentation.test.utils.ClassUtils.getClassNam import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderServerTrace import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import javax.ws.rs.DELETE import javax.ws.rs.GET import javax.ws.rs.HEAD @@ -21,12 +22,13 @@ class JaxRsAnnotations1InstrumentationTest extends AgentInstrumentationSpecifica def "instrumentation can be used as root span and resource is set to METHOD PATH"() { setup: - new Jax() { + def jax = new Jax() { @POST @Path("/a") void call() { } - }.call() + } + jax.call() expect: assertTraces(1) { @@ -34,6 +36,8 @@ class JaxRsAnnotations1InstrumentationTest extends AgentInstrumentationSpecifica span(0) { name "/a" attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" jax.getClass().getName() + "${SemanticAttributes.CODE_FUNCTION.key}" "call" } } } @@ -61,6 +65,8 @@ class JaxRsAnnotations1InstrumentationTest extends AgentInstrumentationSpecifica name "${className}.call" childOf span(0) attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" obj.getClass().getName() + "${SemanticAttributes.CODE_FUNCTION.key}" "call" } } } diff --git a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JerseyTest.groovy b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JerseyTest.groovy index f309e6168ea4..c73c3fe87307 100644 --- a/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JerseyTest.groovy +++ b/instrumentation/jaxrs/jaxrs-1.0/javaagent/src/test/groovy/JerseyTest.groovy @@ -9,6 +9,7 @@ import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderSer import io.dropwizard.testing.junit.ResourceTestRule import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification +import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import org.junit.ClassRule import spock.lang.Shared import spock.lang.Unroll @@ -47,6 +48,8 @@ class JerseyTest extends AgentInstrumentationSpecification { childOf span(0) name controllerName attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" ~/Resource[$]Test*/ + "${SemanticAttributes.CODE_FUNCTION.key}" "hello" } } } @@ -83,6 +86,8 @@ class JerseyTest extends AgentInstrumentationSpecification { name controller1Name kind INTERNAL attributes { + "${SemanticAttributes.CODE_NAMESPACE.key}" ~/Resource[$]Test*/ + "${SemanticAttributes.CODE_FUNCTION.key}" "nested" } } } From 55f0840355a7708f6073878476acd8753939a89c Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 14 Apr 2021 20:57:23 +0300 Subject: [PATCH 4/4] typo --- .../instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java index 0bfd7bc0daef..efeb7793c7d5 100644 --- a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java +++ b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxRsAnnotationsTracer.java @@ -61,7 +61,7 @@ public Context startSpan(Context parentContext, Class target, Method method) public void updateSpanNames( Context context, Span span, Span serverSpan, Class target, Method method) { String pathBasedSpanName = getPathSpanName(target, method); - // If path based name is empty skip appending context path so that path based name would + // 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.