Skip to content
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 amqp instrumentation #4839

Merged
merged 5 commits into from
Nov 1, 2024
Merged

Conversation

piochelepiotr
Copy link
Contributor

What does this PR do?

The amqp instrumentation was not getting the queue name correctly.
In some cases (when using the Get function) it was also not propagating context correctly.

This PR addresses these two issues.

Motivation

Plugin Checklist

Additional Notes

@piochelepiotr piochelepiotr force-pushed the piotr-wolski/fix-amqp-instrumentation branch from 6d9c82c to 2710e7d Compare October 29, 2024 20:41
Copy link

github-actions bot commented Oct 29, 2024

Overall package size

Self size: 7.87 MB
Deduped: 64.89 MB
No deduping: 65.23 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/native-appsec | 8.2.1 | 19.18 MB | 19.19 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.5.0 | 2.51 MB | 2.65 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.0.1 | 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 | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 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 | | int64-buffer | 0.1.10 | 49.18 kB | 49.18 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 | | 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 | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | path-to-regexp | 0.1.10 | 6.38 kB | 6.38 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

@pr-commenter
Copy link

pr-commenter bot commented Oct 29, 2024

Benchmarks

Benchmark execution time: 2024-11-01 19:52:35

Comparing candidate commit e802536 in PR branch piotr-wolski/fix-amqp-instrumentation with baseline commit 57f8a10 in branch master.

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

@piochelepiotr piochelepiotr marked this pull request as ready for review October 30, 2024 16:04
@piochelepiotr piochelepiotr requested review from a team as code owners October 30, 2024 16:04
@@ -54,6 +54,7 @@ jobs:
uses: actions/checkout@v4
with:
repository: 'DataDog/system-tests'
ref: 'piotr-wolski/update-node-js-test'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to be removed before merge

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it goes with this PR: DataDog/system-tests#3346

Not sure in what order I should merge them (probably tests first)

@tlhunter
Copy link
Member

Can you add a test to ensure that the broken scenario is now fixed?

@piochelepiotr piochelepiotr force-pushed the piotr-wolski/fix-amqp-instrumentation branch from 1ce6712 to ecc028e Compare November 1, 2024 15:54
tlhunter
tlhunter previously approved these changes Nov 1, 2024
@piochelepiotr
Copy link
Contributor Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Nov 1, 2024

🚂 MergeQueue: waiting for PR to be ready

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.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

@piochelepiotr
Copy link
Contributor Author

/remove

@dd-devflow
Copy link

dd-devflow bot commented Nov 1, 2024

🚂 Devflow: /remove

@dd-devflow
Copy link

dd-devflow bot commented Nov 1, 2024

⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

If you need support, contact us on Slack #devflow!

@piochelepiotr
Copy link
Contributor Author

I merged DataDog/system-tests#3346 & updated the system test branch to be main branch again.

@piochelepiotr
Copy link
Contributor Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Nov 1, 2024

🚂 MergeQueue: waiting for PR to be ready

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.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

@piochelepiotr piochelepiotr merged commit c03d608 into master Nov 1, 2024
209 checks passed
@piochelepiotr piochelepiotr deleted the piotr-wolski/fix-amqp-instrumentation branch November 1, 2024 21:01
@piochelepiotr piochelepiotr restored the piotr-wolski/fix-amqp-instrumentation branch November 1, 2024 21:01
@piochelepiotr piochelepiotr deleted the piotr-wolski/fix-amqp-instrumentation branch November 1, 2024 21:01
@dd-devflow
Copy link

dd-devflow bot commented Nov 1, 2024

🚂 MergeQueue: This merge request was already merged

This pull request was merged directly.

rochdev pushed a commit that referenced this pull request Nov 6, 2024
* Fix amqp instrumentation
rochdev pushed a commit that referenced this pull request Nov 6, 2024
* Fix amqp instrumentation
rochdev pushed a commit that referenced this pull request Nov 6, 2024
* Fix amqp instrumentation
rochdev pushed a commit that referenced this pull request Nov 6, 2024
* Fix amqp instrumentation
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.

3 participants