feat(telemetry): add stable session identifier headers#7821
Merged
khanayan123 merged 18 commits intomasterfrom Mar 20, 2026
Merged
feat(telemetry): add stable session identifier headers#7821khanayan123 merged 18 commits intomasterfrom
khanayan123 merged 18 commits intomasterfrom
Conversation
Implements the Stable Service Instance Identifier RFC for Node.js. - DD-Session-ID: always set to current runtime_id on all telemetry requests - DD-Root-Session-ID: set when process is a child (value inherited via DD_ROOT_JS_SESSION_ID env var); omitted for root processes - child_session.js: patches child_process.spawn/spawnSync/fork to inject DD_ROOT_JS_SESSION_ID and DD_PARENT_JS_SESSION_ID into child env so forked/spawned processes inherit the session lineage Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Contributor
Overall package sizeSelf size: 5.04 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.0 | 81.15 kB | 815.98 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
2 tasks
This comment has been minimized.
This comment has been minimized.
BenchmarksBenchmark execution time: 2026-03-19 19:53:49 Comparing candidate commit 9a8f4dd in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 229 metrics, 31 unstable metrics. |
- config/index.js: eslint-disable for process.env (internal propagation env var, not in supported-configurations.json) - child_session.js: fix unicorn/no-negated-condition, use spread instead of Array.from, eslint-disable for process.env - send-data.spec.js: add dd-session-id to existing header assertions, add tests for dd-root-session-id presence/absence - child_session.spec.js: new test file covering env injection for spawn, spawnSync, and fork with all argument permutations Co-Authored-By: Claude Opus 4.6 <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7821 +/- ##
==========================================
+ Coverage 80.46% 80.47% +0.01%
==========================================
Files 748 749 +1
Lines 32407 32457 +50
==========================================
+ Hits 26075 26121 +46
- Misses 6332 6336 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Instead of double-shimmer-wrapping child_process methods, use the existing diagnostic channel infrastructure. child_session.js now subscribes to datadog:child_process:execution start events and injects session env vars via context.callArgs. Also adds fork to the instrumented async methods. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…try.enabled - Rename child_session.js → session-propagation.js for clarity - Guard start() with config.telemetry?.enabled check - Add test for telemetry-disabled case Co-Authored-By: Claude Opus 4.6 <[email protected]>
Match the import pattern used by dependencies, endpoints, and telemetryLogger instead of using an inline require. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…tation - Add dedicated fork test section with success, arguments, and error cases - Add callArgs context tests verifying it's present for async and sync methods - Add end-to-end callArgs mutation tests proving subscribers can inject env vars into spawn, spawnSync, and fork Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Fix Windows test failure: process.env keys have different casing on Windows (Path vs PATH), use key count check instead - Remove unused hadSubscribers variable and redundant comments - Simplify config.telemetry && config.telemetry.debug to optional chain - Remove duplicate sinon.assert.calledOnce(start) in child_process test Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Remove obvious comments in child_process.js instrumentation - Simplify createBatchPayload arrow function to implicit return - Fix stray `/` after sendData call in send-data.spec.js Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Remove fork-like callArgs tests from session-propagation (same branches as spawn tests since they share one subscriber) - Remove fork callArgs mutation test (same async wrapper as spawn) - Extract shared injectTestEnv helper in child_process tests Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Break long lines under 120 char max-len limit - Remove blank line before closing brace (padded-blocks) - Remove unnecessary eslint-disable directive in test file Co-Authored-By: Claude Opus 4.6 <[email protected]>
Reverts unrelated comment removals and code style changes in send-data.js, telemetry.js, and child_process.js that were introduced by the code simplifier. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Extract argument shape detection into getArgShape() helper and use a switch statement for clearer control flow. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Export internal functions and call them directly in tests so codecov can track coverage through the source file. Add explicit reason to eslint-disable comments for process.env access. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Collaborator
Author
|
@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: 5634511ede
ℹ️ 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".
execFile supports callback-style calls like execFile(file, args, cb) and execFile(file, cb). The previous implementation would overwrite the callback slot with the options object. Now uses splice to insert options before any trailing callback, preserving the original call semantics. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Collaborator
Author
|
@codex review |
Replace getArgShape switch with a single findOptionsIndex() that locates or determines where the options object belongs. The handler then has two paths: update existing options, or insert new ones (preserving callbacks via splice). Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09bad69bd0
ℹ️ 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".
Unsubscribes from the child_process channel and clears state so the module can be cleanly restarted with new config. Wired into telemetry.stop() alongside endpoints.stop(). Co-Authored-By: Claude Opus 4.6 <[email protected]>
Check args[2] for an existing options object when args[1] is skipped (undefined/null), preserving cwd, stdio, and other flags the caller set. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Collaborator
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Collaborator
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
wconti27
reviewed
Mar 19, 2026
Comment on lines
+123
to
+135
| it('should not include dd-root-session-id header when rootSessionId equals runtime-id', () => { | ||
| sendDataModule.sendData({ | ||
| url: '/test', | ||
| tags: { 'runtime-id': 'same-id' }, | ||
| rootSessionId: 'same-id', | ||
| }, application, 'test', 'req-type') | ||
|
|
||
| sinon.assert.calledOnce(request) | ||
| const options = request.getCall(0).args[1] | ||
|
|
||
| assert.strictEqual(options.headers['dd-session-id'], 'same-id') | ||
| assert.strictEqual(options.headers['dd-root-session-id'], undefined) | ||
| }) |
Contributor
There was a problem hiding this comment.
do we want a fork test here as well?
Collaborator
Author
There was a problem hiding this comment.
I wouldn't say so, this is testing the header logic in send-data.js, the fork propagation is covered in session-propagation.spec.js and child_process.spec.js
Collaborator
Author
There was a problem hiding this comment.
and also the linked system tests
wconti27
approved these changes
Mar 19, 2026
Contributor
wconti27
left a comment
wconti27
left a comment
There was a problem hiding this comment.
LGTM, just the question about if you want / can test session id being present for forks. Not blocking
gh-worker-ownership-write-b05516
bot
removed the request for review
from mabdinur
This was referenced Mar 20, 2026
gh-worker-dd-mergequeue-cf854d bot
pushed a commit
to DataDog/dd-trace-go
that referenced
this pull request
Mar 20, 2026
## Summary Implements the [Stable Service Instance Identifier RFC](https://docs.google.com/document/d/1ECKj9_NnwaKYtFqm3p3Rlpicx5d-OQcdj9kI2jvRqVU) for Go instrumentation telemetry. - **`DD-Session-ID`**: always present on every telemetry request, set to the current `runtime_id` - **`DD-Root-Session-ID`**: present only in child processes, inherited via `_DD_ROOT_GO_SESSION_ID` env var. Omitted when equal to session ID — backend infers root = self when absent - **Auto-propagation**: `globalconfig.init()` sets `_DD_ROOT_GO_SESSION_ID` in `os.Environ()` so child processes spawned via `os/exec` inherit it automatically without any user-side calls ## Changes - `internal/globalconfig/globalconfig.go`: adds `rootSessionID` field, `init()` reads/sets `_DD_ROOT_GO_SESSION_ID` (internal env var, not in supported_configurations), `RootSessionID()` getter - `internal/telemetry/internal/writer.go`: adds `DD-Session-ID` (always) and `DD-Root-Session-ID` (child processes only) to pre-baked telemetry headers - Tests for both globalconfig (including cross-process propagation) and writer ## Related - System-tests PR: DataDog/system-tests#6510 - Node.js PR: DataDog/dd-trace-js#7821 - dd-trace-py fork tracking: DataDog/dd-trace-py#16839 - dd-trace-py spawn tracking: DataDog/dd-trace-py#16842 Co-authored-by: ayan.khan <[email protected]>
dd-octo-sts bot
pushed a commit
that referenced
this pull request
Mar 21, 2026
* feat(telemetry): add stable session identifier headers adds DD-Session-ID and DD-Root-Session-ID headers to telemetry requests so backend can correlate telemetry across parent/child processes without runtime_id fragmentation
Merged
bengl
pushed a commit
that referenced
this pull request
Mar 23, 2026
* feat(telemetry): add stable session identifier headers adds DD-Session-ID and DD-Root-Session-ID headers to telemetry requests so backend can correlate telemetry across parent/child processes without runtime_id fragmentation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the Stable Service Instance Identifier Proposal for Node.js instrumentation telemetry. It adds DD-Session-ID and DD-Root-Session-ID headers to telemetry requests so backend can correlate telemetry across parent/child processes without runtime_id fragmentation
DD-Session-ID: always present on every telemetry request, set to the currentruntime_idDD-Root-Session-ID: present only in child processes (forked/spawned), set to the inherited rootruntime_id. Omitted from root process requests — backend treatsroot_session_id == session_idwhen absentchild_session.jspatcheschild_process.spawn,spawnSync, andforkto injectDD_ROOT_JS_SESSION_IDandDD_PARENT_JS_SESSION_IDenv vars into all spawned processes, so dd-trace in child processes inherits the session lineage at startupChanges
packages/dd-trace/src/config/index.js: readsDD_ROOT_JS_SESSION_IDat module load (falls back to currentRUNTIME_IDfor root processes); stores asconfig.rootSessionIdpackages/dd-trace/src/telemetry/send-data.js: addsDD-Session-ID(always) andDD-Root-Session-ID(child processes only) to all telemetry request headerspackages/dd-trace/src/telemetry/child_session.js(new): patcheschild_processmethods to inject session env vars before spawningpackages/dd-trace/src/telemetry/telemetry.js: callschild_session.start(config)during telemetry initializationRelated