fix(child_process): stabilize promisified exec spans#9180
Conversation
crysmags
added
semver-patch
ai-generated
Overall package sizeSelf size: 6.52 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 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 🎯 Code Coverage (details) 🔗 Commit SHA: 9905af5 | Docs | Datadog PR Page | Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9180 +/- ##
==========================================
+ Coverage 93.63% 93.64% +0.01%
==========================================
Files 896 898 +2
Lines 52220 52371 +151
Branches 12265 12329 +64
==========================================
+ Hits 48895 49044 +149
- Misses 3325 3327 +2
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:
|
BenchmarksBenchmark execution time: 2026-07-02 17:56:18 Comparing candidate commit 9905af5 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2272 metrics, 14 unstable metrics.
|
d69cf0d to
e58552e
Compare
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
This looks great!
This definitely fixes small issues. Mind adding new tests for the asyncEnd changes with the exit code? I believe we are missing a few for that.
| context.error = err | ||
| error.publish(context) | ||
| throw error | ||
| throw err |
There was a problem hiding this comment.
Nice this fixes a real error!
Could we just rename the error channel to errorChannel and keep error here? That way it reads easier AFAIC
There was a problem hiding this comment.
much cleaner, updated in ba1dff7. kept catch (error) and renamed the channel binding to errorChannel.
| // `_DD_APM_TRACING_AGENTLESS_ENABLED`, which switches the tracer to the | ||
| // agentless intake exporter, so spans never reach this agent and every | ||
| // span assertion times out. Force the agent exporter for plugin tests. | ||
| delete process.env._DD_APM_TRACING_AGENTLESS_ENABLED |
There was a problem hiding this comment.
I believe this would never match all theoretical parts that could leak. If we find one, we have to fix it in place :)
So removing this again seems best
There was a problem hiding this comment.
very true, removed this again in ba1dff7.
| // success, where the exit code is 0. | ||
| exitCode = result.status ?? 0 | ||
| } else if (result === undefined && error !== undefined) { | ||
| exitCode = error.status ?? error.code |
There was a problem hiding this comment.
| exitCode = error.status ?? error.code | |
| exitCode = error.status ?? error.code ?? 0 |
There was a problem hiding this comment.
yep, updated to error.status ?? error.code ?? 0 in ba1dff7 and added the asyncEnd unit coverage for status/code/no-code.
2 participants
What
Stabilizes the
child_processplugin tests and fixes issues found in the promisifiedexecFilepath.Root Cause / Reproduction Evidence
This came from the execute workflow run for
child_processon branchapm-ai-toolkit/execute/child-process/20260701-143026.Source signal: https://github.com/DataDog/dd-trace-js/actions/runs/28529738893/job/84575757777
The workflow reproduced with the repo-defined command shape:
unset OTEL_TRACES_EXPORTER OTEL_LOGS_EXPORTER OTEL_METRICS_EXPORTER PLUGINS=child_process npm run test:plugins:ciThe first reproduced failure was environmental:
_DD_APM_TRACING_AGENTLESS_ENABLEDleaked from the instrumented shell into plugin tests. That forced spans through the agentless exporter instead of the mock test agent, soexpectSomeSpan()tests timed out even though spans were created.After that was fixed, one real order-dependent failure remained: the Bluebird concurrent test created 5
echospans but did not drain them. The following rejection test could consume one of those stale success spans instead of its own failingnode -invalidFlagspan.The run also found two production-code issues:
wrapChildProcessCustomPromisifyMethodshadowed the diagnosticerrorchannel withcatch (error), masking synchronous throws.asyncEnd()stringified the promisified{ stdout, stderr }result ascmd.exit_code: "[object Object]".Changes
_DD_APM_TRACING_AGENTLESS_ENABLEDin the plugin test agent harness.cmd.exit_codecorrectly for promisifiedexecFilesuccess/error paths.Validation
Execute workflow final result:
88 passing, 0 failingGenerated with APM Instrumentation Toolkit.