feat(profiling): observe async zlib and crypto events in timeline profiler - #8042
Merged
Conversation
Contributor
Overall package sizeSelf size: 5.67 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.1 | 82.56 kB | 817.39 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8042 +/- ##
==========================================
+ Coverage 77.20% 77.21% +0.01%
==========================================
Files 809 813 +4
Lines 37562 37636 +74
==========================================
+ Hits 28998 29062 +64
- Misses 8564 8574 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 92c54c4 | Docs | Datadog PR Page | Give us feedback! |
Introduce createCallbackInstrumentor(prefix, options) in packages/datadog-instrumentations/src/helpers/callback-instrumentor.js. It owns the shared protocol for callback-style APIs whose work is offloaded to the libuv worker thread pool: bail when no subscribers or no callback, runStores on :start, wrap the callback to publish :error then :finish, and publish :error on synchronous throw. The per-call context object is produced by a caller-supplied buildContext function; the optional captureResult option writes the callback's first non-error argument back onto the context before :finish for plugins that tag spans from the return value. Migrate dns.js to use the helper. Behavior is preserved bit-for-bit: the existing args capture (including rrtype append for resolve shorthands) is built via buildContext, and captureResult restores the ctx.result assignment consumed by the DNS lookup plugin.
Publishes apm:zlib:operation:{start,finish,error} diagnostic channels
for the callback-based async zlib APIs (deflate, deflateRaw, gzip,
gunzip, inflate, inflateRaw, unzip, brotliCompress, brotliDecompress).
These cover the compression operations that are offloaded to the libuv
worker thread pool and will be consumed by the events profiler.
Uses the createCallbackInstrumentor helper, so the instrumentation body
is just the per-method operation name.
Publishes apm:crypto:operation:{start,finish,error} diagnostic channels
for the callback-based async crypto APIs offloaded to the libuv worker
thread pool: pbkdf2, scrypt, randomBytes, randomFill, randomInt,
generateKey, generateKeyPair, hkdf, sign, verify, generatePrime,
checkPrime.
The existing AppSec-oriented datadog:crypto:hashing:start and
datadog:crypto:cipher:start channels are preserved unchanged. Uses the
createCallbackInstrumentor helper; the instrumentation body just
declares per-method argument names to capture on the context.
Subscribes to apm:zlib:operation:{start,finish,error} via a new
ZlibPlugin and emits timeline samples with entry type "zlib" and an
"operation" label carrying the specific compression method name
(gzip, deflate, brotliCompress, etc.). Only the
DatadogInstrumentationEventSource path observes these events; there is
no perf_hooks fallback since Node does not expose a zlib entry type.
Subscribes to apm:crypto:operation:{start,finish,error} via a new
CryptoPlugin and emits timeline samples with entry type "crypto". The
plugin forwards a whitelist of instrumentation-context fields
(operation, algorithm, digest, type, size, keylen, iterations, offset)
as pprof labels; the whitelist mirrors the param names declared in the
crypto instrumentation.
Unit tests publish on the new apm:zlib:operation and apm:crypto:operation diagnostic channels and assert that the events profiler produces samples with the expected event type and labels. Integration tests exercise the full stack: zlibtest.js drives gzip, gunzip, deflate, and brotliCompress; cryptotest.js drives pbkdf2, randomBytes, and randomFill. Both are picked up via gatherTimelineEvents using new ZlibEventProcessor and CryptoEventProcessor helpers that know the crypto-specific label set.
szegedi
force-pushed
the
profiler/events-zlib-crypto
branch
from
April 22, 2026 09:52
f1f0584 to
6302a23
Compare
szegedi
marked this pull request as ready for review
April 22, 2026 14:19
BridgeAR
reviewed
Apr 23, 2026
Comment on lines
+21
to
+32
| checkPrime: [], | ||
| generateKey: ['type'], | ||
| generateKeyPair: ['type'], | ||
| generatePrime: ['size'], | ||
| hkdf: ['digest', null, null, null, 'keylen'], | ||
| pbkdf2: [null, null, 'iterations', 'keylen', 'digest'], | ||
| randomBytes: ['size'], | ||
| randomFill: [null, 'offset', 'size'], | ||
| randomInt: [], | ||
| scrypt: [null, null, 'keylen'], | ||
| sign: ['algorithm'], | ||
| verify: ['algorithm'], |
Member
There was a problem hiding this comment.
What about using maps instead so that the key is the arguments index and we do not need to iterate over null entries?
Contributor
Author
There was a problem hiding this comment.
I'd then still rather use e.g. ['digest',,,,'keylen'] and then an iteration would only include existing indices.
BridgeAR
reviewed
Apr 23, 2026
| return wrapMethod | ||
| } | ||
|
|
||
| function buildAsyncContext (operation, paramNames) { |
Member
There was a problem hiding this comment.
This will have an overhead compared to writing the code manually. I think the performance wins over the simplification, no?
Contributor
Author
There was a problem hiding this comment.
I hardly think an additional level of a function call is a significant enough overhead worth worrying about. We're notoriously bad at intuiting the performance characteristics of code, that's exactly what we use profilers for. If this ever shows up as a significant performance detractor for a customer, I'd consider changing it. Otherwise it's a speculative optimization and I'd rather err towards the side of maintainability in the code.
szegedi
force-pushed
the
profiler/events-zlib-crypto
branch
2 times, most recently
from
April 24, 2026 10:51
e6446d5 to
6302a23
Compare
…ames Replace the null placeholders in asyncParamsByMethod with elided positions (e.g. hkdf: ['digest', , , , 'keylen']) so unused argument slots can be skipped at iteration time rather than tested inside the loop body. buildAsyncContext now walks the populated slots with for-in, pulls the name inline, and breaks once we pass the callback position since indices are yielded in ascending numeric order. for-in is used intentionally here over Object.keys: paramNames is a plain array literal with no prototype additions, so for-in yields the same set of keys without the keys-array allocation. Flagged with the corresponding eslint-disable-line comments on the sparse-array lines since the repo defaults reject holes.
IlyasShabi
reviewed
Apr 26, 2026
IlyasShabi
reviewed
Apr 26, 2026
IlyasShabi
reviewed
Apr 26, 2026
Add zstdCompress and zstdDecompress to the asyncMethods list. Available in Node 22.15+ / 23.8+ / 24+; the existing typeof guard skips them on older runtimes.
Cover the new apm:zlib:operation and apm:crypto:operation diagnostic channels at the instrumentation layer, following the pattern in url.spec.js and child_process.spec.js: subscribe sinon stubs to the channels, drive the wrapped methods, assert on the published context. zlib.spec.js exercises every async compress/decompress pair, the unzip auto-detection path, the on-failure error channel, the no-callback fast path, and zstd when available on the running Node version. crypto.spec.js verifies per-method argument extraction (pbkdf2 iterations/keylen/digest, randomBytes size, randomFill offset+size, generateKeyPair type, hkdf digest+keylen), confirms that non-string non-number arguments at captured positions are filtered out, exercises both the synchronous-throw and asynchronous-callback error paths, and asserts that the existing AppSec datadog:crypto:hashing/cipher channels still fire alongside the new async wrappers.
BenchmarksBenchmark execution time: 2026-04-29 07:27:32 Comparing candidate commit 92c54c4 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 1345 metrics, 98 unstable metrics. scenario:runtime-metrics-with-runtime-metrics-22
|
Merged
BridgeAR
pushed a commit
that referenced
this pull request
Apr 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Extends the timeline events profiler to observe asynchronous compression (
zlib) and encryption (crypto) operations offloaded to the libuv worker thread pool, in addition to the existing DNS, TCP connect, and filesystem events. Along the way, factors the DC-channel protocol shared across these callback-style libuv instrumentations into a single helper and applies it todns.jsas well.Concretely:
createCallbackInstrumentor(prefix, { captureResult })inpackages/datadog-instrumentations/src/helpers/callback-instrumentor.js. It owns the end-to-end protocol: bail when no subscribers or no callback,runStoreson:start, wrap the callback to publish:errorthen:finish, publish:erroron synchronous throw. The optionalcaptureResultflag writes the callback's first non-error argument back onto the context before:finish, for plugins that tag spans from the call's return value.dns.jsmigrated to use the helper. Behavior is preserved bit-for-bit:ctx.argsis built viabuildContext(including therrtypetrailing append for resolve shorthands), andcaptureResultrestores thectx.resultassignment consumed by the DNS lookup plugin. The migration commit doubles as the helper's proof of equivalence on code that's already production-tested.packages/datadog-instrumentations/src/zlib.jspublishingapm:zlib:operation:{start,finish,error}for the callback-based async zlib APIs (deflate{,Raw},inflate{,Raw},gzip,gunzip,unzip,brotliCompress,brotliDecompress).packages/datadog-instrumentations/src/crypto.jsextended withapm:crypto:operation:{start,finish,error}forpbkdf2,scrypt,random{Bytes,Fill,Int},generateKey{,Pair},hkdf,sign,verify,generatePrime,checkPrime. The existing AppSec-orienteddatadog:crypto:hashing:start/datadog:crypto:cipher:startchannels are preserved unchanged.EventPlugin/ decorator pairs (event_plugins/zlib.js,event_plugins/crypto.js) wired intoDatadogInstrumentationEventSourceso the profiler emits timeline samples with event typezlib/cryptoand per-op labels.The commits follow the above structure for easier review.
Motivation
The events profiler lets us correlate libuv thread-pool contention with specific operations and spans. Today that correlation is only possible for DNS, TCP connect, and filesystem work. Compression and encryption are common sources of pool saturation (HTTP response encoding, PBKDF2 / scrypt password hashing, key generation), but we didn't support observing them before. Part of the reason was that the plugins for the already existing three integrations emit the requisite events in their channels, so it was easy to implement them. In contrast, the zlib and crypto plugins had no instrumentation to emit the events. This is now remedied. With this, we now cover all possible built-in Node.js workloads on the libuv thread and have the best possible visibility (without writing a native profiler for them.)
Additional Notes
createGzip,crypto.createCipheriv, etc.) and sync variants are intentionally out of scope for this initial pass — consistent with howfs/netplugins treat streams today. Adding them is a separate follow-up and we can decide per-target.NodeApiEventSourcehas no zlib / cryptoperf_hooksentry type to piggyback on, so when code hotspots are disabled these events are not captured. This matches the asymmetry the other event sources already have for their sub-events and can be filled in later if needed.packages/dd-trace/test/profiling/profilers/events.spec.jsdrive the new channels directly and assert on sample labels. Integration fixturesintegration-tests/profiler/zlibtest.jsandcryptotest.jsplus matching cases inprofiler.spec.jsexercise the full stack end-to-end through a forked tracer process.Potential future work
As mentioned in additional notes, we aren't instrumenting streaming variants.
Also, some 3rd party libraries also use the libuv worker pool; it might be a future expansion of this work to add support for them, especially for those for which we already have plugins. Some known examples of 3rd party libraries using
uv_queue_workare:Jira: PROF-14316