-
Notifications
You must be signed in to change notification settings - Fork 358
fix: failing appsec tests after iitm bump to v2.0.0 #6935
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Overall package sizeSelf size: 13.41 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/pprof | 5.12.0 | 11.19 MB | 11.57 MB | | @datadog/native-iast-taint-tracking | 4.1.0 | 9.01 MB | 9.02 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.83 MB | | @datadog/wasm-js-rewriter | 5.0.1 | 2.82 MB | 3.53 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api-logs | 0.208.0 | 199.48 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 | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | import-in-the-middle | 2.0.0 | 68.46 kB | 797.03 kB | | @datadog/openfeature-node-server | 0.2.0 | 118.51 kB | 437.19 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.1.2 | 90.79 kB | 90.79 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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6935 +/- ##
==========================================
- Coverage 84.81% 84.30% -0.52%
==========================================
Files 513 507 -6
Lines 21521 21455 -66
==========================================
- Hits 18253 18087 -166
- Misses 3268 3368 +100 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
BenchmarksBenchmark execution time: 2025-11-26 14:56:51 Comparing candidate commit bc6fe98 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 290 metrics, 30 unstable metrics. |
ad23362 to
0499959
Compare
BridgeAR
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with the import being more general. The other part is optional.
91e3958 to
cc057b0
Compare
pabloerhard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added dd-trace/initialize.mjs and dd-trace/register.js as flags to consider when checking whether ESM is configured, since these are the options provided in Datadog’s documentation. ESM does not directly expose the module cache, so a direct alternative to require.cache does not currently exist. I added this general comment so we can further discuss any other options or concerns, as the two open comments relate to this change.
@bengl @BridgeAR @uurien
47f2a1a to
42c78f3
Compare
| ? require('import-in-the-middle/lib/get-exports.js') | ||
| : getExportsImporting | ||
|
|
||
| const getExports = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you moving this logic to processModule instead of doing all the logic in getExports method?
BridgeAR
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % @uurien 's comment. It would remove the overhead of checking for that more than once (which will not show up anywhere, but it is still nice).
ed8d73b to
bc6fe98
Compare
What does this PR do?
This PR aims to fix appsec tests failing after bumping iitm to version 2.0.0
Motivation
iitm version 2.0.0 includes changes that include files being rewritten from CommonJS to ESM, which caused the conditions in the isEsmConfigured function to fail. This happened because the files are no longer keys in the require.cache. This PR adds the flag --import dd-trace/initialize.mjs as an early return statement to bypass the issues caused by the iitm change.
Plugin Checklist
Additional Notes