Skip to content

Conversation

ericfirth
Copy link
Contributor

@ericfirth ericfirth commented Jul 29, 2025

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

CleanShot 2025-07-29 at 08 57 07 ### After with rabbitmq CleanShot 2025-07-29 at 08 57 19

Kafka

CleanShot 2025-07-29 at 08 57 32

which i can see when I look at the checkpoints:
test 4 which doesn't work, has the following checkpoints:

{
"key": {
"service": "nodejs-kafka-producer-test4-eric.firth",
"hash": 12593296123675988000,
"parentHash": 0,
"edgeType": "kafka",
"edgeDirection": "out",
"group": "",
"topic": "endian-fix-validation-topic-test4",
"eventType": "",
"exchange": ""
},
"value": {}
},
{
"key": {
"service": "python-kafka-consumer-test4-eric.firth",
"hash": 15170800266638707000,
"parentHash": 11403507064061660000,
"edgeType": "kafka",
"edgeDirection": "in",
"group": "my_group",
"topic": "endian-fix-validation-topic-test4",
"eventType": "",
"exchange": ""
},
"value": {}
}

as you can see the hash of the producer 12593296123675988000 is not the same as the parent hash of the consumer 11403507064061660000
where test5 which does gives the following checkpoints

{
"key": {
"service": "nodejs-kafka-producer-test5-eric.firth",
"hash": 7610290120735904000,
"parentHash": 0,
"edgeType": "kafka",
"edgeDirection": "out",
"group": "",
"topic": "endian-fix-validation-topic-test5",
"eventType": "",
"exchange": ""
},
"value": {}
},
{
"key": {
"service": "python-kafka-consumer-test5-eric.firth",
"hash": 17373314072852003000,
"parentHash": 7610290120735904000,
"edgeType": "kafka",
"edgeDirection": "in",
"group": "my_group",
"topic": "endian-fix-validation-topic-test5",
"eventType": "",
"exchange": ""
},
"value": {}
}

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

@ericfirth ericfirth requested a review from a team as a code owner July 29, 2025 12:45
Copy link

github-actions bot commented Jul 29, 2025

Overall package size

Self size: 11.13 MB
Deduped: 110.71 MB
No deduping: 111.1 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

Copy link

codecov bot commented Jul 29, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.12%. Comparing base (1ec3e85) to head (9302beb).
⚠️ Report is 19 commits behind head on master.

Files with missing lines Patch % Lines
packages/dd-trace/src/datastreams/processor.js 66.66% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pr-commenter
Copy link

pr-commenter bot commented Jul 29, 2025

Benchmarks

Benchmark execution time: 2025-07-31 13:27:04

Comparing candidate commit 9302beb in PR branch eric.firth/fix-dsm-encoding-to-be-compatible-with-other-languages with baseline commit 1ec3e85 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1271 metrics, 52 unstable metrics.

@ericfirth ericfirth force-pushed the eric.firth/fix-dsm-encoding-to-be-compatible-with-other-languages branch from 05e976a to fd1d25a Compare July 29, 2025 13:37
@ericfirth ericfirth requested a review from a team as a code owner July 29, 2025 13:37
simon-id
simon-id previously approved these changes Jul 29, 2025
@simon-id
Copy link
Member

LGTM but could use a review from the team that owns this

wconti27
wconti27 previously approved these changes Jul 29, 2025
Copy link
Contributor

@wconti27 wconti27 left a 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.

@ericfirth ericfirth dismissed stale reviews from wconti27 and simon-id via 5ed85ef July 29, 2025 15:00
@ericfirth ericfirth force-pushed the eric.firth/fix-dsm-encoding-to-be-compatible-with-other-languages branch 6 times, most recently from 484290d to e8f4ba9 Compare July 30, 2025 17:57
@ericfirth
Copy link
Contributor Author

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.

Fixed the tests and system tests

Copy link

@robcarlan-datadog robcarlan-datadog left a 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

Copy link
Member

@rochdev rochdev left a 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

@ericfirth ericfirth force-pushed the eric.firth/fix-dsm-encoding-to-be-compatible-with-other-languages branch from e8f4ba9 to 9302beb Compare July 31, 2025 13:17
@ericfirth
Copy link
Contributor Author

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

This was fixed

@ericfirth ericfirth requested a review from rochdev July 31, 2025 14:12
@ericfirth ericfirth merged commit ed015bc into master Aug 1, 2025
686 checks passed
@ericfirth ericfirth deleted the eric.firth/fix-dsm-encoding-to-be-compatible-with-other-languages branch August 1, 2025 17:20
dd-octo-sts bot pushed a commit that referenced this pull request Aug 2, 2025
@dd-octo-sts dd-octo-sts bot mentioned this pull request Aug 2, 2025
BridgeAR pushed a commit that referenced this pull request Aug 6, 2025
tlhunter pushed a commit that referenced this pull request Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants