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

Attention: Patch coverage is 54.05405% with 17 lines in your changes missing coverage. Please review.

Project coverage is 92.91%. Comparing base (6a38f5a) to head (f835187).
Report is 1234 commits behind head on master.

Files with missing lines	Patch %	Lines
packages/dd-trace/src/plugins/tracing.js	27.27%	8 Missing ⚠️
packages/datadog-instrumentations/src/dns.js	80.00%	4 Missing ⚠️
packages/dd-trace/src/plugins/plugin.js	25.00%	3 Missing ⚠️
packages/dd-trace/src/plugins/outgoing.js	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[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).

[bengl]

This PR needs to be completely redone. Closing.