Skip to content

Conversation

@rochdev
Copy link
Member

@rochdev rochdev commented Oct 29, 2025

What does this PR do?

Add maximum Node version in guardrails.

Motivation

We shouldn't support major versions of Node that don't exist yet in SSI since we automatically inject the library everywhere and if there is any major problem it could crash.

Additional notes

Since Node 25 is already out at this point, I put the upper range at 26.

@github-actions
Copy link

github-actions bot commented Oct 29, 2025

Overall package size

Self size: 13.19 MB
Deduped: 116.11 MB
No deduping: 131.13 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 10.3.0 | 20.73 MB | 20.74 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.12.0 | 11.19 MB | 11.57 MB | | @opentelemetry/resources | 1.30.1 | 557.67 kB | 7.71 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.4 | 2.95 MB | 5.82 MB | | @datadog/wasm-js-rewriter | 4.0.1 | 2.85 MB | 3.58 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api-logs | 0.207.0 | 201.39 kB | 1.42 MB | | @opentelemetry/api | 1.9.0 | 1.22 MB | 1.22 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.15.0 | 127.66 kB | 856.24 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | @datadog/openfeature-node-server | 0.1.0-preview.15 | 106.53 kB | 424.55 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | @isaacs/ttlcache | 2.0.1 | 78.45 kB | 78.45 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 7.0.5 | 63.38 kB | 63.38 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.3 | 23.74 kB | 23.74 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | mutexify | 1.4.0 | 5.71 kB | 8.74 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB | | escape-string-regexp | 5.0.0 | 3.66 kB | 3.66 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@codecov
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.86%. Comparing base (537a4a7) to head (fee12d3).
⚠️ Report is 45 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6788      +/-   ##
==========================================
- Coverage   84.03%   83.86%   -0.18%     
==========================================
  Files         505      506       +1     
  Lines       21223    21346     +123     
==========================================
+ Hits        17834    17901      +67     
- Misses       3389     3445      +56     

☔ View full report in Codecov by Sentry.
📢 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.

@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Oct 29, 2025

⚠️ Tests

⚠️ Warnings

❄️ 10 New flaky tests detected

initialize.mjs as --loader runtime version check when node version is in range of the engines field should initialize the tracer, if no DD_INJECTION_ENABLED from when node version is in range of the engines field (Datadog)
Expected the process to output "manual", but logs only contain: "true
true
"

AssertionError [ERR_ASSERTION]: Expected the process to output "manual", but logs only contain: "true
true
"
    at runAndCheckOutput (integration-tests/helpers/index.js:59:12)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async runAndCheckWithTelemetry (integration-tests/helpers/index.js:79:15)
...

    init.js runtime version check when node version is in range of the engines field should initialize the tracer, if no DD_INJECTION_ENABLED from when node version is in range of the engines field

initialize.mjs as --loader runtime version check when node version is in range of the engines field with DD_INJECTION_ENABLED with debug should initialize the tracer from with debug (Datadog)
Expected the process to output "ssi", but logs only contain: "Application instrumentation bootstrapping complete
true
true
"

AssertionError [ERR_ASSERTION]: Expected the process to output "ssi", but logs only contain: "Application instrumentation bootstrapping complete
true
true
"
    at runAndCheckOutput (integration-tests/helpers/index.js:59:12)
...
View all

ℹ️ Info

🧪 All tests passed

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

@pr-commenter
Copy link

pr-commenter bot commented Oct 30, 2025

Benchmarks

Benchmark execution time: 2025-11-05 22:56:06

Comparing candidate commit fee12d3 in PR branch guardrails-max-node-version with baseline commit 537a4a7 in branch master.

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

@rochdev rochdev marked this pull request as ready for review October 30, 2025 19:35
@rochdev rochdev requested a review from a team as a code owner October 30, 2025 19:35
Copy link
Collaborator

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

I think this requires a stricter regexp and ideally an additional test where we change the upper bound of our package.json to the currently used one to check that it would fail, if changed accordingly.
Another test could verify that our engines field is consistently defined as >=x <y, where x and y may be any of a, a.b or a.b.c

var forced = isTrue(process.env.DD_INJECT_FORCE)
var engines = require('../../../../package.json').engines
var minMajor = parseInt(engines.node.replace(/[^0-9]/g, ''))
var majors = engines.node.match(/\d+/)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This regexp is not very safe. It is likely that the engines field would be changed at some point and we might e.g., provide support from a minor range on.

Copy link
Member Author

Choose a reason for hiding this comment

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

It works with the current release policy and if that changes we can change this code as well. It's also the same expression used in the preinstall script, the old one was inconsistent but this one is now the same. Please propose a specific regular expression if you don't like this one.

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.

3 participants