-
Notifications
You must be signed in to change notification settings - Fork 312
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
[DSM] Fix an issue where RabbitMQ producers when producing a message to the default exchange were setting checkpoints that didn't work in DSM #5150
Conversation
Overall package sizeSelf size: 8.54 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.4.0 | 29.44 MB | 29.44 MB | | @datadog/native-appsec | 8.4.0 | 19.25 MB | 19.26 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.5.0 | 9.8 MB | 10.17 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.6.1 | 2.59 MB | 2.73 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.1.0 | 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 | | 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 | | 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 | | ttl-set | 1.0.0 | 4.61 kB | 9.69 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 |
f88f8ac
to
91473d1
Compare
BenchmarksBenchmark execution time: 2025-01-24 15:08:34 Comparing candidate commit 91473d1 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 912 metrics, 21 unstable metrics. |
310dd03
to
b0338c3
Compare
…to the default exchange were setting checkpoints that didn't work in DSM
b0338c3
to
8c6cfe7
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5150 +/- ##
=======================================
Coverage 81.05% 81.05%
=======================================
Files 477 477
Lines 21307 21308 +1
=======================================
+ Hits 17270 17271 +1
Misses 4037 4037 ☔ View full report in Codecov by Sentry. |
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.
LGTM!
…to the default exchange were setting checkpoints that didn't work in DSM (#5150)
…to the default exchange were setting checkpoints that didn't work in DSM (#5150)
What does this PR do?
The RabbitMQ DSM integration is built with the idea that users will always have an exchange. The Producer plugin sends a checkpoint with the exchange (and no topic). However, there is an API in RabbitMQ where instead of publishing to an exchange w/ a routing key, you send either directly to a queue or you publish to either an undefined or empty string exchange and then the routing key is the queue and the exchange is the default exchange (link, see second sentence of the first bullet). In this case we are unable to show the connection between producer and consumer because we are only sending a checkpoint w/ the exchange which is blank and then the consumer has the topic and the exchange (still blank) and thus we aren't able to correlate them. This is happening w/ a client (unnamed)
see:
debug endpoint w/ checkpoints
trace on producer where we can see that the producer is sending only a routingKey and not an exchange
what the map ends up looking like
To test that this works, I made an experiment using this branch of the tracer on staging w/ the data-streams-dev and sent a message from the producer w/ no exchange. you can see that here
why conditionally send the topic? why not always send the topic?
This fix is for only this case, in a case where we have an exchange, we still want to correlate via the exchange and get the difference between the queue and the routing key (so we only send the topic on the producer if there is a routing key and an empty exchange), otherwise we'd have a connection between two services that looked like this:
Motivation
Plugin Checklist
Additional Notes