DNS with less AsyncResources

[bengl]

No description provided.

[pr-commenter]

Benchmarks

Found 15 performance improvements and 14 performance regressions! Performance is the same for 937 cases.

scenario:plugin-graphql-with-depth-off-14

• 🟥 cpu_user_time [+0.408s; +0.879s] or [+4.117%; +8.873%]
• 🟥 execution_time [+0.377s; +0.748s] or [+3.524%; +6.998%]

scenario:plugin-graphql-with-depth-on-max-14

• 🟥 cpu_user_time [+0.315s; +0.956s] or [+3.276%; +9.958%]
• 🟥 execution_time [+0.209s; +1.166s] or [+2.030%; +11.331%]

scenario:plugin-http-client-with-tracer-14

• 🟥 max_rss_usage [+1.533KB; +1.986KB] or [+2.719%; +3.523%]

scenario:spans-finish-later-14

• 🟩 instructions [-129987732; -67427021] or [-6.143%; -3.186%]

scenario:plugin-dns-with-tracer-14

• 🟩 cpu_usage_percentage [-1.777%; -1.388%]
• 🟩 cpu_user_time [-40.777ms; -25.272ms] or [-9.413%; -5.834%]
• 🟩 execution_time [-0.048s; -0.025s] or [-2.946%; -1.542%]
• 🟩 instructions [-60702601; -57467613] or [-6.382%; -6.042%]

scenario:plugin-http-server-querystring-obfuscation-14

• 🟥 cpu_usage_percentage [+0.801%; +0.923%]
• 🟩 execution_time [-20.782ms; -17.595ms] or [-2.365%; -2.002%]

scenario:net-with-tracer-14

• 🟩 max_rss_usage [-1.773KB; -1.667KB] or [-3.831%; -3.602%]

scenario:plugin-graphql-with-depth-off-16

• 🟥 instructions [+417684928; +732744256] or [+1.932%; +3.389%]

scenario:plugin-graphql-with-depth-on-max-16

• 🟥 execution_time [+0.197s; +0.476s] or [+1.812%; +4.387%]
• 🟩 max_rss_usage [-149.549KB; -109.963KB] or [-15.739%; -11.573%]

scenario:plugin-graphql-with-depth-and-collapse-on-16

• 🟥 instructions [+418318672; +494072812] or [+2.013%; +2.377%]

scenario:plugin-dns-with-tracer-16

• 🟩 cpu_usage_percentage [-1.364%; -1.089%]
• 🟩 cpu_user_time [-41.552ms; -25.698ms] or [-10.082%; -6.235%]
• 🟩 instructions [-45330723; -42981979] or [-5.437%; -5.155%]
• 🟥 max_rss_usage [+1.521KB; +1.796KB] or [+2.966%; +3.502%]

scenario:net-with-tracer-16

• 🟥 cpu_user_time [+8.224ms; +12.123ms] or [+3.628%; +5.348%]
• 🟥 execution_time [+8.183ms; +10.161ms] or [+2.295%; +2.850%]

scenario:scope-manager-base-18

• 🟥 cpu_user_time [+1.459ms; +11.285ms] or [+1.857%; +14.367%]

scenario:plugin-dns-with-tracer-18

• 🟩 cpu_usage_percentage [-2.253%; -1.830%]
• 🟩 cpu_user_time [-55.160ms; -38.703ms] or [-12.846%; -9.013%]
• 🟩 execution_time [-0.055s; -0.025s] or [-3.345%; -1.525%]
• 🟩 instructions [-68548155; -58171438] or [-7.568%; -6.423%]

scenario:plugin-graphql-with-depth-and-collapse-on-18

• 🟥 instructions [+434378264; +610041265] or [+1.911%; +2.684%]

[codecov]

Codecov Report

Merging #2494 (f835187) into master (6a38f5a) will decrease coverage by 0.13%.
Report is 534 commits behind head on master.
The diff coverage is 54.05%.

@@            Coverage Diff             @@
##           master    #2494      +/-   ##
==========================================
- Coverage   93.04%   92.91%   -0.13%     
==========================================
  Files         286      286              
  Lines        9695     9715      +20     
==========================================
+ Hits         9021     9027       +6     
- Misses        674      688      +14     

Files Changed	Coverage Δ
packages/datadog-plugin-dns/src/lookup.js	100.00% <ø> (ø)
packages/datadog-plugin-dns/src/lookup_service.js	100.00% <ø> (ø)
packages/datadog-plugin-dns/src/resolve.js	100.00% <ø> (ø)
packages/datadog-plugin-dns/src/reverse.js	100.00% <ø> (ø)
packages/dd-trace/src/plugins/outgoing.js	18.18% <0.00%> (-4.05%)	⬇️
packages/dd-trace/src/plugins/plugin.js	77.14% <25.00%> (-6.73%)	⬇️
packages/dd-trace/src/plugins/tracing.js	51.28% <27.27%> (-10.79%)	⬇️
packages/datadog-instrumentations/src/dns.js	89.58% <80.00%> (-1.33%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[rochdev]

Can a WeakMap be used instead? If that's possible then it would be possible to just always store the parent store and the additional property wouldn't be needed.

[rochdev]

This should only happen for OutgoingPlugin and InternalPlugin. Even though it doesn't really affect this PR since it only updates an internal plugin, it should still be moved to the right place to avoid issues with other plugins as they are updated to use the new event.

[bengl]

@rochdev There's nothing currently called InternalPlugin, AFAIK.

[tlhunter]

Note that I just force-pushed a rebase on master to run the benchmarks again. @bengl feel free to force push if you get errors when pushing later changes.

[rochdev]

As discussed offline, let's make sure this is wrapped between 2 events so that the subscriber can restore the right context only in the callback, similar to promises and to what is described in nodejs/node#44943 (comment).