Add parallelism information to profiles#4765
Conversation
Overall package sizeSelf size: 8.23 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.2.2 | 29.27 MB | 29.27 MB | | @datadog/native-appsec | 8.3.0 | 19.37 MB | 19.38 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.6.0 | 2.58 MB | 2.72 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.0.1 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 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.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | int64-buffer | 0.1.10 | 49.18 kB | 49.18 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 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 |
6e205e4 to
9d66475
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4765 +/- ##
===========================================
+ Coverage 69.19% 93.90% +24.70%
===========================================
Files 1 106 +105
Lines 198 3312 +3114
Branches 33 0 -33
===========================================
+ Hits 137 3110 +2973
- Misses 61 202 +141 ☔ View full report in Codecov by Sentry. |
szegedi
changed the title
be7f2b1 to
6f50f69
Compare
There was a problem hiding this comment.
UV_THREADPOOL_SIZE might not be defined
There was a problem hiding this comment.
yeah, that's fine – the property will be missing here then
There was a problem hiding this comment.
why not send the default threadpool size when it is not overridden by UV_THREADPOOL_SIZE ?
There was a problem hiding this comment.
So, a hardcoded 4? The comment above that line says it: "we'll let the backend figure out the effective numeric value." I think this is just me instinctively preferring having logic in the backend, 'cause we can more easily fix/modify that than libraries (and have users upgrade to new libraries.) E.g. if libuv in a future Node version changes either the default or the max thread pool size.
I might go for converting early to number though, as JavaScript's parseInt is almost compatible with C atoi that libuv uses to parse it so I wouldn't have to implement an atoi equivalent in Java in the backend.
There was a problem hiding this comment.
Note that if container has a cpu limit enforced through cpu CFS shares, the value will be wrong (eg. with a CPU limit of 0.5CPU on a 96 core system, value will be 96).
Datadog agent metrics (eg. kubernetes.cpu.requests/limits) could be useful.
There was a problem hiding this comment.
Oh I see. This would still be useful, 'cause it could give an upper limit for a suggestion for UV_THREADPOOL_SIZE. The main use I have for this is that I could provide a profiler analysis: if there's more concurrent DNS and filesystem operations (and in future, cryptography and compression) than we have workers in the libuv worker thread pool, we can suggest the customer increase the size of the pool as long as there's space for some extra parallelism in the CPU cores. K8s won't limit parallelism as looking from outside the box, you don't see the threads, it just limits the overall utilization of the CPU, right? Theory being that DNS and filesystem operations cause the libuv threads to be blocked on IO, so they're idle.
There was a problem hiding this comment.
But I could also definitely have use for kubernetes.cpu.requests and kubernetes.cpu.limits too. I don't suppose I can grab those values easily from within the Node app?
There was a problem hiding this comment.
I don't believe there is a way to get kubernetes.cpu.requests and kubernetes.cpu.limits from node app.
My point is that this value will be hard to interpret.
If threads are mostly blocked on network / IO operations, it might be beneficial to increase thread pool size even if available parallelism is low.
c641a92 to
54bfb86
Compare
There was a problem hiding this comment.
Wow, great attention to detail here !
There was a problem hiding this comment.
Sorry for the noise, I did not understand that you were delegating to the backend the computation of the default value.
My first take was that the library was in a better position to determine the default value, but since the node version is part of profile metadata, it might be better to let the backend handle this, since updating code in the backend is far more easier.
There was a problem hiding this comment.
I don't believe there is a way to get kubernetes.cpu.requests and kubernetes.cpu.limits from node app.
My point is that this value will be hard to interpret.
If threads are mostly blocked on network / IO operations, it might be beneficial to increase thread pool size even if available parallelism is low.
54bfb86 to
aa1b8a6
Compare
|
Had to rebase it because it conflicted with #4980 |
2 participants
What does this PR do?
Adds information about available parallelism and size of libuv thread pool to profiles.
Motivation
Combined with timeline events activity, this can allow us to determine in the backend if the libuv thread pool is undersized and offer an analysis/insight for it.
Additional Notes
Jira link: PROF-10686