-
Notifications
You must be signed in to change notification settings - Fork 345
Encode Javascript hashes with little-endian to be compatible with other languages in DSM #6196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Encode Javascript hashes with little-endian to be compatible with other languages in DSM #6196
Conversation
Overall package sizeSelf size: 11.13 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 10.0.1 | 20.3 MB | 20.3 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.9.0 | 9.77 MB | 10.14 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.3 | 2.95 MB | 5.6 MB | | @datadog/wasm-js-rewriter | 4.0.1 | 2.85 MB | 3.58 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.14.2 | 122.36 kB | 850.93 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 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 | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 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 | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 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 #6196 +/- ##
==========================================
+ Coverage 83.10% 83.12% +0.02%
==========================================
Files 477 476 -1
Lines 19693 19719 +26
==========================================
+ Hits 16366 16392 +26
Misses 3327 3327 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
BenchmarksBenchmark execution time: 2025-07-31 13:27:04 Comparing candidate commit 9302beb in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1271 metrics, 52 unstable metrics. |
05e976a
to
fd1d25a
Compare
LGTM but could use a review from the team that owns this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code change looks fine to me, but still quite a few tests to fix. System tests will also need to be fixed, and this PR won't be mergeable until that's complete.
484290d
to
e8f4ba9
Compare
Fixed the tests and system tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most changes make sense, wasn't sure about changes to suite.js
packages/dd-trace/test/datastreams/data_streams_checkpointer.spec.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR currently reverts an unrelated commit, requesting change to avoid an accidental merge.
Note: if a PR is not ready it should be in draft
…er languages in DSM
e8f4ba9
to
9302beb
Compare
This was fixed |
…er languages in DSM (#6196)
…er languages in DSM (#6196)
What does this PR do?
This changes the encoding of hashes from big-endian to little-endian when we are sending the stats to the DSM backend.
Its a relatively small change, but this broke how DSM worked in systems with multiple languages, as every other language uses little endian. This meant that context propagation did not work across languages, the javascript producer would produce a hash in hex format of 8 bytes, broadcast a checkpoint having converted the hash to big endian and the consumer would pull that hash from the message headers, convert it to little endian and the numbers would not match.
It was broken without a visual component for all queue technologies but in rabbitmq, it was also visually broken because of how rabbitmq associations work.
Tests
Before with rabbitmq
Kafka
which i can see when I look at the checkpoints:
test 4 which doesn't work, has the following checkpoints:
as you can see the hash of the producer
12593296123675988000
is not the same as the parent hash of the consumer11403507064061660000
where test5 which does gives the following checkpoints
and in that. the hash of the producer and parent hash of the cnosumer do match (
7610290120735904000
)Motivation
We received a bug report https://datadoghq.atlassian.net/browse/DSMS-98
Plugin Checklist
Additional Notes