From 37ae5c2a23bdac2122f235f317abbd2e6a9daf20 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 6 Dec 2023 13:23:36 +0100 Subject: [PATCH 1/5] R&D week: blackhole span --- .../trace_annotation/DoNotTraceAdvice.java | 26 +++++++++++ .../DoNotTraceAnnotationInstrumentation.java | 44 ++++++++++++++++++ .../test/groovy/TraceAnnotationsTest.groovy | 23 +++++++++- .../test/trace/annotation/DontTraceClass.java | 14 ++++++ .../java/datadog/trace/api/DoNotTrace.java | 12 +++++ .../java/datadog/trace/core/CoreTracer.java | 16 +++++++ .../trace/core/BlackholeSpanTest.groovy | 44 ++++++++++++++++++ .../instrumentation/api/AgentTracer.java | 46 ++++++++++++++++++- 8 files changed, 222 insertions(+), 3 deletions(-) create mode 100644 dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/DoNotTraceAdvice.java create mode 100644 dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/DoNotTraceAnnotationInstrumentation.java create mode 100644 dd-java-agent/instrumentation/trace-annotation/src/test/java/dd/test/trace/annotation/DontTraceClass.java create mode 100644 dd-trace-api/src/main/java/datadog/trace/api/DoNotTrace.java create mode 100644 dd-trace-core/src/test/groovy/datadog/trace/core/BlackholeSpanTest.groovy diff --git a/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/DoNotTraceAdvice.java b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/DoNotTraceAdvice.java new file mode 100644 index 00000000000..7040c9fdcea --- /dev/null +++ b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/DoNotTraceAdvice.java @@ -0,0 +1,26 @@ +package datadog.trace.instrumentation.trace_annotation; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.blackholeSpan; +import static datadog.trace.instrumentation.trace_annotation.TraceDecorator.DECORATE; + +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.implementation.bytecode.assign.Assigner; + +public class DoNotTraceAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope before() { + return activateSpan(blackholeSpan()); + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void after( + @Advice.Enter final AgentScope scope, + @Advice.Return(typing = Assigner.Typing.DYNAMIC, readOnly = false) Object result) { + if (scope != null) { + scope.close(); + result = DECORATE.wrapAsyncResultOrFinishSpan(result, scope.span()); + } + } +} diff --git a/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/DoNotTraceAnnotationInstrumentation.java b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/DoNotTraceAnnotationInstrumentation.java new file mode 100644 index 00000000000..b44e6c90d24 --- /dev/null +++ b/dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/DoNotTraceAnnotationInstrumentation.java @@ -0,0 +1,44 @@ +package datadog.trace.instrumentation.trace_annotation; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.declaresMethod; +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.isAnnotatedWith; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import de.thetaphi.forbiddenapis.SuppressForbidden; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class DoNotTraceAnnotationInstrumentation extends Instrumenter.Tracing + implements Instrumenter.ForTypeHierarchy { + + @SuppressForbidden + public DoNotTraceAnnotationInstrumentation() { + super("not-not-trace", "do-not-trace-annotation"); + } + + @Override + public String hierarchyMarkerType() { + return "datadog.trace.api.DoNotTrace"; + } + + @Override + public ElementMatcher hierarchyMatcher() { + return declaresMethod(isAnnotatedWith(named(hierarchyMarkerType()))); + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".TraceDecorator", + }; + } + + @Override + public void adviceTransformations(AdviceTransformation transformation) { + transformation.applyAdvice( + isAnnotatedWith(named(hierarchyMarkerType())), packageName + ".DoNotTraceAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy index e8acea20b83..a1f1cfd6df6 100644 --- a/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy +++ b/dd-java-agent/instrumentation/trace-annotation/src/test/groovy/TraceAnnotationsTest.groovy @@ -2,6 +2,7 @@ import datadog.trace.agent.test.AgentTestRunner import datadog.trace.agent.test.utils.TraceUtils import datadog.trace.api.Trace import datadog.trace.bootstrap.instrumentation.api.Tags +import dd.test.trace.annotation.DontTraceClass import dd.test.trace.annotation.SayTracedHello import dd.test.trace.annotation.TracedSubClass @@ -510,5 +511,25 @@ class TraceAnnotationsTest extends AgentTestRunner { } } } + def "@DoNotTrace should mute tracing"() { + setup: + TraceUtils.runUnderTrace("parent", () -> { + new DontTraceClass().muted() + }) + expect: + assertTraces(1) { + trace(1) { + span { + hasServiceName() + resourceName "parent" + operationName "parent" + parent() + errored false + tags { + defaultTags() + } + } + } + } + } } - diff --git a/dd-java-agent/instrumentation/trace-annotation/src/test/java/dd/test/trace/annotation/DontTraceClass.java b/dd-java-agent/instrumentation/trace-annotation/src/test/java/dd/test/trace/annotation/DontTraceClass.java new file mode 100644 index 00000000000..ebb875610ac --- /dev/null +++ b/dd-java-agent/instrumentation/trace-annotation/src/test/java/dd/test/trace/annotation/DontTraceClass.java @@ -0,0 +1,14 @@ +package dd.test.trace.annotation; + +import datadog.trace.api.DoNotTrace; +import datadog.trace.api.Trace; + +public class DontTraceClass { + @DoNotTrace + public void muted() { + normallyTraced(); + } + + @Trace + public void normallyTraced() {} +} diff --git a/dd-trace-api/src/main/java/datadog/trace/api/DoNotTrace.java b/dd-trace-api/src/main/java/datadog/trace/api/DoNotTrace.java new file mode 100644 index 00000000000..df27d5de2f0 --- /dev/null +++ b/dd-trace-api/src/main/java/datadog/trace/api/DoNotTrace.java @@ -0,0 +1,12 @@ +package datadog.trace.api; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +/** Set this annotation to a method to mute tracing until its scope is closed */ +@Retention(RUNTIME) +@Target(METHOD) +public @interface DoNotTrace {} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 51216f8849f..6ce4f1cf902 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -888,6 +888,11 @@ public AgentSpan noopSpan() { return AgentTracer.NoopAgentSpan.INSTANCE; } + @Override + public AgentSpan blackholeSpan() { + return new AgentTracer.BlackholeAgentSpan(DDTraceId.from(getTraceId())); + } + @Override public AgentSpan.Context notifyExtensionStart(Object event) { return LambdaHandler.notifyStartInvocation(event, propagationTagsFactory); @@ -1264,6 +1269,17 @@ private void addTerminatedContextAsLinks() { @Override public AgentSpan start() { + AgentSpan.Context pc = parent; + if (pc == null && !ignoreScope) { + final AgentSpan span = activeSpan(); + if (span != null) { + pc = span.context(); + } + } + + if (pc == AgentTracer.BlackholeContext.INSTANCE) { + return new AgentTracer.BlackholeAgentSpan(pc.getTraceId()); + } return buildSpan(); } diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/BlackholeSpanTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/BlackholeSpanTest.groovy new file mode 100644 index 00000000000..b88dc7a6d5d --- /dev/null +++ b/dd-trace-core/src/test/groovy/datadog/trace/core/BlackholeSpanTest.groovy @@ -0,0 +1,44 @@ +package datadog.trace.core + + +import datadog.trace.common.writer.ListWriter +import datadog.trace.core.test.DDCoreSpecification + +class BlackholeSpanTest extends DDCoreSpecification { + def "should mute tracing"() { + setup: + def writer = new ListWriter() + def tracer = tracerBuilder().writer(writer).build() + when: + def child = null + def bh = null + def ignored = null + def root = tracer.startSpan("test", "root") + def scope1 = tracer.activateSpan(root) + try { + bh = tracer.blackholeSpan() + def scope2 = tracer.activateSpan(bh) + try { + ignored = tracer.startSpan("test", "ignored") + ignored.finish() + } finally { + bh.finish() + scope2.close() + } + child = tracer.startSpan("test", "child") + child.finish() + } finally { + root.finish() + scope1.close() + } + then: + writer.waitForTraces(1) + assert writer.firstTrace().size() == 2 + assert writer.firstTrace().containsAll([root, child]) + assert !writer.firstTrace().contains(bh) + assert !writer.firstTrace().contains(ignored) + cleanup: + writer.close() + tracer.close() + } +} 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 0e9c9564ddf..1bbc85b9ef7 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 @@ -149,6 +149,10 @@ public static AgentSpan noopSpan() { return get().noopSpan(); } + public static AgentSpan blackholeSpan() { + return get().blackholeSpan(); + } + public static final TracerAPI NOOP_TRACER = new NoopTracerAPI(); private static volatile TracerAPI provider = NOOP_TRACER; @@ -240,6 +244,8 @@ AgentSpan startSpan( AgentSpan noopSpan(); + AgentSpan blackholeSpan(); + /** Deprecated. Use {@link #buildSpan(String, CharSequence)} instead. */ @Deprecated default SpanBuilder buildSpan(CharSequence spanName) { @@ -395,6 +401,11 @@ public AgentSpan noopSpan() { return NoopAgentSpan.INSTANCE; } + @Override + public AgentSpan blackholeSpan() { + return NoopAgentSpan.INSTANCE; // no-op tracer stays no-op + } + @Override public SpanBuilder buildSpan(final String instrumentationName, final CharSequence spanName) { return null; @@ -519,7 +530,32 @@ public AgentHistogram newHistogram(double relativeAccuracy, int maxNumBins) { } } - public static final class NoopAgentSpan implements AgentSpan { + public static final class BlackholeAgentSpan extends NoopAgentSpan { + private final DDTraceId ddTraceId; + + public BlackholeAgentSpan(final DDTraceId ddTraceId) { + this.ddTraceId = ddTraceId; + } + + @Override + public boolean isSameTrace(final AgentSpan otherSpan) { + return otherSpan != null + && ((ddTraceId != null && ddTraceId.equals(otherSpan.getTraceId())) + || otherSpan.getTraceId() == null); + } + + @Override + public DDTraceId getTraceId() { + return ddTraceId; + } + + @Override + public Context context() { + return BlackholeContext.INSTANCE; + } + } + + public static class NoopAgentSpan implements AgentSpan { public static final NoopAgentSpan INSTANCE = new NoopAgentSpan(); private NoopAgentSpan() {} @@ -909,7 +945,13 @@ public AgentSpan getSpan() { } } - public static final class NoopContext implements Context.Extracted { + public static final class BlackholeContext extends NoopContext { + public static final BlackholeContext INSTANCE = new BlackholeContext(); + + private BlackholeContext() {} + } + + public static class NoopContext implements Context.Extracted { public static final NoopContext INSTANCE = new NoopContext(); private NoopContext() {} From af0cddb0bc7740d48c95e1e9978716295987cd11 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 8 Dec 2023 10:35:23 +0100 Subject: [PATCH 2/5] blackhole manual tracing API --- .../java/datadog/trace/api/GlobalTracer.java | 7 ++++ .../main/java/datadog/trace/api/Tracer.java | 3 ++ .../datadog/trace/context/NoopTraceScope.java | 42 +++++++++++++++++++ .../datadog/trace/api/EventTrackerTest.groovy | 6 +++ .../java/datadog/trace/core/CoreTracer.java | 6 +++ .../java/datadog/opentracing/DDTracer.java | 6 +++ .../datadog/opentracing/DDTracerTest.groovy | 30 +++++++++++++ .../instrumentation/api/AgentTracer.java | 6 +++ 8 files changed, 106 insertions(+) create mode 100644 dd-trace-api/src/main/java/datadog/trace/context/NoopTraceScope.java diff --git a/dd-trace-api/src/main/java/datadog/trace/api/GlobalTracer.java b/dd-trace-api/src/main/java/datadog/trace/api/GlobalTracer.java index ccbd53e12d6..600c7ecf313 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/GlobalTracer.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/GlobalTracer.java @@ -2,6 +2,8 @@ import datadog.trace.api.interceptor.TraceInterceptor; import datadog.trace.api.internal.InternalTracer; +import datadog.trace.context.NoopTraceScope; +import datadog.trace.context.TraceScope; import java.util.ArrayList; import java.util.Collection; @@ -28,6 +30,11 @@ public String getSpanId() { public boolean addTraceInterceptor(TraceInterceptor traceInterceptor) { return false; } + + @Override + public TraceScope muteTracing() { + return NoopTraceScope.INSTANCE; + } }; private static final Collection installationCallbacks = new ArrayList<>(); diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Tracer.java b/dd-trace-api/src/main/java/datadog/trace/api/Tracer.java index d46c89f8cc0..a2583f91d8b 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Tracer.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Tracer.java @@ -1,6 +1,7 @@ package datadog.trace.api; import datadog.trace.api.interceptor.TraceInterceptor; +import datadog.trace.context.TraceScope; /** A class with Datadog tracer features. */ public interface Tracer { @@ -21,4 +22,6 @@ public interface Tracer { * @return false if an interceptor with same priority exists. */ boolean addTraceInterceptor(TraceInterceptor traceInterceptor); + + TraceScope muteTracing(); } diff --git a/dd-trace-api/src/main/java/datadog/trace/context/NoopTraceScope.java b/dd-trace-api/src/main/java/datadog/trace/context/NoopTraceScope.java new file mode 100644 index 00000000000..69f6b8ee895 --- /dev/null +++ b/dd-trace-api/src/main/java/datadog/trace/context/NoopTraceScope.java @@ -0,0 +1,42 @@ +package datadog.trace.context; + +public class NoopTraceScope implements TraceScope { + public static final NoopTraceScope INSTANCE = new NoopTraceScope(); + + public static class NoopContinuation implements Continuation { + public static final NoopContinuation INSTANCE = new NoopContinuation(); + + private NoopContinuation() {} + + @Override + public TraceScope activate() { + return NoopTraceScope.INSTANCE; + } + + @Override + public void cancel() {} + } + + private NoopTraceScope() {} + + @Override + public Continuation capture() { + return NoopContinuation.INSTANCE; + } + + @Override + public Continuation captureConcurrent() { + return null; + } + + @Override + public void close() {} + + @Override + public boolean isAsyncPropagating() { + return false; + } + + @Override + public void setAsyncPropagation(boolean value) {} +} diff --git a/dd-trace-api/src/test/groovy/datadog/trace/api/EventTrackerTest.groovy b/dd-trace-api/src/test/groovy/datadog/trace/api/EventTrackerTest.groovy index eec5b70a4b1..fc40112c24a 100644 --- a/dd-trace-api/src/test/groovy/datadog/trace/api/EventTrackerTest.groovy +++ b/dd-trace-api/src/test/groovy/datadog/trace/api/EventTrackerTest.groovy @@ -6,6 +6,7 @@ import datadog.trace.api.interceptor.TraceInterceptor import datadog.trace.api.internal.InternalTracer import datadog.trace.api.internal.TraceSegment import datadog.trace.api.profiling.Profiling +import datadog.trace.context.TraceScope import datadog.trace.test.util.DDSpecification class EventTrackerTest extends DDSpecification { @@ -154,6 +155,11 @@ class EventTrackerTest extends DDSpecification { return false } + @Override + TraceScope muteTracing() { + return null + } + @Override DataStreamsCheckpointer getDataStreamsCheckpointer() { return null diff --git a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java index 6ce4f1cf902..61d99f23f95 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java @@ -70,6 +70,7 @@ import datadog.trace.common.writer.Writer; import datadog.trace.common.writer.WriterFactory; import datadog.trace.common.writer.ddintake.DDIntakeTraceInterceptor; +import datadog.trace.context.TraceScope; import datadog.trace.core.datastreams.DataStreamContextInjector; import datadog.trace.core.datastreams.DataStreamsMonitoring; import datadog.trace.core.datastreams.DefaultDataStreamsMonitoring; @@ -1037,6 +1038,11 @@ public boolean addTraceInterceptor(final TraceInterceptor interceptor) { } } + @Override + public TraceScope muteTracing() { + return activateSpan(blackholeSpan()); + } + @Override public DataStreamsCheckpointer getDataStreamsCheckpointer() { return this.dataStreamsMonitoring; diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index b857f458df4..be2db34e368 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -14,6 +14,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import datadog.trace.common.sampling.Sampler; import datadog.trace.common.writer.Writer; +import datadog.trace.context.TraceScope; import datadog.trace.core.CoreTracer; import datadog.trace.core.DDSpanContext; import datadog.trace.core.propagation.ExtractedContext; @@ -438,6 +439,11 @@ public boolean addTraceInterceptor(final TraceInterceptor traceInterceptor) { return tracer.addTraceInterceptor(traceInterceptor); } + @Override + public TraceScope muteTracing() { + return tracer.muteTracing(); + } + @Override public DataStreamsCheckpointer getDataStreamsCheckpointer() { return tracer.getDataStreamsCheckpointer(); diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDTracerTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDTracerTest.groovy index 023df969e74..f076131b057 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDTracerTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDTracerTest.groovy @@ -1,6 +1,8 @@ package datadog.opentracing + import datadog.trace.common.writer.DDAgentWriter +import datadog.trace.common.writer.ListWriter import datadog.trace.test.util.DDSpecification class DDTracerTest extends DDSpecification { @@ -47,4 +49,32 @@ class DDTracerTest extends DDSpecification { cleanup: tracer.close() } + + def "should produce blackhole scopes"() { + setup: + def writer = new ListWriter() + def tracer = DDTracer.builder().writer(writer).build() + + when: + def span = tracer.buildSpan("some name").start() + def scope = tracer.scopeManager().activate(span) + def muteScope = tracer.muteTracing() + def blackholed = tracer.buildSpan("hidden span").start() + blackholed.finish() + muteScope.close() + def visibleSpan = tracer.buildSpan("visible span").start() + visibleSpan.finish() + scope.close() + span.finish() + + then: + writer.waitForTraces(1) + assert writer.size() == 1 + assert writer.firstTrace().size() == 2 + assert Long.toString(writer.firstTrace()[0].context().getSpanId()) == span.context().toSpanId() + assert Long.toString(writer.firstTrace()[1].context().getSpanId()) == visibleSpan.context().toSpanId() + + cleanup: + tracer.close() + } } 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 1bbc85b9ef7..379dff9cced 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 @@ -24,6 +24,7 @@ import datadog.trace.api.sampling.SamplingRule; import datadog.trace.api.scopemanager.ScopeListener; import datadog.trace.bootstrap.instrumentation.api.AgentSpan.Context; +import datadog.trace.context.TraceScope; import java.nio.ByteBuffer; import java.util.Collections; import java.util.LinkedHashMap; @@ -459,6 +460,11 @@ public boolean addTraceInterceptor(final TraceInterceptor traceInterceptor) { return false; } + @Override + public TraceScope muteTracing() { + return NoopAgentScope.INSTANCE; + } + @Override public DataStreamsCheckpointer getDataStreamsCheckpointer() { return getDataStreamsMonitoring(); From 25b195ad076bae233d47f5435193df8ee5253d7a Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 8 Dec 2023 14:22:47 +0100 Subject: [PATCH 3/5] Decouple httpclient and aws sdk instrumentations with blackhole --- .../apachehttpclient/HelperMethods.java | 25 +++---------------- .../apachehttpclient5/HelperMethods.java | 24 +++--------------- .../aws/v0/TracingRequestHandler.java | 13 ++++++---- .../aws/v2/TracingExecutionInterceptor.java | 8 +++--- .../trace/agent/test/AgentTestRunner.groovy | 4 +-- 5 files changed, 21 insertions(+), 53 deletions(-) diff --git a/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/HelperMethods.java b/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/HelperMethods.java index 345b9beed38..959510b9699 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/HelperMethods.java +++ b/dd-java-agent/instrumentation/apache-httpclient-4/src/main/java/datadog/trace/instrumentation/apachehttpclient/HelperMethods.java @@ -7,7 +7,6 @@ import static datadog.trace.instrumentation.apachehttpclient.ApacheHttpClientDecorator.HTTP_REQUEST; import static datadog.trace.instrumentation.apachehttpclient.HttpHeadersInjectAdapter.SETTER; -import datadog.trace.api.Config; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -19,46 +18,30 @@ import org.apache.http.client.methods.HttpUriRequest; public class HelperMethods { - private static final boolean AWS_LEGACY_TRACING = - Config.get().isLegacyTracingEnabled(false, "aws-sdk"); - public static AgentScope doMethodEnter(final HttpUriRequest request) { - boolean awsClientCall = request.containsHeader("amz-sdk-invocation-id"); - if (!AWS_LEGACY_TRACING && awsClientCall) { - // avoid creating an extra HTTP client span beneath the AWS client call - return null; - } - final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpClient.class); if (callDepth > 0) { return null; } - - return activateHttpSpan(request, awsClientCall); + return activateHttpSpan(request); } public static AgentScope doMethodEnter(HttpHost host, HttpRequest request) { - boolean awsClientCall = request.containsHeader("amz-sdk-invocation-id"); - if (!AWS_LEGACY_TRACING && awsClientCall) { - // avoid creating an extra HTTP client span beneath the AWS client call - return null; - } - final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpClient.class); if (callDepth > 0) { return null; } - return activateHttpSpan(new HostAndRequestAsHttpUriRequest(host, request), awsClientCall); + return activateHttpSpan(new HostAndRequestAsHttpUriRequest(host, request)); } - private static AgentScope activateHttpSpan( - final HttpUriRequest request, final boolean awsClientCall) { + private static AgentScope activateHttpSpan(final HttpUriRequest request) { final AgentSpan span = startSpan(HTTP_REQUEST); final AgentScope scope = activateSpan(span); DECORATE.afterStart(span); DECORATE.onRequest(span, request); + final boolean awsClientCall = request.containsHeader("amz-sdk-invocation-id"); // AWS calls are often signed, so we can't add headers without breaking the signature. if (!awsClientCall) { diff --git a/dd-java-agent/instrumentation/apache-httpclient-5/src/main/java/datadog/trace/instrumentation/apachehttpclient5/HelperMethods.java b/dd-java-agent/instrumentation/apache-httpclient-5/src/main/java/datadog/trace/instrumentation/apachehttpclient5/HelperMethods.java index c1d7df18d9c..ebbde1b1ba6 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-5/src/main/java/datadog/trace/instrumentation/apachehttpclient5/HelperMethods.java +++ b/dd-java-agent/instrumentation/apache-httpclient-5/src/main/java/datadog/trace/instrumentation/apachehttpclient5/HelperMethods.java @@ -7,7 +7,6 @@ import static datadog.trace.instrumentation.apachehttpclient5.ApacheHttpClientDecorator.HTTP_REQUEST; import static datadog.trace.instrumentation.apachehttpclient5.HttpHeadersInjectAdapter.SETTER; -import datadog.trace.api.Config; import datadog.trace.bootstrap.CallDepthThreadLocalMap; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -18,47 +17,32 @@ import org.apache.hc.core5.http.HttpResponse; public class HelperMethods { - private static final boolean AWS_LEGACY_TRACING = - Config.get().isLegacyTracingEnabled(false, "aws-sdk"); - public static AgentScope doMethodEnter(final HttpRequest request) { - boolean awsClientCall = request.containsHeader("amz-sdk-invocation-id"); - if (!AWS_LEGACY_TRACING && awsClientCall) { - // avoid creating an extra HTTP client span beneath the AWS client call - return null; - } - final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpClient.class); if (callDepth > 0) { return null; } - return activateHttpSpan(request, awsClientCall); + return activateHttpSpan(request); } public static AgentScope doMethodEnter(HttpHost host, HttpRequest request) { - boolean awsClientCall = request.containsHeader("amz-sdk-invocation-id"); - if (!AWS_LEGACY_TRACING && awsClientCall) { - // avoid creating an extra HTTP client span beneath the AWS client call - return null; - } - final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(HttpClient.class); if (callDepth > 0) { return null; } - return activateHttpSpan(new HostAndRequestAsHttpUriRequest(host, request), awsClientCall); + return activateHttpSpan(new HostAndRequestAsHttpUriRequest(host, request)); } - private static AgentScope activateHttpSpan( - final HttpRequest request, final boolean awsClientCall) { + private static AgentScope activateHttpSpan(final HttpRequest request) { final AgentSpan span = startSpan(HTTP_REQUEST); final AgentScope scope = activateSpan(span); DECORATE.afterStart(span); DECORATE.onRequest(span, request); + final boolean awsClientCall = request.containsHeader("amz-sdk-invocation-id"); // AWS calls are often signed, so we can't add headers without breaking the signature. if (!awsClientCall) { propagate().inject(span, request, SETTER); diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java index bff4430f717..894b63270ff 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.aws.v0; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.blackholeSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.propagate; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; import static datadog.trace.core.datastreams.TagsProcessor.DIRECTION_IN; @@ -53,7 +53,7 @@ public void beforeRequest(final Request request) { if (!AWS_LEGACY_TRACING && isPollingRequest(request.getOriginalRequest())) { // SQS messages spans are created by aws-java-sqs-1.0 - replace client scope with no-op, // so we can tell when receive call is complete without affecting the rest of the trace - span = noopSpan(); + activateSpan(blackholeSpan()); } else { span = requestSpanStore.remove(request.getOriginalRequest()); if (span != null) { @@ -74,10 +74,13 @@ public void beforeRequest(final Request request) { log.warn("Unable to inject trace header", e); } } + // This scope will be closed by AwsHttpClientInstrumentation + if (AWS_LEGACY_TRACING) { + activateSpan(span); + } else { + activateSpan(blackholeSpan()); + } } - - // This scope will be closed by AwsHttpClientInstrumentation - activateSpan(span); } @Override diff --git a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java index 6713a594c51..830319df7c3 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java @@ -1,7 +1,7 @@ package datadog.trace.instrumentation.aws.v2; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; -import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.blackholeSpan; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.propagate; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; import static datadog.trace.instrumentation.aws.v2.AwsSdkClientDecorator.AWS_LEGACY_TRACING; @@ -85,10 +85,8 @@ public SdkHttpRequest modifyHttpRequest( public void beforeTransmission( final Context.BeforeTransmission context, final ExecutionAttributes executionAttributes) { final AgentSpan span; - if (!AWS_LEGACY_TRACING && isPollingRequest(context.request())) { - // SQS messages spans are created by aws-java-sqs-2.0 - replace client scope with no-op, - // so we can tell when receive call is complete without affecting the rest of the trace - span = noopSpan(); + if (!AWS_LEGACY_TRACING) { + span = blackholeSpan(); } else { span = executionAttributes.getAttribute(SPAN_ATTRIBUTE); } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.groovy index 252c48e0691..b52c2ed28ea 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.groovy @@ -327,14 +327,14 @@ abstract class AgentTestRunner extends DDSpecification implements AgentBuilder.L boolean enabledFinishTimingChecks = this.enabledFinishTimingChecks() TEST_TRACER.startSpan(*_) >> { - DDSpan agentSpan = callRealMethod() + AgentSpan agentSpan = callRealMethod() TEST_SPANS.add(agentSpan.spanId) if (!enabledFinishTimingChecks) { return agentSpan } // rest of closure if for checking duplicate finishes and tags set after finish - DDSpan spiedAgentSpan = Spy(agentSpan) + AgentSpan spiedAgentSpan = Spy(agentSpan) originalToSpySpan[agentSpan] = spiedAgentSpan def handleFinish = { MockInvocation mi -> def depth = CallDepthThreadLocalMap.incrementCallDepth(DDSpan) From 5508303793026e68e3e7d21facf5b33aec89a0a3 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 15 Dec 2023 12:52:22 +0100 Subject: [PATCH 4/5] exclude blackhole from coverage --- internal-api/build.gradle | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal-api/build.gradle b/internal-api/build.gradle index 5c5fb509a1f..3d0208e1b70 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -57,9 +57,11 @@ excludedClassesCoverage += [ "datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopAgentPropagation", "datadog.trace.bootstrap.instrumentation.api.AgentTracer", "datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopContext", + "datadog.trace.bootstrap.instrumentation.api.AgentTracer.BlackholeContext", "datadog.trace.bootstrap.instrumentation.api.InstrumentationTags", "datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopContinuation", "datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopAgentSpan", + "datadog.trace.bootstrap.instrumentation.api.AgentTracer.BlackholeAgentSpan", "datadog.trace.bootstrap.instrumentation.api.DDComponents", "datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopAgentScope", "datadog.trace.bootstrap.instrumentation.api.AgentTracer.NoopTracerAPI", From 47f4209e36dba43ba1f9e7142fd89b0537dca965 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 9 Jan 2024 10:42:54 +0100 Subject: [PATCH 5/5] exlcude no-op from coverage --- dd-trace-api/build.gradle | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dd-trace-api/build.gradle b/dd-trace-api/build.gradle index 228298c5cf6..249e9671da3 100644 --- a/dd-trace-api/build.gradle +++ b/dd-trace-api/build.gradle @@ -35,6 +35,8 @@ excludedClassesCoverage += [ 'datadog.trace.api.experimental.DataStreamsContextCarrier', 'datadog.trace.api.experimental.DataStreamsContextCarrier.NoOp', 'datadog.appsec.api.blocking.*', + 'datadog.trace.context.NoopTraceScope.NoopContinuation', + 'datadog.trace.context.NoopTraceScope', ] description = 'dd-trace-api'