Skip to content

Commit cced1e7

Browse files
committed
fix(native-spans): correct flush protocol and WASM reclamation
Addresses review findings across the native span pipeline. Export protocol: - Send one HTTP request per flush. libdatadog stages one chunk per `prepareChunk` and `sendPreparedChunk` drains all of them into a single multi-trace payload, so the previous stage-then-send-per-group chain issued N sequential round trips per flush. Verified against the shipped 0.18.1 binding: three traces in one flush now produce one /v0.4/traces POST carrying three chunks. - Count `.requests`/`.responses` and the OTLP export counters once per request again, the same scale as `.errors`. - Bound payload size. `flush()` takes whole chunks up to a 10k-span soft limit and leaves the remainder for the next send, and `export()` forces a flush at the same limit. Previously a single flush could build an unbounded payload, because a backlog accumulated for the whole duration of an in-flight send. WASM memory: - Free the `WasmSpanState` that `setAgentUrl` replaces. Each state owns an 8 MB change queue in linear memory, which never shrinks, so every rebuild leaked 8 MB. A route on the http client `blocklist` rebuilt state on each filtered request and aborted the process after roughly 4000 of them; 300 rebuilds reached 2428 MB, against a flat 18 MB now. The free is deferred while a send or stats flush still borrows the state. - Amortize reclamation over 10k dropped spans rather than rebuilding per dropped trace. 6000 blocklisted requests now cause zero rebuilds. - Remove the JS-side native discard path. It could not run on any released binding, and on 0.18.1 its `prepareChunk(0, ...)` "unstage" was a no-op, so every dropped trace would have been transmitted on the next flush. libdatadog applies its own client-side p0 drop, so a sampler-rejected trace still never reaches the wire. Startup: - Degrade to the JS pipeline when the runtime has no WebAssembly, not only when libdatadog is absent. Under `node --jitless` the loader throws a bare ReferenceError, which fell through to a silent NoopTracer and lost all tracing. - Guard `setUrl` on the exporter, which the Lambda stdout exporter does not implement. Also from the same review pass: restore the stdout exporter for a Lambda with no local agent, remove the dead agentless encoder and intake modules, seed `error.type` before the OTel HTTP remap so it is no longer overwritten by the status code, and drop a stale CODEOWNERS entry.
1 parent eb9dc11 commit cced1e7

40 files changed

Lines changed: 1965 additions & 1634 deletions

