Skip to content

ci(nodejs): lower default shard duration to 300 seconds - #7690

Open
rochdev wants to merge 1 commit into
rochdev/nodejs-express-weblogsfrom
rochdev/nodejs-desired-execution-time
Open

ci(nodejs): lower default shard duration to 300 seconds#7690
rochdev wants to merge 1 commit into
rochdev/nodejs-express-weblogsfrom
rochdev/nodejs-desired-execution-time

Conversation

@rochdev

@rochdev rochdev commented Sep 10, 2026

Copy link
Copy Markdown
Member

Motivation

PR #7689 reduces redundant Node.js weblog/scenario combinations. That leaves
enough job capacity to shorten the system-tests internal Node.js shard target
from 600 seconds to 300 seconds and improve feedback time.

This PR is stacked on #7689.

Changes

  • Use a 300-second desired execution time when Node.js is the only selected
    library.
  • Keep the existing 600-second target for every other single-library change.
  • Keep the existing 3600-second target when multiple libraries are selected.
  • Add a focused test for Node.js duration selection while retaining the
    existing Python and C expectations.

Runtime measurements

The comparison uses two successful Node.js-only runs:

They are not strictly apples-to-apples. The 600-second run selected
DEFAULT,TEST_THE_TEST; the 300-second run additionally selected PARAMETRIC
and TRACE_STATS_COMPUTATION_OBFUSCATION_DISABLED. The end-to-end timings
below are still useful, but a same-scenario rerun is needed for a controlled
comparison.

Environment Target E2E shards E2E completion Longest shard E2E runner time
Production 600s 19 17m 41s 17m 22s 2h 58m 04s
Production 300s 35 11m 39s 11m 20s 3h 16m 29s
Development 600s 19 21m 52s 21m 33s 3h 17m 53s
Development 300s 35 12m 42s 12m 15s 3h 33m 54s

Observed end-to-end critical-path improvements:

  • Production end-to-end: 6m 02s faster (34.1%).
  • Development end-to-end: 9m 10s faster (41.9%).

The 300-second production system-tests workflow actually completed in 13m 25s
because its unsharded parametric job took 13m 06s and became the critical
path. Parametric did not run in the baseline and is unaffected by this change,
so including it produced the misleading 24.1% figure.

The observed cost is 38 to 70 end-to-end jobs and 6h 15m 57s to 6h 50m 23s
of aggregate end-to-end runner time, an increase of 34m 26s (9.2%). That cost
comparison also includes the extra end-to-end scenario in the 300-second run.
All 70 end-to-end shards passed. These are single-run measurements, so normal
CI variance still applies.

Job-count comparison

For the end_to_end,open_telemetry groups, per prod/dev invocation:

Both remain below the 256-job ceiling. Once #7689 is merged, system-tests CI
will run approximately 70 Node.js end-to-end jobs across prod and dev at the
new target.

Validation

Full pytest collection requires a running Docker daemon, which was unavailable
locally.

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on your PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from
      R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • Anything but tests/ or manifests/ is modified? I have the approval
    from R&P team
  • A docker base image is modified?
    • The relevant build-XXX-image label is present
  • A scenario is added, removed or renamed?

Generated by Codex.

@github-actions

Copy link
Copy Markdown
Contributor

CODEOWNERS have been resolved as:

tests/test_the_test/test_compute_libraries_and_scenarios.py             @DataDog/system-tests-core
utils/scripts/compute_libraries_and_scenarios.py                        @DataDog/system-tests-core

@rochdev
rochdev force-pushed the rochdev/nodejs-desired-execution-time branch from 1f3baaa to d426203 Compare September 10, 2026 04:19
@rochdev
rochdev marked this pull request as ready for review September 10, 2026 04:26
@rochdev
rochdev requested a review from a team as a code owner September 10, 2026 04:26

@cbeauchesne cbeauchesne left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This script is only used by system-tests' own CI and is not used by other repositories.

System-tests is already a significant consumer of GitHub runners. We have tried increasing the level of parallelism in the past, and observed that it amplified issues with external calls, rate limits, and other infrastructure dependencies.

I understand that this change comes together with a reduction in the overall number of jobs, which definitely changes the trade-off. That said, the current CI runtime is already relatively low, around ~12 minutes, and because there is a fixed cost associated with spawning each job, doubling the parallelism won't necessarily translate into cutting the CI time in half.

Could you measure the actual runtime improvement? That would give us the data needed to properly evaluate whether the gain is worth the trade-off.

@rochdev
rochdev changed the base branch from main to rochdev/nodejs-express-weblogs September 11, 2026 13:15
@rochdev
rochdev added this pull request to stack #7701 September 11, 2026 13:15
@rochdev

rochdev commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

@cbeauchesne We have less runners to begin with for Node for the same amount of time. Worth noting that we are used to a very fast feedback loop in Node, way faster than other languages. This is why for us 10 minutes just for the scenario is too much, we try to always be under 10 minutes total run time. All of this is why I went for 300s and not lower too, so that in system-tests it's more balanced, and in dd-trace we're going for a lower value because we always only run one language so we can be more aggressive.

As for actual numbers, working on it.

@rochdev
rochdev force-pushed the rochdev/nodejs-desired-execution-time branch from d426203 to 532d8ad Compare September 11, 2026 13:16
@rochdev
rochdev requested a review from a team as a code owner September 11, 2026 13:16
@rochdev
rochdev requested review from pierotibou and removed request for a team September 11, 2026 13:16
@rochdev
rochdev force-pushed the rochdev/nodejs-desired-execution-time branch from 532d8ad to 9b7c91b Compare September 11, 2026 13:18
@rochdev rochdev changed the title ci(nodejs): lower default shard duration to 300 seconds [nodejs] ci(nodejs): lower default shard duration to 300 seconds Sep 11, 2026
@rochdev rochdev closed this Sep 11, 2026
@rochdev rochdev reopened this Sep 11, 2026
@rochdev
rochdev removed this pull request from stack #7701 September 11, 2026 15:16
@rochdev
rochdev changed the base branch from rochdev/nodejs-express-weblogs to main September 11, 2026 15:16
@rochdev rochdev closed this Sep 11, 2026
@rochdev rochdev reopened this Sep 11, 2026
@rochdev rochdev changed the title [nodejs] ci(nodejs): lower default shard duration to 300 seconds ci(nodejs): lower default shard duration to 300 seconds Sep 11, 2026
@rochdev
rochdev changed the base branch from main to rochdev/nodejs-express-weblogs September 11, 2026 15:33
@rochdev
rochdev added this pull request to stack #7705 September 11, 2026 15:33
@rochdev

rochdev commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

@cbeauchesne Results are in, for prod+dev combined we go from 22 minutes to 12 minutes total run time. Still a bit high, but way more reasonable and close enough to our 10 minutes target without too many jobs. I think this is the right balance for now.

@rochdev

rochdev commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

Looking at the numbers for other languages (for production), even with 35 shards, we're still way under the next 2 other best languages (.NET at 51 shards and Go at 142 shards). Given that, I think we could push this even further. For example, at 4 minutes we'd be around 10 minutes total run time with 43 shards, and at 3 minutes we'd be around 9 minutes run time with 53 shards. Of course these end up being diminishing return so I wouldn't push lower than 5 minutes right away and instead would look into what else takes up the additional time (why 12 minutes total if the tests are done in 5 minutes?).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants