fix(config): extract relevant env vars from pm2_env JSON before tracer init#8863
fix(config): extract relevant env vars from pm2_env JSON before tracer init#8863tlhunter wants to merge 6 commits into
Conversation
In PM2 cluster mode, per-app env vars are serialized into a single `pm2_env` JSON string via `cluster.fork()`. PM2's ProcessContainer.js unpacks them into process.env, but only after NODE_OPTIONS `--require` preloads have already run — so dd-trace initialized with DD_SERVICE undefined and fell back to "node". `applyPm2ClusterEnv()` is called once at the top of `getConfig()` before `new Config()` reads process.env. It parses the pm2_env blob and copies any DD_*/OTEL_* keys that are not already set, so host-level overrides still take precedence. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Overall package sizeSelf size: 6.2 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.2 | 85.93 kB | 825.11 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 |
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 83370aa | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-06-11 17:39:26 Comparing candidate commit 83370aa in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1448 metrics, 19 unstable metrics. |
- process.env access is only allowed in config/helper.js per the eslint-process-env rule; move applyPm2ClusterEnv there and re-export - Replace assert.doesNotThrow() (banned by no-restricted-syntax) with direct calls per the project's test style Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…spec.js Tests via proxyquire (in index.spec.js) don't guarantee reliable coverage attribution for helper.js function bodies in codecov's patch check. Adding direct-require tests in helper.spec.js ensures the new lines are always covered by the file that owns them. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![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: 41e1117416
ℹ️ 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".
|
The codecov check is buggy. It passes and fails between different runs even when code hasn't changed. |
tlhunter
changed the title
index.js decides noop vs full proxy by reading DD_TRACE_ENABLED and OTEL_TRACES_EXPORTER via getValueFromEnvSources() at require() time — before getConfig() is ever invoked. In PM2 cluster mode those values sit in the pm2_env blob, so they were invisible to that early read. Calling applyPm2ClusterEnv() as a module-level side-effect in helper.js guarantees that process.env is populated from the blob before index.js's first line executes (index.js already requires helper.js as its first import). The call in getConfig() is kept for correctness when helper.js is loaded before pm2_env is set (e.g. tests via proxyquire). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nfig The module-level call in helper.js fires when the module is first required, which is earlier than getConfig() in all real code paths. In tests, proxyquire.noPreserveCache() re-runs the module-level call on each getConfig() invocation — and all test cases already set pm2_env before calling getConfig() — so the explicit call inside getConfig() adds no value. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83370aa0cf
ℹ️ 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".
| } | ||
|
|
||
| for (const [key, value] of Object.entries(pm2Config)) { | ||
| if ((key.startsWith('DD_') || key.startsWith('OTEL_')) && !(key in process.env) && value != null) { |
There was a problem hiding this comment.
In PM2 cluster mode the worker later unpacks pm2_env by assigning its keys into process.env, so per-app values in the PM2 config override any inherited daemon/shell environment. With this guard, if the PM2 daemon was started with DD_SERVICE=host but the app's ecosystem config sets DD_SERVICE=app, --require dd-trace/init keeps the inherited host value and caches the tracer config before PM2 performs its normal overwrite, so traces are reported under the wrong service for that app.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This seems a legitimate issue
| if ((key.startsWith('DD_') || key.startsWith('OTEL_')) && !(key in process.env) && value != null) { | |
| if (key.startsWith('DD_') || key.startsWith('OTEL_') || aliasToCanonical.has(key)) { |
| } | ||
|
|
||
| for (const [key, value] of Object.entries(pm2Config)) { | ||
| if ((key.startsWith('DD_') || key.startsWith('OTEL_')) && !(key in process.env) && value != null) { |
There was a problem hiding this comment.
This only extracts DD_/OTEL_ names, but DATADOG_API_KEY is a supported alias for DD_API_KEY in supported-configurations.json. In PM2 cluster mode, users who put DATADOG_API_KEY in the app's PM2 env and preload the tracer will still get config.apiKey === undefined until PM2 unpacks the blob later, so agentless/LLMObs/log-submission paths can be disabled or drop data even though the app was configured with a valid supported key.
Useful? React with 👍 / 👎.
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
This will likely fix the overall problem for most users, while our guardrails will not know about DD_INJECT_FORCE and DD_INJECTION_ENABLED.
I am unsure if that is an issue. Next to that: we currently also access process.env using webpack, esbuild, and cypress and I guess they could also be called, while unlikely?
| } | ||
| }, | ||
|
|
||
| applyPm2ClusterEnv, |
There was a problem hiding this comment.
Please do not expose the internal helper and test it with proxyquire instead. That way we prevent changing production code for tests.
| /** | ||
| * In PM2 cluster mode, per-app env vars (DD_SERVICE, DD_ENV, etc.) are not | ||
| * passed as individual environment variables. Instead PM2 serializes the entire | ||
| * process config into a single `pm2_env` JSON string and passes only that to |
There was a problem hiding this comment.
😢
Should we potentially open a PR against PM2 to stop this and instead just sets the envs when starting the child accordingly?
| } | ||
|
|
||
| for (const [key, value] of Object.entries(pm2Config)) { | ||
| if ((key.startsWith('DD_') || key.startsWith('OTEL_')) && !(key in process.env) && value != null) { |
There was a problem hiding this comment.
This seems a legitimate issue
| if ((key.startsWith('DD_') || key.startsWith('OTEL_')) && !(key in process.env) && value != null) { | |
| if (key.startsWith('DD_') || key.startsWith('OTEL_') || aliasToCanonical.has(key)) { |
|
I'm converting this into a draft while we wait on PM2 to reply to the PR. |
2 participants
What does this PR do?
This allows the tracer to run in pm2 cluster mode. Notably pm2 passes in a JSON-encoded env var that is merges into
process.envafter--requireruns, so this change extracts any tracer-relevant entries first.Motivation
This is from a user-reported error.