feat: add DSM support for Amazon EventBridge#8476
Conversation
Overall package sizeSelf size: 6.78 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.1 | 122.62 kB | 438.86 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8476 +/- ##
========================================
Coverage 98.34% 98.35%
========================================
Files 924 924
Lines 123056 123197 +141
Branches 10869 10536 -333
========================================
+ Hits 121024 121165 +141
Misses 2032 2032 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07e96ad374
ℹ️ 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".
BenchmarksBenchmark execution time: 2026-07-17 12:23:20 Comparing candidate commit 0138f3f in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2313 metrics, 45 unstable metrics.
|
|
A new linting rule just landed in |
robcarlan-datadog
left a comment
There was a problem hiding this comment.
(req changes pending some internal discussion on data model)
BridgeAR
left a comment
There was a problem hiding this comment.
This looks very solid!
Could you just please adjust the tests to be more real world like and also add an end to end test to verify the hash is calculated as expected? :)
When a Kinesis record exceeds the 1 MiB cap, injectToMessage bailed after setDSMCheckpoint had already run: it recorded a produce checkpoint, advanced the pathway context, and tagged pathway.hash on the span, yet params.Data was never written so nothing shipped on the wire. The producer left a dangling pathway edge no consumer completes and a span tagged with a hash for a message it never sent. Gate on the trace-context payload before setDSMCheckpoint so a record we cannot ship records no checkpoint, advances no pathway, and tags no hash. The bounded dd-pathway-ctx-base64 bytes are added after the gate; they cannot realistically tip a record that already cleared 1 MiB with room for them. EventBridge shares this checkpoint-before-gate pattern and is tracked separately in #8476; this change is Kinesis-only.
…9183) * fix(aws-sdk): record kinesis DSM checkpoint only when context ships When a Kinesis record exceeds the 1 MiB cap, injectToMessage bailed after setDSMCheckpoint had already run: it recorded a produce checkpoint, advanced the pathway context, and tagged pathway.hash on the span, yet params.Data was never written so nothing shipped on the wire. The producer left a dangling pathway edge no consumer completes and a span tagged with a hash for a message it never sent. Gate on the trace-context payload before setDSMCheckpoint so a record we cannot ship records no checkpoint, advances no pathway, and tags no hash. The bounded dd-pathway-ctx-base64 bytes are added after the gate; they cannot realistically tip a record that already cleared 1 MiB with room for them. EventBridge shares this checkpoint-before-gate pattern and is tracked separately in #8476; this change is Kinesis-only. * refactor(aws-sdk): track message injection with a boolean, drop isEmpty The kinesis, sqs, and sns producers decided whether to attach `_datadog` by re-scanning the freshly built `ddInfo` carrier with an `isEmpty` for-in helper. A local `injected` flag set at each population site carries the same fact without the per-send rescan or the shared helper, and matches `DsmPathwayCodec.encode`'s own `!dataStreamsContext.hash` guard on the sns path so an empty pathway context no longer counts as injected. Replace the faked kinesis inject-to-message spec — an `Object.create(Kinesis.prototype)` instance with a stubbed tracer — with a real agent-loaded regression in kinesis.dsm.spec.js: a record that only clears the 1 MiB cap before the trace context is attached must record no produce checkpoint, asserted through the `putRecord` span carrying no `pathway.hash`. Drop the sqs unit case that asserted a carrier state (`injectTraceContext` true, `inject` a no-op) no real caller reaches. * refactor(core): return whether inject wrote anything into the carrier `Tracer.inject` and every propagator now report whether they wrote any context, so the AWS SDK messaging producers stop attaching an empty `_datadog` message attribute when nothing was injected. Previously they assumed a write happened whenever injection was attempted, which shipped an empty carrier under `DD_TRACE_PROPAGATION_STYLE_INJECT=none` and whenever a disabled-APM-tracing (standalone ASM) trace was stripped after injection. The standalone ASM strip runs on the `dd-trace:span:inject` channel after the propagator writes, so the text_map propagator re-checks the `_dd.p.ts` trace tag when APM tracing is disabled to keep the returned value truthful without rescanning the carrier. * test: align inject test doubles with the boolean carrier-write return `Tracer.inject` now reports whether it wrote context into the carrier, and the AWS SDK producers read that instead of rescanning with `isEmpty`. Four expectations still modelled the old `undefined` return and failed in CI: 1. sqs-inject-to-message: stubs mutate the carrier but returned `undefined`, so the producer dropped `_datadog`; return `true` from the writing stubs and `false` from the no-op default. 2. dd-trace-api: `inject` forwards its return through the public API, now `false` for the empty test args; assert `false`. 3. pathway: type the no-hash `encode` argument concretely to clear jsdoc/reject-any-type. 4. kinesis.dsm: `assert.match(resource, /^putRecord/)` clears eslint-prefer-assert-match. * fix(aws-sdk): reserve the DSM pathway field before the kinesis size gate The size gate ran on the trace-context payload before DsmPathwayCodec.encode appended the fixed-size dd-pathway-ctx-base64 field. A record sitting within 55 bytes of the 1 MiB cap cleared the gate, recorded a checkpoint, and then wrote an over-cap params.Data that Kinesis rejects, failing the user's putRecord. Reserve the pathway field's bytes in the gate so a record that only fits without it records no checkpoint and never ships over the cap. * fix(types): mirror inject boolean return in the v5 type surface Tracer.inject now reports whether it wrote context into the carrier, and index.d.ts was updated to return boolean. index.d.v5.ts still declared void. inject exists on v5 and this return is not v6-only, so the v5 release type swap (scripts/release/swap-v5-types.js) would ship a stale void declaration and hide the return value from v5 TypeScript users even though the runtime provides it. Mirror the signature and JSDoc so the two public surfaces diverge only by the intended v6 cleanups. Refs: #9183 (comment) * fix(types): keep public inject void on v5/v6, expose boolean on v7 The prior commit widened Tracer.inject's declared return from void to boolean across the public surface. That is a breaking TypeScript change: under strict mode `const x: void = tracer.inject(...)` and returning `tracer.inject(...)` from a void-typed function stop compiling. It must not ship on a shipping major. The runtime already returns the boolean and the internal AWS SDK producers consume it through the unwrapped DatadogTracer (a plugin's `this.tracer` resolves to `_tracer._tracer`, and the DSM checkpointer holds that inner tracer directly), never through the public proxy. So the boolean stays an internal signal regardless of what the public `.d.ts` promises. Freeze the contract per major: 1. index.d.v5.ts and the new index.d.v6.ts declare `inject(): void` — the current shipping majors keep their contract. 2. index.d.ts (master is 7.0.0-pre) declares `inject(): boolean` as the next-major public API, documented in MIGRATING.md under 6.0 to 7.0. Generalize the release type swap: swap-v5-types.js only handled v5, so a v6 release would publish the v7 index.d.ts. Rename it to swap-legacy-types.js and pick index.d.v<major>.ts by the release major; the newest major has no frozen file and is a no-op. Adding a future frozen surface is now a single file drop. The .v<major>.ts files stay out of the npm `files` array by design — the swap copies the chosen one over index.d.ts at pack time, so only index.d.ts ships. Refs: #9183 (comment) * fix(propagation): return lazily created carriers Standalone ASM can remove trace headers while leaving baggage in the carrier. A boolean result reported false for that non-empty carrier, causing AWS messaging producers to drop valid context. Return the carrier internally after its first write while keeping the public tracer API void at the proxy boundary. Kinesis limits cover Data plus PartitionKey. Include both before recording a DSM checkpoint and allow records at the exact default limit. No-write injection measured 11.96-12.21 ns eager vs 6.92-6.98 ns lazy; one write in eight measured 29.30-29.46 ns vs 26.09-26.66 ns on Node 24.18.0 / V8 13.6.233.17, 5M iterations x 7 trials with best and worst dropped.
…9183) * fix(aws-sdk): record kinesis DSM checkpoint only when context ships When a Kinesis record exceeds the 1 MiB cap, injectToMessage bailed after setDSMCheckpoint had already run: it recorded a produce checkpoint, advanced the pathway context, and tagged pathway.hash on the span, yet params.Data was never written so nothing shipped on the wire. The producer left a dangling pathway edge no consumer completes and a span tagged with a hash for a message it never sent. Gate on the trace-context payload before setDSMCheckpoint so a record we cannot ship records no checkpoint, advances no pathway, and tags no hash. The bounded dd-pathway-ctx-base64 bytes are added after the gate; they cannot realistically tip a record that already cleared 1 MiB with room for them. EventBridge shares this checkpoint-before-gate pattern and is tracked separately in #8476; this change is Kinesis-only. * refactor(aws-sdk): track message injection with a boolean, drop isEmpty The kinesis, sqs, and sns producers decided whether to attach `_datadog` by re-scanning the freshly built `ddInfo` carrier with an `isEmpty` for-in helper. A local `injected` flag set at each population site carries the same fact without the per-send rescan or the shared helper, and matches `DsmPathwayCodec.encode`'s own `!dataStreamsContext.hash` guard on the sns path so an empty pathway context no longer counts as injected. Replace the faked kinesis inject-to-message spec — an `Object.create(Kinesis.prototype)` instance with a stubbed tracer — with a real agent-loaded regression in kinesis.dsm.spec.js: a record that only clears the 1 MiB cap before the trace context is attached must record no produce checkpoint, asserted through the `putRecord` span carrying no `pathway.hash`. Drop the sqs unit case that asserted a carrier state (`injectTraceContext` true, `inject` a no-op) no real caller reaches. * refactor(core): return whether inject wrote anything into the carrier `Tracer.inject` and every propagator now report whether they wrote any context, so the AWS SDK messaging producers stop attaching an empty `_datadog` message attribute when nothing was injected. Previously they assumed a write happened whenever injection was attempted, which shipped an empty carrier under `DD_TRACE_PROPAGATION_STYLE_INJECT=none` and whenever a disabled-APM-tracing (standalone ASM) trace was stripped after injection. The standalone ASM strip runs on the `dd-trace:span:inject` channel after the propagator writes, so the text_map propagator re-checks the `_dd.p.ts` trace tag when APM tracing is disabled to keep the returned value truthful without rescanning the carrier. * test: align inject test doubles with the boolean carrier-write return `Tracer.inject` now reports whether it wrote context into the carrier, and the AWS SDK producers read that instead of rescanning with `isEmpty`. Four expectations still modelled the old `undefined` return and failed in CI: 1. sqs-inject-to-message: stubs mutate the carrier but returned `undefined`, so the producer dropped `_datadog`; return `true` from the writing stubs and `false` from the no-op default. 2. dd-trace-api: `inject` forwards its return through the public API, now `false` for the empty test args; assert `false`. 3. pathway: type the no-hash `encode` argument concretely to clear jsdoc/reject-any-type. 4. kinesis.dsm: `assert.match(resource, /^putRecord/)` clears eslint-prefer-assert-match. * fix(aws-sdk): reserve the DSM pathway field before the kinesis size gate The size gate ran on the trace-context payload before DsmPathwayCodec.encode appended the fixed-size dd-pathway-ctx-base64 field. A record sitting within 55 bytes of the 1 MiB cap cleared the gate, recorded a checkpoint, and then wrote an over-cap params.Data that Kinesis rejects, failing the user's putRecord. Reserve the pathway field's bytes in the gate so a record that only fits without it records no checkpoint and never ships over the cap. * fix(types): mirror inject boolean return in the v5 type surface Tracer.inject now reports whether it wrote context into the carrier, and index.d.ts was updated to return boolean. index.d.v5.ts still declared void. inject exists on v5 and this return is not v6-only, so the v5 release type swap (scripts/release/swap-v5-types.js) would ship a stale void declaration and hide the return value from v5 TypeScript users even though the runtime provides it. Mirror the signature and JSDoc so the two public surfaces diverge only by the intended v6 cleanups. Refs: #9183 (comment) * fix(types): keep public inject void on v5/v6, expose boolean on v7 The prior commit widened Tracer.inject's declared return from void to boolean across the public surface. That is a breaking TypeScript change: under strict mode `const x: void = tracer.inject(...)` and returning `tracer.inject(...)` from a void-typed function stop compiling. It must not ship on a shipping major. The runtime already returns the boolean and the internal AWS SDK producers consume it through the unwrapped DatadogTracer (a plugin's `this.tracer` resolves to `_tracer._tracer`, and the DSM checkpointer holds that inner tracer directly), never through the public proxy. So the boolean stays an internal signal regardless of what the public `.d.ts` promises. Freeze the contract per major: 1. index.d.v5.ts and the new index.d.v6.ts declare `inject(): void` — the current shipping majors keep their contract. 2. index.d.ts (master is 7.0.0-pre) declares `inject(): boolean` as the next-major public API, documented in MIGRATING.md under 6.0 to 7.0. Generalize the release type swap: swap-v5-types.js only handled v5, so a v6 release would publish the v7 index.d.ts. Rename it to swap-legacy-types.js and pick index.d.v<major>.ts by the release major; the newest major has no frozen file and is a no-op. Adding a future frozen surface is now a single file drop. The .v<major>.ts files stay out of the npm `files` array by design — the swap copies the chosen one over index.d.ts at pack time, so only index.d.ts ships. Refs: #9183 (comment) * fix(propagation): return lazily created carriers Standalone ASM can remove trace headers while leaving baggage in the carrier. A boolean result reported false for that non-empty carrier, causing AWS messaging producers to drop valid context. Return the carrier internally after its first write while keeping the public tracer API void at the proxy boundary. Kinesis limits cover Data plus PartitionKey. Include both before recording a DSM checkpoint and allow records at the exact default limit. No-write injection measured 11.96-12.21 ns eager vs 6.92-6.98 ns lazy; one write in eight measured 29.30-29.46 ns vs 26.09-26.66 ns on Node 24.18.0 / V8 13.6.233.17, 5M iterations x 7 trials with best and worst dropped.
…9183) * fix(aws-sdk): record kinesis DSM checkpoint only when context ships When a Kinesis record exceeds the 1 MiB cap, injectToMessage bailed after setDSMCheckpoint had already run: it recorded a produce checkpoint, advanced the pathway context, and tagged pathway.hash on the span, yet params.Data was never written so nothing shipped on the wire. The producer left a dangling pathway edge no consumer completes and a span tagged with a hash for a message it never sent. Gate on the trace-context payload before setDSMCheckpoint so a record we cannot ship records no checkpoint, advances no pathway, and tags no hash. The bounded dd-pathway-ctx-base64 bytes are added after the gate; they cannot realistically tip a record that already cleared 1 MiB with room for them. EventBridge shares this checkpoint-before-gate pattern and is tracked separately in #8476; this change is Kinesis-only. * refactor(aws-sdk): track message injection with a boolean, drop isEmpty The kinesis, sqs, and sns producers decided whether to attach `_datadog` by re-scanning the freshly built `ddInfo` carrier with an `isEmpty` for-in helper. A local `injected` flag set at each population site carries the same fact without the per-send rescan or the shared helper, and matches `DsmPathwayCodec.encode`'s own `!dataStreamsContext.hash` guard on the sns path so an empty pathway context no longer counts as injected. Replace the faked kinesis inject-to-message spec — an `Object.create(Kinesis.prototype)` instance with a stubbed tracer — with a real agent-loaded regression in kinesis.dsm.spec.js: a record that only clears the 1 MiB cap before the trace context is attached must record no produce checkpoint, asserted through the `putRecord` span carrying no `pathway.hash`. Drop the sqs unit case that asserted a carrier state (`injectTraceContext` true, `inject` a no-op) no real caller reaches. * refactor(core): return whether inject wrote anything into the carrier `Tracer.inject` and every propagator now report whether they wrote any context, so the AWS SDK messaging producers stop attaching an empty `_datadog` message attribute when nothing was injected. Previously they assumed a write happened whenever injection was attempted, which shipped an empty carrier under `DD_TRACE_PROPAGATION_STYLE_INJECT=none` and whenever a disabled-APM-tracing (standalone ASM) trace was stripped after injection. The standalone ASM strip runs on the `dd-trace:span:inject` channel after the propagator writes, so the text_map propagator re-checks the `_dd.p.ts` trace tag when APM tracing is disabled to keep the returned value truthful without rescanning the carrier. * test: align inject test doubles with the boolean carrier-write return `Tracer.inject` now reports whether it wrote context into the carrier, and the AWS SDK producers read that instead of rescanning with `isEmpty`. Four expectations still modelled the old `undefined` return and failed in CI: 1. sqs-inject-to-message: stubs mutate the carrier but returned `undefined`, so the producer dropped `_datadog`; return `true` from the writing stubs and `false` from the no-op default. 2. dd-trace-api: `inject` forwards its return through the public API, now `false` for the empty test args; assert `false`. 3. pathway: type the no-hash `encode` argument concretely to clear jsdoc/reject-any-type. 4. kinesis.dsm: `assert.match(resource, /^putRecord/)` clears eslint-prefer-assert-match. * fix(aws-sdk): reserve the DSM pathway field before the kinesis size gate The size gate ran on the trace-context payload before DsmPathwayCodec.encode appended the fixed-size dd-pathway-ctx-base64 field. A record sitting within 55 bytes of the 1 MiB cap cleared the gate, recorded a checkpoint, and then wrote an over-cap params.Data that Kinesis rejects, failing the user's putRecord. Reserve the pathway field's bytes in the gate so a record that only fits without it records no checkpoint and never ships over the cap. * fix(types): mirror inject boolean return in the v5 type surface Tracer.inject now reports whether it wrote context into the carrier, and index.d.ts was updated to return boolean. index.d.v5.ts still declared void. inject exists on v5 and this return is not v6-only, so the v5 release type swap (scripts/release/swap-v5-types.js) would ship a stale void declaration and hide the return value from v5 TypeScript users even though the runtime provides it. Mirror the signature and JSDoc so the two public surfaces diverge only by the intended v6 cleanups. Refs: #9183 (comment) * fix(types): keep public inject void on v5/v6, expose boolean on v7 The prior commit widened Tracer.inject's declared return from void to boolean across the public surface. That is a breaking TypeScript change: under strict mode `const x: void = tracer.inject(...)` and returning `tracer.inject(...)` from a void-typed function stop compiling. It must not ship on a shipping major. The runtime already returns the boolean and the internal AWS SDK producers consume it through the unwrapped DatadogTracer (a plugin's `this.tracer` resolves to `_tracer._tracer`, and the DSM checkpointer holds that inner tracer directly), never through the public proxy. So the boolean stays an internal signal regardless of what the public `.d.ts` promises. Freeze the contract per major: 1. index.d.v5.ts and the new index.d.v6.ts declare `inject(): void` — the current shipping majors keep their contract. 2. index.d.ts (master is 7.0.0-pre) declares `inject(): boolean` as the next-major public API, documented in MIGRATING.md under 6.0 to 7.0. Generalize the release type swap: swap-v5-types.js only handled v5, so a v6 release would publish the v7 index.d.ts. Rename it to swap-legacy-types.js and pick index.d.v<major>.ts by the release major; the newest major has no frozen file and is a no-op. Adding a future frozen surface is now a single file drop. The .v<major>.ts files stay out of the npm `files` array by design — the swap copies the chosen one over index.d.ts at pack time, so only index.d.ts ships. Refs: #9183 (comment) * fix(propagation): return lazily created carriers Standalone ASM can remove trace headers while leaving baggage in the carrier. A boolean result reported false for that non-empty carrier, causing AWS messaging producers to drop valid context. Return the carrier internally after its first write while keeping the public tracer API void at the proxy boundary. Kinesis limits cover Data plus PartitionKey. Include both before recording a DSM checkpoint and allow records at the exact default limit. No-write injection measured 11.96-12.21 ns eager vs 6.92-6.98 ns lazy; one write in eight measured 29.30-29.46 ns vs 26.09-26.66 ns on Node 24.18.0 / V8 13.6.233.17, 5M iterations x 7 trials with best and worst dropped.
BridgeAR
left a comment
There was a problem hiding this comment.
Please rebase and I have two minor comments right now :)
| const ddInfo = {} | ||
| if (injectTraceContext) { | ||
| this.tracer.inject(span, 'text_map', ddInfo) |
There was a problem hiding this comment.
To identify in all cases when something got injected or not, I changed the API. Calling inject will now return an context object if something got injected and return undefined if not in case no context was provided as third argument.
There was a problem hiding this comment.
@BridgeAR could I just double check what change your asking for here. Do I now need to ddInfo = this.tracer.inject(span, 'text_map') because the context is going to be returned?
What does this PR do?
Extends the DSM auto checkpointing and injection support into Amazon Event Bridge. Replicating the same way the DSM propagation works for SQS/SNS/Kinesis
Motivation
Customers working with serverless often use Amazon EventBridge as a messaging technology, extend DSM support to cover all AWS native messaging technologies.
Additional Notes