.github/CODEOWNERS

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,8 @@
115115
/packages/dd-trace/test/plugins/util/test.spec.js @DataDog/ci-app-libraries
116116
/packages/dd-trace/test/plugins/util/test-environment.spec.js @DataDog/ci-app-libraries
117117
/packages/dd-trace/src/encode/agentless-ci-visibility.js @DataDog/ci-app-libraries
118-
/packages/dd-trace/src/encode/agentless-json.js @DataDog/ci-app-libraries
119118
/packages/dd-trace/src/encode/coverage-ci-visibility.js @DataDog/ci-app-libraries
120119
/packages/dd-trace/src/encode/tags-processors.js @DataDog/ci-app-libraries
121-
/packages/dd-trace/src/exporters/agentless/ @DataDog/ci-app-libraries
122120
/packages/dd-trace/src/git_metadata.js @DataDog/ci-app-libraries
123121
/packages/dd-trace/src/git_metadata_tagger.js @DataDog/ci-app-libraries
124122
/packages/dd-trace/src/plugins/util/ci.js @DataDog/ci-app-libraries
@@ -340,9 +338,9 @@
340338
/benchmark/sirun/async_hooks/ @DataDog/lang-platform-js
341339
/benchmark/sirun/dogstatsd/ @DataDog/lang-platform-js
342340
/benchmark/sirun/encoding/ @DataDog/lang-platform-js
343-
/benchmark/sirun/exporting-pipeline/ @DataDog/lang-platform-js
344341
/benchmark/sirun/id/ @DataDog/lang-platform-js
345342
/benchmark/sirun/log/ @DataDog/lang-platform-js
343+
/benchmark/sirun/native-span-drain.js @DataDog/lang-platform-js
346344
/benchmark/sirun/runtime-metrics/ @DataDog/lang-platform-js
347345
/benchmark/sirun/scope/ @DataDog/lang-platform-js
348346
/benchmark/sirun/shimmer-runtime/ @DataDog/lang-platform-js
@@ -372,7 +370,6 @@
372370
/packages/datadog-core/ @DataDog/lang-platform-js
373371
/packages/datadog-shimmer/ @DataDog/lang-platform-js
374372
/packages/dd-trace/*/crashtracking/ @DataDog/lang-platform-js
375-
/benchmark/sirun/native-spans/ @DataDog/lang-platform-js
376373
/packages/dd-trace/src/native/ @DataDog/lang-platform-js
377374
/packages/dd-trace/src/exporters/native/ @DataDog/lang-platform-js
378375
/packages/dd-trace/test/native/ @DataDog/lang-platform-js
@@ -388,7 +385,6 @@
388385
/packages/dd-trace/test/dogstatsd.spec.js @DataDog/lang-platform-js
389386
/packages/dd-trace/test/encode/ @DataDog/lang-platform-js
390387
/packages/dd-trace/test/esm-named-exports.spec.js @DataDog/lang-platform-js
391-
/packages/dd-trace/test/exporter.spec.js @DataDog/lang-platform-js
392388
/packages/dd-trace/test/exporters/ @DataDog/lang-platform-js
393389
/packages/dd-trace/test/external-logger/ @DataDog/lang-platform-js
394390
/packages/dd-trace/test/flare.spec.js @DataDog/lang-platform-js

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ Temporary Items
3333
logs
3434
*.log
3535
node-*-junit.xml
36+
.junit-tmp/
3637
npm-debug.log*
3738
yarn-debug.log*
3839
yarn-error.log*

benchmark/sirun/collect-overview.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ const SG_FILE = path.join(require('os').tmpdir(), 'sg-overview.txt')
2929
// Curated per-bench judgment the run cannot measure.
3030
const HIGH_MEANING = new Set([
3131
'shimmer-runtime', 'shimmer-startup', 'scope', 'id', 'spans', 'encoding',
32-
'native-spans', 'propagation', 'async_hooks', 'url', 'startup', 'fs',
32+
'propagation', 'async_hooks', 'url', 'startup', 'fs',
3333
])
3434
const LOW_MEANING = new Set(['plugin-dns'])
3535

3636
const CRITICAL_PATH = new Set([
3737
'shimmer-runtime', 'shimmer-startup', 'scope', 'id', 'spans', 'encoding',
38-
'native-spans', 'propagation', 'async_hooks', 'startup',
38+
'propagation', 'async_hooks', 'startup',
3939
])
4040
const LIVE = new Set(['appsec', 'appsec-iast', 'plugin-http', 'plugin-net'])
4141
const BACKGROUND = new Set(['runtime-metrics', 'profiler', 'log', 'llmobs', 'debugger'])

benchmark/sirun/native-span-drain.js

Lines changed: 140 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,46 +2,164 @@
22

33
const DEFAULT_DRAIN_THRESHOLD = 5000
44

5+
/**
6+
* A local root span leads its chunk so the WASM pipeline treats it as the chunk
7+
* root. Mirror of `#isLocalRoot` in packages/dd-trace/src/exporters/native/index.js.
8+
*
9+
* @param {object} span
10+
* @returns {boolean}
11+
*/
12+
function isLocalRoot (span) {
13+
const context = span.context()
14+
15+
if (!context._parentId) return true
16+
if (context._isRemote) return true
17+
18+
const trace = context._trace
19+
return Boolean(trace) && trace.started.length > 0 && trace.started[0] === span
20+
}
21+
22+
/**
23+
* Mirror of `#syncTraceTags` in the native exporter: trace-level tags live on
24+
* the trace object and are stamped onto the chunk's local root before export.
25+
*
26+
* @param {object} span
27+
*/
28+
function syncTraceTags (span) {
29+
const context = span.context()
30+
const traceTags = context._trace?.tags
31+
32+
if (!traceTags) return
33+
34+
for (const [key, value] of Object.entries(traceTags)) {
35+
// Don't overwrite existing span tags.
36+
if (value !== undefined && value !== null && !context.hasTag(key)) {
37+
context.setTag(key, value)
38+
}
39+
}
40+
}
41+
42+
/**
43+
* Split staged chunks into one `flushSpansGrouped` group per trace, local root
44+
* first. Mirror of `#groupsFromSpanChunks(spanChunks, true)` in the native
45+
* exporter, which is the shape the shipped flush path uses.
46+
*
47+
* @param {Array<Array<object>>} spanChunks
48+
* @returns {Array<{spanIds: Uint8Array[], firstIsLocalRoot: boolean}>}
49+
*/
50+
function groupsFromSpanChunks (spanChunks) {
51+
const groups = []
52+
for (const spans of spanChunks) {
53+
const byTrace = new Map()
54+
for (const span of spans) {
55+
const trace = span.context()._trace
56+
let group = byTrace.get(trace)
57+
if (group === undefined) { group = []; byTrace.set(trace, group) }
58+
group.push(span)
59+
}
60+
61+
for (const group of byTrace.values()) {
62+
const root = group.find(isLocalRoot)
63+
const firstIsLocalRoot = root !== undefined
64+
let ordered = group
65+
if (firstIsLocalRoot) {
66+
syncTraceTags(root)
67+
if (group[0] !== root) {
68+
ordered = [root, ...group.filter(span => span !== root)]
69+
}
70+
}
71+
groups.push({
72+
spanIds: ordered.map(span => span.context()._nativeSpanId),
73+
firstIsLocalRoot,
74+
})
75+
}
76+
}
77+
return groups
78+
}
79+
80+
/**
81+
* Periodically move finished native spans out of WASM storage so a long bench
82+
* loop does not grow the native span map without bound.
83+
*
84+
* Staging mirrors the shipped export path: each processor export call is kept as
85+
* its own trace chunk, every chunk is split into one group per trace with the
86+
* local root first, and the groups go through the public
87+
* `nativeSpans.flushSpansGrouped`. Staging a single chunk for all pending spans
88+
* instead would skip the per-trace `prepareChunk` and the per-chunk trace-tag
89+
* stamping production pays on every flush, so the bench would report the cost of
90+
* a pipeline we do not ship.
91+
*
92+
* @param {object} tracer Initialized tracer
93+
* @param {number} [threshold] Pending spans that trigger a drain
94+
*/
595
function createNativeSpanDrain (tracer, threshold = DEFAULT_DRAIN_THRESHOLD) {
696
const nativeSpans = tracer._tracer._nativeSpans
7-
const pendingSpanIds = nativeSpans ? [] : null
97+
// JS-only mode has nothing in native storage: every entry point stays a no-op.
98+
const pendingChunks = nativeSpans ? [] : null
99+
let pendingCount = 0
100+
let flushedGroups = 0
101+
let problems = 0
102+
const reported = new Set()
8103

9-
function add (span) {
10-
if (pendingSpanIds) {
11-
pendingSpanIds.push(span.context()._nativeSpanId)
12-
}
104+
// A silent catch would let a run that never staged or sent a single chunk
105+
// report clean numbers, hiding exactly the work these benches claim to
106+
// measure. Print the first occurrence of each distinct failure, count the rest
107+
// and summarize at exit, so a broken drain is visible without flooding the
108+
// sirun output on every one of the hundreds of drains a run performs.
109+
function report (message) {
110+
problems++
111+
if (reported.has(message)) return
112+
reported.add(message)
113+
process.stderr.write(`native span drain: ${message}\n`)
114+
}
115+
116+
if (pendingChunks) {
117+
process.on('exit', () => {
118+
if (problems > 0) {
119+
process.stderr.write(
120+
`native span drain: ${problems} failed drain(s), ${flushedGroups} trace group(s) flushed\n`
121+
)
122+
} else if (flushedGroups === 0) {
123+
process.stderr.write('native span drain: no trace group was ever flushed\n')
124+
}
125+
})
13126
}
14127

15128
function addAll (spans) {
16-
if (!pendingSpanIds) return
129+
if (!pendingChunks || spans.length === 0) return
17130

18-
for (const span of spans) {
19-
pendingSpanIds.push(span.context()._nativeSpanId)
20-
}
131+
// SpanProcessor reassigns `trace.started` rather than mutating it, so
132+
// holding this array is safe — the real exporter buffers it the same way.
133+
pendingChunks.push(spans)
134+
pendingCount += spans.length
21135
}
22136

23137
async function drain () {
24-
if (!pendingSpanIds || pendingSpanIds.length === 0) return
138+
if (!pendingChunks || pendingCount === 0) return
25139

26-
nativeSpans.flushChangeQueue()
140+
const groups = groupsFromSpanChunks(pendingChunks)
141+
pendingChunks.length = 0
142+
pendingCount = 0
27143

28-
const spanIds = Buffer.allocUnsafe(pendingSpanIds.length * 8)
29-
let offset = 0
30-
for (const spanId of pendingSpanIds) {
31-
spanIds.set(spanId, offset)
32-
offset += 8
144+
try {
145+
// flushSpansGrouped drains the change queue itself, then prepares one
146+
// chunk per group and sends them as a single request.
147+
const response = await nativeSpans.flushSpansGrouped(groups)
148+
if (response === 'no spans to flush') {
149+
report(`staged no chunk for ${groups.length} trace group(s)`)
150+
} else {
151+
flushedGroups += groups.length
152+
}
153+
} catch (err) {
154+
report(`flushSpansGrouped rejected: ${err?.message ?? err}`)
33155
}
34-
35-
nativeSpans._state.prepareChunk(pendingSpanIds.length, false, spanIds)
36-
await nativeSpans._state.sendPreparedChunk().catch(() => {})
37-
pendingSpanIds.length = 0
38156
}
39157

40158
function needsDrain () {
41-
return pendingSpanIds && pendingSpanIds.length >= threshold
159+
return pendingCount >= threshold
42160
}
43161

44-
return { add, addAll, drain, needsDrain }
162+
return { addAll, drain, needsDrain }
45163
}
46164

47165
module.exports = { createNativeSpanDrain }

benchmark/sirun/spans/spans.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,16 @@ nock('http://127.0.0.1:8126').persist().put(/.*/).reply(200, '{}').post(/.*/).re
1212
const tracer = require('../../..').init({ hostname: '127.0.0.1', port: 8126 })
1313
const nativeSpanDrain = createNativeSpanDrain(tracer)
1414

15-
tracer._tracer._processor.process = function process (span) {
16-
const trace = span.context()._trace
17-
nativeSpanDrain.add(span)
18-
this._erase(trace, [])
15+
// Replace only the exporter, not the processor: the whole per-span cost this
16+
// bench measures (priority/span sampling, trace-tag sync to native, span
17+
// formatting and the final meta/metrics batch in syncFinalTagsToNative) lives in
18+
// SpanProcessor#process. Overriding process() would drop all of it and the
19+
// with-tags variants would measure a tag-less span. The collector keeps real
20+
// network I/O out of the measurement while native spans still get drained.
21+
tracer._tracer._processor._exporter = {
22+
export (spans) {
23+
nativeSpanDrain.addAll(spans)
24+
},
1925
}
2026

2127
const { FINISH, SHAPE = 'plain' } = process.env

ext/exporters.d.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
declare const exporters: {
22
AGENT: 'agent',
3-
AGENTLESS: 'agentless',
43
DATADOG: 'datadog',
54
AGENT_PROXY: 'agent_proxy',
65
JEST_WORKER: 'jest_worker',

ext/exporters.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use strict'
22
module.exports = {
33
AGENT: 'agent',
4-
AGENTLESS: 'agentless',
54
DATADOG: 'datadog',
65
AGENT_PROXY: 'agent_proxy',
76
CI_VALIDATION: 'ci_validation',

0 commit comments

Comments
 (0)