diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java index d40f574c85c..47b52acc51e 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java @@ -21,6 +21,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.InternalSpanTypes; import datadog.trace.bootstrap.instrumentation.api.PathwayContext; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; @@ -34,6 +35,7 @@ import java.util.BitSet; import java.util.LinkedHashMap; import java.util.Map; +import java.util.concurrent.ExecutionException; import java.util.function.BiFunction; import java.util.function.Function; import java.util.function.Supplier; @@ -292,7 +294,7 @@ public AgentSpan onResponseStatus(final AgentSpan span, final int status) { // even if the server chooses not to respond with 5xx to an error. // Anyway, we def don't want it applied to blocked requests if (!BlockingException.class.getName().equals(span.getTag("error.type"))) { - span.setError(SERVER_ERROR_STATUSES.get(status)); + span.setError(SERVER_ERROR_STATUSES.get(status), ErrorPriorities.HTTP_SERVER_DECORATOR); } if (SHOULD_SET_404_RESOURCE_NAME && status == 404) { @@ -363,6 +365,16 @@ private AgentSpan.Context.Extracted callIGCallbackStart(AgentSpan.Context.Extrac return context; } + @Override + public AgentSpan onError(final AgentSpan span, final Throwable throwable) { + if (throwable != null) { + span.addThrowable( + throwable instanceof ExecutionException ? throwable.getCause() : throwable, + ErrorPriorities.HTTP_SERVER_DECORATOR); + } + return span; + } + private Flow callIGCallbackRequestHeaders(AgentSpan span, REQUEST_CARRIER carrier) { CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC); RequestContext requestContext = span.getRequestContext(); diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy index 095bba51400..d720a061f5b 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/BaseDecoratorTest.groovy @@ -11,6 +11,9 @@ class BaseDecoratorTest extends DDSpecification { @Shared def decorator = newDecorator() + @Shared + def errorPriority = null as Byte + def span = Mock(AgentSpan) def "test afterStart"() { @@ -59,7 +62,11 @@ class BaseDecoratorTest extends DDSpecification { then: if (error) { - 1 * span.addThrowable(error) + if (errorPriority != null) { + 1 * span.addThrowable(error, errorPriority) + } else { + 1 * span.addThrowable(error) + } } 0 * _ diff --git a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy index 79c69018886..79635d9248a 100644 --- a/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy +++ b/dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy @@ -13,6 +13,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.AgentTracer import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI import datadog.trace.bootstrap.instrumentation.api.ContextVisitors +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter @@ -35,6 +36,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { void setup() { origAppSecActive = ActiveSubsystems.APPSEC_ACTIVE ActiveSubsystems.APPSEC_ACTIVE = true + errorPriority = ErrorPriorities.HTTP_SERVER_DECORATOR } void cleanup() { @@ -303,7 +305,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest { 1 * this.span.setHttpStatusCode(status) } if (resp) { - 1 * this.span.setError(error) + 1 * this.span.setError(error, ErrorPriorities.HTTP_SERVER_DECORATOR) } if (status == 404) { 1 * this.span.setResourceName({ it as String == "404" }, ResourceNamePriorities.HTTP_404) diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java index 87d26466956..009c45fcc37 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/DispatcherServletInstrumentation.java @@ -17,6 +17,7 @@ import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import java.util.List; import net.bytebuddy.asm.Advice; import org.springframework.context.ApplicationContext; @@ -121,7 +122,7 @@ public static void nameResource(@Advice.Argument(3) final Exception exception) { // We rely on a decorator to set the error state based on response code. (5xx -> error) // Status code might not be set though if the span isn't the server span. // Meaning the error won't be set by the status code. (Probably ok since not measured.) - span.setError(alreadyError); + span.setError(alreadyError, ErrorPriorities.HTTP_SERVER_DECORATOR); } } } diff --git a/dd-java-agent/instrumentation/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ErrorHandlerAdvice.java b/dd-java-agent/instrumentation/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ErrorHandlerAdvice.java index d921e6c8ddf..77c6449ae4e 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ErrorHandlerAdvice.java +++ b/dd-java-agent/instrumentation/spring-webmvc-6.0/src/main/java17/datadog/trace/instrumentation/springweb6/ErrorHandlerAdvice.java @@ -3,6 +3,7 @@ import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import net.bytebuddy.asm.Advice; public class ErrorHandlerAdvice { @@ -16,7 +17,7 @@ public static void nameResource(@Advice.Argument(3) final Exception exception) { // We rely on a decorator to set the error state based on response code. (5xx -> error) // Status code might not be set though if the span isn't the server span. // Meaning the error won't be set by the status code. (Probably ok since not measured.) - span.setError(alreadyError); + span.setError(alreadyError, ErrorPriorities.HTTP_SERVER_DECORATOR); } } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java index 016fdce358a..a92bdcdf689 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java @@ -22,6 +22,7 @@ import datadog.trace.api.sampling.SamplingMechanism; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AttachableWrapper; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.PathwayContext; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import java.io.PrintWriter; @@ -245,7 +246,12 @@ public final void publish() { @Override public DDSpan setError(final boolean error) { - context.setErrorFlag(error); + return setError(error, ErrorPriorities.DEFAULT); + } + + @Override + public DDSpan setError(final boolean error, final byte priority) { + context.setErrorFlag(error, priority); return this; } @@ -313,6 +319,11 @@ public DDSpan setErrorMessage(final String errorMessage) { @Override public DDSpan addThrowable(final Throwable error) { + return addThrowable(error, ErrorPriorities.DEFAULT); + } + + @Override + public DDSpan addThrowable(Throwable error, byte errorPriority) { if (null != error) { String message = error.getMessage(); if (!"broken pipe".equalsIgnoreCase(message) @@ -322,7 +333,7 @@ public DDSpan addThrowable(final Throwable error) { // which might happen because the application is overloaded // or warming up - capturing the stack trace and keeping // the trace may exacerbate existing problems. - setError(true); + setError(true, errorPriority); final StringWriter errorString = new StringWriter(); error.printStackTrace(new PrintWriter(errorString)); setTag(DDTags.ERROR_STACK, errorString.toString()); @@ -331,7 +342,6 @@ public DDSpan addThrowable(final Throwable error) { setTag(DDTags.ERROR_MSG, message); setTag(DDTags.ERROR_TYPE, error.getClass().getName()); } - return this; } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 0cd13607955..12ce0f7983c 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -16,6 +16,7 @@ import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.sampling.SamplingMechanism; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.PathwayContext; import datadog.trace.bootstrap.instrumentation.api.ProfilerContext; import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration; @@ -113,6 +114,8 @@ public class DDSpanContext /** True indicates that the span reports an error */ private volatile boolean errorFlag; + private volatile byte errorFlagPriority = ErrorPriorities.UNSET; + private volatile boolean measured; private volatile boolean topLevel; @@ -388,9 +391,10 @@ public boolean getErrorFlag() { return errorFlag; } - public void setErrorFlag(final boolean errorFlag) { - if (errorFlag != this.errorFlag) { + public void setErrorFlag(final boolean errorFlag, final byte priority) { + if (priority >= this.errorFlagPriority) { this.errorFlag = errorFlag; + this.errorFlagPriority = priority; } } diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index 17e108b4c04..727f56ca9f9 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -24,6 +24,7 @@ import datadog.trace.api.env.CapturedEnvironment; import datadog.trace.api.normalize.HttpResourceNames; import datadog.trace.api.sampling.SamplingMechanism; +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities; import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags; import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities; import datadog.trace.bootstrap.instrumentation.api.Tags; @@ -186,7 +187,7 @@ private boolean interceptDbStatement(DDSpanContext span, Object value) { } private boolean interceptError(DDSpanContext span, Object value) { - span.setErrorFlag(asBoolean(value)); + span.setErrorFlag(asBoolean(value), ErrorPriorities.DEFAULT); return true; } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextTest.groovy index 71c3a345098..0ea53cdd3fb 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanContextTest.groovy @@ -3,6 +3,7 @@ package datadog.trace.core import datadog.trace.api.DDTags import datadog.trace.api.DDTraceId import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration import datadog.trace.common.writer.ListWriter import datadog.trace.core.propagation.ExtractedContext @@ -43,7 +44,7 @@ class DDSpanContextTest extends DDCoreSpecification { when: context.setTag("some.tag", "asdf") context.setTag(name, null) - context.setErrorFlag(true) + context.setErrorFlag(true, ErrorPriorities.DEFAULT) span.finish() writer.waitForTraces(1) diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy index aeb16fe45d0..ecccd01274d 100644 --- a/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/DDSpanTest.groovy @@ -7,6 +7,7 @@ import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.api.sampling.PrioritySampling import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopPathwayContext +import datadog.trace.bootstrap.instrumentation.api.ErrorPriorities import datadog.trace.bootstrap.instrumentation.api.TagContext import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString import datadog.trace.common.sampling.RateByServiceTraceSampler @@ -431,4 +432,32 @@ class DDSpanTest extends DDCoreSpecification { 0.5 | 100 0.25 | Integer.MAX_VALUE } + + def "error priorities should be respected"() { + setup: + def span = tracer.buildSpan("testSpan").start() as DDSpan + + expect: + !span.isError() + + when: + span.setError(true) + then: + span.isError() + + when: + span.setError(false) + then: + !span.isError() + + when: + span.setError(true, ErrorPriorities.HTTP_SERVER_DECORATOR) + then: + !span.isError() + + when: + span.setError(true, Byte.MAX_VALUE) + then: + span.isError() + } } diff --git a/internal-api/build.gradle b/internal-api/build.gradle index 7f307138838..49b8971998e 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -65,6 +65,7 @@ excludedClassesCoverage += [ "datadog.trace.bootstrap.instrumentation.api.TagContext.HttpHeaders", "datadog.trace.bootstrap.instrumentation.api.ForwardedTagContext", "datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities", + "datadog.trace.bootstrap.instrumentation.api.ErrorPriorities", "datadog.trace.api.civisibility.ci.CIInfo", "datadog.trace.api.civisibility.ci.CIInfo.Builder", "datadog.trace.api.civisibility.InstrumentationBridge", diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java index 710b3338762..59dc808ada2 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentSpan.java @@ -62,12 +62,16 @@ public interface AgentSpan extends MutableSpan, IGSpanInfo { @Override AgentSpan setError(boolean error); + AgentSpan setError(boolean error, byte priority); + AgentSpan setMeasured(boolean measured); AgentSpan setErrorMessage(String errorMessage); AgentSpan addThrowable(Throwable throwable); + AgentSpan addThrowable(Throwable throwable, byte errorPriority); + @Override AgentSpan getLocalRootSpan(); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java index 8be8e2b5682..2f9d4a6ed81 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java @@ -669,6 +669,11 @@ public AgentSpan setError(final boolean error) { return this; } + @Override + public AgentSpan setError(boolean error, byte priority) { + return this; + } + @Override public AgentSpan setMeasured(boolean measured) { return this; @@ -689,6 +694,11 @@ public AgentSpan addThrowable(final Throwable throwable) { return this; } + @Override + public AgentSpan addThrowable(Throwable throwable, byte errorPriority) { + return this; + } + @Override public AgentSpan setHttpStatusCode(int statusCode) { return this; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ErrorPriorities.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ErrorPriorities.java new file mode 100644 index 00000000000..3e48582a1c5 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ErrorPriorities.java @@ -0,0 +1,8 @@ +package datadog.trace.bootstrap.instrumentation.api; + +public class ErrorPriorities { + public static final byte UNSET = Byte.MIN_VALUE; + public static final byte HTTP_SERVER_DECORATOR = -1; + + public static final byte DEFAULT = 0; +}