-
Notifications
You must be signed in to change notification settings - Fork 340
remove async resource usage from dns/fs/net integrations #5673
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
Conversation
Overall package sizeSelf size: 9.31 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.5.1 | 29.73 MB | 29.73 MB | | @datadog/native-appsec | 8.5.2 | 19.33 MB | 19.34 MB | | @datadog/native-iast-taint-tracking | 3.3.1 | 13.99 MB | 13.99 MB | | @datadog/pprof | 5.7.1 | 9.51 MB | 9.88 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.4.0 | 2.77 MB | 5.42 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 | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.13.1 | 117.64 kB | 839.26 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | dc-polyfill | 0.1.8 | 25.08 kB | 25.08 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.2 | 23.54 kB | 23.54 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 | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5673 +/- ##
==========================================
- Coverage 79.13% 79.09% -0.04%
==========================================
Files 513 513
Lines 23500 23530 +30
==========================================
+ Hits 18596 18611 +15
- Misses 4904 4919 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
BenchmarksBenchmark execution time: 2025-05-08 02:26:19 Comparing candidate commit 90801fa in PR branch Found 16 performance improvements and 0 performance regressions! Performance is the same for 1255 metrics, 52 unstable metrics. scenario:appsec-iast-startup-time-iast-enabled-22
scenario:plugin-dns-with-tracer-18
scenario:plugin-dns-with-tracer-20
scenario:plugin-dns-with-tracer-22
scenario:runtime-metrics-with-runtime-metrics-20
scenario:runtime-metrics-with-runtime-metrics-22
|
Datadog ReportBranch report: ✅ 0 Failed, 935 Passed, 0 Skipped, 15m 34.33s Total Time |
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.
This looks really great!
Just two profiler tests are still failing and it still needs some additional tests to cover the current changed code :)
I'm going to pull this into EDIT: The net and dns tests pass on that branch after pulling in these commits. Once This PR lands, I'll remove them from the |
This fixes an issue in Node 24. It will become tested once we enable the tests for Node 24. Meanwhile, @bengl has validated above that it fixes the issue. |
* remove async resource usage from net integration * remove async resource usage from dns integration * code cleanup * fix fs and profiling
* remove async resource usage from net integration * remove async resource usage from dns integration * code cleanup * fix fs and profiling
What does this PR do?
Remove
AsyncResource
usage fromdns
,fs
andnet
integrations.Motivation
Using
AsyncResource
for this is incorrect as it will change the underlying resource for all stores and not just ours. There is also a bug in Node 24 when usingAsyncResource
that isn't triggered when usingrunStores
instead.