-
Notifications
You must be signed in to change notification settings - Fork 400
fix(config): extract relevant env vars from pm2_env JSON before tracer init #8863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
cc82106
f23fb9d
dcb7c9d
41e1117
3daebd1
83370aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -148,6 +148,39 @@ function validateAccess (name) { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * 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 | ||||||
| * `cluster.fork()`. pm2 later unpacks it into process.env, but that happens | ||||||
| * inside the main module, after --require dd-trace/init` has already run. | ||||||
| * | ||||||
| * This function copies only DD_* and OTEL_* keys out of the pm2_env blob into | ||||||
| * process.env before config is read, so the tracer sees them at init time. | ||||||
| * Keys already present in process.env are not overwritten, so any host-level | ||||||
| * or explicit env var still takes precedence. | ||||||
| */ | ||||||
|
tlhunter marked this conversation as resolved.
|
||||||
| function applyPm2ClusterEnv () { | ||||||
| if (typeof process.env.pm2_env !== 'string') return | ||||||
|
|
||||||
| let pm2Config | ||||||
| try { | ||||||
| pm2Config = JSON.parse(process.env.pm2_env) | ||||||
| } catch { | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In PM2 cluster mode the worker later unpacks Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems a legitimate issue
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This only extracts Useful? React with 👍 / 👎. |
||||||
| process.env[key] = String(value) | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Run at module load so that early reads in index.js (DD_TRACE_ENABLED, | ||||||
| // OTEL_TRACES_EXPORTER) see the correct values before getConfig() is called. | ||||||
| applyPm2ClusterEnv() | ||||||
|
|
||||||
| module.exports = { | ||||||
| /** | ||||||
| * Expose raw stable config maps and warnings for consumers that need | ||||||
|
|
@@ -264,4 +297,6 @@ module.exports = { | |||||
| } | ||||||
| } | ||||||
| }, | ||||||
|
|
||||||
| applyPm2ClusterEnv, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do not expose the internal helper and test it with proxyquire instead. That way we prevent changing production code for tests. |
||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😢
Should we potentially open a PR against PM2 to stop this and instead just sets the envs when starting the child accordingly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unitech/pm2#6121