collect and propagate process tags in first chunk span#6733
Conversation
Overall package sizeSelf size: 13.64 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 10.3.0 | 20.73 MB | 20.74 MB | | @datadog/pprof | 5.12.0 | 11.19 MB | 11.57 MB | | @datadog/native-iast-taint-tracking | 4.1.0 | 9.01 MB | 9.02 MB | | @opentelemetry/resources | 1.30.1 | 557.67 kB | 7.71 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.4 | 2.95 MB | 5.83 MB | | @datadog/wasm-js-rewriter | 5.0.1 | 2.82 MB | 3.53 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api-logs | 0.208.0 | 199.48 kB | 1.42 MB | | @opentelemetry/api | 1.9.0 | 1.22 MB | 1.22 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.15.0 | 127.66 kB | 856.24 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | @datadog/openfeature-node-server | 0.2.0 | 118.51 kB | 437.19 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | @isaacs/ttlcache | 2.1.3 | 90.79 kB | 90.79 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 7.0.5 | 63.38 kB | 63.38 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.3 | 23.74 kB | 23.74 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 | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB | | escape-string-regexp | 5.0.0 | 3.66 kB | 3.66 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 #6733 +/- ##
==========================================
+ Coverage 84.80% 84.81% +0.01%
==========================================
Files 514 515 +1
Lines 21987 22015 +28
==========================================
+ Hits 18645 18673 +28
Misses 3342 3342 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected * Fix with Cursor requires Datadog plugin ≥v2.17.0 🔗 Commit SHA: 72ae510 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
f80d984 to
48dcde7
Compare
48dcde7 to
79d07ac
Compare
337d9f1 to
f067df3
Compare
df701f0 to
1035fc3
Compare
1035fc3 to
3e47727
Compare
| function sanitize (value) { | ||
| return String(value) | ||
| .toLowerCase() | ||
| .replaceAll(/[^a-zA-Z0-9/_.-]+/g, '_') |
There was a problem hiding this comment.
It's not written in the RFC, but I think : is also an authorized character
There was a problem hiding this comment.
It looks like today we decided to disallow : characters to not disrupt key/value parsing.
There was a problem hiding this comment.
Just to verify, has the RFC been updated to reflect everything here?
There was a problem hiding this comment.
| this._processTags = config.propagateProcessTags?.enabled | ||
| ? getProcessTags().serialized | ||
| : false |
There was a problem hiding this comment.
How much can you expose global constants ? process tags will not change during the lifetime of the program. What we do in python is we exposed a constant (which is None or the value depending of the config) and the other modules can grab this constant (which also prevents recomputing the process tags several times)
There was a problem hiding this comment.
It's easily achievable but is considered a "sin" in Node.js to create globals. However we can essentially have "namespaced globals" by exporting a singleton instance in a file (module) and requiring that file elsewhere.
That said the way I've set up the code we should only ever calculate the process tags a single time for the lifetime of the process so it does benefit from the never-changing behavior of process tags.
| const tags = [ | ||
| // the immediate parent directory name of the entrypoint script, e.g. /foo/bar/baz/banana.js -> baz | ||
| ['entrypoint.basedir', ENTRYPOINT_PATH === '' ? undefined : path.basename(path.dirname(ENTRYPOINT_PATH))], | ||
|
|
||
| // the entrypoint script filename without the extension, e.g. /foo/bar/baz/banana.js -> banana | ||
| ['entrypoint.name', path.basename(ENTRYPOINT_PATH, path.extname(ENTRYPOINT_PATH)) || undefined], | ||
|
|
||
| // always script for JavaScript applications | ||
| ['entrypoint.type', 'script'], | ||
|
|
||
| // last segment of the current working directory, e.g. /foo/bar/baz/ -> baz | ||
| ['entrypoint.workdir', path.basename(CURRENT_WORKING_DIRECTORY) || undefined], | ||
|
|
||
| // the .name field from the application's package.json | ||
| ['package.json.name', pkg.name || undefined] | ||
| ] |
There was a problem hiding this comment.
Can this code throw ? It seems it's not the case but wanted to double check
There was a problem hiding this comment.
In all my testing I only found a situation where require.main.filename could throw as require.main is missing in certain situations where the tracer is used in CI during test runs but I addressed that. Based on where this code is placed it should be running in nearly all of our CI jobs and those aren't failing.
3e47727 to
3f40669
Compare
|
Apparently the network requests have 3 layers instead of 2. There's the outer network request, which contains an array of "chunks", which contains an array of spans. Chunks contain all the spans related to a single trace. Currently the code is written to assume a chunk is synonymous with a network request. We only need to send this data in the first span of the first chunk of the network request. That means this PR in its current form is sending redundant data. It's not the end of the world and is mergeable but I'll be working on removing the redundancy. |
3f40669 to
30589ac
Compare
30589ac to
e272bb8
Compare
bengl
left a comment
There was a problem hiding this comment.
You'll also need to make the linter happy.
| // Add process tags to the first span of the first trace in a payload | ||
| if (shouldAddProcessTags && processTags && span.meta) { | ||
| const { TRACING_FIELD_NAME } = require('./process-tags') | ||
| span.meta[TRACING_FIELD_NAME] = processTags |
There was a problem hiding this comment.
Seems to me that all this should be done in span_format.js, no?
There was a problem hiding this comment.
I reverted all the changes in this file
| function sanitize (value) { | ||
| return String(value) | ||
| .toLowerCase() | ||
| .replaceAll(/[^a-zA-Z0-9/_.-]+/g, '_') |
There was a problem hiding this comment.
Just to verify, has the RFC been updated to reflect everything here?
| // entrypoint.basedir = baz | ||
| // package.json.name = <from package.json> | ||
|
|
||
| module.exports = function getProcessTags () { |
There was a problem hiding this comment.
Should the result of this function be cached here?
There was a problem hiding this comment.
It's a property of SpanProcessor which is attached to the tracer singleton:
packages/dd-trace/src/opentracing/tracer.js
5:const SpanProcessor = require('../span_processor')
34: this._processor = new SpanProcessor(this._exporter, this._prioritySampler, config)
This reverts commit e272bb8.
|
I forgot that I had pushed a half-baked commit |
- collects basic process tags - tag serializer - send in first chunk span every time we send traces to the agent - create DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=false to configure - TODO: as a follow-up this should only add tags to the first span of the first chunk of a network request
- collects basic process tags - tag serializer - send in first chunk span every time we send traces to the agent - create DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=false to configure - TODO: as a follow-up this should only add tags to the first span of the first chunk of a network request
- collects basic process tags - tag serializer - send in first chunk span every time we send traces to the agent - create DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=false to configure - TODO: as a follow-up this should only add tags to the first span of the first chunk of a network request
What does this PR do?
DD_EXPERIMENTAL_PROPAGATE_PROCESS_TAGS_ENABLED=falseto configureMotivation
Plugin Checklist
Additional Notes