Document when we manage a span's lifecycle without scopes (#8628)

There are a few instrumentations where the instrumented code does not align with scopes.
In such situations the span is activated, but the scope is thrown away. The span is then
closed in another section of code using 'closeActive()'.

This PR adds a 'activateSpanWithoutScope(span)' method to self-document this behaviour
and explicitly tie it to the use of 'closeActive()'.

Author: mcculls

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java

@@ -1,7 +1,7 @@
 package datadog.trace.civisibility.domain;
 
 import static datadog.json.JsonMapper.toJson;
-import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpanWithoutScope;
 import static datadog.trace.civisibility.Constants.CI_VISIBILITY_INSTRUMENTATION_NAME;
 
 import datadog.trace.api.Config;
@@ -116,7 +116,7 @@ public TestImpl(
 
     span = spanBuilder.start();
 
-    activateSpan(span);
+    activateSpanWithoutScope(span);
 
     span.setSpanType(InternalSpanTypes.TEST);
     span.setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_TEST);

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java

@@ -1,7 +1,7 @@
 package datadog.trace.civisibility.domain;
 
 import static datadog.json.JsonMapper.toJson;
-import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpanWithoutScope;
 import static datadog.trace.civisibility.Constants.CI_VISIBILITY_INSTRUMENTATION_NAME;
 
 import datadog.trace.api.Config;
@@ -130,7 +130,7 @@ public TestSuiteImpl(
     testDecorator.afterStart(span);
 
     if (!parallelized) {
-      activateSpan(span);
+      activateSpanWithoutScope(span);
     }
 
     metricCollector.add(CiVisibilityCountMetric.EVENT_CREATED, 1, instrumentation, EventType.SUITE);

dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java

@@ -2,7 +2,7 @@
 
 import static datadog.trace.api.datastreams.DataStreamsContext.create;
 import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.XRAY_TRACING_CONCERN;
-import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpanWithoutScope;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.blackholeSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
 import static datadog.trace.core.datastreams.TagsProcessor.DIRECTION_IN;
@@ -55,7 +55,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
-      activateSpan(blackholeSpan());
+      activateSpanWithoutScope(blackholeSpan());
     } else {
       span = requestSpanStore.remove(request.getOriginalRequest());
       if (span != null) {
@@ -78,9 +78,9 @@ public void beforeRequest(final Request<?> request) {
       }
       // This scope will be closed by AwsHttpClientInstrumentation
       if (AWS_LEGACY_TRACING) {
-        activateSpan(span);
+        activateSpanWithoutScope(span);
       } else {
-        activateSpan(blackholeSpan());
+        activateSpanWithoutScope(blackholeSpan());
       }
     }
   }

dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java

@@ -2,6 +2,7 @@
 
 import static datadog.trace.bootstrap.instrumentation.api.AgentPropagation.XRAY_TRACING_CONCERN;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpanWithoutScope;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.blackholeSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
 import static datadog.trace.instrumentation.aws.v2.AwsSdkClientDecorator.AWS_LEGACY_TRACING;
@@ -93,7 +94,7 @@ public void beforeTransmission(
     if (span != null) {
       // This scope will be closed by AwsHttpClientInstrumentation since ExecutionInterceptor API
       // doesn't provide a way to run code in same thread after transmission has been scheduled.
-      activateSpan(span);
+      activateSpanWithoutScope(span);
     }
   }
 

dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java

@@ -22,7 +22,6 @@
 import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.bootstrap.ContextStore;
-import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
 import datadog.trace.bootstrap.instrumentation.api.Tags;
@@ -198,7 +197,7 @@ public boolean beforeStep(Step step, ScenarioRuntime sr) {
       return true;
     }
     AgentSpan span = AgentTracer.startSpan("karate", KARATE_STEP_SPAN_NAME);
-    AgentScope scope = AgentTracer.activateSpan(span);
+    AgentTracer.activateSpanWithoutScope(span);
     String stepName = step.getPrefix() + " " + step.getText();
     span.setResourceName(stepName);
     span.setTag(Tags.COMPONENT, "karate");

dd-java-agent/instrumentation/undertow/undertow-2.0/src/main/java/datadog/trace/instrumentation/undertow/HttpRequestParserInstrumentation.java

@@ -4,7 +4,6 @@
 import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
-import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.closeActive;
 import static datadog.trace.instrumentation.undertow.UndertowDecorator.DECORATE;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
@@ -92,7 +91,7 @@ public static void afterRequestParse(
           if (scope != null) {
             scope.close();
           } else {
-            closeActive();
+            // span was already active, scope will be closed by HandlerInstrumentation
           }
         }
       }

dd-trace-core/src/main/java/datadog/trace/core/CoreTracer.java

@@ -916,6 +916,12 @@ public AgentScope activateManualSpan(final AgentSpan span) {
     return scopeManager.activateManualSpan(span);
   }
 
+  @Override
+  @SuppressWarnings("resource")
+  public void activateSpanWithoutScope(AgentSpan span) {
+    scopeManager.activateSpan(span);
+  }
+
   @Override
   public AgentScope.Continuation captureActiveSpan() {
     return scopeManager.captureActiveSpan();

internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/AgentTracer.java

@@ -87,6 +87,16 @@ public static AgentScope activateSpan(final AgentSpan span) {
     return get().activateSpan(span);
   }
 
+  /**
+   * Activate a span which will be closed by {@link #closeActive()} instead of a scope.
+   *
+   * @deprecated This should only be used when the instrumented code doesn't align with a scope.
+   */
+  @Deprecated
+  public static void activateSpanWithoutScope(final AgentSpan span) {
+    get().activateSpanWithoutScope(span);
+  }
+
   /**
    * When asynchronous propagation is enabled, prevent the currently active trace from reporting
    * until the returned Continuation is either activated (and the returned scope is closed) or the
@@ -142,8 +152,8 @@ public static void rollbackActiveToCheckpoint() {
   /**
    * Closes the scope for the currently active span.
    *
-   * @deprecated This should only be used when an instrumentation does not have access to the
-   *     original scope returned by {@link #activateSpan}.
+   * @deprecated This should only be used when the span was previously activated with {@link
+   *     #activateSpanWithoutScope} because the instrumented code didn't align with a scope.
    */
   @Deprecated
   public static void closeActive() {
@@ -338,6 +348,9 @@ AgentSpan startSpan(
     /** Activate a span from outside auto-instrumentation, i.e. a manual or custom span. */
     AgentScope activateManualSpan(AgentSpan span);
 
+    /** Activate a span which will be closed by {@link #closeActive()} instead of a scope. */
+    void activateSpanWithoutScope(AgentSpan span);
+
     @Override
     AgentScope.Continuation captureActiveSpan();
 
@@ -483,6 +496,9 @@ public AgentScope activateManualSpan(final AgentSpan span) {
       return NoopScope.INSTANCE;
     }
 
+    @Override
+    public void activateSpanWithoutScope(final AgentSpan span) {}
+
     @Override
     public AgentScope.Continuation captureActiveSpan() {
       return NoopContinuation.INSTANCE;