Stop flagging JAX-RS/Spring MVC spans as errors when the exception maps to a non-5xx response

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.
+ *
+ * <p>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<String, String> 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() {}
+}

dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/src/main/java/datadog/trace/instrumentation/jakarta3/JakartaRsAnnotationsDecorator.java

@@ -2,16 +2,20 @@
 
 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 datadog.trace.bootstrap.instrumentation.decorator.ConfiguredResponseStatusExceptions;
 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 +57,37 @@ 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) {
+      status = ConfiguredResponseStatusExceptions.extractStatus(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) {
 

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,185 @@ 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 "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() {
@@ -164,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()
   }

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,21 +2,25 @@
 
 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;
 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;
 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 +45,37 @@ 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) {
+      status = ConfiguredResponseStatusExceptions.extractStatus(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) {