fix: make DD_TRACING_ENABLED an alias of DD_APM_TRACING_ENABLED#8140
fix: make DD_TRACING_ENABLED an alias of DD_APM_TRACING_ENABLED#8140
Conversation
It was accidentially made an alias of DD_TRACE_ENABLED, which acts differently.
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8140 +/- ##
==========================================
+ Coverage 74.11% 77.19% +3.07%
==========================================
Files 786 813 +27
Lines 36400 37658 +1258
==========================================
+ Hits 26979 29069 +2090
+ Misses 9421 8589 -832 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:
|
![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: 55b8d1a7c7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
BenchmarksBenchmark execution time: 2026-04-29 13:39:06 Comparing candidate commit 5d1e09d in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1345 metrics, 99 unstable metrics. |
Overall package sizeSelf size: 5.67 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.1 | 82.56 kB | 817.39 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d244269a6b
ℹ️ 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".
It was accidentially made an alias of DD_TRACE_ENABLED, which acts differently.
| 'should capture and send expected payload when a log line probe is triggered', | ||
| testBasicInputWithoutDD.bind(null, t) | ||
| ) | ||
| it('should capture and send expected payload when a log line probe is triggered', testBasicInput.bind(null, t)) |
There was a problem hiding this comment.
I'm not sure we want to remove testBasicInputWithoutDD. I would assume that's still relevant here?
There was a problem hiding this comment.
In standalone APM mode (DD_APM_TRACING_ENABLED=false / DD_TRACING_ENABLED=false) the tracer is fully active locally, it just stops shipping spans the agent does not need.
The active span returned by tracer.scope().active()?.context() is still a real span with a trace_id and span_id, so the dd field IS populated in the debugger payload. That's why the new test correctly uses testBasicInput (which asserts the dd field is present and equal to the local span's ids) instead of testBasicInputWithoutDD.
The only scenario where the helper would be relevant is DD_TRACE_ENABLED=false, but after this PR that flag unconditionally short-circuits to the noop proxy, which means DI is never started in the test app and the breakpoint would never fire — so the helper had no remaining caller and I removed it.
I will open a follow-up that adds an explicit assertion that DI does not start when DD_TRACE_ENABLED=false, so the new behavior is locked down by a test.
There was a problem hiding this comment.
I thought that when DD_TRACING_ENABLED=false then there would be no trace/span created?
There was a problem hiding this comment.
The spans are still created, they are just not send out later. Span creation is only turned off with DD_TRACE_ENABLED being false.
The template integration test compared its output against a hardcoded `util.inspect` string and version-gated the bracketed `[byteLength]` form on `>=24.11.1`. The same change was later backported to Node.js v22.22.1, which broke `node-active` on master once GitHub Actions started serving v22.22.x to that job (PR CI happened to land on v22.22.0 and stayed green). Probe the current `util.inspect` output for `[byteLength]` instead, so the assertion tracks Node's actual behavior on whatever patch the runner picks up. This avoids having to enumerate every release line that backports the change. Refs: nodejs/node#60131 Refs: #8140
The template integration test compared its output against a hardcoded `util.inspect` string and version-gated the bracketed `[byteLength]` form on `>=24.11.1`. The same change was later backported to Node.js v22.22.1, which broke `node-active` on master once GitHub Actions started serving v22.22.x to that job (PR CI happened to land on v22.22.0 and stayed green). Probe the current `util.inspect` output for `[byteLength]` instead, so the assertion tracks Node's actual behavior on whatever patch the runner picks up. This avoids having to enumerate every release line that backports the change. Refs: nodejs/node#60131 Refs: #8140
It was accidentially made an alias of DD_TRACE_ENABLED, which acts differently.
The template integration test compared its output against a hardcoded `util.inspect` string and version-gated the bracketed `[byteLength]` form on `>=24.11.1`. The same change was later backported to Node.js v22.22.1, which broke `node-active` on master once GitHub Actions started serving v22.22.x to that job (PR CI happened to land on v22.22.0 and stayed green). Probe the current `util.inspect` output for `[byteLength]` instead, so the assertion tracks Node's actual behavior on whatever patch the runner picks up. This avoids having to enumerate every release line that backports the change. Refs: nodejs/node#60131 Refs: #8140
3 participants
It was accidentially made an alias of DD_TRACE_ENABLED, which acts differently.