Skip to content

Commit 6821684

Browse files
claponcetdevflow.devflow-routing-intake
claponcet
and
devflow.devflow-routing-intake
authored
Stamp _dd.apm.enabled on every exported chunk when APM tracing is disabled (#12028)
Stamp _dd.apm.enabled on every exported chunk when APM tracing is disabled Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Mark the root-most span of each exported chunk with _dd.apm.enabled:0 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Cover root-less chunk fallback in ApmTracingDisabledChunkMarkerTest Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Correct getLocalRootSpanTags comment to say root-most span Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> add standalone tag on every single span Tighten smoke-test to every-span assertion and clarify apmTracingEnabled javadoc Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Merge branch 'master' into clara.poncet/apm-enabled-service-entry-span fix late outbound controller formatting Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent fc47338 commit 6821684

6 files changed

Lines changed: 262 additions & 5 deletions

File tree

‎dd-smoke-tests/apm-tracing-disabled/application/src/main/java/datadog/smoketest/apmtracingdisabled/Controller.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package datadog.smoketest.apmtracingdisabled;
22

3+
import io.opentracing.Scope;
34
import io.opentracing.Span;
45
import io.opentracing.util.GlobalTracer;
56
import java.io.IOException;
67
import java.util.Map;
78
import javax.servlet.http.HttpServletResponse;
9+
import org.slf4j.Logger;
10+
import org.slf4j.LoggerFactory;
811
import org.springframework.http.MediaType;
912
import org.springframework.http.ResponseEntity;
1013
import org.springframework.web.bind.annotation.*;
@@ -14,6 +17,8 @@
1417
@RequestMapping("/rest-api")
1518
public class Controller {
1619

20+
private static final Logger log = LoggerFactory.getLogger(Controller.class);
21+
1722
@GetMapping("/greetings")
1823
public String greetings(
1924
@RequestParam(name = "url", required = false) String url,
@@ -71,6 +76,33 @@ public void write(
7176
}
7277
}
7378

79+
@GetMapping("/late-outbound")
80+
public String lateOutbound(@RequestParam(name = "url") String url) {
81+
final Span span = GlobalTracer.get().activeSpan();
82+
// Thread synchronization relies on waitForTraceCount rather than Thread completion, no race
83+
// issue.
84+
Thread thread =
85+
new Thread(
86+
() -> {
87+
try {
88+
// Sleep past PendingTraceBuffer's 500ms flush delay so the root chunk exports
89+
// before this late child.
90+
Thread.sleep(3000);
91+
} catch (InterruptedException e) {
92+
Thread.currentThread().interrupt();
93+
return;
94+
}
95+
try (Scope scope = GlobalTracer.get().activateSpan(span)) {
96+
new RestTemplate().getForObject(url, String.class);
97+
} catch (Exception e) {
98+
log.debug("late outbound call to {} failed", url, e);
99+
}
100+
});
101+
thread.setDaemon(true);
102+
thread.start();
103+
return "late-outbound";
104+
}
105+
74106
private String forceKeepSpan() {
75107
final Span span = GlobalTracer.get().activeSpan();
76108
if (span != null) {

‎dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/AbstractApmTracingDisabledSmokeTest.groovy‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,20 @@ abstract class AbstractApmTracingDisabledSmokeTest extends AbstractServerSmokeTe
7878
}
7979

8080
protected hasApmDisabledTag(DecodedTrace trace) {
81-
return trace.spans[0].metrics['_dd.apm.enabled'] == 0
81+
return trace.spans.any { it.metrics['_dd.apm.enabled'] == 0 }
82+
}
83+
84+
protected hasApmDisabledTagOnEverySpan(DecodedTrace trace) {
85+
return trace.spans.every { it.metrics['_dd.apm.enabled'] == 0 }
86+
}
87+
88+
/** The chunk holding the delayed outbound client span (service == serviceName, http.url == url). */
89+
protected DecodedTrace getOutboundChunk(String serviceName, String url) {
90+
return traces.find { trace ->
91+
trace.spans.any { span ->
92+
span.service == serviceName && span.meta['http.url'] == url
93+
}
94+
}
8295
}
8396

8497
protected hasASMEvents(DecodedTrace trace){

‎dd-smoke-tests/apm-tracing-disabled/src/test/groovy/datadog/smoketest/apmtracingdisabled/ApmTracingDisabledSmokeTest.groovy‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,28 @@ abstract class ApmTracingDisabledSmokeTest extends AbstractApmTracingDisabledSmo
5050
!hasApmDisabledTag(getServiceTrace(APM_ENABLED_SERVICE_NAME))
5151
}
5252

53+
void 'When APM is disabled, a delayed outbound child flushed in its own chunk must still carry _dd.apm.enabled:0'() {
54+
setup:
55+
final targetUrl = "http://localhost:${httpPorts[1]}/rest-api/greetings"
56+
final url = "http://localhost:${httpPorts[0]}/rest-api/late-outbound?url=${URLEncoder.encode(targetUrl, 'UTF-8')}"
57+
final request = new Request.Builder().url(url).get().build()
58+
59+
when:
60+
final response = client.newCall(request).execute()
61+
62+
then: 'the request root, the delayed outbound child, and the downstream service each report a chunk'
63+
response.successful
64+
waitForTraceCount(3)
65+
// The delayed outbound client span belongs to the APM-disabled service but is exported alone,
66+
// in a separate chunk from the /late-outbound service-entry span that carries the marker.
67+
def outboundChunk = getOutboundChunk(APM_TRACING_DISABLED_SERVICE_NAME, targetUrl)
68+
assert outboundChunk != null
69+
assert outboundChunk.spans.every { it.resource != 'GET /rest-api/late-outbound' }
70+
71+
then: 'every span of the delayed child chunk must carry the billing marker'
72+
hasApmDisabledTagOnEverySpan(outboundChunk)
73+
}
74+
5375
void 'When APM is disabled, libraries must completely disable the generation of APM trace metrics'(){
5476
setup:
5577
final url1 = "http://localhost:${httpPorts[0]}/rest-api/greetings"

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package datadog.trace.core;
22

3+
import static datadog.trace.api.DDTags.APM_ENABLED;
34
import static datadog.trace.api.DDTags.DJM_ENABLED;
45
import static datadog.trace.api.DDTags.DSM_ENABLED;
56
import static datadog.trace.api.DDTags.PROFILING_CONTEXT_ENGINE;
@@ -194,6 +195,12 @@ public static CoreTracerBuilder builder() {
194195

195196
private final boolean localRootSpanTagsNeedIntercept;
196197

198+
/**
199+
* When {@code false}, every exported span is stamped with the {@code _dd.apm.enabled:0} billing
200+
* marker — see {@link #write(SpanList)}.
201+
*/
202+
private final boolean apmTracingEnabled;
203+
197204
/** A set of tags that are added to every span */
198205
private final TagMap defaultSpanTags;
199206

@@ -668,6 +675,7 @@ private CoreTracer(
668675
this.serviceName = serviceName;
669676

670677
this.initialConfig = config;
678+
this.apmTracingEnabled = config.isApmTracingEnabled();
671679
this.initialSampler = sampler;
672680

673681
// Get initial Trace Sampling Rules from config
@@ -1302,6 +1310,13 @@ void write(final SpanList trace) {
13021310
spanToSample.forceKeep(forceKeep);
13031311
boolean published = forceKeep || traceCollector.sample(spanToSample);
13041312
if (published) {
1313+
if (!apmTracingEnabled) {
1314+
// Stamp the billing marker on every span of each exported chunk so the intake does not bill
1315+
// APM host usage.
1316+
for (int i = 0; i < writtenTrace.size(); i++) {
1317+
writtenTrace.get(i).setTag(APM_ENABLED, 0);
1318+
}
1319+
}
13051320
writer.write(writtenTrace);
13061321
} else {
13071322
// with span streaming this won't work - it needs to be changed

‎dd-trace-core/src/test/java/datadog/trace/core/ApmTracingDisabledChunkMarkerTest.java‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
package datadog.trace.core;
2+
3+
import static datadog.trace.api.DDTags.APM_ENABLED;
4+
import static org.junit.jupiter.api.Assertions.assertEquals;
5+
import static org.junit.jupiter.api.Assertions.assertNull;
6+
import static org.junit.jupiter.api.Assertions.assertTrue;
7+
8+
import datadog.trace.common.writer.ListWriter;
9+
import datadog.trace.test.junit.utils.config.WithConfig;
10+
import java.util.List;
11+
import java.util.concurrent.TimeoutException;
12+
import org.junit.jupiter.api.AfterEach;
13+
import org.junit.jupiter.api.BeforeEach;
14+
import org.junit.jupiter.api.Test;
15+
16+
/**
17+
* When APM tracing is disabled ({@code DD_APM_TRACING_ENABLED=false}), the intake bills APM host
18+
* usage for every exported trace <em>chunk</em> that does not carry the {@code _dd.apm.enabled:0}
19+
* marker. To make sure no chunk gets flushed without the correct billing tag, it gets added to
20+
* every single span. These tests lock that in.
21+
*/
22+
@WithConfig(key = "apm.tracing.enabled", value = "false")
23+
class ApmTracingDisabledChunkMarkerTest extends DDCoreJavaSpecification {
24+
25+
private ListWriter writer;
26+
private CoreTracer tracer;
27+
28+
@Override
29+
protected boolean useStrictTraceWrites() {
30+
// Production standalone-ASM uses the delaying PendingTraceBuffer, which is what lets a buffered
31+
// root be written on its own before a late child arrives. Strict writes would instead select
32+
// the discarding buffer and coalesce both spans into a single chunk, hiding the bug.
33+
return false;
34+
}
35+
36+
@BeforeEach
37+
void setup() {
38+
writer = new ListWriter();
39+
tracer = tracerBuilder().writer(writer).build();
40+
}
41+
42+
@AfterEach
43+
void cleanup() {
44+
if (tracer != null) {
45+
tracer.close();
46+
}
47+
}
48+
49+
@Test
50+
void everyExportedChunkCarriesApmDisabledMarker() throws InterruptedException, TimeoutException {
51+
DDSpan root = (DDSpan) tracer.buildSpan("test", "root").start();
52+
DDSpan child = (DDSpan) tracer.buildSpan("test", "child").asChildOf(root.spanContext()).start();
53+
54+
PendingTrace trace = (PendingTrace) root.spanContext().getTraceCollector();
55+
56+
// Root finishes while the child is still running -> root is buffered, not yet written.
57+
root.finish();
58+
assertTrue(writer.isEmpty());
59+
60+
// Write the buffered root out on its own chunk. This sets rootSpanWritten=true and is exactly
61+
// what the PendingTraceBuffer worker does when a buffered root ages out (SEND_DELAY_NS).
62+
trace.write();
63+
writer.waitForTraces(1);
64+
65+
// The long-lived child finishes later. Because the root was already written, it flushes alone.
66+
child.finish();
67+
trace.write();
68+
writer.waitForTraces(2);
69+
70+
List<DDSpan> rootChunk = writer.get(0);
71+
List<DDSpan> childChunk = writer.get(1);
72+
assertEquals(1, rootChunk.size(), "expected the root to flush alone in the first chunk");
73+
assertEquals(1, childChunk.size(), "expected the child to flush alone in the second chunk");
74+
assertEquals(root, rootChunk.get(0), "first chunk should be the local root");
75+
assertEquals(child, childChunk.get(0), "second chunk should be the delayed child");
76+
77+
// Sanity: the child really is a child of the local root (a local, non-remote parent) — this is
78+
// exactly the case that stamping the marker only on the local root span would miss.
79+
assertEquals(root.getSpanId(), child.getParentId());
80+
81+
// Both chunks must carry the billing marker. The delayed child chunk is load-bearing: without
82+
// the marker the intake would bill APM host usage for this otherwise-unmarked chunk.
83+
assertAllSpansMarked(rootChunk, "root chunk");
84+
assertAllSpansMarked(childChunk, "delayed child chunk");
85+
}
86+
87+
@Test
88+
void singleChunkTraceCarriesApmDisabledMarker() throws InterruptedException, TimeoutException {
89+
// Positive control: when the whole trace is exported as a single chunk (child finishes before
90+
// the root, so nothing is written until the root closes), the marker is present on every span.
91+
DDSpan root = (DDSpan) tracer.buildSpan("test", "root").start();
92+
DDSpan child = (DDSpan) tracer.buildSpan("test", "child").asChildOf(root.spanContext()).start();
93+
94+
child.finish();
95+
assertTrue(writer.isEmpty(), "trace must not be written while the root is still open");
96+
root.finish();
97+
writer.waitForTraces(1);
98+
99+
assertEquals(1, writer.size(), "expected the whole trace in a single chunk");
100+
List<DDSpan> chunk = writer.get(0);
101+
assertEquals(2, chunk.size(), "expected both spans in the same chunk");
102+
assertAllSpansMarked(chunk, "single-chunk trace");
103+
}
104+
105+
@Test
106+
void rootlessMultiSpanChunkMarksEverySpan() throws InterruptedException, TimeoutException {
107+
// A multi-span chunk exported WITHOUT its local root (a partial flush / orphaned subtree): the
108+
// root is written first, then its descendants flush together in a later, root-less chunk. Every
109+
// span in that chunk must still carry the marker.
110+
DDSpan root = (DDSpan) tracer.buildSpan("test", "root").start();
111+
DDSpan childA =
112+
(DDSpan) tracer.buildSpan("test", "childA").asChildOf(root.spanContext()).start();
113+
DDSpan grandchild =
114+
(DDSpan) tracer.buildSpan("test", "grandchild").asChildOf(childA.spanContext()).start();
115+
116+
PendingTrace trace = (PendingTrace) root.spanContext().getTraceCollector();
117+
118+
// Flush the root on its own first (rootSpanWritten=true), so the descendants can only export in
119+
// a later, root-less chunk.
120+
root.finish();
121+
trace.write();
122+
writer.waitForTraces(1);
123+
assertEquals(1, writer.get(0).size(), "expected the root to flush alone in the first chunk");
124+
125+
// Both descendants finish, then flush together in a single root-less chunk.
126+
childA.finish();
127+
grandchild.finish();
128+
trace.write();
129+
writer.waitForTraces(2);
130+
131+
List<DDSpan> chunk = writer.get(1);
132+
assertEquals(2, chunk.size(), "expected childA and grandchild in one root-less chunk");
133+
assertTrue(chunk.contains(childA) && chunk.contains(grandchild), "chunk must hold both spans");
134+
assertTrue(!chunk.contains(root), "the local root must not be part of this chunk");
135+
136+
// Every span in the root-less chunk carries the marker, including the inner grandchild whose
137+
// parent is present in the chunk.
138+
assertAllSpansMarked(chunk, "root-less descendant chunk");
139+
}
140+
141+
@Test
142+
@WithConfig(key = "apm.tracing.enabled", value = "true")
143+
void apmTracingEnabledLeavesChunkUnmarked() throws InterruptedException, TimeoutException {
144+
// Negative control: the method-level override flips APM tracing back on (overriding the
145+
// class-level false before setup() builds the tracer), so the billing marker must NOT be
146+
// stamped on any span. Guards against a regression that marks unconditionally, ignoring the
147+
// apm.tracing.enabled flag.
148+
DDSpan root = (DDSpan) tracer.buildSpan("test", "root").start();
149+
DDSpan child = (DDSpan) tracer.buildSpan("test", "child").asChildOf(root.spanContext()).start();
150+
151+
child.finish();
152+
root.finish();
153+
writer.waitForTraces(1);
154+
155+
List<DDSpan> chunk = writer.get(0);
156+
assertEquals(2, chunk.size(), "expected both spans in the same chunk");
157+
for (DDSpan span : chunk) {
158+
assertNull(
159+
span.getTag(APM_ENABLED),
160+
"span '"
161+
+ span.getOperationName()
162+
+ "' must not carry _dd.apm.enabled when APM tracing is enabled");
163+
}
164+
}
165+
166+
/** Every span of an exported chunk must carry the numeric {@code _dd.apm.enabled:0} marker. */
167+
private static void assertAllSpansMarked(List<DDSpan> chunk, String description) {
168+
for (DDSpan span : chunk) {
169+
assertEquals(
170+
Integer.valueOf(0),
171+
span.getTag(APM_ENABLED),
172+
description + " span '" + span.getOperationName() + "' is missing _dd.apm.enabled:0");
173+
}
174+
}
175+
}

‎internal-api/src/main/java/datadog/trace/api/Config.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,6 @@
195195
import static datadog.trace.api.ConfigDefaults.DEFAULT_WEBSOCKET_MESSAGES_SEPARATE_TRACES;
196196
import static datadog.trace.api.ConfigDefaults.DEFAULT_WEBSOCKET_TAG_SESSION_ID;
197197
import static datadog.trace.api.ConfigSetting.NON_DEFAULT_SEQ_ID;
198-
import static datadog.trace.api.DDTags.APM_ENABLED;
199198
import static datadog.trace.api.DDTags.HOST_TAG;
200199
import static datadog.trace.api.DDTags.INTERNAL_HOST_NAME;
201200
import static datadog.trace.api.DDTags.LANGUAGE_TAG_KEY;
@@ -5198,9 +5197,10 @@ public TagMap getLocalRootSpanTags() {
51985197
result.put(LANGUAGE_TAG_KEY, LANGUAGE_TAG_VALUE);
51995198
result.put(SCHEMA_VERSION_TAG_KEY, SpanNaming.instance().version());
52005199
result.put(DDTags.PROFILING_ENABLED, isProfilingEnabled() ? 1 : 0);
5201-
if (!isApmTracingEnabled()) {
5202-
result.put(APM_ENABLED, 0);
5203-
}
5200+
// The _dd.apm.enabled:0 billing marker is intentionally NOT set here. When APM tracing is
5201+
// disabled it is stamped on every span of each exported chunk (see CoreTracer.write), so that
5202+
// chunks flushed without their local root span (e.g. a late child span) still opt out of APM
5203+
// host billing.
52045204

52055205
if (reportHostName) {
52065206
final String hostName = getHostName();

0 commit comments

Comments
 (0)