-
Notifications
You must be signed in to change notification settings - Fork 318
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
Fix MongoDB DBM propagation issue where trace comments are not properly injected into commands #5306
Conversation
…ommand is modified before execution
Overall package sizeSelf size: 8.77 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.3.0 | 13.77 MB | 13.78 MB | | @datadog/pprof | 5.5.1 | 9.79 MB | 10.17 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.8.0 | 2.6 MB | 2.74 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 | 835.4 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 | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 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 | | 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 | | 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 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5306 +/- ##
=======================================
Coverage 80.94% 80.94%
=======================================
Files 488 488
Lines 21844 21848 +4
=======================================
+ Hits 17681 17685 +4
Misses 4163 4163 ☔ View full report in Codecov by Sentry. |
Datadog ReportBranch report: ✅ 0 Failed, 666 Passed, 0 Skipped, 11m 4.99s Total Time |
BenchmarksBenchmark execution time: 2025-02-24 19:56:59 Comparing candidate commit 536e0d6 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 915 metrics, 18 unstable metrics. |
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.
The change itself is LGTM. The tests could AFAIC be a bit more resilient by checking for the change after the start call to verify the input is manipulated accordingly instead of checking for the injectDbmCommand method.
We could probably also just return the promise in the test instead of using promises and callbacks.
@BridgeAR, I updated the tests to inspect |
/merge |
View all feedbacks in Devflow UI.
This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
zhengda.lu@datadoghq.com unqueued this merge request |
/remove |
View all feedbacks in Devflow UI.
|
…ly injected into commands (#5306) * comment has to be assigned back to the input ops to ensure incoming command is modified before execution * input comment to injectDbmCommand * rename injectDbmCommand to injectDbmComment
…ly injected into commands (#5306) * comment has to be assigned back to the input ops to ensure incoming command is modified before execution * input comment to injectDbmCommand * rename injectDbmCommand to injectDbmComment
What does this PR do?
This PR fixes an issue where MongoDB DBM trace comments were not properly injected into the command object. The root cause was that the modified comment was assigned to a new copied object
dbmTracedCommand = { ...command }
, which did not affect the original ops object outside ofstart()
. This fix ensures that the comment is correctly assigned back to the original ops object to maintain proper DBM propagation.Motivation
Fix a bug where DBM trace comments were missing from MongoDB commands due to improper reassignment.
Plugin Checklist
Additional Notes