test(coverage): collect integration coverage with native V8 instead of istanbul#9091
Conversation
Overall package sizeSelf size: 6.59 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.1 | 122.62 kB | 437.94 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: 7d3b45e | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-07-07 14:12:54 Comparing candidate commit 7d3b45e in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2253 metrics, 33 unstable metrics.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9091 +/- ##
==========================================
+ Coverage 93.48% 96.51% +3.02%
==========================================
Files 901 905 +4
Lines 53308 119150 +65842
Branches 12583 20211 +7628
==========================================
+ Hits 49833 114993 +65160
- Misses 3475 4157 +682 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:
|
…f istanbul The integration coverage harness instrumented dd-trace's source to count lines: nyc's require-hook in every spawned child, plus a pack-time istanbul rewrite of the sandbox tarball. That transform is the dominant cost of a coverage run (~4x wall time on spawn-heavy suites, enough to time out child_process tests on slower runners) and it breaks source-map resolution, which is why several fixtures opt out entirely. Switch collection to native V8 coverage (c8). Every process in the tree writes a raw V8 profile into one shared collector directory via NODE_V8_COVERAGE; after mocha exits, merge-lcov converts each profile through v8-to-istanbul into the same coverage/node-<version> lcov the upload step already reads. No source is transformed, so ESM and source-mapped files are covered without a hook. 1. patch-child-process.js now injects NODE_V8_COVERAGE (plus a minimal bootstrap require that only re-installs the patch) into children, including those that replace env; the opt-out blanks NODE_V8_COVERAGE rather than omitting it, because Node copies the parent's value to a child otherwise. 2. child-bootstrap.js drops nyc.wrap()/the pre-instrumented writer and instead flushes via v8.takeCoverage() on SIGTERM/SIGINT, so a long-running fixture the harness stops with a signal still contributes coverage. 3. The in-process test:*:ci suites move from nyc to c8 via scripts/c8-ci.js. 4. scripts/patch-v8-to-istanbul.js corrects V8's line-coverage over-report on multi-line statements: V8 leaves an indented, un-taken ternary/logical arm reported as covered, so the patch zeroes a line covered only from its first non-whitespace column. This keeps c8's line numbers aligned with the statement-granular istanbul baseline Codecov was built against. Deletes the pack-time instrumenter, the pre-instrumented writer, the sandbox nyc config, and the per-sandbox finalize step — all of which existed only to get istanbul counters into child processes.
`v8-to-istanbul` is a c8 transitive pinned at `^9.0.0` in the lockfile; declaring it as a direct dep at `^9.3.0` left a range the lock had no key for, so every CI job failed to install with `v8-to-istanbul@^9.3.0 failed to resolve`. Align the declared range with the existing resolution (9.3.0 satisfies both).
…e c8 CLI c8's bundled yargs entrypoint throws `require is not defined in ES module scope` when its bin loads under newer Node, so every `test:*:ci` job died at startup. Drop the c8 CLI from scripts/c8-ci.js and instead set NODE_V8_COVERAGE directly, then convert the profiles with the same patched v8-to-istanbul pipeline the integration harness uses (extracted as `convertV8DirToReport` from merge-lcov). Both coverage paths now report through one code path with no CLI dependency.
… chains The `:ci` scripts now invoke `node scripts/c8-ci.js <script>` instead of `nyc -- npm run <script>`. verify-exercised-tests.js only followed `npm run` / `yarn` chains, so it could no longer trace a `:ci` script to the glob that selects its specs and reported every in-process suite's tests as unexercised. Recognize `node scripts/c8-ci.js <script>` as equivalent to `npm run <script>`.
…verage The coverage harness self-test runs in the integration-tests suite, which the Platform job executes via `test:integration:coverage` — i.e. inside the very harness it tests. The ambient NODE_V8_COVERAGE then redirects the children these cases spawn, so a test asserting on its own explicit coverage directory found nothing. Read coverage from whichever directory actually received the profile (the ambient collector when active, the explicit dir otherwise), exercise the shared `convertV8DirToReport` directly instead of the shared merge output dir, and assert the report/`.skipped` invariant rather than a specific source file that the harness legitimately excludes.
Integration tests install dd-trace as a packed tarball inside a throwaway sandbox and run against it, so V8 records coverage under `<sandbox>/node_modules/dd-trace/…`. Two things then dropped every one of those entries at merge time: the sandbox path failed the REPO_ROOT/`node_modules` include check, and the sandbox is deleted before the merge runs so the file is no longer loadable. All dd-trace code exercised only from sandboxes — the jest, cypress, playwright and vitest plugins, the profiler CLI — therefore reported zero coverage, which the previous istanbul harness had captured by rebasing sandbox paths back onto the repo tree. Map the segment after `node_modules/dd-trace/` onto REPO_ROOT before the include check, and load the still-present repo copy (whose bytes are identical to the sandbox copy V8 measured, the tarball shipping source verbatim) so the entry is counted under its repo path and merges with in-process coverage.
ed7b908 to
7f8f64b
Compare
The codeowners audit only scoped `**/*.spec.js`, `benchmark/sirun/**`, and the agent config dirs, so every non-spec file under integration-tests/ — fixtures, helpers, forwarder scripts — could land unowned without tripping the lint gate. The new coverage-merge-lcov.spec.js was the first to surface it. Add an `integration-tests/**` glob to the audit and assign every path it flags: the appsec graphql and standalone-asm fixtures to asm-js, the pino fixture to apm-idm-js, the log_injection / remote_config fixtures and telemetry forwarder to apm-sdk-capabilities-js, and the shared helpers, init, memory-leak, package-guardrails, startup and tsconfig entries to lang-platform-js. The package-guardrails directory entry replaces the file-by-file flush.js rule the audit flagged as fragile.
The project status was off, so a PR could land changed lines at the 95% patch target while quietly lowering total coverage with no gate to stop it. Turn the project status back on with `target: auto` and `threshold: 0%` so any drop below the PR's base fails `codecov/project`, while patch stays at 95% — new code may sit at 95% as long as it does not pull the overall number down. It keeps the existing `master-coverage` flag / only_pulls scoping, so non-master PRs auto-pass. Also bump both All Green upload fan-outs (Datadog and Codecov) from -P 10 to -P 20. The upload step is still a noticeable chunk of that job and the earlier parallelism bump held up, so this raises it further to see if it shortens the step without overwhelming either backend.
sabrenner
left a comment
There was a problem hiding this comment.
two non-blocking questions, gave it a general look over but afaict looks ok to me
`NODE_V8_COVERAGE` is not private to the harness — Node's own test runner (`node --test --experimental-test-coverage`), c8 and other tools read it from the child env. Overwriting it unconditionally redirected such a fixture's own coverage into our collector, usually leaving its assertions green against empty data. When a child already carries a directory, leave it in place and record the collector in a copy-back marker instead; the per-spawn opt-out likewise blanks `NODE_V8_COVERAGE` only when it is the value the harness injected, preserving a directory the child set for itself. A forked child's own profile is folded into the collector on its `exit`, after V8's single teardown write, so `v8.takeCoverage()` is never called in a gracefully-exiting child — calling it splits the child's execution counters (each call resets them), which would corrupt the very coverage the child's own tooling reads. The forceful stop paths still flush in-child, where the reset is moot because the process is being terminated. Reintroduce the Windows IPC flush sentinel in `stopProc`/`child-bootstrap`: Windows SIGTERM is forceful and skips the signal hook, so a connected child is asked to flush and exit cleanly on the sentinel, and its preserved-directory profiles are copied on the ensuing exit.
bengl
left a comment
There was a problem hiding this comment.
This is fine, but let's make sure all of this patching of devDeps actually happens upstream at some point, in some way.
bengl
left a comment
There was a problem hiding this comment.
This is fine, but let's make sure all of this patching of devDeps actually happens upstream at some point, in some way.
…f istanbul (#9091) The istanbul harness made coverage runs spend most of their time rewriting source and installing require hooks in child processes, which timed out spawn-heavy suites and made source-mapped fixtures opt out of coverage entirely. 1. Replace the integration and in-process coverage paths with shared NODE_V8_COVERAGE collection and v8-to-istanbul conversion. 2. Preserve a child's own NODE_V8_COVERAGE directory and copy those profiles back so fixture-level coverage tools keep seeing their own data. 3. Rebase sandbox dd-trace paths onto the repo copy before merging, so packed-tarball integration tests still count toward repo coverage. 4. Patch V8's multi-line line coverage over-reporting to keep Codecov line numbers aligned with the istanbul baseline. 5. Restore coverage CI guardrails by enabling project coverage drops as failures and auditing integration-test ownership.
…f istanbul (#9091) The istanbul harness made coverage runs spend most of their time rewriting source and installing require hooks in child processes, which timed out spawn-heavy suites and made source-mapped fixtures opt out of coverage entirely. 1. Replace the integration and in-process coverage paths with shared NODE_V8_COVERAGE collection and v8-to-istanbul conversion. 2. Preserve a child's own NODE_V8_COVERAGE directory and copy those profiles back so fixture-level coverage tools keep seeing their own data. 3. Rebase sandbox dd-trace paths onto the repo copy before merging, so packed-tarball integration tests still count toward repo coverage. 4. Patch V8's multi-line line coverage over-reporting to keep Codecov line numbers aligned with the istanbul baseline. 5. Restore coverage CI guardrails by enabling project coverage drops as failures and auditing integration-test ownership.
…f istanbul (#9091) The istanbul harness made coverage runs spend most of their time rewriting source and installing require hooks in child processes, which timed out spawn-heavy suites and made source-mapped fixtures opt out of coverage entirely. 1. Replace the integration and in-process coverage paths with shared NODE_V8_COVERAGE collection and v8-to-istanbul conversion. 2. Preserve a child's own NODE_V8_COVERAGE directory and copy those profiles back so fixture-level coverage tools keep seeing their own data. 3. Rebase sandbox dd-trace paths onto the repo copy before merging, so packed-tarball integration tests still count toward repo coverage. 4. Patch V8's multi-line line coverage over-reporting to keep Codecov line numbers aligned with the istanbul baseline. 5. Restore coverage CI guardrails by enabling project coverage drops as failures and auditing integration-test ownership.
…f istanbul (#9091) The istanbul harness made coverage runs spend most of their time rewriting source and installing require hooks in child processes, which timed out spawn-heavy suites and made source-mapped fixtures opt out of coverage entirely. 1. Replace the integration and in-process coverage paths with shared NODE_V8_COVERAGE collection and v8-to-istanbul conversion. 2. Preserve a child's own NODE_V8_COVERAGE directory and copy those profiles back so fixture-level coverage tools keep seeing their own data. 3. Rebase sandbox dd-trace paths onto the repo copy before merging, so packed-tarball integration tests still count toward repo coverage. 4. Patch V8's multi-line line coverage over-reporting to keep Codecov line numbers aligned with the istanbul baseline. 5. Restore coverage CI guardrails by enabling project coverage drops as failures and auditing integration-test ownership.
Summary
Replace the istanbul-based integration coverage harness with native V8 coverage (c8).
The old harness instrumented dd-trace's source to count lines — nyc's require-hook in every spawned child plus a pack-time istanbul rewrite of the sandbox tarball. That transform is the dominant cost of a coverage run (~4x wall time on spawn-heavy suites, enough to time out
child_processtests on slower runners) and it breaks--enable-source-maps, which is why several fixtures opt out of coverage entirely.Now every process in the tree writes a raw V8 profile into one shared collector directory via
NODE_V8_COVERAGE; after mocha exits,merge-lcov.jsconverts each profile throughv8-to-istanbulinto the samecoverage/node-<version>lcov the upload step already reads. No source is transformed, so ESM and source-mapped files are covered without a hook.patch-child-process.jsinjectsNODE_V8_COVERAGE(plus a minimal bootstrap require that only re-installs the patch) into children, including those that replaceenv. The per-spawn opt-out blanksNODE_V8_COVERAGErather than omitting it, because Node copies the parent's value to a child otherwise.child-bootstrap.jsdropsnyc.wrap()/ the pre-instrumented writer and instead flushes viav8.takeCoverage()onSIGTERM/SIGINT, so a long-running fixture the harness stops with a signal still contributes coverage.test:*:cisuites move from nyc to c8 viascripts/c8-ci.js.scripts/patch-v8-to-istanbul.jscorrects V8's line-coverage over-report on multi-line statements (an indented, un-taken ternary/logical arm is otherwise reported as covered), keeping c8's line numbers aligned with the statement-granular istanbul baseline Codecov was built against.Deletes the pack-time instrumenter, the pre-instrumented writer, the sandbox nyc config, and the per-sandbox finalize step — all of which existed only to get istanbul counters into child processes.
Why
V8 coverage removes the instrumentation transform that motivated the entire pre-instrumented-tarball machinery, dissolving both its ~4x overhead (the root cause of the
child_processCI timeouts) and its source-map breakage. The line-coverage patch matches the existing istanbulgetLineCoveragepatch in spirit so the migration does not move Codecov's line numbers under teams.Depending on #9094