Skip to content

ci: reduce system tests execution time, split AppSec ubuntu matrix, rework All Green report uploads - #9197

Open
rochdev wants to merge 37 commits into
masterfrom
rochdev/system-tests-execution-time
Open

ci: reduce system tests execution time, split AppSec ubuntu matrix, rework All Green report uploads#9197
rochdev wants to merge 37 commits into
masterfrom
rochdev/system-tests-execution-time

Conversation

@rochdev

@rochdev rochdev commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Sets desired_execution_time for the System Tests workflow from 300s (5 min) to 180s (3 min).
  • Splits the AppSec ubuntu job into an [oldest, maintenance, active, latest] matrix so the four Node versions run in parallel instead of sequentially in one job.
  • Reworks how All Green downloads, merges, and uploads junit/coverage reports, to cut both wall-clock time and the number of CLI/network calls issued per commit:
    • Per-workflow Codecov pipelining: each sibling workflow's coverage is downloaded, merged, and uploaded to Codecov as soon as that workflow finishes, instead of waiting for every workflow to complete (scripts/all-green.mjs, scripts/download-artifacts.mjs, scripts/upload-coverage.mjs).
    • Batched junit and Datadog coverage uploads: unlike Codecov, junit and Datadog's coverage upload have no per-workflow constraint (Codecov needs a distinct flag per workflow; junit differentiates by a node_version tag instead, and Datadog's coverage flag is the same for every run), so both are merged across every sibling workflow and uploaded once each, after all workflows finish, instead of once per workflow (scripts/upload-junit.mjs, scripts/group-coverage.mjs, scripts/upload-coverage.mjs). This was motivated by evidence that many CLI processes (datadog-ci/codecovcli) spawning at once when several sibling workflows finished in the same window were contending for CPU on the runner and inflating every upload's wall-clock time, regardless of payload size; reducing the number of invocations (rather than just capping concurrency, which only serializes the same work) fixes that at the source.
    • Junit XML pre-merge: every matrix cell's junit report is merged into a single XML document before upload instead of uploading one file per cell; each testcase's node_version property keeps cells distinguishable afterward (scripts/upload-junit.mjs).
    • Coverage merging by run instead of by integration: scripts/group-coverage.mjs merges each workflow run's lcov reports into a single file, replacing the previous per-integration grouping (~100 groups uploaded per commit) with a real per-file merge (summing DA/FNDA/BRDA hit counts) rather than passing each cell's report through unmerged.
    • Dropped istanbul JSON support, lcov only for Codecov too: Codecov originally read branch/function coverage from a separately-merged istanbul JSON report, since Datadog only ingests lcov. That JSON merge (istanbul-lib-coverage's merge) turned out to be the actual bottleneck on reruns — far slower than the lcov merge on a run with many matrix cells — for coverage detail only Codecov's branch/function view used. Rather than caching around that cost, both uploads now go through the same merged lcov file and JSON support was removed outright (scripts/group-coverage.mjs, scripts/upload-coverage.mjs, .github/actions/upload-coverage-artifact/action.yml). The trade-off: Codecov reports line coverage only, no branch/function coverage, going forward — a conscious choice given how much rerun time the JSON merge cost for detail nothing else consumed.
    • Removed the per-run Codecov cache: with lcov merging now cheap, the persisted run-id cache that let a rerun skip re-uploading already-succeeded runs to Codecov (a JSON file plus an actions/cache step) no longer earns its complexity — every rerun now just redoes download, merge, and upload for every run again (scripts/all-green.mjs, .github/workflows/all-green.yml).
    • Codecov commit/report registration added: codecovcli do-upload requires create-commit/create-report to already exist for the SHA; scripts/upload-coverage.mjs registers the commit/report once per run (memoized, since every sibling workflow shares the same commit), and every codecovcli call now passes --fail-on-error so a real failure fails the job instead of being silently swallowed.
    • Manual notification trigger: .codecov.yml sets notify.manual_trigger so Codecov waits for a final send-notifications call once every sibling workflow's uploads have landed, instead of posting its status after the first upload.
    • Retry with backoff: scripts/run-upload.mjs retries each codecovcli upload call (2 retries, 2s backoff) before giving up, since --fail-on-error can now fail the whole job on a transient blip.
    • Per-workflow Codecov flags: each sibling workflow's Codecov upload carries a flag derived from the workflow name, instead of the shared per-integration group name, so Codecov's per-flag breakdown reflects each workflow separately.
    • Bounded, retried artifact downloads: scripts/download-artifacts.mjs caps how many artifacts it downloads concurrently and retries failed downloads with backoff, instead of firing every artifact's download at once — a run with 100+ artifacts finishing at once was observed hitting GitHub API connection/rate limits and failing every download for that run with a generic fetch failed.
    • Consolidated logging: each workflow run prints one summary line covering its artifact download and coverage upload, instead of several scattered log lines per run; the batched junit/coverage-to-Datadog uploads print one more summary line covering the whole commit. Upload CLI output is buffered and only dumped on failure.

Test plan

  • ./node_modules/.bin/mocha scripts/group-coverage.spec.mjs scripts/upload-junit.spec.mjs passes (updated for lcov-only merging)
  • npx eslint clean on all changed files
  • Confirmed via Codecov's public API that a commit run through the pipeline is fully ingested (state: complete, upload sessions present, coverage computed)
  • Confirmed codecov/patch and codecov/project PR status checks post successfully
  • Verified on this PR's own All Green runs that report processing produces one consolidated log line per sibling workflow, plus one batched line for the commit-wide junit/coverage upload
  • Verified via real CI logs that the batched junit+coverage upload takes ~12s total for the whole commit, versus up to ~28.8s/24.6s for a single workflow's calls under the old per-workflow design
  • Verified via a run where several sibling workflows finished at once that the concurrency cap + retry on artifact downloads prevents the fetch failed failures that an uncapped burst previously caused

Generated by Claude Code.

@dd-octo-sts

dd-octo-sts Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Overall package size

Self size: 8.03 MB
Deduped: 8.69 MB
No deduping: 8.69 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.3 | 125.43 kB | 441.68 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

@datadog-official

datadog-official Bot commented Jul 2, 2026

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 97.96% (-0.60%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: c74af92 | Docs | Datadog PR Page | Give us feedback!

@pr-commenter

pr-commenter Bot commented Jul 2, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-08-13 00:32:48

Comparing candidate commit c74af92 in PR branch rochdev/system-tests-execution-time with baseline commit f72b07e in branch master.

📊 Benchmarking dashboard

Found 0 performance improvements and 0 performance regressions! Performance is the same for 2319 metrics, 39 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Unstable benchmarks

These benchmarks have a confidence interval too wide to call a change; treat them as noise rather than signal.

scenario:appsec-appsec-enabled-24

  • unstable execution_time [-210.999ms; +202.153ms] or [-7.771%; +7.445%]

scenario:appsec-appsec-enabled-26

  • unstable execution_time [-239.058ms; +226.346ms] or [-9.231%; +8.740%]

scenario:appsec-appsec-enabled-with-attacks-24

  • unstable execution_time [-169.011ms; +155.429ms] or [-5.394%; +4.960%]

scenario:appsec-appsec-enabled-with-attacks-26

  • unstable execution_time [-190.406ms; +175.609ms] or [-6.454%; +5.952%]

scenario:appsec-control-20

  • unstable execution_time [-126.931ms; +115.415ms] or [-7.615%; +6.924%]

scenario:appsec-control-24

  • unstable execution_time [-113738.384µs; +113445.141µs] or [-9.054%; +9.031%]

scenario:appsec-control-26

  • unstable execution_time [-126.085ms; +123.749ms] or [-10.049%; +9.862%]

scenario:appsec-iast-no-vulnerability-iast-enabled-default-config-20

  • unstable execution_time [-15.562ms; +10.652ms] or [-6.072%; +4.157%]

scenario:child_process-shell-string-24

  • unstable execution_time [-18.117ms; +22.783ms] or [-5.549%; +6.979%]
  • unstable throughput [-200366.500op/s; +169676.146op/s] or [-5.598%; +4.741%]

scenario:debugger-line-probe-with-snapshot-default-26

  • unstable cpu_user_time [-2667.749ms; +4232.653ms] or [-27.991%; +44.410%]
  • unstable execution_time [-2813.824ms; +4390.654ms] or [-27.430%; +42.802%]
  • unstable instructions [-23.6G instructions; +37.5G instructions] or [-29.623%; +47.124%]
  • unstable max_rss_usage [-8.615MB; +13.514MB] or [-5.426%; +8.511%]
  • unstable throughput [-843.508op/s; +549.479op/s] or [-26.020%; +16.950%]

scenario:debugger-line-probe-with-snapshot-minimal-26

  • unstable cpu_user_time [-4634.195ms; +3105.935ms] or [-41.664%; +27.924%]
  • unstable execution_time [-4704.441ms; +3130.283ms] or [-39.645%; +26.380%]
  • unstable instructions [-41.2G instructions; +27.4G instructions] or [-44.016%; +29.253%]
  • unstable max_rss_usage [-14.640MB; +10.532MB] or [-8.910%; +6.410%]
  • unstable throughput [-616.374op/s; +906.522op/s] or [-21.066%; +30.982%]

scenario:debugger-line-probe-without-snapshot-24

  • unstable cpu_user_time [-996.529ms; +1403.951ms] or [-12.631%; +17.796%]
  • unstable execution_time [-997.918ms; +1364.878ms] or [-11.586%; +15.846%]
  • unstable instructions [-8.6G instructions; +12.0G instructions] or [-13.442%; +18.746%]
  • unstable throughput [-474.475op/s; +372.190op/s] or [-12.631%; +9.908%]

scenario:debugger-line-probe-without-snapshot-26

  • unstable cpu_user_time [-3652.275ms; +3662.551ms] or [-35.350%; +35.450%]
  • unstable execution_time [-3661.113ms; +3680.819ms] or [-33.158%; +33.336%]
  • unstable instructions [-32708.1M instructions; +32700.6M instructions] or [-37.817%; +37.809%]
  • unstable max_rss_usage [-12956.406KB; +12054.806KB] or [-8.035%; +7.476%]
  • unstable throughput [-730.507op/s; +707.147op/s] or [-23.681%; +22.924%]

scenario:dogstatsd-with-tags-20

  • unstable cpu_user_time [-458.744ms; +227.529ms] or [-9.354%; +4.640%]
  • unstable execution_time [-456.576ms; +228.609ms] or [-9.167%; +4.590%]
  • unstable throughput [-80819.223op/s; +163551.723op/s] or [-4.798%; +9.709%]

scenario:plugin-claude-agent-sdk-compact-stream-scan-24

  • unstable cpu_usage_percentage [-5.704%; +4.456%]

scenario:plugin-claude-agent-sdk-compact-stream-scan-26

  • unstable cpu_usage_percentage [-6.064%; +4.109%]

scenario:plugin-graphql-long-with-depth-off-20

  • unstable max_rss_usage [-7818.040KB; +6434.040KB] or [-5.986%; +4.926%]

scenario:plugin-graphql-long-with-depth-off-26

  • unstable max_rss_usage [-35.425MB; +16.236MB] or [-19.698%; +9.028%]

scenario:plugin-mongodb-core-plain-find-26

  • unstable execution_time [-150.383ms; +155.187ms] or [-7.229%; +7.459%]
  • unstable throughput [-223744.109op/s; +245347.332op/s] or [-5.624%; +6.167%]

scenario:test-optimization-large-suite-20

  • unstable max_rss_usage [-4520.391KB; +6179.724KB] or [-5.680%; +7.764%]

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.56%. Comparing base (f72b07e) to head (c74af92).
⚠️ Report is 44 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9197      +/-   ##
==========================================
  Coverage   98.56%   98.56%              
==========================================
  Files         972      972              
  Lines      140837   140837              
  Branches    12116       74   -12042     
==========================================
+ Hits       138811   138817       +6     
+ Misses       2026     2020       -6     
Flag Coverage Δ
ai-guard 60.38% <ø> (?)
aiguard ?
aiguard-integration ?
apm-bucket-0 ?
apm-bucket-1 ?
apm-bucket-2 ?
apm-bucket-3 ?
apm-capabilities 62.50% <ø> (?)
apm-capabilities-tracing ?
apm-integrations 76.21% <ø> (?)
apm-integrations-aerospike ?
apm-integrations-confluentinc-kafka-javascript ?
apm-integrations-couchbase ?
apm-integrations-http ?
apm-integrations-kafkajs ?
apm-integrations-next ?
apm-integrations-prisma ?
appsec 59.87% <ø> (-12.23%) ⬇️
appsec-express_fastify_graphql ?
appsec-integration ?
appsec-kafka_ldapjs_lodash ?
appsec-mongodb-core_mongoose_mysql ?
appsec-next ?
appsec-node-serialize_passport_postgres ?
appsec-sourcing_stripe_template ?
debugger 64.23% <ø> (-0.02%) ⬇️
instrumentation 53.99% <ø> (?)
instrumentations-bucket-0 ?
instrumentations-bucket-1 ?
instrumentations-bucket-10 ?
instrumentations-bucket-11 ?
instrumentations-bucket-12 ?
instrumentations-bucket-13 ?
instrumentations-bucket-14 ?
instrumentations-bucket-2 ?
instrumentations-bucket-3 ?
instrumentations-bucket-4 ?
instrumentations-bucket-5 ?
instrumentations-bucket-6 ?
instrumentations-bucket-7 ?
instrumentations-bucket-8 ?
instrumentations-bucket-9 ?
instrumentations-instrumentation-couchbase ?
instrumentations-integration-esbuild ?
llmobs 73.02% <ø> (?)
llmobs-ai_anthropic_bedrock ?
llmobs-bucket-1 ?
llmobs-openai ?
llmobs-openai-agents_vertex-ai ?
llmobs-sdk ?
master-coverage 98.56% <ø> (?)
openfeature 59.98% <ø> (+4.30%) ⬆️
openfeature-unit ?
platform 63.92% <ø> (?)
platform-core_esbuild_instrumentations-misc ?
platform-integration ?
platform-shimmer_unit-guardrails_webpack ?
plugins-bucket-0 ?
plugins-bucket-1 ?
plugins-bucket-11 ?
plugins-bucket-14 ?
plugins-bucket-17 ?
plugins-bucket-18 ?
plugins-bucket-19 ?
plugins-bucket-20 ?
plugins-bucket-4 ?
plugins-bullmq_cassandra_cookie ?
plugins-cookie-parser_crypto_dd-trace-api ?
plugins-fetch_fs_generic-pool ?
plugins-google-cloud-pubsub_grpc_handlebars ?
plugins-hapi_hono_ioredis ?
plugins-knex_langgraph_ldapjs ?
plugins-light-my-request_lodash_mariadb ?
plugins-memcached_mercurius_microgateway-core ?
plugins-mongodb-core_mongoose_multer ?
plugins-mysql_mysql2_nats ?
plugins-pino_postgres_process ?
plugins-pug_redis_router ?
plugins-url_valkey_vm ?
plugins-winston_ws ?
profiling 61.51% <ø> (-0.03%) ⬇️
serverless 64.17% <ø> (?)
serverless-aws-sdk-aws-sdk ?
serverless-aws-sdk-base-inject-field ?
serverless-aws-sdk-bedrockruntime ?
serverless-aws-sdk-client ?
serverless-aws-sdk-dynamodb ?
serverless-aws-sdk-eventbridge ?
serverless-aws-sdk-kinesis ?
serverless-aws-sdk-lambda ?
serverless-aws-sdk-s3 ?
serverless-aws-sdk-serverless-peer-service ?
serverless-aws-sdk-sns ?
serverless-aws-sdk-sqs ?
serverless-aws-sdk-stepfunctions ?
serverless-aws-sdk-util ?
serverless-bucket-0 ?
serverless-bucket-1 ?
test-optimization 82.57% <ø> (?)
test-optimization-cucumber ?
test-optimization-cypress ?
test-optimization-jest ?
test-optimization-mocha ?
test-optimization-playwright-playwright-atr ?
test-optimization-playwright-playwright-efd ?
test-optimization-playwright-playwright-final-status ?
test-optimization-playwright-playwright-impacted-tests ?
test-optimization-playwright-playwright-reporting ?
test-optimization-playwright-playwright-test-management ?
test-optimization-playwright-playwright-test-span ?
test-optimization-selenium ?
test-optimization-testopt ?
test-optimization-vitest ?
test-optimization-vitest-browser ?
test-optimization-webdriverio ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rochdev rochdev changed the title ci: reduce system tests desired execution time to 120s ci: reduce system tests execution time, split AppSec ubuntu matrix, speed up All Green coverage uploads Jul 2, 2026
@rochdev
rochdev force-pushed the rochdev/system-tests-execution-time branch from 0f4fc4f to 239c810 Compare July 8, 2026 16:24
@rochdev
rochdev marked this pull request as ready for review July 8, 2026 18:40
@rochdev
rochdev requested review from a team as code owners July 8, 2026 18:40
@rochdev
rochdev marked this pull request as draft July 8, 2026 18:40

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6dd8d9a919

ℹ️ 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".

Comment thread scripts/upload-coverage.mjs Outdated

@datadog-official datadog-official Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datadog Autotest: PASS

More details

The per-run concurrent upload architecture in all-green.mjs and the rewritten group-coverage.mjs are correct: processedRunIds prevents double-processing, downloadArtifacts is safe for concurrent calls (paths isolated by run ID), mergeLcov correctly concatenates lcov records including duplicate source files (Codecov sums server-side), and runUpload always resolves (never rejects) while reliably setting process.exitCode on non-zero exits. All 8 official tests and all 10 adversarial scenarios pass.

Was this helpful? React 👍 or 👎

📊 Validated against 18 scenarios · Open Bits AI session

🤖 Datadog Autotest · Commit 6dd8d9a · What is Autotest? · Any feedback? Reach out in #autotest

@rochdev rochdev changed the title ci: reduce system tests execution time, split AppSec ubuntu matrix, speed up All Green coverage uploads ci: reduce system tests execution time, split AppSec ubuntu matrix, rework All Green report uploads Jul 8, 2026
@rochdev
rochdev marked this pull request as ready for review July 8, 2026 22:53

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b994d9fdd

ℹ️ 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".

Comment thread scripts/all-green.mjs

@datadog-official datadog-official Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datadog Autotest: FAIL

The new spawnUpload function in scripts/run-upload.mjs is missing a child.on('error', ...) listener. Node.js treats an unhandled error event on a ChildProcess as an uncaught exception, crashing all-green.mjs immediately instead of returning a failed UploadResult. The old shell-script approach (bash exit 127) handled this gracefully; the new spawn-based approach does not. Fix applied: add child.on('error', err => { output += err.message }) and use code ?? 1 in the close handler to cover the null code that spawn failures emit.

View proposed fix
📊 Validated against 17 scenarios · Open Bits AI session

🤖 Datadog Autotest · Commit 0b994d9 · What is Autotest? · Any feedback? Reach out in #autotest

Comment thread scripts/run-upload.mjs
@rochdev
rochdev force-pushed the rochdev/system-tests-execution-time branch from 6c4bedc to 79166f3 Compare July 10, 2026 15:33
@rochdev
rochdev force-pushed the rochdev/system-tests-execution-time branch from 79166f3 to 7a3eabe Compare July 27, 2026 23:32
rochdev added a commit that referenced this pull request Jul 28, 2026
Each matrix cell in a sibling workflow (Node.js version, plugin partition)
writes its own complete lcov report, so a shared source file gets an `SF:`
block from every cell. Concatenating those blocks, as group-coverage.mjs did,
produced a report with duplicate `SF:` sections per file; Codecov keeps only
the last block for a file rather than summing across duplicates, which
silently discarded almost all branch/function coverage once per-cell uploads
were merged into one per-workflow upload (PR #9197's branch dropped from
10802 to 65 branches as a result). mergeLcov now sums DA:/FNDA:/BRDA: hit
counts per file across cells instead, the way `lcov --add-tracefile` does.

Generated by Claude Code.
@rochdev
rochdev marked this pull request as draft July 29, 2026 19:11
rochdev added a commit that referenced this pull request Aug 10, 2026
Each matrix cell in a sibling workflow (Node.js version, plugin partition)
writes its own complete lcov report, so a shared source file gets an `SF:`
block from every cell. Concatenating those blocks, as group-coverage.mjs did,
produced a report with duplicate `SF:` sections per file; Codecov keeps only
the last block for a file rather than summing across duplicates, which
silently discarded almost all branch/function coverage once per-cell uploads
were merged into one per-workflow upload (PR #9197's branch dropped from
10802 to 65 branches as a result). mergeLcov now sums DA:/FNDA:/BRDA: hit
counts per file across cells instead, the way `lcov --add-tracefile` does.

Generated by Claude Code.
@rochdev
rochdev force-pushed the rochdev/system-tests-execution-time branch from ca32fde to 3152da2 Compare August 10, 2026 03:12
@rochdev
rochdev marked this pull request as ready for review August 10, 2026 21:12

@datadog-official datadog-official Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datadog Autotest: FAIL

The per-call limiter does not bound the aggregate burst when several workflows settle together, recreating the API pressure this change intends to remove. The custom lcov merger also silently collapses valid same-named functions from different source lines.

📊 Validated against 8 scenarios · Open Bits AI session

🤖 Datadog Autotest · Commit 926d33c · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

Comment thread scripts/download-artifacts.mjs Outdated
Comment thread scripts/group-coverage.mjs Outdated
rochdev added a commit that referenced this pull request Aug 12, 2026
…med lcov function merge

Addresses two Codex/Datadog Autotest review findings on PR #9197:

- The per-`downloadArtifacts`-call worker pool let each concurrently processed
  sibling workflow open its own 10-download burst, recreating the aggregate
  GitHub API pressure the cap was meant to prevent. A module-scoped Semaphore
  now bounds every in-flight call together.
- `mergeLcovRecord` keyed functions by name only, so two functions sharing a
  name at different lines (e.g. two closures both named `shared`) collapsed
  into one, undercounting FNF/FNH. FN/FNDA are now paired positionally and
  keyed by `line,name`.

Generated by Claude Code.
The ubuntu job ran all four Node versions sequentially in a single
job. Splitting into a matrix lets them run concurrently, cutting
wall-clock time.
group-coverage.mjs bucketed cell reports per integration (~101 groups),
each needing its own datadog-ci/codecov CLI invocation. .codecov.yml
only gates the separate master-coverage flag, so per-integration flags
carried no gating weight of their own - they just fed a coverage-by-plugin
UI breakdown. Grouping by top-level area instead (appsec, apm-integrations,
etc.) cuts that to ~12 groups, and bumping xargs -P from 10 to 20 lets all
of them upload in a single concurrent batch instead of two sequential ones.
rochdev added 21 commits August 12, 2026 12:58
…nd fail loudly

do-upload requires create-commit/create-report to already exist; without them it
silently failed (exit 0, no report ever created) since --fail-on-error wasn't set.
Add --fail-on-error to every codecovcli call and retry each with backoff so a
transient failure doesn't immediately fail the whole All Green job.

Generated by Claude Code
Downloading artifacts and merging coverage each logged their own line, so a
single workflow run's report processing produced 4-5 scattered entries.
Silence those intermediate steps and report their outcome (artifact count)
in the same one-line summary the uploads already produce.

Generated by Claude Code
sendCodecovNotifications fired unconditionally once all sibling
workflows reached a final state, regardless of outcome. A failing
suite's coverage run is expected to be lower than a healthy one, so
notifying Codecov on a red commit reports a misleadingly low status
against what should otherwise be a normal, unaffected coverage trend.

Generated by Claude Code
Handle the 'error' event on spawned upload processes so an ENOENT
doesn't crash All Green via an uncaught exception, and skip the
Codecov notification when no run ever registered a commit/report
(e.g. Dependabot PRs, whose coverage artifacts are skipped).

Generated by Claude Code
Each matrix cell in a sibling workflow (Node.js version, plugin partition)
writes its own complete lcov report, so a shared source file gets an `SF:`
block from every cell. Concatenating those blocks, as group-coverage.mjs did,
produced a report with duplicate `SF:` sections per file; Codecov keeps only
the last block for a file rather than summing across duplicates, which
silently discarded almost all branch/function coverage once per-cell uploads
were merged into one per-workflow upload (PR #9197's branch dropped from
10802 to 65 branches as a result). mergeLcov now sums DA:/FNDA:/BRDA: hit
counts per file across cells instead, the way `lcov --add-tracefile` does.

Generated by Claude Code.
…n flag

Every sibling workflow's merged coverage upload shared the same `coverage`
flag, so Codecov's per-flag breakdown couldn't distinguish one workflow's
coverage from another's. flagOf derives a Codecov-safe flag from the
workflow's name (e.g. `test-optimization`, `appsec`) so each of the ~15
per-run uploads gets its own flag at no extra upload cost.

Generated by Claude Code.
Dropping the JSON report in ffe3919 (lcov only) removed Codecov's
branch-coverage total from ~10802 to near zero: Codecov's own
cross-session merge overwrites rather than sums a shared file's lcov
BRDA records when more than one sibling workflow's upload reports it,
so a later zero-branch session (e.g. AppSec, whose branch coverage is
broken by a separate pre-existing bug) clobbers an earlier session's
real branch data. Restoring the merged istanbul JSON report — which
Codecov reads branch/function coverage from and reconciles more
reliably across sessions — for Codecov's upload only. Datadog keeps
reading lcov, unchanged.

Generated by Claude Code.
…them

A sibling workflow's upload bundles every matrix cell's junit XML into one
datadog-ci call, which only tags the whole batch with one CI job name/ID -
there was no way to tell which Node version a given test result came from
once uploaded. mocha-junit-reporter now stamps each testsuite with a
node_version property, and the upload lifts it into a real test.node_version
tag via datadog-ci's documented property xpath pattern.

Generated by Claude Code.
Each sibling workflow's junit upload previously let datadog-ci's
--auto-discovery upload every matrix cell's XML file separately, one
HTTP request per file. Merge every cell's report into a single XML
document per run before uploading instead, now that each testcase
carries a node_version property (and derived tag) to stay
distinguishable after merging.

Generated by Claude Code.
Uploading each sibling workflow's junit and Datadog coverage report as
soon as that workflow finished meant firing datadog-ci/codecovcli child
processes for every workflow at once whenever several finished in the
same poll cycle, contending for the runner's CPU regardless of payload
size. Neither junit tags nor Datadog's coverage flag vary per workflow
(unlike Codecov's per-workflow flag, which still needs a call per run),
so batch those two into one upload each, issued once every run is done,
instead of one call per workflow.

Generated by Claude Code.
A live rerun that forced several sibling workflows to finish at once
downloaded 100+ artifacts in one uncapped burst; every fetch for one
run failed with a generic "fetch failed" while the others succeeded,
matching a connection/rate limit being hit under that burst size.
Bound concurrency and retry transient failures, mirroring the
existing upload retry pattern in run-upload.mjs.

Generated by Claude Code
datadog-ci junit upload reads GITHUB_WORKFLOW/GITHUB_RUN_ID/GITHUB_RUN_NUMBER/
GITHUB_RUN_ATTEMPT from the process environment to attribute uploaded tests to
a pipeline. Because all-green batched every sibling workflow's junit reports
into one upload running from inside the all-green job, every test in Test
Optimization showed up attributed to the "all-green" workflow instead of the
one that actually produced it.

Upload junit per sibling workflow run instead, tagged with that run's own
metadata (already available from the GitHub API response all-green already
fetches), alongside the existing per-run Codecov upload. Datadog coverage has
no comparable per-test attribution to lose, so it stays batched into one call
after every run finishes.

Generated by Claude Code
datadog-ci junit upload reads GITHUB_WORKFLOW/GITHUB_RUN_ID/GITHUB_RUN_NUMBER
from its own process env to set each test's Pipeline/Job facets. Uploading
every sibling workflow's junit results from one batched call inside All
Green attributed every test to the "All Green" workflow instead of the one
that produced it.

Revert the per-run-upload workaround from a previous commit (extra
datadog-ci invocations, one per sibling workflow) in favor of stamping each
job's own CI metadata as junit XML properties at mocha-run time (while its
own GITHUB_* env vars are still correct), then lifting them into real
per-test ci.pipeline.*/ci.job.name tags via --xpath-tag at upload time -
mirroring the existing node_version tagging pattern, restoring the single
batched upload.

Also drop upload-junit.mjs's custom XML merging: --auto-discovery already
recursively finds and uploads every matching file in one CLI invocation, so
merging reports into one document first added regex-based XML surgery for
no benefit.

Generated by Claude Code.
…oads

datadog-ci coverage upload recursively discovers every report file under a
given path by default, so uploading each sibling workflow's already
per-run-merged lcov file directly (one datadog-ci invocation over
coverage-upload/) makes the extra merge into a single final lcov file
redundant, mirroring the same simplification already applied to junit
uploads.

Generated by Claude Code.
…r-test pipeline tags

getCISpanTags() derives ci.pipeline.name/id/number and ci.job.name straight
from GITHUB_WORKFLOW/GITHUB_JOB/GITHUB_RUN_ID/GITHUB_RUN_NUMBER, applying the
same value to every file in the batched junit upload -- overwriting the
per-test values the --xpath-tag mappings lift from each testsuite's own
stamped properties. Blanking just those four env vars for the datadog-ci
child process leaves every other GITHUB_*-derived tag (repository URL, PR
association, commit SHA) untouched, since none of those collide with what
the xpath mappings set.

Generated by Claude Code.
…d.mjs

NodeJS.ProcessEnv isn't a recognized global type for this project's JSDoc
lint setup; Record<string, string> matches the existing pattern used
elsewhere in scripts/ for plain env-var-shaped objects.

Generated by Claude Code.
Reruns of an already-fully-uploaded All Green job were re-running every
sibling workflow's Codecov upload from scratch, saturating the runner
with concurrent codecovcli processes. Persist the set of run IDs whose
Codecov upload already succeeded (cached per-commit, updated after each
run's own upload, not gated on the job finishing) so a rerun skips only
the codecovcli call for runs already confirmed uploaded, while still
redownloading/remerging their reports for the unconditional Datadog
batch upload.

Generated by Claude Code.
Skipping a cached run's codecovcli upload still paid for merging its
istanbul JSON report, which only Codecov reads — istanbul-lib-coverage's
merge is far slower than the lcov merge on a run with many cells, so it
dominated rerun time even with the upload itself skipped. Pass skipJson
through to mergeRunCoverage so a cached run only redoes the cheap lcov
merge that the Datadog batch upload still needs.

Generated by Claude Code.
istanbul JSON merging was the actual bottleneck on reruns (far slower
than lcov's merge on runs with many cells), for coverage detail
(branch/function) only Codecov read. Dropping JSON support outright
removes that cost instead of caching around it, at the cost of losing
that detail in Codecov.

With the merge now cheap, the per-run Codecov-upload cache (a
persisted run-id list plus an actions/cache step) no longer earns its
complexity — every rerun just redoes download, merge, and upload for
every run again.

Generated by Claude Code.
…med lcov function merge

Addresses two Codex/Datadog Autotest review findings on PR #9197:

- The per-`downloadArtifacts`-call worker pool let each concurrently processed
  sibling workflow open its own 10-download burst, recreating the aggregate
  GitHub API pressure the cap was meant to prevent. A module-scoped Semaphore
  now bounds every in-flight call together.
- `mergeLcovRecord` keyed functions by name only, so two functions sharing a
  name at different lines (e.g. two closures both named `shared`) collapsed
  into one, undercounting FNF/FNH. FN/FNDA are now paired positionally and
  keyed by `line,name`.

Generated by Claude Code.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bbd0a1681a

ℹ️ 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".

Comment thread scripts/download-artifacts.mjs Outdated
Comment thread scripts/download-artifacts.spec.mjs Outdated
@rochdev
rochdev force-pushed the rochdev/system-tests-execution-time branch from bbd0a16 to 2837855 Compare August 12, 2026 17:00
Addresses Codex review feedback on bbd0a16: exporting Semaphore from
download-artifacts.mjs existed only so its spec could reach in and
instantiate it directly, expanding that module's API for test-only
purposes. Moving it to scripts/semaphore.mjs makes it a real, independently
testable utility instead. Also replaces the spec's real 5ms sleeps with
deterministic microtask-flush assertions on promise resolution state, so
the concurrency-cap tests no longer depend on wall-clock timing.

Generated by Claude Code.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0d7a8829e

ℹ️ 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".

Comment thread scripts/upload-coverage.mjs
Comment thread scripts/all-green.mjs Outdated
Comment thread scripts/upload-junit.mjs
…ow status

`checkAllGreen` only checked GitHub workflow-run conclusions before notifying
Codecov, so a Codecov/coverage upload failure (which never throws, only sets
`process.exitCode`) went unnoticed and the run still got treated as fully
uploaded. Track upload failures in `run-upload.mjs` and gate on them too.

Generated by Claude Code.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba598e9d03

ℹ️ 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".

Comment thread scripts/all-green.mjs
Comment thread scripts/run-upload.spec.mjs
Comment thread scripts/group-coverage.mjs Outdated
…s OSes

An artifact download that exhausts its retries only affected the log label,
not the Codecov notification gate, so a transient GitHub outage on an
otherwise-green run could still notify Codecov over a partial report. Track
download failures the same way upload failures already are.

Also normalize `SF:` paths before merging lcov records: istanbul's lcov
writer uses `path.relative()`, which returns backslashes on a Windows matrix
cell (e.g. AppSec's windows-2022 job) but forward slashes on Linux/macOS
cells in the same run, so the same file's coverage was splitting into two
separate records instead of summing.

Generated by Claude Code.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7e7789799f

ℹ️ 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".

Comment thread scripts/all-green.mjs
A thrown processRun error (e.g. the artifact listing or lcov merge itself
failing) only set process.exitCode, so that run's report never reaching
Codecov/Datadog went unnoticed by the notification gate. Generalize the
existing incomplete-download flag to cover this path as well.

Generated by Claude Code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants