feat(native-spans): native span pipeline via libdatadog wasm - #9139
feat(native-spans): native span pipeline via libdatadog wasm#9139bengl wants to merge 170 commits into
Conversation
… span types + exporter) Introduces the native-spans pipeline as a self-contained subsystem. Spans flow through @DataDog/libdatadog's WASM-backed TraceExporter via a change-buffer protocol instead of being formatted in JS and shipped over the agent's HTTP endpoint. native/index.js — libdatadog pipeline loader. - Lazy require + maybeLoad; exposes `available`, OpCode, WasmSpanState, wasmMemory, plus lazy class re-exports via a loadWithNoop helper that prevents fs-instrumentation recursion during module loading. - Split try/catch: MODULE_NOT_FOUND is silent (expected on platforms without libdatadog); other require errors get log.warn (corrupt package / EACCES on the .node binary / etc.); pipeline.init or setStorage failures get log.error. native/native_spans.js — NativeSpansInterface, the JS-side bridge. - Slot allocator (u32 slot indices, not raw spanIds). - String table with rollback-on-failure ordering: WASM-side insert runs before the JS-side map set, so a thrown insert never leaves the JS map claiming a string is interned at a dangling id. - Change-buffer wire format (header + per-op records). Three queue methods: queueOp, queueCreateSpan (op=13), plus the batch helpers queueBatchMeta (op=15) / queueBatchMetrics (op=16). The class-level JSDoc documents the byte layout. - Detach-safety invariant: every WASM call that can grow memory is followed by #checkDetach() to refresh the cached _cqbView / _cqbBytes views. No entry-time check on queue methods; the inner getStringId loop self-heals before any view writes happen. - Atomic setAgentUrl(): builds the new WasmSpanState before clearing JS-side bookkeeping, so a thrown WasmSpanState constructor leaves the existing state consistent. - flushChangeQueue() and flushSpans() rethrow after resetting JS-side state + refreshing views, so callers get a loud signal instead of a silent half-flushed buffer. - Periodic stats flush registered via globalThis dd-trace beforeExitHandlers with a process.once fallback. native/span.js + native/span_context.js — NativeDatadogSpan and NativeSpanContext. - NativeDatadogSpan extends DatadogSpan, overriding only the methods that need native-storage sync (constructor, _createContext, setTag, _addTags, finish). Baggage, links/events, scope, util.inspect, sanitization helpers, and the rest of the OpenTracing surface are inherited unchanged. - NativeSpanContext extends DatadogSpanContext. Slot-indexed; `_name` setter queues SetName via _syncNameToNative, with a construction- time no-op shadow so the parent constructor's initial name write doesn't double-emit alongside queueCreateSpan. - Batched-sync hot path for _addTags: a plain-object input writes directly to the JS cache + syncToNativeOnly in one pass. Priority short-circuits prioritySampler.sample() once a priority is decided. - #serializeSpanLinks / #serializeSpanEvents apply MAX_META_VALUE_LENGTH truncation matching the JS exporter path; oversized payloads would otherwise be silently rejected by the agent. - _createContext throws + frees the slot if a NativeSpanContext is passed as fields.context — re-wrapping would either leak the slot or duplicate the span. exporters/native/index.js — NativeExporter. - Batches raw span objects (not pre-formatted msgpack) for chunked export via NativeSpansInterface.flushSpans. - Atomic setUrl(): parse first, only assign this._url after the native setAgentUrl succeeds. - In-flight serialization: a second flush() while the first is unresolved buffers spans rather than starting a parallel send. Drains _pendingSpans on both success and rejection. - beforeExit handler registered on the dd-trace shared handler set (with process.once fallback), preventing listener leaks on repeated tracer construction. The subsystem compiles in isolation but is not wired into any tracer or exporter path yet — see the follow-up integration commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, and OTel bridge
Wires the native subsystem into the existing tracer paths. The tracer
constructs the native pipeline whenever libdatadog is available; on
platforms where the native module can't be loaded, the tracer logs a
warn and falls back to the JS implementation.
opentracing/tracer.js:
- Lazy getNativeModule() so installs whose tracer never starts don't
pull libdatadog into memory.
- Native init in the DatadogTracer constructor constructs
NativeSpansInterface + NativeExporter + SpanProcessor with the
native interface attached when libdatadog is available.
- Warns once at init when libdatadog is unavailable so the JS
fallback isn't silent.
- Warns once at init when DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED
is set together with native mode (the JS-side spanFormat() path is
what emits process tags; the native exporter doesn't yet).
span_processor.js:
- Detects native mode via the nativeSpans constructor arg.
- _sampleNative: runs JS-side sampling (manual overrides first,
otherwise the standard priority sampler), then mirrors the
resulting priority/mechanism into native storage via
_syncSamplingToNative.
- getNativeOpCode is a lazy resolver; caches only on non-null,
log.errors once and returns null if OpCode is missing so the
caller short-circuits instead of NPE-ing in the sampling hot path.
opentelemetry/span.js:
- The OTel-API bridge constructs a NativeDatadogSpan instead of a
DatadogSpan when _tracer._nativeSpans is set; both pass to
super(ddSpan) so BridgeSpanBase still owns the OTel surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds 86 tests across five native specs plus targeted updates to span_processor.spec.js, covering the native subsystem and its integration points. - integration.spec.js: end-to-end against the real NativeSpansInterface (skipped when libdatadog is unavailable on the platform) — tracer wiring, full span lifecycle, double-finish protection, parent->child via active scope, service/resource/type via tracer.trace, error propagation, inject+extract round-trip. - native_spans.spec.js: queue methods, change-buffer detach-safety, flushChangeQueue rethrow + state reset, flushSpans rejection-path recovery, getStringId WASM-first ordering rollback, atomic setAgentUrl swap (including preservation on ctor failure). - span.spec.js: NativeDatadogSpan native-only behavior — combined queueCreateSpan op on construction, no-double SetName on init, slot-free + throw on duplicate-NativeSpanContext wrapping, syncOneTagToNative / syncToNativeOnly call assertions, priority short-circuit on tag application, SetDuration on finish. Behavior inherited from DatadogSpan is exercised by packages/dd-trace/test/opentracing/span.spec.js. - exporter.spec.js: in-flight serialization, drain on success and rejection, beforeExitHandlers registration. - span_context.spec.js: setTag side-effects to the WASM pipeline (covering SetServiceName / SetResourceName / SetType / SetError / SetMetaAttr / SetMetricAttr / SetTraceMetaAttr / SetTraceMetricsAttr / SetTraceOrigin), _syncNameToNative, nativeSpanId getter. The strengthened "should reset queue state when prepareChunk throws" test isolates the catch arm by stubbing flushChangeQueue to a no-op so the only observable cleanup path is the flushSpans catch — without this, the success-path reset inside flushChangeQueue would mask whether the catch arm runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Non-runtime support for the native-spans feature. Benchmarks: - benchmark/sirun/native-spans/* — creation, tagging, parent-child, pipeline, get-tag, and verify scenarios for the native pipeline. - benchmark/sirun/spans/* — small adjustments to align with the new per-span hot paths. Build/test plumbing: - package.json: test:trace:core glob includes the new `native` directory. - test/setup/core.js: clears OTEL_EXPORTER_OTLP_* env vars at suite start so plugin tests can stub http.request without observability tooling shells (Claude Code, etc.) hijacking traces through OTLP. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`@DataDog/libdatadog` moves out of `optionalDependencies` into `dependencies` — native spans are the only supported span pipeline going forward, and the WASM runtime that backs them must always be installed. With the optional install path gone, the JS-side fallback exporter, the JS-side span/stats formatter, and the OTLP-trace fallback exporter are all unreachable; this change deletes them and collapses the remaining call sites onto the native path. The dependency change is a stopgap. `@DataDog/libdatadog@0.9.3` is ~36 MB unpacked due to the bundled native binary set, which is what motivated optional-dep status originally. A smaller alternative is being explored elsewhere. Production modules deleted (no callers outside the JS-fallback branch): - packages/dd-trace/src/exporter.js (the JS-exporter resolver) - packages/dd-trace/src/exporters/agentless/ (index.js + writer.js) - packages/dd-trace/src/exporters/log/ - packages/dd-trace/src/exporters/span-stats/ (index.js + writer.js) - packages/dd-trace/src/span_format.js (JS-side span formatter) - packages/dd-trace/src/span_stats.js (JS-side stats processor) - packages/dd-trace/src/opentelemetry/trace/ (OTLP-trace exporter: index.js + otlp_http_trace_exporter.js + otlp_transformer.js) Production fallback branches removed: - packages/dd-trace/src/opentracing/tracer.js — drop the `if (getNativeModule().available)` outer gate, the surrounding try/catch, the JS-side `Exporter` + `SpanProcessor` construction, the `OTEL_TRACES_EXPORTER === 'otlp'` branch, and the unused `getExporter` import. - packages/dd-trace/src/opentelemetry/span.js — drop the JS `DatadogSpan` branch in the constructor; always build a `NativeDatadogSpan`. - packages/dd-trace/src/span_processor.js — drop the `_nativeSpans === null` branch in `sample()`, the `useJsFormatter` branch and JS-stats fallback in `process()`, the `SpanStatsProcessor` setup in the constructor, the `_stats` field, and the `spanFormat` import. The `nativeSpans` parameter is now required. - packages/dd-trace/src/native/index.js — `@DataDog/libdatadog` becomes a top-level require. Pipeline loading is deferred to first access (via a lazy `getPipeline()` helper) so importing this module from a unit test doesn't require a working pipeline binary, but any use throws hard if the pipeline can't load. The `available` boolean is gone. Test files deleted (one-to-one with deleted production): - packages/dd-trace/test/exporter.spec.js - packages/dd-trace/test/exporters/agentless/ (entire dir) - packages/dd-trace/test/exporters/log/ - packages/dd-trace/test/exporters/span-stats/ - packages/dd-trace/test/span_format.spec.js - packages/dd-trace/test/span_stats.spec.js - packages/dd-trace/test/opentelemetry/traces.spec.js Test files updated: - packages/dd-trace/test/span_processor.spec.js — rewritten around the native-only path; now seeds `nativeSpans` and `trace.tags` so `_sampleNative` / `_addDecisionMaker` have valid inputs, and asserts raw-span export instead of spanFormat output. Switches to `proxyquire.noCallThru()` so stubbing `./native` doesn't trigger the real pipeline load. - packages/dd-trace/test/opentracing/tracer.spec.js — rewritten to stub `NativeSpansInterface`, `NativeDatadogSpan`, and `NativeExporter` via proxyquire instead of asserting on the old AgentExporter / JS-exporter selection path. - packages/dd-trace/test/opentelemetry/span.spec.js — drops the now-deleted `span_format` import; the link- and exception-format assertions read the span context tags directly (the native span path serializes `_dd.span_links` / `ERROR_*` onto the context during finish). - packages/dd-trace/test/native/integration.spec.js — removes the `(skipped)` branch and de-indents the unconditional describe block. - packages/dd-trace/test/native/native_spans.spec.js and packages/dd-trace/test/native/span_context.spec.js — switch to `proxyquire.noCallThru()` so the in-test `./index` stubs no longer trigger proxyquire's callThru fallback into the real `src/native/index.js`. Bug also fixed in this change: `NativeDatadogSpan._addTags` was using `for-in`, which silently skipped Symbol-keyed entries like `IGNORE_OTEL_ERROR` (set by the OTel bridge's `applyOtelStatus`). The JS `DatadogSpan` parent uses `Object.assign`, which handles Symbols; the native subclass now does the same. `syncToNativeOnly` in `span_context.js` switches from `for-in` to `Object.keys` for the same reason and to honor the project's no-`for-in` rule. Notes: - `experimental.exporter` stays in the config schema: `ci_plugin.js` still reads it for CI worker-framework detection, and `_DD_APM_TRACING_AGENTLESS_ENABLED` still writes it (the write is a no-op now, but doesn't hurt). - Native span context already mirrors error/span-link encoding behaviorally; comments referencing `span_format.js` in `src/native/span.js` and `src/native/span_context.js` are left in place as historical references. - `Span` import (`./span` = JS DatadogSpan) is kept in `src/opentracing/tracer.js`: `NativeDatadogSpan` extends it, and `inject()` uses an `instanceof Span` check that still has to work for native spans. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The legacy agent writer publishes `dd-trace:exporter:first-flush` on its first flush; the native exporter introduced in this branch never did, so subscribers (notably the aborted-integrations log) never fire under the native span pipeline. Publish exactly once on the first successful native flush, gated on a private flag. The rejection path stays silent so transient agent errors don't trip the signal. Signed-off-by: Bryan English <bryan.english@datadoghq.com>
The protocol-version block in #applyEnvironment flipped OTEL_TRACES_EXPORTER from 'otlp' to 'none' whenever DD_TRACE_AGENT_PROTOCOL_VERSION was set. It originally guarded the JS-side OTLP traces exporter, which has been removed alongside the move to the native-span pipeline. With no OTLP exporter to disable, the only remaining effect was suppressing the OTel-sampler default for users who opted into 'otlp' on a non-default agent protocol version. Drop the block. The sampler-default branch downstream now fires consistently whenever OTEL_TRACES_EXPORTER='otlp' is set, regardless of DD_TRACE_AGENT_PROTOCOL_VERSION.
After the JS span pipeline was removed, two pre-existing comments in master-side files still named the deleted `span_format.js` as if it were live code: - `test/opentelemetry/context_manager.spec.js`: rationale for the numeric-startTime assertion attributed to span_format's `Math.round(startTime * 1e6)`. Reworded to point at the downstream ms-conversion generally; the test still guards the same shape. - `src/service-naming/extra-services.js`: comment claimed the cache exists for span_format's per-span hot path. The whole module has no production caller now (only tests + the global mocha clear hook); noted that explicitly so a future reader knows the cache is currently dormant rather than serving a hidden caller.
span_format.js previously copied _dd.rule_psr, _dd.limit_psr, and _dd.agent_psr from context._trace[KEY] onto the root span at format time. The native pipeline dropped that path. The native WASM pipeline supports these via SetTraceMetricsAttr (OpCode 11), which writes into trace.metrics and copies onto the chunk root span at flush. This restores the wiring by queuing SetTraceMetricsAttr ops for each sampling-decision metric present on spanContext._trace.
Native pipeline expects the caller to emit _dd.span_sampling.{mechanism,rule_rate,max_per_second} per-span metrics. These were previously set by span_format.setSingleSpanIngestionTags(), which was removed when the native pipeline replaced span_format.
Emit the three metrics inline in SpanSampler.sample() via queueBatchMetrics on the native spans interface, in the same location where _spanSampling is stamped.
registerExtraService was previously called from span_format.extractTags() (per span at format time). With the JS format pipeline removed in favor of the native WASM exporter, the extra_services Set was always empty and remote-config could no longer see per-service registrations, impacting AppSec service routing and other RC-driven service routing. Restore by calling registerExtraService in SpanProcessor.process() while iterating finished spans before export, matching the original timing.
This removes the unused _DD_APM_TRACING_AGENTLESS_ENABLED environment variable and all associated dead code. The feature was experimental and never fully shipped. The primary effect is removing the config block from `packages/dd-trace/src/config/index.js`.
AGENTS.md disallows async/await in production code outside test files
and worker threads (`packages/dd-trace/src/debugger/devtools_client/`).
`flushSpans` was async and awaited `_state.sendPreparedChunk()` inside
a try/catch.
Rewrite as a non-async function that returns a Promise via .then() /
.catch(). Behavior is identical:
- Same return shape: Promise<string> resolving to the agent response,
or 'no spans to flush' for empty slots.
- Same cleanup on either prepareChunk-throw or sendPreparedChunk-rejection:
resetChangeQueue + #checkDetach + log.error('Error flushing spans to
agent:', e), then propagate the error.
Test files keep their `await flushSpans()` calls; `await` works on any
thenable, and tests are exempt from the no-async/await rule.
The native span pipeline removed the JS span formatter (span_format.js) which previously added _dd.tags.process to the local root span. Restore it by adding the tag in the native exporter's #syncTraceTags method, guarded by DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED. Tag value comes from process-tags/index.js.
Old span_format.js automatically marked spans with span.kind (except internal) as measured so the agent computes metrics. This was lost when the JS formatter was removed. Add 'span.kind' to SPECIAL_KEYS to bypass the fast path, and emit SetMetricAttr for _dd.measured in the switch case.
The native span_context setTag path diverged from the deleted JS formatter (span_format.js) in three ways: - Plain object tag values were stringified to "[object Object]" instead of being flattened one level into key.prop entries. - NaN number metrics were emitted as f64 NaN; the old formatter dropped them entirely. - tracer.js warned that DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED was unsupported, but the native exporter already emits _dd.tags.process on the local-root span. Remove the stale, contradictory warning. Introduce a shared appendTag helper mirroring the legacy addTag coercion (strings to meta, finite numbers to metrics, booleans to 0/1, plain objects flattened one level, arrays/Buffer/URL stringified as meta) and route all four value-handling sites through it to avoid divergence.
The native exporter discarded the value resolved by flushSpans. That value is the agent's response body, which carries rate_by_service used for adaptive (agent-driven) priority sampling. Without it the JS sampler never adapted to agent feedback and fell back to static rates \u2014 a regression from the legacy AgentWriter (which called prioritySampler.update(rate_by_service) on every response). The native sendPreparedChunk already surfaces this body to JS: it resolves 'unchanged' when the rates payload-version header matches the prior flush (no body), otherwise the raw JSON body. Parse the latter and forward rate_by_service to the priority sampler, swallowing/logging any malformed response so it never disrupts the flush cycle.
…rotocol The native change-buffer protocol moved from slot-indexed ([opcode:u64][slotIndex:u32]) to span_id-addressed ([opcode:u16][span_id:u64]) in the @DataDog/libdatadog bindings built against libdatadog main. Port the JS native layer to match. src: - op header: u16 opcode + the 8-byte LE span id (_nativeSpanId) as the handle; drop the u32 slot index and all slot allocation (allocSlot/freeSlots/_slotIndex removed across span_context, span, span_processor, span_sampler, native exporter) - CreateSpan: span_id moves to the header; new segment_id arg (trace_id u128, segment_id u64, parent_id u64, name_id u32, start i64). One segment_id per local trace, allocated on the shared _trace object and reused by children (required: native chunk flush keys by segment) - flush chunk buffer carries u64 span ids (8B) instead of u32 slots - batch meta/metric ops re-headered the same way tests (native_spans, span, span_context, exporter, span_sampler specs): - updated to the span_id protocol: u16 opcode offsets, span_id (Uint8Array) handle, segment allocator, queueCreateSpan signature, span-id flush arrays, and _nativeSpanId mock handles in the sampler spec. Verified: lint clean; the four test/native specs pass (67 tests); and an end-to-end run against the real span_id wasm (create root+child, Set*/Batch ops, flush) had the Rust decoder accept the chunk and emit a 460-byte payload. span_sampler.spec.js is ported + lint-clean but exercised by CI (it needs the built vendor/dist, unavailable locally).
meta_struct (AppSec, Code Origin, Dynamic Instrumentation) was dropped on the native path \u2014 there was no binding to set it, so structured per-span data never reached the agent. Add a setMetaStruct wrapper on NativeSpansInterface that drains the change queue first (the WASM binding flushes internally, so the JS-side queue offsets must stay in sync) and forwards the value to the new WasmSpanState.setMetaStruct, folding the 8-byte handle big-endian to the numeric span id. NativeDatadogSpan.finish now msgpack-encodes each qualifying meta_struct entry and forwards it, matching the legacy encoder's map<string, bin> wire shape and value filter (string/number/non-null object). Requires the companion WasmSpanState.setMetaStruct binding in libdatadog-nodejs.
A `unix://` agent URL already flows through to libdatadog's exporter and works once the wasm transport honors the socket path. Windows named pipes are represented as `unix://./pipe/...` (the legacy exporter's form), but ddcommon's parse_uri expects the `windows:` scheme for pipes. Normalize the agent URL in the single WasmSpanState construction path so the pipe form is rewritten to `windows://./pipe/...`; UDS and http(s) URLs pass through unchanged.
The native exporter emitted no tracer health metrics, so operators lost the datadog.tracer.node.exporter.agent.* counters the legacy AgentWriter produces (libdatadog's telemetry/health-metrics are native-only, cfg(not wasm32), and weren't wired from JS). Emit, around each native flush, matching the legacy metric names: - .requests on send - .responses on success - .errors + .errors.by.name (+ .errors.by.code) on failure .responses.by.status is intentionally omitted \u2014 sendPreparedChunk does not surface the HTTP status (libdatadog owns the transport). Covered by two new exporter.spec.js tests.
When DD_TRACE_NATIVE_SPAN_EVENTS is enabled (matching the legacy 0.4 encoder's gate), serialize each span event through the new native addSpanEvent setter so it lands in libdatadog's top-level v0.4 span_events field with typed attributes \u2014 no truncation. When the flag is off, keep the existing (lossy) _dd.span_events meta-tag fallback. Attributes are encoded into the flat little-endian buffer the pipeline crate decodes (per-value type tags String=0, Boolean=1, Integer=2, Double=3, Array=4); integer-valued numbers go out as i64, the rest as f64. The native_spans wrapper drains the change queue and refreshes detached views, mirroring setMetaStruct.
When DD_TRACE_AGENT_PROTOCOL_VERSION resolves to 0.5, the native exporter now fetches the agent /info (reusing agent/info.js) and, if the agent advertises /v0.5/traces, switches the native trace exporter to v0.5 via setUseV05(). v0.5 is opt-in and capability-gated: the v0.5 wire schema has no slot for meta_struct (or top-level span_events), so libdatadog silently drops them in v0.5 mode, matching dd-trace-js master's v0.5 encoder. Gating on both explicit config and agent advertisement avoids dropping that data for anyone who did not ask for v0.5. Negotiation is async; until it resolves the exporter stays on v0.4 (the safe default), and the selection is preserved across setAgentUrl().
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 730b231550
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e channel, dm gating - span_context: skip SetError for error.type when IGNORE_OTEL_ERROR is set, so otel recordException() no longer flips the span error bit (only setStatus(ERROR) does). Mirrors span_format.js. - span: publish dd-trace:span:tags:update after native addTags so subscribers (e.g. the wall profiler's web-tag refresh) still fire on the fast path. - span_processor: write _dd.p.dm only for kept traces (priority >= AUTO_KEEP), matching the legacy priority sampler, instead of whenever a mechanism is set. Adds regression tests for all three.
|
@codex review |
`addEvent` and the OTel bridge do not type-check `name`, so an untyped caller can pass a non-string. On the native path that value reached the WASM string parameter during `finish()` and threw out into application code. The legacy v0.4 span_events encoder drops these events and keeps encoding the rest of the trace (encode/0.4.js), so apply the same guard before calling `addSpanEvent`. The `events` meta fallback is unaffected: it JSON-stringifies the whole array and never throws. Reported by Codex review as P2.
At `flushInterval: 0` a coalesced flush sends one request per group so each trace keeps its own payload. Every response carries its own `rate_by_service`, but only whatever the chain settled with reached `#updateSamplingRates`. An early request returning fresh rates followed by a later `unchanged` left agent-driven sampling stale. Feed each response to the sampler as it arrives instead. Reported by Codex review as P2.
The agent returns `Datadog-Container-Tags-Hash` whenever the request carried a container id, and the legacy writer feeds it to the propagation hash so DBM SQL comments and DSM pathway hashes correlate with container tags. The native path never read it, so those hashes kept using process tags alone. libdatadog's transport already offers a response-header observer, so register one when the pipeline is initialised. It takes Node's flat `rawHeaders` array, and it hangs off the module rather than the span state, so it survives the `setAgentUrl` rebuild. Reported by Codex review as P2.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3786df7073
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| exporter._writer?.flush() | ||
| exporter.flush() |
There was a problem hiding this comment.
In AWS Lambda without the extension or mini-agent, the tracer selects LogExporter, which only implements export(), but this now calls exporter.flush() unconditionally. Any OpenTelemetry user invoking TracerProvider.forceFlush() in that environment gets a synchronous TypeError, and the active span processor is never flushed; preserve the prior optional behavior or add a compatible flush implementation.
AGENTS.md reference: AGENTS.md:L222-L224
Useful? React with 👍 / 👎.
| this.#nativeSpans.queueOp(OpCode.SetName, spanId, String(formatted.name)) | ||
| this.#nativeSpans.queueOp(OpCode.SetResourceName, spanId, String(formatted.resource)) | ||
| if (typeof formatted.service === 'string') { | ||
| this.#nativeSpans.queueOp(OpCode.SetServiceName, spanId, formatted.service) |
There was a problem hiding this comment.
For spans whose operation or service exceeds 100 characters, or whose resource exceeds 5,000 characters, these values bypass the existing normalizeSpan() step used by every legacy encoder and are forwarded unchanged to native storage. This removes the established defaults and intake-limit truncation from the default pipeline, so high-cardinality route/resource values can produce oversized or rejected spans; reuse the existing normalization rules before queuing these fields rather than maintaining a divergent path.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| // `DD_TRACE_NATIVE_SPAN_EVENTS` gate), append each event to the top-level | ||
| // v0.4 `span_events` field via the native setter — no truncation, typed | ||
| // attributes. Otherwise fall back to the `events` meta tag (plain JSON). | ||
| if (this.tracer()._config.DD_TRACE_NATIVE_SPAN_EVENTS) { |
There was a problem hiding this comment.
When OTEL_TRACES_EXPORTER=otlp is used with the default DD_TRACE_NATIVE_SPAN_EVENTS=false, this branch serializes events into the legacy meta.events JSON tag instead of the native top-level event field. The removed OTLP transformer converted raw span events regardless of this agent-protocol flag, so OTLP collectors now receive a string attribute rather than structured OTLP events, breaking consumers of exception and event data; use the native event path whenever the destination is OTLP.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| // Datadog HTTP tags out of the WASM store and syncs the OTel-named ones at | ||
| // finish (WASM has no remove-meta op, so eagerly-synced DD keys couldn't be | ||
| // dropped). Read on the hot tag-sync path, so keep it a plain field. | ||
| this.otelSemanticsEnabled = options.otelSemanticsEnabled || false |
There was a problem hiding this comment.
With both OTLP export and DD_TRACE_OTEL_SEMANTICS_ENABLED=true, this option is retained only for the JS HTTP-tag remap and is never forwarded into WasmSpanState or its OTLP mapper. The removed transformer used this flag to omit Datadog-only service.name, operation.name, resource.name, span.type, and error.message attributes while retaining their dedicated OTLP fields; the native mapper therefore emits the non-semantic attributes even in the requested pure-OTel mode. Forward the flag to the native mapper or suppress those fields before mapping.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| const useCustomLookup = typeof config.lookup === 'function' && | ||
| lookupOrigin !== 'default' && | ||
| !config.isCiVisibility && | ||
| !useElectronExporter |
There was a problem hiding this comment.
When a user configures both OTEL_TRACES_EXPORTER=otlp and a custom lookup, this predicate selects the JS pipeline, whose ternary constructs AgentExporter; the OTLP endpoint is never configured and every span is sent to the Datadog agent instead. Fresh evidence beyond the earlier OTLP fix is that useCustomLookup does not exclude useOtlpExporter, despite OTLP otherwise taking precedence in Lambda and native setup. Preserve OTLP routing for this combination rather than treating the collector configuration as an agent lookup fallback.
Useful? React with 👍 / 👎.
| export (spans) { | ||
| if (this.#disabled) return |
There was a problem hiding this comment.
After a NativeExporterBuildError, #disabled remains true for the tracer lifetime, so this early return drops every later export without calling prepareChunk or otherwise removing those spans from WasmSpanState; _discardNativeSpans and the idle reset are also disabled by the same flag. A service that keeps running after one fatal exporter-build failure therefore accumulates every subsequent native span until process exit, even though their contexts are marked exported. Discard/reset native state on this path or stop allocating native spans once export is disabled.
AGENTS.md reference: AGENTS.md:L194-L200
Useful? React with 👍 / 👎.
The custom-`lookup` guard did not exclude OTLP, so `OTEL_TRACES_EXPORTER=otlp` plus a configured `lookup` selected the JS pipeline, which builds an `AgentExporter`. The OTLP endpoint was never configured and every span went to the Datadog agent instead of the collector. OTLP export lives in libdatadog, so the JS pipeline cannot do it at all. Give OTLP precedence, exactly as the Lambda pipeline already does, and warn that the `lookup` cannot be honoured rather than dropping it in silence. Reported by Codex review as P1, against the commit that added the guard.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 956052e382
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #releaseState (state) { | ||
| if (this.#sendInFlight === null) { | ||
| state.free() | ||
| return | ||
| } | ||
| const free = () => state.free() | ||
| this.#sendInFlight.then(free, free) |
There was a problem hiding this comment.
When native stats are enabled, either the 10-second interval or an explicit flushStats() can still be awaiting _state.flushStats() when tracer.setUrl() replaces the state. This release guard tracks only #sendInFlight, so it immediately calls free() on the old state during an in-flight asynchronous stats operation; that operation can then access released WASM state, losing stats or surfacing a runtime failure. Track stats-flush promises alongside trace sends before freeing the superseded state.
AGENTS.md reference: AGENTS.md:L194-L200
Useful? React with 👍 / 👎.
| this._nativeSpans.addSpanEvent( | ||
| this._spanContext._nativeSpanId, | ||
| event.name, | ||
| BigInt(Math.round(event.startTime * 1e6)), |
There was a problem hiding this comment.
With DD_TRACE_NATIVE_SPAN_EVENTS=true, an untyped caller can pass Infinity as the startTime to span.addEvent(), which reaches this conversion and makes BigInt(Infinity) throw synchronously from span.finish(). The legacy encoder accepts the numeric value without throwing, so native mode can now crash an otherwise successful instrumented operation; validate the computed timestamp and drop or normalize invalid events before the BigInt/native call.
AGENTS.md reference: AGENTS.md:L222-L225
Useful? React with 👍 / 👎.
| const createStartTime = fields.startTime === undefined | ||
| ? spanContext._trace.startTime + now() - spanContext._trace.ticks | ||
| : fields.startTime | ||
| fields.startTime = createStartTime |
There was a problem hiding this comment.
When startSpan() receives startTime: 0, this branch queues the native Create with an epoch start and leaves fields.startTime as zero, but the superclass subsequently treats that falsy value as absent and replaces it with _getTime(). The duration is therefore calculated from the current JS start while the exported native span still starts at epoch zero, producing an inconsistent timestamp that can be rejected or reported incorrectly; normalize the value using the superclass's existing semantics before queuing it.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
`TracerProvider.forceFlush()` called `exporter.flush()` unconditionally. In a Lambda with neither the extension layer nor the mini agent the tracer selects the stdout exporter, which writes synchronously and implements only `export()`, so any OpenTelemetry user calling `forceFlush()` there got a synchronous TypeError and the active span processor was never flushed either. Reported by Codex review as P2.
The v0.4 encoder runs `normalizeSpan` on every span as it encodes (`encode/0.4.js` selects it as the per-span formatter), so the JS pipeline never ships a span missing the intake defaults or exceeding the 100-character caps on service, name and type. The native path wrote `formatted.name` / `.service` / `.type` straight into WASM, making it the only pipeline that could emit un-normalized core fields — so a high-cardinality route name went out at full length. Apply the same pass at the native write, after the stats snapshot, which matches the legacy ordering where normalization happens at encode time rather than at finish. Reported by Codex review as P2. Note the report also mentions the 5,000 character resource cap; that is `truncateSpan`, which the v0.4 agent path does not apply either (only the electron and agentless encoders do), so it is deliberately left alone.
The `meta.events` JSON fallback exists for agents that cannot read the native `span_events` field, gated on `DD_TRACE_NATIVE_SPAN_EVENTS`. That gate is about the agent protocol, so with `OTEL_TRACES_EXPORTER=otlp` and the default flag value every event reached the collector as a JSON string attribute instead of a structured OTLP event, breaking consumers of exception data. The deleted OTLP transformer converted events regardless of the flag. Take the native path whenever the destination is OTLP. Reported by Codex review as P2.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f3148e54b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const resourceBefore = typeof tags['resource.name'] === 'string' ? tags['resource.name'] : undefined | ||
| const errorBefore = tags.error ? 1 : 0 | ||
| const view = { meta, metrics, error: errorBefore, resource: resourceBefore } |
There was a problem hiding this comment.
When DD_TRACE_OTEL_SEMANTICS_ENABLED is enabled and an HTTP span has both an error: Error tag and an error status (for example, a server 500), spanFormat() has already produced and synced the exception-derived error.type, but this rebuilt view starts only from the raw tag map and therefore omits that generated field. applyHttpOtelSemantics() consequently treats the error type as absent and emits the status code as error.type, overwriting the exception class in native storage; seed the remap from the formatted span or otherwise preserve its generated error metadata.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| .then(applyResponse), | ||
| Promise.resolve('no spans to flush') | ||
| ) | ||
| : this._nativeSpans.flushSpansGrouped(groups).then(applyResponse) |
There was a problem hiding this comment.
With the default nonzero flushInterval, every trace accumulated during the interval is passed to one flushSpansGrouped() call, with no equivalent of the legacy encoder's 8 MiB soft flush or 50 MiB hard payload cap. A high-throughput service or a batch containing large traces can therefore produce a request above the agent's 50 MiB intake limit; because all groups have already been extracted from native storage before the request is rejected, the entire interval's traces are lost. Split or flush groups before the assembled native payload crosses the established limit.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| // to OTLP only in that case by leaving the native concentrator disabled. | ||
| statsEnabled: (config.stats?.DD_TRACE_STATS_COMPUTATION_ENABLED && | ||
| !config.OTEL_TRACES_SPAN_METRICS_ENABLED) || false, | ||
| hostname: config.hostname || os.hostname(), |
There was a problem hiding this comment.
When native client-computed stats are enabled, this passes config.hostname, which is the trace-agent endpoint host and defaults to 127.0.0.1, as the v0.6 stats payload hostname. The replaced SpanStatsProcessor deliberately used os.hostname() for the payload while using config.hostname only to contact the agent, so native stats from nearly every process are now attributed to the agent address (and container deployments commonly all become datadog-agent) instead of the application host. Pass os.hostname() here independently of the agent URL configuration.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
I briefly benchmarked an example express app and this turned out to be 50% slower.
This adds a ton of code and complexity and I fear the approach can not beat the JS baseline performance wise.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bca8cc820d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Check if parent was remote (from context propagation) | ||
| // In that case, this span is the local root | ||
| if (context._isRemote) return true |
There was a problem hiding this comment.
With the default nonzero flush interval, an inbound span that continues an extracted context cannot satisfy this check: NativeDatadogSpan._createContext() clears _isRemote, and SpanProcessor._erase() empties trace.started before the delayed exporter evaluates the fallback. The service-entry span is therefore sent to prepareChunk with firstIsLocalRoot: false, so libdatadog does not apply local-root chunk handling to normal distributed traces. Preserve remote-parent provenance separately or compute this flag before erasing the trace.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| // prepared chunks don't accumulate faster than they can be sent, which | ||
| // would cause unbounded memory growth proportional to total requests. | ||
| // Note: flushChangeQueue is called inside flushSpansGrouped. | ||
| runtimeMetrics.increment(`${METRIC_PREFIX}.requests`, true) |
There was a problem hiding this comment.
When OTEL_TRACES_EXPORTER=otlp, this unconditional increment labels every collector request as datadog.tracer.node.exporter.agent.requests, while the removed OTLP exporter’s otel.traces_export_attempts and otel.traces_export_successes counters are no longer emitted anywhere. OTLP users consequently lose exporter-success telemetry and produce false agent-export activity; branch these counters on the configured transport and retain the established OTLP signals.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| if (trace.record === false) { | ||
| this._discardNativeSpans(started) | ||
| this._erase(trace, []) | ||
| this._exporter._resetNativeStateWhenIdle?.() |
There was a problem hiding this comment.
When request filtering marks a trace unrecorded and native stats are disabled, _discardNativeSpans() already removes its spans through discardSpansGrouped, but this unconditionally schedules a full WasmSpanState rebuild anyway. Each state owns an 8 MiB change queue, and concurrent filtered requests accumulate one callback apiece that is replayed as repeated allocate/free cycles when the exporter becomes idle, making blocklists and request filters an expensive hot path. Propagate the discard result and reset only when native discard is unavailable or fails.
AGENTS.md reference: AGENTS.md:L194-L200
Useful? React with 👍 / 👎.
| ? groups.reduce( | ||
| (previous, group) => previous | ||
| .then(() => this._nativeSpans.flushSpansGrouped([group])) | ||
| .then(applyResponse), |
There was a problem hiding this comment.
When flushInterval === 0 and several chunks accumulated during an in-flight send, this promise chain short-circuits on the first rejected group. _pendingSpanChunks was already cleared before the chain started, so all later groups are neither retried nor passed to #finishSend; their traces are silently lost and their materialized native spans remain allocated indefinitely. Requeue the unattempted groups or continue the sequence while recording the first error.
Useful? React with 👍 / 👎.
| if (isFirstSpanInChunk && this._config.apmTracingEnabled === false) { | ||
| span.context().setTag(APM_TRACING_ENABLED_KEY, 0) |
There was a problem hiding this comment.
When APM tracing is disabled and the tracer selects the JS fallback pipeline—for example because of a custom DNS lookup, AWS Lambda, or Electron—this condition adds _dd.apm.enabled: 0 only to the first finished span in a multi-span chunk. The replaced processor stamped every span, and the native processor still does so because standalone ASM requires each emitted span to carry the APM-disabled classification; subsequent JS-fallback spans now lose that marker. Remove the isFirstSpanInChunk restriction while retaining it only for actual chunk-level tags.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b2aeab4a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (err.code) { | ||
| runtimeMetrics.increment(`${METRIC_PREFIX}.errors.by.code`, `code:${err.code}`, true) | ||
| } | ||
| log.error('Error sending spans to agent via native exporter:', err) |
There was a problem hiding this comment.
When the default native agent export rejects because the agent is unreachable, this handler records runtime metrics and a normal log entry but never calls the existing startup-log.logAgentError() path used by exporters/agent/writer.js. Consequently errors.agentError remains unset, so the telemetry app-started payload and DD_TRACE_STARTUP_LOGS diagnostics no longer report agent connectivity failures for native users; invoke the established helper here for agent transport errors.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| } | ||
| this.#flushInFlight = true | ||
| sendGrouped | ||
| .then((response) => { |
There was a problem hiding this comment.
With DD_TRACE_STARTUP_LOGS=true on the default native agent path, completing the first export never invokes logIntegrations(). The replaced agent writer called that existing helper after its first request, and a repo-wide search shows no other production caller, so native users permanently lose the DATADOG TRACER INTEGRATIONS LOADED startup record; call the helper once when a native agent send completes.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| offset += 8 | ||
| } | ||
|
|
||
| nativeSpans._state.prepareChunk(pendingSpanIds.length, false, spanIds) |
There was a problem hiding this comment.
In the spans and plugin-redis-traced benchmarks, this array contains thousands of independently created root spans, each with a different native segment, but it is passed to one prepareChunk call. The native flush contract in native_spans.js requires each prepared group to contain exactly one trace/segment; combining them applies one segment's chunk handling to unrelated traces and then clears every ID from the helper, so native state may not be reclaimed correctly and the measured memory/time results are invalid. Group IDs by trace segment and prepare each group separately.
Useful? React with 👍 / 👎.
| set (v) { | ||
| nameValue = v | ||
| this._syncNameToNative(v) |
There was a problem hiding this comment.
This mock's _name setter calls _syncNameToNative, but the production NativeSpanContext setter only assigns its symbol-backed value and never makes that call. The setOperationName spec therefore passes against behavior no real native span executes and cannot catch a broken or missing native name update; construct the real context/native path or make the mock exactly match production before asserting this contract.
AGENTS.md reference: AGENTS.md:L128-L129
Useful? React with 👍 / 👎.
| let serialized = JSON.stringify(events) | ||
| if (serialized.length > MAX_META_VALUE_LENGTH) { | ||
| serialized = `${serialized.slice(0, MAX_META_VALUE_LENGTH)}...` |
There was a problem hiding this comment.
With the default DD_TRACE_NATIVE_SPAN_EVENTS=false, a span whose serialized events exceed 25,000 characters is sliced at an arbitrary character and suffixed with ..., producing invalid JSON in meta.events; the agent can no longer decode any event in that value. The replaced legacy formatter wrote the complete stringifySpanEvents() result rather than truncating the JSON text, so preserve that behavior or apply a size policy that removes complete events while keeping the array valid.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 429a2c19d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setTimeout(() => { | ||
| const exported = exportedSpans.find(s => s.context()._name === 'lifecycle') | ||
| assert.ok(exported, 'finished span should reach the exporter') | ||
| done() | ||
| }, 50) |
There was a problem hiding this comment.
This test spin-waits for 5 ms and then delays its assertion with a real 50 ms timer even though the stubbed export() records spans synchronously. This unnecessarily slows every run and makes the test depend on wall-clock scheduling; assert the export immediately and use Sinon fake timers if elapsed duration must be controlled.
AGENTS.md reference: AGENTS.md:L131-L133
Useful? React with 👍 / 👎.
| sendGrouped = this._config.flushInterval === 0 && groups.length > 1 | ||
| ? groups.reduce( | ||
| (previous, group) => previous | ||
| .then(() => this._nativeSpans.flushSpansGrouped([group])) | ||
| .then(applyResponse), |
There was a problem hiding this comment.
When flushInterval === 0 and multiple groups accumulate behind an in-flight send, this reduction invokes flushSpansGrouped() once per group, so it performs multiple HTTP requests, but the surrounding code increments the request counter only once and increments the response counter only once after the entire chain. Runtime exporter health metrics therefore undercount traffic in this common Lambda/zero-interval backlog case, and if a later request fails they also omit responses from earlier successful requests; record request and response metrics around each per-group send.
Useful? React with 👍 / 👎.
| (previous, group) => previous | ||
| .then(() => this._nativeSpans.flushSpansGrouped([group])) | ||
| .then(applyResponse), |
There was a problem hiding this comment.
The default APM exporter now constructs a Promise chain for every native flush and an additional pair of promises for every queued zero-interval group. Besides the allocation and microtask overhead on the production export path, this directly violates the repository's prohibition on promises in npm production code; expose or use a callback-compatible native send path instead.
AGENTS.md reference: AGENTS.md:L196-L206
Useful? React with 👍 / 👎.
| Tracer = require('../../src/tracer') | ||
| tracer = new Tracer(config) | ||
|
|
There was a problem hiding this comment.
The suite describes itself as an end-to-end test against the real libdatadog pipeline, but this global stub replaces NativeExporter.export() in every test with an array append. As a result, the lifecycle assertions never exercise grouping, chunk preparation, native serialization, span reclamation, or transport, and therefore cannot catch regressions in the boundary the suite claims to cover; retain the real exporter and assert observable mock-agent output, or scope this stub only to tests that explicitly need it.
AGENTS.md reference: AGENTS.md:L128-L129
Useful? React with 👍 / 👎.
Keep the existing flush timer authoritative after an in-flight native send and trigger an early flush at 2,000 pending spans. The native exporter waited for the two-second interval before its first send, then bypassed batching after every settlement. On the Express/PostgreSQL workload, bounded batching reduced CPU/request by 18.6%, increased throughput by 25.1%, and reduced RSS from 1,344 MiB to 366 MiB. - Run the native exporter unit tests. - Verify every changed production branch with c8. - Run the full repository lint. - Run three fresh-process 50,000-request Express/PostgreSQL trials with exact trace and query counts.
All spans in a trace share an immutable ID, but the native path rebuilt its 16-byte representation for every child. Reusing the shared representation reduced buildNativeTraceId self-time from 94.6 to 30.8 ms in a 50,000-request Express/PostgreSQL profile and its isolated seven-span path from 1,081 to 169 ns/trace.
Assigning and deleting the construction-time name hook put every native span context on a slow object shape even though final synchronization already owns name writes. Removing the stale hook reduced CPU per request from 112.41 to 99.77 µs and raised throughput from 11,039 to 12,175 requests/s in the Express/PostgreSQL workload; isolated construction fell from 75.1 to 6.4 ns/span.
## Summary Keep resource names intact in native fast final sync, matching the JS v0.4 and v0.5 encoders. ## Why The tracer applied the agent's 5,000-character normalization limit before native export. Long SQL resources then differed between exporters and failed the existing PostgreSQL wire assertions. ## Test plan - ./node_modules/.bin/mocha packages/dd-trace/test/native/span_context.spec.js - Existing PostgreSQL long-query cases for pg 8.0.3 and 8.22 (2 passing)
## Summary Mirror the canonical formatter's base-service inference in native fast final sync and remove the now-redundant configured-service field from native contexts. ## Why The fast path registered an overridden service but skipped _dd.base_service. Raw spans and WASM output therefore lost the configured service whenever a span selected a different one. ## Test plan - Native tracer and span-context suites (45 passing) - Native plugin wire assertions for base-service propagation (3 passing)
## Summary Change-queue flushing consumed 95.8% of the deferred-finish profile and kept the eight-sample CI variant running when the 30-minute job expired. ## Why The benchmark is meant to isolate span construction and finish, but it applied and exported every queued native mutation. Discarding those mutations reduced the same 250,000-span process from 23.11 s to 0.60 s. Native event samples still drain because libdatadog applies events directly. ## Test plan - Run all span variants through three fresh sirun matrices. - Run changed-line coverage and full lint.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbf2cbf085
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| _discardNativeSpans (spans) { | ||
| if (this.#disabled || this.#nativeStatsEnabled() || !spans?.length) return false |
There was a problem hiding this comment.
When DD_TRACE_STATS_COMPUTATION_ENABLED=true and a trace is suppressed by an HTTP client allowlist or request filter, this guard prevents _discardNativeSpans() from preparing or removing its native spans. The processor still marks their contexts exported, and _resetNativeStateWhenIdle() is disabled by the same stats condition, so every filtered span remains queued or materialized in WASM indefinitely; a service with recurring filtered traffic can therefore grow native memory without bound. Preserve any required stats while still reclaiming these spans.
AGENTS.md reference: AGENTS.md:L194-L200
Useful? React with 👍 / 👎.
| const fastSynced = formattedSpan === undefined && span._tryFastNativeFinalSync?.() === true | ||
| if (!fastSynced) { |
There was a problem hiding this comment.
With native stats disabled—the default—a normal span containing only supported primitive tags makes _tryFastNativeFinalSync() succeed here, bypassing spanFormat(). That fast path never writes the formatter's unconditional meta.language = 'javascript', so these common spans lose the established language value and may instead be reported using the native request language (nodejs), while spans that take the slow path retain javascript. Add the formatter-owned metadata to the fast path before skipping the shared formatter.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| nativeSpans.resetChangeQueue() | ||
| queuedSpans = 0 | ||
| } | ||
| this._erase(trace, []) |
There was a problem hiding this comment.
Every measured span finish reaches this monkey-patched processor, but neither the current SpanProcessor nor JsSpanProcessor defines _erase; that logic was extracted to span-processor-state.js. Consequently the initial sanitySpan.finish() throws TypeError: this._erase is not a function before any benchmark variant enters its measured loop. Invoke the shared eraser directly instead of calling the removed private method.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a127870943
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return | ||
| } | ||
| // Invalid endpoints fail loudly during native exporter construction. | ||
| this._nativeSpans.setOtlpEndpoint(endpoint) |
There was a problem hiding this comment.
When OTEL_EXPORTER_OTLP_TRACES_TIMEOUT (or its generic alias) is set, the native OTLP setup forwards the endpoint, protocol, and headers but never the resolved timeout. The replaced JS exporter passed this value to OtlpHttpExporterBase, so native-mode users now silently receive libdatadog's default timeout instead of their configured request bound; forward the timeout through the native transport API, adding that capability upstream if necessary.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| hostname: config.hostname || os.hostname(), | ||
| env: config.env || '', | ||
| appVersion: config.version || '', | ||
| runtimeId: config.tags?.['runtime-id'] || '', |
There was a problem hiding this comment.
With native OTLP export, custom entries from OTEL_RESOURCE_ATTRIBUTES or DD_TAGS are not passed as resource configuration here; only selected fields such as env, version, and runtime ID reach the native state, while config.tags are applied later as ordinary span tags. The replaced buildResourceAttributes() explicitly copied every filtered config tag into the OTLP Resource, so collectors grouping or filtering on custom resource attributes now see them at the wrong scope or not at all; forward the complete resource-attribute map through the native mapper.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| return error?.code === 'MODULE_NOT_FOUND' && | ||
| /^Cannot find module ['"]@datadog\/libdatadog['"]/.test(String(error.message)) |
There was a problem hiding this comment.
On a supported Node.js platform for which @datadog/libdatadog has no matching pipeline binary/WASM artifact, the package itself resolves but libdatadog.load('pipeline') throws a non-MODULE_NOT_FOUND error. This predicate therefore treats the native pipeline as corrupt and rethrows during tracer construction instead of selecting the JS fallback, leaving the proxy as a no-op tracer and disabling all tracing for that process; classify the binding's unsupported-platform/no-artifact error as native unavailability.
AGENTS.md reference: AGENTS.md:L222-L225
Useful? React with 👍 / 👎.
| statsEnabled: (config.stats?.DD_TRACE_STATS_COMPUTATION_ENABLED && | ||
| !config.OTEL_TRACES_SPAN_METRICS_ENABLED) || false, |
There was a problem hiding this comment.
For v5 configurations using experimental.appsec.standalone.enabled, apmTracingEnabled is flipped to false only after the config logic that defaults stats on GCP/Azure, so DD_TRACE_STATS_COMPUTATION_ENABLED can remain true here. The replaced processor explicitly excluded config.appsec.standalone.enabled, but this native-state option does not, causing standalone ASM deployments in those environments to aggregate and send ordinary APM stats even though APM tracing is disabled; retain the standalone guard when enabling the native concentrator.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
2 participants
Important
Blocking before merge:
.github/workflows/system-tests.ymlcurrently pins the reusable system-tests workflow toref: bengl/parametric-native-stats-flushinstead ofmain. That branch is DataDog/system-tests#7293, which adds the parametric/trace/stats/flushhook the native stats path needs. Do not merge this PR until #7293 is merged and thatref:line is removed. Until then the parametric jobs here are testing against a fork branch, not system-testsmain.What does this PR do?
Adds the native span pipeline to dd-trace-js. Regular APM spans are stored, serialized, and exported through
@datadog/libdatadoginstead of the JSspan_format/ encoder /AgentWriterpath.Highlights:
packages/dd-trace/src/native/*, using the libdatadog span-id change-buffer protocol.packages/dd-trace/src/exporters/native/*, including v0.4/v0.5 agent export, OTLP HTTP trace export, grouped trace flushing, first-flush signaling, stats flushing, and health metrics.meta_struct, top-levelspan_events, error bits/meta, span sampling, process tags, service/resource/type tagging, UDS / Windows named-pipe URLs, client-computed stats, and native client stats flush.@datadog/libdatadogto the published0.18.1package.Motivation
This branch moves dd-trace-js onto the libdatadog native span pipeline so the Node tracer can share the same lower-level span storage/export implementation as the rest of the Datadog native stack. The PR also closes the feature-parity gaps found while running native mode against the full dd-trace-js CI/system-tests matrix.
Breaking changes
Deliberate, but they change the public surface and need release notes:
experimental.exporter: 'log'is gone.LOGwas removed fromext/exporters.jsandext/exporters.d.ts, and theindex.d.tsunion is now'agent' | 'datadog' | 'electron'. An unsupported value logsNative spans mode ignores unsupported experimental exporter "%s"; using native agent exporterand falls back to the agent./opt/extensions/datadog-agent, no mini-agent) gets the stdout exporter from that probe instead of fromexporter: 'log'. An explicitexporter: 'agent'still wins, matching howgetExportermatched the configured name before it probed._DD_APM_TRACING_AGENTLESS_ENABLEDis no longer read. (exporters.AGENTLESSstill exists as a constant but nothing consumes it;index.d.tsalready drops it from the union.)datadog.tracer.node.exporter.agent.responses.by.statusis not emitted on the native path. libdatadog does not surface the HTTP status code to the host, so only.responses(plus.errors,.errors.by.name,.errors.by.code) are reported. The legacy agent writer still emits it on the JS pipeline.Reviewer notes
packages/dd-trace/src/native/*packages/dd-trace/src/exporters/native/*packages/dd-trace/src/opentracing/tracer.jspackages/dd-trace/src/span_processor.js/span_sampler.jspackages/dd-trace/test/native/*and affected plugin/system-test fixtures.sendPreparedChunkthat ships all staged chunks as one multi-trace payload — the same shape the legacyAgentWriterproduced.traces[0][0]and the Azure Functions integration test assertspayload.length === 2, so changing how many traces a payload carries (by batching differently, or by gating the post-send drain) breaks them in opposite directions. Treat the flush/drain cadence inexporters/native/index.jsas observable behaviour, not an implementation detail.setAgentUrlfrees theWasmSpanStateit replaces, deferring the free while a send still borrows it. Each state owns an 8 MB change queue in linear memory, which never shrinks, so this matters: without it, a route on the documented http clientblocklistrebuilt state on every filtered request and walked into the wasm32 4 GB ceiling, aborting the process after roughly 4000 requests (300 rebuilds reached 2428 MB; it is flat 18 MB with the free).lookup(custom DNS resolution for the agent host) routes that process onto the JS pipeline. libdatadog's transport builds its ownhttp.requestoptions and exposes no hook for them, so on the native path the callback was silently dropped. The check keys offconfig.getOrigin('lookup')rather than comparing todns.lookup, because the dns plugin wraps that in place.WebAssembly(node --jitless, hardened or JIT-disabled deployments), the tracer degrades to the JS pipeline instead of throwing. libdatadog's loader raises a bareReferenceErrorthere, which previously propagated and left a silentNoopTracerwith no log at default level._writer.flush()is intentionally a compatibility shim for weblog/parametric flush paths: it waits for in-flight trace sends, then force-flushes native stats.flushStats()is separate from normal traceflush()to avoid shipping partial 10s stats buckets on every trace flush.DD_TRACE_NATIVE_SPAN_EVENTScontrols whether span events are sent as native top-levelspan_events; when disabled, they fall back to legacymeta.events.native-spans-*microbenchmarks are not regressing after the hot-path fixes.Validation
Merge base with
masteris01da8d650eccc066e9177ca7d9d2c01734393f95.Local validation at the current head:
npm run test:trace:core— 4185 passing, 10 pending. The single failure ispackages/dd-trace/test/process-tags.spec.js:103, which assertsworkdirTag[1] === 'dd-trace-js'and so only passes when the checkout directory is literally nameddd-trace-js; it passes in CI and fails in a differently-named worktree.npx eslinton every changed file — clean.packages/dd-trace/test/native/*plusopentracing/tracer.spec.js— 198 passing.fetchplugin suite (30 passing) andnpm run test:lambda(44 passing), both of which exerciseAWS_LAMBDA_FUNCTION_NAMEwith an explicit agent exporter.CI on this PR is the source of truth for the full matrix; check the checks tab for the current head rather than any count quoted here.
Follow-up
refpin once DataDog/system-tests#7293 merges (see the note at the top).error.typebefore the OTel HTTP remap so the status code cannot overwrite it, and per-request OTLP export telemetry counters.prepareChunkis the only call that releases a span and it stages whatever it releases. A libdatadog API to release spans without staging them would remove the need for rebuilds entirely.