From c9f92b78a5defedefe66a97d9b68b122c5d0a48a Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Wed, 19 Mar 2025 16:32:37 -0700 Subject: [PATCH 1/3] Experimental change to prevent JAX-RS instrumentation from overriding play framework span resource names. --- .../instrumentation/decorator/HttpServerDecorator.java | 1 + .../trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java | 2 +- .../java/datadog/trace/instrumentation/play26/PlayAdvice.java | 2 +- .../groovy/datadog/trace/agent/test/base/HttpServerTest.groovy | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java index bed9b4d2178..726b8cce90b 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java @@ -358,6 +358,7 @@ public AgentSpan onResponseStatus(final AgentSpan span, final int status) { } if (SHOULD_SET_404_RESOURCE_NAME && status == 404) { + // TODO span.setResourceName(NOT_FOUND_RESOURCE_NAME, ResourceNamePriorities.HTTP_404); } 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 0b37c0da9fb..522884dbc72 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 @@ -57,7 +57,7 @@ public void onJaxRsSpan( // This check ensures that we only use the route from the first JAX-RS annotated method that // is executed if (parent.getLocalRootSpan().getResourceNamePriority() - < ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE) { + < ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE) { // TODO HTTP_RESOURCE_DECORATOR.withRoute( parent.getLocalRootSpan(), httpMethodAndRoute.getLeft(), httpMethodAndRoute.getRight()); parent.getLocalRootSpan().setTag(Tags.COMPONENT, "jax-rs"); diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayAdvice.java b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayAdvice.java index ecd5b903e19..d15ea8b7676 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayAdvice.java +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayAdvice.java @@ -81,7 +81,7 @@ public static void stopTraceOnResponse( // set the resource name on the upstream akka/netty span if there is one if (rootSpan != null && playControllerSpan.getResourceName() != null) { rootSpan.setResourceName( - playControllerSpan.getResourceName(), ResourceNamePriorities.HTTP_PATH_NORMALIZER); + playControllerSpan.getResourceName(), ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE); } } } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index afd26bc61bc..da819705a44 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -191,7 +191,7 @@ abstract class HttpServerTest extends WithHttpServer { String expectedResourceName(ServerEndpoint endpoint, String method, URI address) { if (endpoint.status == 404 && (changesAll404s() || endpoint.path == "/not-found")) { - return "404" + return "404" // TODO } else if (endpoint.hasPathParam) { return "$method ${testPathParam()}" } From b2eac2fef391f0014d5901685570d5c83d5c8920 Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Wed, 19 Mar 2025 17:08:30 -0700 Subject: [PATCH 2/3] Add debug for setResourceName --- .../src/main/java/datadog/trace/core/DDSpanContext.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index b28247c6914..dc130640f05 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -427,6 +427,14 @@ public byte getResourceNamePriority() { } public void setResourceName(final CharSequence resourceName, byte priority) { + if (log.isDebugEnabled()) { + log.debug( + "setResourceName `{}`->`{}` with priority {}->{}", + this.resourceName, + resourceName, + resourceNamePriority, + priority); + } if (null == resourceName) { return; } From 8fa65e094d8449896a75633f27350dc2e4272580 Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Mon, 9 Jun 2025 13:40:53 -0700 Subject: [PATCH 3/3] Clean up. Fix 404 handling. --- .../instrumentation/decorator/HttpServerDecorator.java | 3 +-- .../instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java | 2 +- dd-java-agent/instrumentation/play-2.6/build.gradle | 4 ++-- .../instrumentation/play26/PlayHttpServerDecorator.java | 7 +++++++ .../instrumentation/play26/RequestCompleteCallback.java | 5 ++++- .../datadog/trace/agent/test/base/HttpServerTest.groovy | 2 +- .../src/main/java/datadog/trace/core/DDSpanContext.java | 6 ++++-- gradle/java_no_deps.gradle | 4 ++++ 8 files changed, 24 insertions(+), 9 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java index 726b8cce90b..153825738e4 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java @@ -73,7 +73,7 @@ public abstract class HttpServerDecorator result) { if (result.isFailure()) { DECORATE.onError(span, result.failed().get()); } else { + Result response = result.get(); if (REPORT_HTTP_STATUS) { - DECORATE.onResponse(span, result.get()); + DECORATE.onResponse(span, response); + } else { + DECORATE.updateOn404Only(span, response); } } DECORATE.beforeFinish(span); diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index da819705a44..afd26bc61bc 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -191,7 +191,7 @@ abstract class HttpServerTest extends WithHttpServer { String expectedResourceName(ServerEndpoint endpoint, String method, URI address) { if (endpoint.status == 404 && (changesAll404s() || endpoint.path == "/not-found")) { - return "404" // TODO + return "404" } else if (endpoint.hasPathParam) { return "$method ${testPathParam()}" } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index dc130640f05..a1a253ca9f5 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -429,11 +429,13 @@ public byte getResourceNamePriority() { public void setResourceName(final CharSequence resourceName, byte priority) { if (log.isDebugEnabled()) { log.debug( - "setResourceName `{}`->`{}` with priority {}->{}", + "setResourceName `{}`->`{}` with priority {}->{} for traceId={} spanId={}", this.resourceName, resourceName, resourceNamePriority, - priority); + priority, + traceId, + spanId); } if (null == resourceName) { return; diff --git a/gradle/java_no_deps.gradle b/gradle/java_no_deps.gradle index 51d7225dce3..184ed76c259 100644 --- a/gradle/java_no_deps.gradle +++ b/gradle/java_no_deps.gradle @@ -298,16 +298,20 @@ def isJavaVersionAllowedForProperty(JavaVersion version, String propertyPrefix = def definedMin = project.hasProperty(minProp) def definedMax = project.hasProperty(maxProp) if (definedMin && project.getProperty(minProp).compareTo(version) > 0) { + logger.info("isJavaVersionAllowedForProperty is false b/o minProp=${minProp} is defined and greater than version=${version}") return false } //default to the general min if defined and specific one is was not defined if (!propertyPrefix.isEmpty() && !definedMin && project.hasProperty('minJavaVersionForTests') && project.getProperty('minJavaVersionForTests').compareTo(version) > 0) { + logger.info("isJavaVersionAllowedForProperty is false b/o minJavaVersionForTests=${project.getProperty('minJavaVersionForTests')} is defined and greater than version=${version}") return false } if (definedMax && project.getProperty(maxProp).compareTo(version) < 0) { + logger.info("isJavaVersionAllowedForProperty is false b/o maxProp=${project.getProperty(maxProp)} is defined and lower than version=${version}") return false } if (!propertyPrefix.isEmpty() && !definedMax && project.hasProperty('maxJavaVersionForTests') && project.getProperty('maxJavaVersionForTests').compareTo(version) < 0) { + logger.info("isJavaVersionAllowedForProperty is false b/o maxJavaVersionForTests=${project.getProperty('maxJavaVersionForTests')} is defined and lower than version=${version}") return false } return true