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

Instrument AMQP batched publishing #2659

Merged
merged 1 commit into from
May 13, 2024

Conversation

lcobucci
Copy link
Contributor

@lcobucci lcobucci commented May 8, 2024

Description

This fixes #2644 by configuring the extension to trace the necessary methods.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@lcobucci lcobucci requested review from a team as code owners May 8, 2024 08:17
@codecov-commenter
Copy link

codecov-commenter commented May 8, 2024

Codecov Report

Attention: Patch coverage is 0% with 44 lines in your changes are missing coverage. Please review.

Project coverage is 8.19%. Comparing base (d60b961) to head (01d3595).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #2659       +/-   ##
============================================
- Coverage     79.18%   8.19%   -70.99%     
- Complexity     2208    2210        +2     
============================================
  Files           199     104       -95     
  Lines         22019    9189    -12830     
============================================
- Hits          17435     753    -16682     
- Misses         4584    8436     +3852     
Flag Coverage Δ
tracer-extension ?
tracer-php 8.19% <0.00%> (-72.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/DDTrace/Integrations/AMQP/AMQPIntegration.php 0.00% <0.00%> (-98.25%) ⬇️

... and 173 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d60b961...01d3595. Read the comment docs.

@bwoebi
Copy link
Collaborator

bwoebi commented May 8, 2024

That's awesome! Thanks @lcobucci :-)

We'll review it soon and make sure to include it in 1.0.0.

@PROFeNoM Can you please have a look, at a glance it looks great to me, but maybe you have a comment?

@lcobucci
Copy link
Contributor Author

lcobucci commented May 8, 2024

Thanks, happy to help.

Do let me know if there's any change required 👍

Copy link
Contributor

@PROFeNoM PROFeNoM left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution 😃 It looks very good to me. There would only be some minor, semantical changes to be made ,and then it's all good to me 👍

PS: Sorry for the delay reviewing, I was OOO last week ;-)

src/DDTrace/Integrations/AMQP/AMQPIntegration.php Outdated Show resolved Hide resolved
src/DDTrace/Integrations/AMQP/AMQPIntegration.php Outdated Show resolved Hide resolved
This fixes DataDog#2644 by configuring the extension to trace the necessary
methods.
@lcobucci lcobucci force-pushed the add-trace-to-batch-amqp-publish branch from 63fe756 to 01d3595 Compare May 13, 2024 08:33
@lcobucci lcobucci requested a review from PROFeNoM May 13, 2024 08:34
Copy link
Contributor

@PROFeNoM PROFeNoM left a comment

Choose a reason for hiding this comment

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

LGTM! 😃 Thanks a lot for your superb contribution 🤗

@PROFeNoM PROFeNoM merged commit 2e11319 into DataDog:master May 13, 2024
550 of 554 checks passed
@bwoebi bwoebi added this to the 1.0.0 milestone May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Missing instrumentation for batched AMQP publishing
4 participants