From 279d2b4453cded59913caf7c22a3a22512849314 Mon Sep 17 00:00:00 2001 From: Jeremy Katz Date: Thu, 20 Aug 2026 15:24:32 -0400 Subject: [PATCH 1/2] Don't flag JAX-RS/Spring MVC spans as errors for exceptions that map to a non-5xx response JAX-RS and Spring MVC resource/controller methods commonly signal a non-2xx response by throwing an exception (e.g. NotFoundException, ResponseStatusException, or a custom exception annotated with @ResponseStatus) that the framework's own exception-mapping machinery turns into a normal HTTP response. The tracer previously flagged the resource/controller span as an error unconditionally whenever such an exception was thrown, even when the framework maps it to a routine 4xx (or other non-5xx) status - flooding error tracking with non-actionable "errors" for expected control flow. These exceptions already carry their intended status (WebApplicationException's embedded Response, ResponseStatusException/ErrorResponse, or the @ResponseStatus annotation), so the fix decides the error flag from that status against the same "server error" set used for the root HTTP span, instead of unconditionally erroring. @ResponseStatus's value() and code() attributes are @AliasFor each other, but plain reflection on the annotation proxy doesn't resolve that aliasing - if a caller sets only code(), value() still reports its own default (INTERNAL_SERVER_ERROR) rather than the value mirrored from code(). Both Spring decorators now read code() first (reflectively on the Spring 3.1 classpath, which predates code()'s introduction in 4.2) and fall back to value() only when code() is left at its default. --- .../JakartaRsAnnotationsDecorator.java | 31 ++++ ...taRsAnnotations3InstrumentationTest.groovy | 113 +++++++++++++++ .../jaxrs1/JaxRsAnnotationsDecorator.java | 31 ++++ ...axRsAnnotations1InstrumentationTest.groovy | 100 +++++++++++++ .../jaxrs2/JaxRsAnnotationsDecorator.java | 31 ++++ ...axRsAnnotations2InstrumentationTest.groovy | 101 +++++++++++++ ...WebHttpServerDecoratorLatestDepTest.groovy | 77 ++++++++++ .../SpringWebHttpServerDecorator.java | 92 ++++++++++++ .../SpringWebHttpServerDecoratorTest.groovy | 124 ++++++++++++++++ .../SpringWebHttpServerDecorator.java | 55 +++++++ .../SpringWebHttpServerDecoratorTest.groovy | 134 ++++++++++++++++++ 11 files changed, 889 insertions(+) create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorLatestDepTest.groovy create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorTest.groovy create mode 100644 dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecoratorTest.groovy diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/JakartaRsAnnotationsDecorator.java b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/JakartaRsAnnotationsDecorator.java index 57d41ff0012..a2b3c66acea 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/JakartaRsAnnotationsDecorator.java +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/JakartaRsAnnotationsDecorator.java @@ -2,16 +2,19 @@ import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR; +import datadog.trace.api.Config; import datadog.trace.api.GenericClassValue; import datadog.trace.api.Pair; import datadog.trace.bootstrap.ClassHierarchyIterable; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.BaseDecorator; import jakarta.ws.rs.HttpMethod; import jakarta.ws.rs.Path; +import jakarta.ws.rs.WebApplicationException; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.Map; @@ -53,6 +56,34 @@ protected CharSequence component() { return JAKARTA_RS_CONTROLLER; } + @Override + protected void doOnError(final AgentSpan span, final Throwable throwable, byte errorPriority) { + // If the mapped status isn't one of the configured "server error" statuses, this isn't really + // an error from the caller's point of view (e.g. a 404 NotFoundException), even though a Java + // exception was thrown to get there. + Integer status = extractResponseStatus(throwable); + if (status != null) { + span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); + span.setError( + Config.get().getHttpServerErrorStatuses().get(status), + ErrorPriorities.HTTP_SERVER_DECORATOR); + return; + } + super.doOnError(span, throwable, errorPriority); + } + + // Walk the cause chain looking for a WebApplicationException, which carries the response the + // framework will actually send. + private static Integer extractResponseStatus(final Throwable throwable) { + Throwable current = throwable; + for (int depth = 0; current != null && depth < 5; depth++, current = current.getCause()) { + if (current instanceof WebApplicationException) { + return ((WebApplicationException) current).getResponse().getStatus(); + } + } + return null; + } + public void onJakartaRsSpan( final AgentSpan span, final AgentSpan parent, final Class target, final Method method) { diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/groovy/JakartaRsAnnotations3InstrumentationTest.groovy b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/groovy/JakartaRsAnnotations3InstrumentationTest.groovy index 2d68787a746..baea5c1fc88 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/groovy/JakartaRsAnnotations3InstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/groovy/JakartaRsAnnotations3InstrumentationTest.groovy @@ -11,6 +11,8 @@ import jakarta.ws.rs.PATCH import jakarta.ws.rs.POST import jakarta.ws.rs.PUT import jakarta.ws.rs.Path +import jakarta.ws.rs.WebApplicationException +import jakarta.ws.rs.core.Response import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace @@ -140,6 +142,117 @@ class JakartaRsAnnotations3InstrumentationTest extends InstrumentationSpecificat className = JakartaRsAnnotationsDecorator.DECORATE.className(obj.class) } + def "resource method exception with an embedded non-5xx status is not flagged as an error"() { + setup: + // jakarta-rs-annotations-3.0's test classpath has no JAX-RS runtime implementation, so + // WebApplicationException's convenience constructors (which build a Response via + // RuntimeDelegate) can't be used here. Stub the Response instead. + def response = Stub(Response) { + getStatus() >> 404 + getStatusInfo() >> Response.Status.NOT_FOUND + } + def obj = new Jakarta() { + @GET + @Path("/not-found") + void call() { + throw new WebApplicationException(response) + } + } + + when: + obj.call() + + then: + thrown(WebApplicationException) + assertTraces(1) { + trace(1) { + span { + operationName "jakarta-rs.request" + resourceName "GET /not-found" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "jakarta-rs-controller" + "$Tags.HTTP_ROUTE" "/not-found" + errorTags(WebApplicationException, "HTTP 404 Not Found") + defaultTags() + } + } + } + } + } + + def "resource method exception with an embedded 5xx status is flagged as an error"() { + setup: + def response = Stub(Response) { + getStatus() >> 500 + getStatusInfo() >> Response.Status.INTERNAL_SERVER_ERROR + } + def obj = new Jakarta() { + @GET + @Path("/internal-error") + void call() { + throw new WebApplicationException(response) + } + } + + when: + obj.call() + + then: + thrown(WebApplicationException) + assertTraces(1) { + trace(1) { + span { + operationName "jakarta-rs.request" + resourceName "GET /internal-error" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "jakarta-rs-controller" + "$Tags.HTTP_ROUTE" "/internal-error" + errorTags(WebApplicationException, "HTTP 500 Internal Server Error") + defaultTags() + } + } + } + } + } + + def "resource method with a plain exception is still flagged as an error"() { + setup: + def obj = new Jakarta() { + @GET + @Path("/boom") + void call() { + throw new IllegalStateException("boom") + } + } + + when: + obj.call() + + then: + thrown(IllegalStateException) + assertTraces(1) { + trace(1) { + span { + operationName "jakarta-rs.request" + resourceName "GET /boom" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "jakarta-rs-controller" + "$Tags.HTTP_ROUTE" "/boom" + errorTags(IllegalStateException, "boom") + defaultTags() + } + } + } + } + } + + def "no annotations has no effect"() { setup: def obj = new Jakarta() { diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java index 7186c507c13..caf106f9e14 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java @@ -2,10 +2,12 @@ import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR; +import datadog.trace.api.Config; import datadog.trace.api.GenericClassValue; import datadog.trace.api.Pair; import datadog.trace.bootstrap.ClassHierarchyIterable; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import datadog.trace.bootstrap.instrumentation.api.Tags; @@ -17,6 +19,7 @@ import java.util.concurrent.ConcurrentHashMap; import javax.ws.rs.HttpMethod; import javax.ws.rs.Path; +import javax.ws.rs.WebApplicationException; public class JaxRsAnnotationsDecorator extends BaseDecorator { public static JaxRsAnnotationsDecorator DECORATE = new JaxRsAnnotationsDecorator(); @@ -41,6 +44,34 @@ protected CharSequence component() { return JAX_RS_CONTROLLER; } + @Override + protected void doOnError(final AgentSpan span, final Throwable throwable, byte errorPriority) { + // If the mapped status isn't one of the configured "server error" statuses, this isn't really + // an error from the caller's point of view (e.g. a 404 NotFoundException), even though a Java + // exception was thrown to get there. + Integer status = extractResponseStatus(throwable); + if (status != null) { + span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); + span.setError( + Config.get().getHttpServerErrorStatuses().get(status), + ErrorPriorities.HTTP_SERVER_DECORATOR); + return; + } + super.doOnError(span, throwable, errorPriority); + } + + // Walk the cause chain looking for a WebApplicationException, which carries the response the + // framework will actually send. + private static Integer extractResponseStatus(final Throwable throwable) { + Throwable current = throwable; + for (int depth = 0; current != null && depth < 5; depth++, current = current.getCause()) { + if (current instanceof WebApplicationException) { + return ((WebApplicationException) current).getResponse().getStatus(); + } + } + return null; + } + public void onJaxRsSpan( final AgentSpan span, final AgentSpan parent, final Class target, final Method method) { diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy index f54ddde50f5..057f533d177 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy @@ -13,6 +13,7 @@ import javax.ws.rs.OPTIONS import javax.ws.rs.POST import javax.ws.rs.PUT import javax.ws.rs.Path +import javax.ws.rs.WebApplicationException class JaxRsAnnotations1InstrumentationTest extends InstrumentationSpecification { @@ -140,6 +141,105 @@ class JaxRsAnnotations1InstrumentationTest extends InstrumentationSpecification className = JaxRsAnnotationsDecorator.DECORATE.className(obj.class) } + def "resource method exception with an embedded non-5xx status is not flagged as an error"() { + setup: + def obj = new Jax() { + @GET + @Path("/not-found") + void call() { + throw new WebApplicationException(404) + } + } + + when: + obj.call() + + then: + thrown(WebApplicationException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /not-found" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/not-found" + errorTags(WebApplicationException) + defaultTags() + } + } + } + } + } + + def "resource method exception with an embedded 5xx status is flagged as an error"() { + setup: + def obj = new Jax() { + @GET + @Path("/internal-error") + void call() { + throw new WebApplicationException(500) + } + } + + when: + obj.call() + + then: + thrown(WebApplicationException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /internal-error" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/internal-error" + errorTags(WebApplicationException) + defaultTags() + } + } + } + } + } + + def "resource method with a plain exception is still flagged as an error"() { + setup: + def obj = new Jax() { + @GET + @Path("/boom") + void call() { + throw new IllegalStateException("boom") + } + } + + when: + obj.call() + + then: + thrown(IllegalStateException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /boom" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/boom" + errorTags(IllegalStateException, "boom") + defaultTags() + } + } + } + } + } + def "no annotations has no effect"() { setup: def obj = new Jax() { diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java index a7c8e82f2d8..be4933c798f 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java @@ -2,10 +2,12 @@ import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR; +import datadog.trace.api.Config; import datadog.trace.api.GenericClassValue; import datadog.trace.api.Pair; import datadog.trace.bootstrap.ClassHierarchyIterable; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import datadog.trace.bootstrap.instrumentation.api.Tags; @@ -17,6 +19,7 @@ import java.util.concurrent.ConcurrentHashMap; import javax.ws.rs.HttpMethod; import javax.ws.rs.Path; +import javax.ws.rs.WebApplicationException; public class JaxRsAnnotationsDecorator extends BaseDecorator { @@ -52,6 +55,34 @@ protected CharSequence component() { return JAX_RS_CONTROLLER; } + @Override + protected void doOnError(final AgentSpan span, final Throwable throwable, byte errorPriority) { + // If the mapped status isn't one of the configured "server error" statuses, this isn't really + // an error from the caller's point of view (e.g. a 404 NotFoundException), even though a Java + // exception was thrown to get there. + Integer status = extractResponseStatus(throwable); + if (status != null) { + span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); + span.setError( + Config.get().getHttpServerErrorStatuses().get(status), + ErrorPriorities.HTTP_SERVER_DECORATOR); + return; + } + super.doOnError(span, throwable, errorPriority); + } + + // Walk the cause chain looking for a WebApplicationException, which carries the response the + // framework will actually send. + private static Integer extractResponseStatus(final Throwable throwable) { + Throwable current = throwable; + for (int depth = 0; current != null && depth < 5; depth++, current = current.getCause()) { + if (current instanceof WebApplicationException) { + return ((WebApplicationException) current).getResponse().getStatus(); + } + } + return null; + } + public void onJaxRsSpan( final AgentSpan span, final AgentSpan parent, final Class target, final Method method) { diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy index fe542a6baac..e8d85955019 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy @@ -7,10 +7,12 @@ import io.dropwizard.jersey.PATCH import javax.ws.rs.DELETE import javax.ws.rs.GET import javax.ws.rs.HEAD +import javax.ws.rs.NotFoundException import javax.ws.rs.OPTIONS import javax.ws.rs.POST import javax.ws.rs.PUT import javax.ws.rs.Path +import javax.ws.rs.WebApplicationException import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace @@ -140,6 +142,105 @@ class JaxRsAnnotations2InstrumentationTest extends InstrumentationSpecification className = JaxRsAnnotationsDecorator.DECORATE.className(obj.class) } + def "resource method exception with an embedded non-5xx status is not flagged as an error"() { + setup: + def obj = new Jax() { + @GET + @Path("/not-found") + void call() { + throw new NotFoundException() + } + } + + when: + obj.call() + + then: + thrown(NotFoundException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /not-found" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/not-found" + errorTags(NotFoundException, "HTTP 404 Not Found") + defaultTags() + } + } + } + } + } + + def "resource method exception with an embedded 5xx status is flagged as an error"() { + setup: + def obj = new Jax() { + @GET + @Path("/internal-error") + void call() { + throw new WebApplicationException(500) + } + } + + when: + obj.call() + + then: + thrown(WebApplicationException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /internal-error" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/internal-error" + errorTags(WebApplicationException, "HTTP 500 Internal Server Error") + defaultTags() + } + } + } + } + } + + def "resource method with a plain exception is still flagged as an error"() { + setup: + def obj = new Jax() { + @GET + @Path("/boom") + void call() { + throw new IllegalStateException("boom") + } + } + + when: + obj.call() + + then: + thrown(IllegalStateException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /boom" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/boom" + errorTags(IllegalStateException, "boom") + defaultTags() + } + } + } + } + } + def "no annotations has no effect"() { setup: def obj = new Jax() { diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorLatestDepTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorLatestDepTest.groovy new file mode 100644 index 00000000000..a9342137ca7 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/latestDepTest/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorLatestDepTest.groovy @@ -0,0 +1,77 @@ +package datadog.trace.instrumentation.springweb + +import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.bootstrap.instrumentation.api.AgentTracer +import datadog.trace.bootstrap.instrumentation.api.Tags +import org.springframework.http.HttpStatus +import org.springframework.web.server.ResponseStatusException + +import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE + +// ResponseStatusException was added in Spring 5.0, which is only guaranteed to be on the +// classpath for the latestDepTest suite (this module's base test classpath is Spring 3.1). This +// exercises the reflective ResponseStatusException handling in SpringWebHttpServerDecorator. +class SpringWebHttpServerDecoratorLatestDepTest extends InstrumentationSpecification { + + def "ResponseStatusException with a non-5xx status is not flagged as an error"() { + setup: + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + def throwable = new ResponseStatusException(HttpStatus.NOT_FOUND) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(ResponseStatusException, throwable.message) + defaultTags() + } + } + } + } + } + + def "ResponseStatusException with a 5xx status is flagged as an error"() { + setup: + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + def throwable = new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(ResponseStatusException, throwable.message) + defaultTags() + } + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java index 473b7b2b050..9325e7643c0 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java @@ -3,8 +3,11 @@ import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR; import datadog.context.Context; +import datadog.trace.api.Config; +import datadog.trace.bootstrap.ClassHierarchyIterable; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; @@ -12,7 +15,9 @@ import javax.servlet.Servlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.springframework.http.HttpStatus; import org.springframework.web.HttpRequestHandler; +import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.ModelAndView; @@ -21,6 +26,38 @@ public class SpringWebHttpServerDecorator extends HttpServerDecorator { + // ResponseStatusException was added in Spring 5.0; this module also supports Spring 3.1-4.x, + // so it can't be referenced directly. Resolved reflectively, once, and reused. + private static final Method RESPONSE_STATUS_EXCEPTION_GET_STATUS = + findResponseStatusExceptionGetStatus(); + + private static Method findResponseStatusExceptionGetStatus() { + try { + Class responseStatusExceptionClass = + Class.forName( + "org.springframework.web.server.ResponseStatusException", + false, + SpringWebHttpServerDecorator.class.getClassLoader()); + return responseStatusExceptionClass.getMethod("getStatus"); + } catch (ClassNotFoundException | NoSuchMethodException e) { + return null; + } + } + + // ResponseStatus#code() was added in Spring 4.2 as an alias of value(); this module compiles + // against Spring 3.1, so it can't be referenced directly. Resolved reflectively, once, and + // reused. Plain reflection doesn't resolve Spring's @AliasFor, so if a caller only set code(), + // value() still reports its own default rather than the value mirrored from code(). + private static final Method RESPONSE_STATUS_CODE = findResponseStatusCode(); + + private static Method findResponseStatusCode() { + try { + return ResponseStatus.class.getMethod("code"); + } catch (NoSuchMethodException e) { + return null; + } + } + private static final CharSequence SPRING_HANDLER = UTF8BytesString.create("spring.handler"); public static final CharSequence RESPONSE_RENDER = UTF8BytesString.create("response.render"); @@ -97,6 +134,61 @@ protected String getRequestHeader(final HttpServletRequest request, String key) return request.getHeader(key); } + @Override + protected void doOnError(final AgentSpan span, final Throwable throwable, byte errorPriority) { + // Walk the cause chain looking for a status the exception itself carries (@ResponseStatus, or + // a ResponseStatusException on Spring 5+). If the mapped status isn't one of the configured + // "server error" statuses, this isn't really an error from the caller's point of view (e.g. a + // 404 mapping), even though a Java exception was thrown to get there. + Integer status = extractResponseStatus(throwable); + if (status != null) { + span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); + span.setError( + Config.get().getHttpServerErrorStatuses().get(status), + ErrorPriorities.HTTP_SERVER_DECORATOR); + return; + } + super.doOnError(span, throwable, errorPriority); + } + + private static Integer extractResponseStatus(final Throwable throwable) { + Throwable current = throwable; + for (int depth = 0; current != null && depth < 5; depth++, current = current.getCause()) { + if (RESPONSE_STATUS_EXCEPTION_GET_STATUS != null + && RESPONSE_STATUS_EXCEPTION_GET_STATUS.getDeclaringClass().isInstance(current)) { + try { + Object httpStatus = RESPONSE_STATUS_EXCEPTION_GET_STATUS.invoke(current); + if (httpStatus instanceof HttpStatus) { + return ((HttpStatus) httpStatus).value(); + } + } catch (Throwable ignored) { + // fall through to the @ResponseStatus check below + } + } + for (Class type : new ClassHierarchyIterable(current.getClass())) { + ResponseStatus responseStatus = type.getAnnotation(ResponseStatus.class); + if (responseStatus != null) { + return responseStatusCode(responseStatus); + } + } + } + return null; + } + + private static int responseStatusCode(final ResponseStatus responseStatus) { + if (RESPONSE_STATUS_CODE != null) { + try { + HttpStatus code = (HttpStatus) RESPONSE_STATUS_CODE.invoke(responseStatus); + if (code != HttpStatus.INTERNAL_SERVER_ERROR) { + return code.value(); + } + } catch (Throwable ignored) { + // fall through to value() below + } + } + return responseStatus.value().value(); + } + @Override protected void doOnRequest( final AgentSpan span, diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorTest.groovy new file mode 100644 index 00000000000..31509a58362 --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorTest.groovy @@ -0,0 +1,124 @@ +package datadog.trace.instrumentation.springweb + +import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.bootstrap.instrumentation.api.AgentTracer +import datadog.trace.bootstrap.instrumentation.api.Tags +import org.springframework.http.HttpStatus +import org.springframework.web.bind.annotation.ResponseStatus + +import static datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator.DECORATE + +// ResponseStatusException was added in Spring 5.0 and isn't available on this module's base +// (Spring 3.1) test classpath. Its handling is covered separately, on the latestDepTest +// classpath, by SpringWebHttpServerDecoratorLatestDepTest. +class SpringWebHttpServerDecoratorTest extends InstrumentationSpecification { + + @ResponseStatus(HttpStatus.NOT_FOUND) + static class CustomNotFoundException extends RuntimeException { + } + + @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) + static class CustomServerErrorException extends RuntimeException { + } + + // Spring's @AliasFor("value") on ResponseStatus#code isn't honored by plain reflection, so + // setting only code() (rather than value()) exercises a separate code path. + @ResponseStatus(code = HttpStatus.NOT_FOUND) + static class CustomNotFoundExceptionUsingCodeAttribute extends RuntimeException { + } + + def "exception with an embedded non-5xx status is not flagged as an error"() { + setup: + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(throwable.class) + defaultTags() + } + } + } + } + + where: + throwable << [new CustomNotFoundException(), new CustomNotFoundExceptionUsingCodeAttribute()] + } + + def "exception with an embedded 5xx status is flagged as an error"() { + setup: + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + def throwable = new CustomServerErrorException() + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(CustomServerErrorException) + defaultTags() + } + } + } + } + } + + def "plain exception is still flagged as an error"() { + setup: + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + def throwable = new IllegalStateException("boom") + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(IllegalStateException, "boom") + defaultTags() + } + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecorator.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecorator.java index 17f0e93bbe2..eb784cc6ba0 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecorator.java @@ -3,8 +3,11 @@ import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR; import datadog.context.Context; +import datadog.trace.api.Config; +import datadog.trace.bootstrap.ClassHierarchyIterable; import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; @@ -12,7 +15,11 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import java.lang.reflect.Method; +import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.web.ErrorResponse; import org.springframework.web.HttpRequestHandler; +import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.ModelAndView; @@ -101,6 +108,54 @@ protected int status(final HttpServletResponse httpServletResponse) { return httpServletResponse.getStatus(); } + @Override + protected void doOnError(final AgentSpan span, final Throwable throwable, byte errorPriority) { + // Walk the cause chain looking for a status the exception itself carries (@ResponseStatus, + // ResponseStatusException, or any other ErrorResponse such as NoResourceFoundException). If + // the mapped status isn't one of the configured "server error" statuses, this isn't really an + // error from the caller's point of view (e.g. a 404 mapping), even though a Java exception + // was thrown to get there. + Integer status = extractResponseStatus(throwable); + if (status != null) { + span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); + span.setError( + Config.get().getHttpServerErrorStatuses().get(status), + ErrorPriorities.HTTP_SERVER_DECORATOR); + return; + } + super.doOnError(span, throwable, errorPriority); + } + + private static Integer extractResponseStatus(final Throwable throwable) { + Throwable current = throwable; + for (int depth = 0; current != null && depth < 5; depth++, current = current.getCause()) { + if (current instanceof ErrorResponse) { + HttpStatusCode statusCode = ((ErrorResponse) current).getStatusCode(); + if (statusCode != null) { + return statusCode.value(); + } + } + for (Class type : new ClassHierarchyIterable(current.getClass())) { + ResponseStatus responseStatus = type.getAnnotation(ResponseStatus.class); + if (responseStatus != null) { + return responseStatusCode(responseStatus); + } + } + } + return null; + } + + private static int responseStatusCode(final ResponseStatus responseStatus) { + // value() and code() are @AliasFor each other, but plain reflection doesn't resolve + // @AliasFor: if a caller only set code(), value() still reports its own default rather than + // the value mirrored from code(). Prefer code() whenever it was explicitly set. + HttpStatus code = responseStatus.code(); + if (code != HttpStatus.INTERNAL_SERVER_ERROR) { + return code.value(); + } + return responseStatus.value().value(); + } + @Override protected void doOnRequest( final AgentSpan span, diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecoratorTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecoratorTest.groovy new file mode 100644 index 00000000000..c082169631c --- /dev/null +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecoratorTest.groovy @@ -0,0 +1,134 @@ +package datadog.trace.instrumentation.springweb6 + +import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.bootstrap.instrumentation.api.AgentTracer +import datadog.trace.bootstrap.instrumentation.api.Tags +import org.springframework.http.HttpStatus +import org.springframework.web.ErrorResponseException +import org.springframework.web.bind.annotation.ResponseStatus +import org.springframework.web.server.ResponseStatusException + +import static datadog.trace.instrumentation.springweb6.SpringWebHttpServerDecorator.DECORATE + +class SpringWebHttpServerDecoratorTest extends InstrumentationSpecification { + + @ResponseStatus(HttpStatus.NOT_FOUND) + static class CustomNotFoundException extends RuntimeException { + } + + @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) + static class CustomServerErrorException extends RuntimeException { + } + + // Spring's @AliasFor("value") on ResponseStatus#code isn't honored by plain reflection, so + // setting only code() (rather than value()) exercises a separate code path. + @ResponseStatus(code = HttpStatus.NOT_FOUND) + static class CustomNotFoundExceptionUsingCodeAttribute extends RuntimeException { + } + + def "exception with an embedded non-5xx status is not flagged as an error"() { + setup: + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(throwable.class, throwable.message) + defaultTags() + } + } + } + } + + where: + throwable << [ + new CustomNotFoundException(), + new CustomNotFoundExceptionUsingCodeAttribute(), + new ResponseStatusException(HttpStatus.NOT_FOUND), + new ErrorResponseException(HttpStatus.NOT_FOUND) + ] + } + + def "exception with an embedded 5xx status is flagged as an error"() { + setup: + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(throwable.class, throwable.message) + defaultTags() + } + } + } + } + + where: + throwable << [ + new CustomServerErrorException(), + new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR), + new ErrorResponseException(HttpStatus.INTERNAL_SERVER_ERROR) + ] + } + + def "plain exception is still flagged as an error"() { + setup: + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + def throwable = new IllegalStateException("boom") + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(IllegalStateException, "boom") + defaultTags() + } + } + } + } + } +} From 93e9b3bc3cbcec1f773d007f4d923142065b71f1 Mon Sep 17 00:00:00 2001 From: Jeremy Katz Date: Thu, 20 Aug 2026 15:29:28 -0400 Subject: [PATCH 2/2] Add opt-in config to recognize custom exception types that carry an HTTP status Some applications signal a response status via their own exception hierarchy and a generic exception-handling advice, rather than any of the JAX-RS/Spring conventions already handled (WebApplicationException, ResponseStatusException, ErrorResponse, @ResponseStatus). Those exceptions still get unconditionally flagged as errors even when they map to a routine non-5xx response, since the tracer has no way to know what status they carry. DD_TRACE_RESPONSE_STATUS_EXCEPTIONS / trace.response-status.exceptions lets users declare a list of fully.qualified.ExceptionClass#accessorMethod entries; when a thrown exception (or a subclass of one) matches, the named no-arg accessor is invoked reflectively and its numeric return value is used the same way as the built-in status extraction. This keeps the change fully opt-in and precise - it only ever reflects on classes/methods a user explicitly named, rather than guessing at common accessor names across arbitrary exceptions and risking a genuine error being silently cleared. A misconfigured accessor could return a value outside the valid HTTP status range (e.g. -1 as an unknown-status sentinel). That value would otherwise flow straight into Config#getHttpServerErrorStatuses, a BitSet indexed by status code, which throws on a negative index, aborting normal error handling for the span entirely. The accessor return value is now validated as a plausible HTTP status (100-599) before use, falling back to normal error handling otherwise. --- .../ConfiguredResponseStatusExceptions.java | 60 ++++++++++ .../JakartaRsAnnotationsDecorator.java | 4 + ...taRsAnnotations3InstrumentationTest.groovy | 80 +++++++++++++ .../jaxrs1/JaxRsAnnotationsDecorator.java | 4 + ...axRsAnnotations1InstrumentationTest.groovy | 80 +++++++++++++ .../jaxrs2/JaxRsAnnotationsDecorator.java | 4 + ...axRsAnnotations2InstrumentationTest.groovy | 80 +++++++++++++ .../SpringWebHttpServerDecorator.java | 4 + .../SpringWebHttpServerDecoratorTest.groovy | 111 ++++++++++++++++++ .../SpringWebHttpServerDecorator.java | 4 + .../SpringWebHttpServerDecoratorTest.groovy | 77 ++++++++++++ .../config/TraceInstrumentationConfig.java | 9 ++ .../main/java/datadog/trace/api/Config.java | 11 ++ .../datadog/trace/api/ConfigTest.groovy | 24 ++++ metadata/supported-configurations.json | 8 ++ 15 files changed, 560 insertions(+) create mode 100644 dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ConfiguredResponseStatusExceptions.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ConfiguredResponseStatusExceptions.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ConfiguredResponseStatusExceptions.java new file mode 100644 index 00000000000..e15254ba2c9 --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ConfiguredResponseStatusExceptions.java @@ -0,0 +1,60 @@ +package datadog.trace.bootstrap.instrumentation.decorator; + +import datadog.trace.api.Config; +import datadog.trace.bootstrap.ClassHierarchyIterable; +import java.util.Map; + +/** + * Lets users teach the tracer about their own, framework-agnostic exception types via {@code + * trace.response-status.exceptions} ({@code DD_TRACE_RESPONSE_STATUS_EXCEPTIONS}): a list of {@code + * fully.qualified.ExceptionClass#accessorMethod} entries. When a configured exception (or a + * subclass of one) is thrown from a request handler, the named no-arg accessor is invoked + * reflectively and its numeric return value is used as the HTTP status for deciding whether the + * span should be flagged as an error, instead of unconditionally marking it as an error. + * + *

Only ever reflects on classes/methods the user explicitly named, unlike a generic heuristic + * that would probe arbitrary exceptions for common accessor names and risk silently clearing a + * genuine error whose exception happens to have a same-named, unrelated method. + */ +public final class ConfiguredResponseStatusExceptions { + + public static Integer extractStatus(final Throwable throwable) { + Map accessors = Config.get().getResponseStatusExceptionAccessors(); + if (accessors.isEmpty()) { + return null; + } + Throwable current = throwable; + for (int depth = 0; current != null && depth < 5; depth++, current = current.getCause()) { + for (Class type : new ClassHierarchyIterable(current.getClass())) { + String methodName = accessors.get(type.getName()); + if (methodName != null) { + Integer status = invoke(current, methodName); + if (status != null) { + return status; + } + } + } + } + return null; + } + + private static Integer invoke(final Throwable throwable, final String methodName) { + try { + Object result = throwable.getClass().getMethod(methodName).invoke(throwable); + if (result instanceof Number) { + int status = ((Number) result).intValue(); + // Guard against sentinel values (e.g. -1 for "unknown") reflectively returned by a + // misconfigured accessor: an out-of-range status would otherwise throw when later used + // to index into Config#getHttpServerErrorStatuses, aborting normal error handling. + if (status >= 100 && status <= 599) { + return status; + } + } + } catch (Throwable ignored) { + // misconfigured entry (wrong method name, non-numeric return, etc.) -- fall through + } + return null; + } + + private ConfiguredResponseStatusExceptions() {} +} diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/JakartaRsAnnotationsDecorator.java b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/JakartaRsAnnotationsDecorator.java index a2b3c66acea..22615f863ea 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/JakartaRsAnnotationsDecorator.java +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/JakartaRsAnnotationsDecorator.java @@ -12,6 +12,7 @@ import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.BaseDecorator; +import datadog.trace.bootstrap.instrumentation.decorator.ConfiguredResponseStatusExceptions; import jakarta.ws.rs.HttpMethod; import jakarta.ws.rs.Path; import jakarta.ws.rs.WebApplicationException; @@ -62,6 +63,9 @@ protected void doOnError(final AgentSpan span, final Throwable throwable, byte e // an error from the caller's point of view (e.g. a 404 NotFoundException), even though a Java // exception was thrown to get there. Integer status = extractResponseStatus(throwable); + if (status == null) { + status = ConfiguredResponseStatusExceptions.extractStatus(throwable); + } if (status != null) { span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); span.setError( diff --git a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/groovy/JakartaRsAnnotations3InstrumentationTest.groovy b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/groovy/JakartaRsAnnotations3InstrumentationTest.groovy index baea5c1fc88..933f79bc3fd 100644 --- a/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/groovy/JakartaRsAnnotations3InstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/test/groovy/JakartaRsAnnotations3InstrumentationTest.groovy @@ -253,6 +253,74 @@ class JakartaRsAnnotations3InstrumentationTest extends InstrumentationSpecificat } + def "resource method exception recognized via configured custom accessor with a non-5xx status is not flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def obj = new Jakarta() { + @GET + @Path("/custom-not-found") + void call() { + throw new CustomStatusException(404) + } + } + + when: + obj.call() + + then: + thrown(CustomStatusException) + assertTraces(1) { + trace(1) { + span { + operationName "jakarta-rs.request" + resourceName "GET /custom-not-found" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "jakarta-rs-controller" + "$Tags.HTTP_ROUTE" "/custom-not-found" + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + + def "resource method exception recognized via configured custom accessor with a 5xx status is flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def obj = new Jakarta() { + @GET + @Path("/custom-internal-error") + void call() { + throw new CustomStatusException(500) + } + } + + when: + obj.call() + + then: + thrown(CustomStatusException) + assertTraces(1) { + trace(1) { + span { + operationName "jakarta-rs.request" + resourceName "GET /custom-internal-error" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "jakarta-rs-controller" + "$Tags.HTTP_ROUTE" "/custom-internal-error" + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + def "no annotations has no effect"() { setup: def obj = new Jakarta() { @@ -277,6 +345,18 @@ class JakartaRsAnnotations3InstrumentationTest extends InstrumentationSpecificat } } + static class CustomStatusException extends RuntimeException { + private final int status + + CustomStatusException(int status) { + this.status = status + } + + int httpCode() { + return status + } + } + interface Jakarta { void call() } diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java index caf106f9e14..b440b5b6f4b 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java @@ -13,6 +13,7 @@ import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.BaseDecorator; +import datadog.trace.bootstrap.instrumentation.decorator.ConfiguredResponseStatusExceptions; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.Map; @@ -50,6 +51,9 @@ protected void doOnError(final AgentSpan span, final Throwable throwable, byte e // an error from the caller's point of view (e.g. a 404 NotFoundException), even though a Java // exception was thrown to get there. Integer status = extractResponseStatus(throwable); + if (status == null) { + status = ConfiguredResponseStatusExceptions.extractStatus(throwable); + } if (status != null) { span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); span.setError( diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy index 057f533d177..83c2f9f70aa 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-1.1.1/src/test/groovy/JaxRsAnnotations1InstrumentationTest.groovy @@ -240,6 +240,74 @@ class JaxRsAnnotations1InstrumentationTest extends InstrumentationSpecification } } + def "resource method exception recognized via configured custom accessor with a non-5xx status is not flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def obj = new Jax() { + @GET + @Path("/custom-not-found") + void call() { + throw new CustomStatusException(404) + } + } + + when: + obj.call() + + then: + thrown(CustomStatusException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /custom-not-found" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/custom-not-found" + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + + def "resource method exception recognized via configured custom accessor with a 5xx status is flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def obj = new Jax() { + @GET + @Path("/custom-internal-error") + void call() { + throw new CustomStatusException(500) + } + } + + when: + obj.call() + + then: + thrown(CustomStatusException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /custom-internal-error" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/custom-internal-error" + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + def "no annotations has no effect"() { setup: def obj = new Jax() { @@ -264,6 +332,18 @@ class JaxRsAnnotations1InstrumentationTest extends InstrumentationSpecification } } + static class CustomStatusException extends RuntimeException { + private final int status + + CustomStatusException(int status) { + this.status = status + } + + int httpCode() { + return status + } + } + interface Jax { void call() } diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java index be4933c798f..397665b59ab 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/main/java/datadog/trace/instrumentation/jaxrs2/JaxRsAnnotationsDecorator.java @@ -13,6 +13,7 @@ import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.BaseDecorator; +import datadog.trace.bootstrap.instrumentation.decorator.ConfiguredResponseStatusExceptions; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.Map; @@ -61,6 +62,9 @@ protected void doOnError(final AgentSpan span, final Throwable throwable, byte e // an error from the caller's point of view (e.g. a 404 NotFoundException), even though a Java // exception was thrown to get there. Integer status = extractResponseStatus(throwable); + if (status == null) { + status = ConfiguredResponseStatusExceptions.extractStatus(throwable); + } if (status != null) { span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); span.setError( diff --git a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy index e8d85955019..e2c7074e4b1 100644 --- a/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/src/test/groovy/JaxRsAnnotations2InstrumentationTest.groovy @@ -241,6 +241,74 @@ class JaxRsAnnotations2InstrumentationTest extends InstrumentationSpecification } } + def "resource method exception recognized via configured custom accessor with a non-5xx status is not flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def obj = new Jax() { + @GET + @Path("/custom-not-found") + void call() { + throw new CustomStatusException(404) + } + } + + when: + obj.call() + + then: + thrown(CustomStatusException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /custom-not-found" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/custom-not-found" + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + + def "resource method exception recognized via configured custom accessor with a 5xx status is flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def obj = new Jax() { + @GET + @Path("/custom-internal-error") + void call() { + throw new CustomStatusException(500) + } + } + + when: + obj.call() + + then: + thrown(CustomStatusException) + assertTraces(1) { + trace(1) { + span { + operationName "jax-rs.request" + resourceName "GET /custom-internal-error" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "jax-rs-controller" + "$Tags.HTTP_ROUTE" "/custom-internal-error" + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + def "no annotations has no effect"() { setup: def obj = new Jax() { @@ -265,6 +333,18 @@ class JaxRsAnnotations2InstrumentationTest extends InstrumentationSpecification } } + static class CustomStatusException extends RuntimeException { + private final int status + + CustomStatusException(int status) { + this.status = status + } + + int httpCode() { + return status + } + } + interface Jax { void call() } diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java index 9325e7643c0..96c33e8f3cc 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecorator.java @@ -10,6 +10,7 @@ import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import datadog.trace.bootstrap.instrumentation.decorator.ConfiguredResponseStatusExceptions; import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; import java.lang.reflect.Method; import javax.servlet.Servlet; @@ -141,6 +142,9 @@ protected void doOnError(final AgentSpan span, final Throwable throwable, byte e // "server error" statuses, this isn't really an error from the caller's point of view (e.g. a // 404 mapping), even though a Java exception was thrown to get there. Integer status = extractResponseStatus(throwable); + if (status == null) { + status = ConfiguredResponseStatusExceptions.extractStatus(throwable); + } if (status != null) { span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); span.setError( diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorTest.groovy index 31509a58362..a49c90d9132 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorTest.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/datadog/trace/instrumentation/springweb/SpringWebHttpServerDecoratorTest.groovy @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.springweb import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.api.config.TraceInstrumentationConfig import datadog.trace.bootstrap.instrumentation.api.AgentTracer import datadog.trace.bootstrap.instrumentation.api.Tags import org.springframework.http.HttpStatus @@ -27,6 +28,18 @@ class SpringWebHttpServerDecoratorTest extends InstrumentationSpecification { static class CustomNotFoundExceptionUsingCodeAttribute extends RuntimeException { } + static class CustomStatusException extends RuntimeException { + private final int status + + CustomStatusException(int status) { + this.status = status + } + + int httpCode() { + return status + } + } + def "exception with an embedded non-5xx status is not flagged as an error"() { setup: def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") @@ -91,6 +104,104 @@ class SpringWebHttpServerDecoratorTest extends InstrumentationSpecification { } } + def "exception recognized via configured custom accessor with a non-5xx status is not flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + def throwable = new CustomStatusException(404) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + + def "exception recognized via configured custom accessor with a 5xx status is flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + def throwable = new CustomStatusException(500) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + + def "exception recognized via configured custom accessor with an out-of-range status falls back to normal error handling"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + // -1 is a plausible "unknown status" sentinel a misconfigured accessor could return; it must + // not be used to index into the configured server-error statuses. + def throwable = new CustomStatusException(-1) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + def "plain exception is still flagged as an error"() { setup: def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecorator.java b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecorator.java index eb784cc6ba0..14252a48042 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecorator.java @@ -10,6 +10,7 @@ import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import datadog.trace.bootstrap.instrumentation.decorator.ConfiguredResponseStatusExceptions; import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator; import jakarta.servlet.Servlet; import jakarta.servlet.http.HttpServletRequest; @@ -116,6 +117,9 @@ protected void doOnError(final AgentSpan span, final Throwable throwable, byte e // error from the caller's point of view (e.g. a 404 mapping), even though a Java exception // was thrown to get there. Integer status = extractResponseStatus(throwable); + if (status == null) { + status = ConfiguredResponseStatusExceptions.extractStatus(throwable); + } if (status != null) { span.addThrowable(throwable, ErrorPriorities.HTTP_SERVER_DECORATOR); span.setError( diff --git a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecoratorTest.groovy b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecoratorTest.groovy index c082169631c..3e139b8bb7b 100644 --- a/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecoratorTest.groovy +++ b/dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/SpringWebHttpServerDecoratorTest.groovy @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.springweb6 import datadog.trace.agent.test.InstrumentationSpecification +import datadog.trace.api.config.TraceInstrumentationConfig import datadog.trace.bootstrap.instrumentation.api.AgentTracer import datadog.trace.bootstrap.instrumentation.api.Tags import org.springframework.http.HttpStatus @@ -26,6 +27,18 @@ class SpringWebHttpServerDecoratorTest extends InstrumentationSpecification { static class CustomNotFoundExceptionUsingCodeAttribute extends RuntimeException { } + static class CustomStatusException extends RuntimeException { + private final int status + + CustomStatusException(int status) { + this.status = status + } + + int httpCode() { + return status + } + } + def "exception with an embedded non-5xx status is not flagged as an error"() { setup: def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") @@ -101,6 +114,70 @@ class SpringWebHttpServerDecoratorTest extends InstrumentationSpecification { ] } + def "exception recognized via configured custom accessor with a non-5xx status is not flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + def throwable = new CustomStatusException(404) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored false + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + + def "exception recognized via configured custom accessor with a 5xx status is flagged as an error"() { + setup: + injectSysConfig(TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS, "${CustomStatusException.name}#httpCode") + def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") + def scope = AgentTracer.activateSpan(testSpan) + DECORATE.afterStart(testSpan) + def throwable = new CustomStatusException(500) + + when: + DECORATE.onError(testSpan, throwable) + DECORATE.beforeFinish(testSpan) + scope.close() + testSpan.finish() + + then: + assertTraces(1) { + trace(1) { + span { + operationName "spring.handler" + spanType "web" + errored true + tags { + "$Tags.COMPONENT" "spring-web-controller" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + errorTags(CustomStatusException) + defaultTags() + } + } + } + } + } + def "plain exception is still flagged as an error"() { setup: def testSpan = AgentTracer.startSpan("spring-web-controller", "spring.handler") diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java index 855a7243caf..b18ce173231 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java @@ -198,6 +198,15 @@ public final class TraceInstrumentationConfig { "trace.jax-rs.exception-as-error.enabled"; public static final String JAX_RS_ADDITIONAL_ANNOTATIONS = "trace.jax-rs.additional.annotations"; + /** + * Comma-separated list of {@code fully.qualified.ExceptionClass#accessorMethod} entries. When a + * JAX-RS/Spring MVC handler throws an exception matching one of these entries (or a subclass of + * one), the named no-arg accessor method is invoked reflectively and its numeric return value is + * used as the HTTP status for deciding whether the span should be flagged as an error, instead of + * unconditionally marking it as an error. + */ + public static final String RESPONSE_STATUS_EXCEPTIONS = "trace.response-status.exceptions"; + /** If set, the instrumentation will set its resource name on the local root too. */ public static final String AXIS_PROMOTE_RESOURCE_NAME = "trace.axis.promote.resource-name"; diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index fade2b4c417..dcc68f1a99d 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -624,6 +624,7 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.RABBIT_PROPAGATION_DISABLED_QUEUES; import static datadog.trace.api.config.TraceInstrumentationConfig.RESILIENCE4J_MEASURED_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.RESILIENCE4J_TAG_METRICS_ENABLED; +import static datadog.trace.api.config.TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS; import static datadog.trace.api.config.TraceInstrumentationConfig.SERVLET_ASYNC_TIMEOUT_ERROR; import static datadog.trace.api.config.TraceInstrumentationConfig.SERVLET_PRINCIPAL_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.SERVLET_ROOT_CONTEXT_SERVICE_NAME; @@ -1305,6 +1306,8 @@ public static String getHostName() { private final Set jmsPropagationDisabledQueues; private final int jmsUnacknowledgedMaxAge; + private final Map responseStatusExceptionAccessors; + private final boolean rabbitPropagationEnabled; private final Set rabbitPropagationDisabledQueues; private final Set rabbitPropagationDisabledExchanges; @@ -3097,6 +3100,8 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) tryMakeImmutableSet(configProvider.getList(JMS_PROPAGATION_DISABLED_QUEUES)); jmsUnacknowledgedMaxAge = configProvider.getInteger(JMS_UNACKNOWLEDGED_MAX_AGE, 3600); + responseStatusExceptionAccessors = configProvider.getMergedMap(RESPONSE_STATUS_EXCEPTIONS, '#'); + rabbitPropagationEnabled = isPropagationEnabled(true, "rabbit", "rabbitmq"); rabbitPropagationDisabledQueues = tryMakeImmutableSet(configProvider.getList(RABBIT_PROPAGATION_DISABLED_QUEUES)); @@ -3690,6 +3695,10 @@ public Map getResponseHeaderTags() { return responseHeaderTags; } + public Map getResponseStatusExceptionAccessors() { + return responseStatusExceptionAccessors; + } + public boolean isRequestHeaderTagsCommaAllowed() { return requestHeaderTagsCommaAllowed; } @@ -6809,6 +6818,8 @@ public String toString() { + jmsPropagationDisabledTopics + ", jmsPropagationDisabledQueues=" + jmsPropagationDisabledQueues + + ", responseStatusExceptionAccessors=" + + responseStatusExceptionAccessors + ", rabbitPropagationEnabled=" + rabbitPropagationEnabled + ", rabbitPropagationDisabledQueues=" diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index ef95d5e902c..9d649d55ce8 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -101,6 +101,7 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN +import static datadog.trace.api.config.TraceInstrumentationConfig.RESPONSE_STATUS_EXCEPTIONS import static datadog.trace.api.config.TraceInstrumentationConfig.RUNTIME_CONTEXT_FIELD_INJECTION import static datadog.trace.api.config.TraceInstrumentationConfig.TRACE_ENABLED import static datadog.trace.api.config.TracerConfig.AGENT_HOST @@ -1523,6 +1524,29 @@ class ConfigTest extends DDSpecification { // spotless:on } + def "verify response status exceptions config on tracer"() { + setup: + System.setProperty(PREFIX + RESPONSE_STATUS_EXCEPTIONS, propString) + def props = new Properties() + props.setProperty(RESPONSE_STATUS_EXCEPTIONS, propString) + + when: + def config = new Config() + def propConfig = Config.get(props) + + then: + config.responseStatusExceptionAccessors == expected + propConfig.responseStatusExceptionAccessors == expected + + where: + // spotless:off + propString | expected + "" | [:] + "some.pkg.MyException#httpCode" | ["some.pkg.MyException": "httpCode"] + "some.pkg.MyException#httpCode,other.pkg.OtherException#getStatusCode" | ["some.pkg.MyException": "httpCode", "other.pkg.OtherException": "getStatusCode"] + // spotless:on + } + def "verify integer range configs on tracer"() { setup: System.setProperty(PREFIX + HTTP_SERVER_ERROR_STATUSES, value) diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index a1459dc3453..487230022c3 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -9577,6 +9577,14 @@ "aliases": [] } ], + "DD_TRACE_RESPONSE_STATUS_EXCEPTIONS": [ + { + "version": "A", + "type": "map", + "default": null, + "aliases": [] + } + ], "DD_TRACE_RESTEASY_ENABLED": [ { "version": "A",