diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index 3d5b82cd407..9d42f196f25 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -29,5 +29,7 @@ public interface AgentSpan { void setSpanName(String spanName); + boolean hasResourceName(); + interface Context {} } diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index fb992597d5f..856bca5b429 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -203,6 +203,11 @@ public String getSpanName() { @Override public void setSpanName(final String spanName) {} + + @Override + public boolean hasResourceName() { + return false; + } } static class NoopAgentScope implements AgentScope { diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java index 77fa060595f..214725f1741 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/OpenTracing32.java @@ -234,6 +234,14 @@ public void setSpanName(final String spanName) { span.setOperationName(spanName); } + @Override + public boolean hasResourceName() { + if (span instanceof DDSpan) { + return ((DDSpan) span).context().hasResourceName(); + } + return false; + } + private Span getSpan() { return span; } diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java index bd39af4b7b1..8cbaf29c78e 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java +++ b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java @@ -36,10 +36,11 @@ protected String component() { return "jax-rs-controller"; } - public void onControllerStart( + public void onJaxRsSpan( final AgentSpan span, final AgentSpan parent, final Class target, final Method method) { + final String resourceName = getPathResourceName(target, method); - updateParent(parent, resourceName); + updateRootSpan(parent, resourceName); span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_SERVER); @@ -48,19 +49,24 @@ public void onControllerStart( if (isRootScope && !resourceName.isEmpty()) { span.setTag(DDTags.RESOURCE_NAME, resourceName); } else { - span.setTag(DDTags.RESOURCE_NAME, DECORATE.spanNameForClass(target) + "." + method.getName()); + span.setTag( + DDTags.RESOURCE_NAME, + DECORATE.spanNameForClass(target) + (method == null ? "" : "." + method.getName())); } } - private void updateParent(AgentSpan span, final String resourceName) { + private void updateRootSpan(AgentSpan span, final String resourceName) { if (span == null) { return; } span = span.getLocalRootSpan(); - span.setTag(Tags.COMPONENT, "jax-rs"); - if (!resourceName.isEmpty()) { - span.setTag(DDTags.RESOURCE_NAME, resourceName); + if (!span.hasResourceName()) { + span.setTag(Tags.COMPONENT, "jax-rs"); + + if (!resourceName.isEmpty()) { + span.setTag(DDTags.RESOURCE_NAME, resourceName); + } } } @@ -72,6 +78,7 @@ private void updateParent(AgentSpan span, final String resourceName) { */ private String getPathResourceName(final Class target, final Method method) { Map classMap = resourceNames.get(target); + if (classMap == null) { resourceNames.putIfAbsent(target, new ConcurrentHashMap()); classMap = resourceNames.get(target); diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java index 0c71b362153..f13c86c7af3 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java +++ b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsInstrumentation.java @@ -84,7 +84,7 @@ public static AgentScope nameSpan( final AgentSpan parent = activeSpan(); final AgentSpan span = startSpan(JAX_ENDPOINT_OPERATION_NAME); - DECORATE.onControllerStart(span, parent, target.getClass(), method); + DECORATE.onJaxRsSpan(span, parent, target.getClass(), method); DECORATE.afterStart(span); final AgentScope scope = activateSpan(span, true); diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/test/groovy/JerseyTest.groovy b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/test/groovy/JerseyTest.groovy index daeeca315c9..6ffbadc507a 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/test/groovy/JerseyTest.groovy +++ b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/test/groovy/JerseyTest.groovy @@ -57,4 +57,53 @@ class JerseyTest extends AgentTestRunner { "/test2/hello/bob" | "POST /test2/hello/{name}" | "Test2.hello" | "Test2 bob!" "/test3/hi/bob" | "POST /test3/hi/{name}" | "Test3.hello" | "Test3 bob!" } + + def "test nested call"() { + + when: + // start a trace because the test doesn't go through any servlet or other instrumentation. + def response = runUnderTrace("test.span") { + resources.client().resource(resource).post(String) + } + + then: + response == expectedResponse + + assertTraces(1) { + trace(0, 3) { + span(0) { + operationName "test.span" + resourceName parentResourceName + tags { + "$Tags.COMPONENT" "jax-rs" + defaultTags() + } + } + span(1) { + childOf span(0) + operationName "jax-rs.request" + resourceName controller1Name + spanType DDSpanTypes.HTTP_SERVER + tags { + "$Tags.COMPONENT" "jax-rs-controller" + defaultTags() + } + } + span(2) { + childOf span(1) + operationName "jax-rs.request" + resourceName controller2Name + spanType DDSpanTypes.HTTP_SERVER + tags { + "$Tags.COMPONENT" "jax-rs-controller" + defaultTags() + } + } + } + } + + where: + resource | parentResourceName | controller1Name | controller2Name | expectedResponse + "/test3/nested" | "POST /test3/nested" | "Test3.nested" | "Test3.hello" | "Test3 nested!" + } } diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/test/java/Resource.java b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/test/java/Resource.java index 0a505ee7945..73df13fd493 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-1/src/test/java/Resource.java +++ b/dd-java-agent/instrumentation/jax-rs-annotations-1/src/test/java/Resource.java @@ -39,5 +39,11 @@ class Test3 implements SubResource { public String hello(@PathParam("name") final String name) { return "Test3 " + name + "!"; } + + @POST + @Path("/nested") + public String nested() { + return hello("nested"); + } } } diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java index c229a6c4924..70cba43faf7 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java +++ b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java @@ -1,5 +1,7 @@ package datadog.trace.instrumentation.jaxrs2; +import static datadog.trace.bootstrap.WeakMap.Provider.newWeakMap; + import datadog.trace.agent.tooling.ClassHierarchyIterable; import datadog.trace.api.DDSpanTypes; import datadog.trace.api.DDTags; @@ -25,8 +27,7 @@ public class JaxRsAnnotationsDecorator extends BaseDecorator { public static final JaxRsAnnotationsDecorator DECORATE = new JaxRsAnnotationsDecorator(); - private final WeakMap, Map> resourceNames = - WeakMap.Provider.newWeakMap(); + private final WeakMap, Map> resourceNames = newWeakMap(); @Override protected String[] instrumentationNames() { @@ -47,7 +48,7 @@ public void onJaxRsSpan( final AgentSpan span, final AgentSpan parent, final Class target, final Method method) { final String resourceName = getPathResourceName(target, method); - updateParent(parent, resourceName); + updateRootSpan(parent, resourceName); span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_SERVER); @@ -62,15 +63,18 @@ public void onJaxRsSpan( } } - private void updateParent(AgentSpan span, final String resourceName) { + private void updateRootSpan(AgentSpan span, final String resourceName) { if (span == null) { return; } span = span.getLocalRootSpan(); - span.setTag(Tags.COMPONENT, "jax-rs"); - if (!resourceName.isEmpty()) { - span.setTag(DDTags.RESOURCE_NAME, resourceName); + if (!span.hasResourceName()) { + span.setTag(Tags.COMPONENT, "jax-rs"); + + if (!resourceName.isEmpty()) { + span.setTag(DDTags.RESOURCE_NAME, resourceName); + } } } diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/test/groovy/JaxRsFilterTest.groovy b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/test/groovy/JaxRsFilterTest.groovy index b11687316cd..eaf8ab7559c 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/test/groovy/JaxRsFilterTest.groovy +++ b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/test/groovy/JaxRsFilterTest.groovy @@ -96,6 +96,62 @@ abstract class JaxRsFilterTest extends AgentTestRunner { "/test3/hi/bob" | false | true | "test.span" | "PrematchRequestFilter.filter" | "Aborted Prematch" } + def "test nested call"() { + given: + simpleRequestFilter.abort = false + prematchRequestFilter.abort = false + + when: + def responseText + def responseStatus + + // start a trace because the test doesn't go through any servlet or other instrumentation. + runUnderTrace("test.span") { + (responseText, responseStatus) = makeRequest(resource) + } + + then: + responseStatus == Response.Status.OK.statusCode + responseText == expectedResponse + + assertTraces(1) { + trace(0, 3) { + span(0) { + operationName "test.span" + resourceName parentResourceName + tags { + "$Tags.COMPONENT" "jax-rs" + defaultTags() + } + } + span(1) { + childOf span(0) + operationName "jax-rs.request" + resourceName controller1Name + spanType DDSpanTypes.HTTP_SERVER + tags { + "$Tags.COMPONENT" "jax-rs-controller" + defaultTags() + } + } + span(2) { + childOf span(1) + operationName "jax-rs.request" + resourceName controller2Name + spanType DDSpanTypes.HTTP_SERVER + tags { + "$Tags.COMPONENT" "jax-rs-controller" + defaultTags() + } + } + } + } + + where: + resource | parentResourceName | controller1Name | controller2Name | expectedResponse + "/test3/nested" | "POST /test3/nested" | "Test3.nested" | "Test3.hello" | "Test3 nested!" + } + @Provider class SimpleRequestFilter implements ContainerRequestFilter { boolean abort = false diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/test/java/Resource.java b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/test/java/Resource.java index 0a505ee7945..73df13fd493 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-2/src/test/java/Resource.java +++ b/dd-java-agent/instrumentation/jax-rs-annotations-2/src/test/java/Resource.java @@ -39,5 +39,11 @@ class Test3 implements SubResource { public String hello(@PathParam("name") final String name) { return "Test3 " + name + "!"; } + + @POST + @Path("/nested") + public String nested() { + return hello("nested"); + } } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java index 240c4358d73..cc029507fb8 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpanContext.java @@ -174,6 +174,10 @@ public boolean isResourceNameSet() { return !(resourceName == null || resourceName.isEmpty()); } + public boolean hasResourceName() { + return isResourceNameSet() || tags.containsKey(DDTags.RESOURCE_NAME); + } + public void setResourceName(final String resourceName) { this.resourceName = resourceName; }