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(pubsub): correctly trace modacks in the unary pull #14049

Merged
merged 5 commits into from
Apr 24, 2024

Conversation

alevenberg
Copy link
Member

@alevenberg alevenberg commented Apr 23, 2024

  • refactor(pubsub): create pull manager impl to wrap the modack rpc
  • test(pubsub): use the mock pull lease manager impl in the tracing unittest
  • refactor(pubsub): remove tracing lease manager class
  • test(pubsub): add a test for the default pull lease manager impl

I split up the commits for easier reading, if reviewers prefer I can merge them lmk.

Fixes #14020


This change is Reviewable

@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Apr 23, 2024
Copy link

codecov bot commented Apr 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (main@8747e14). Click here to learn what that means.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14049   +/-   ##
=======================================
  Coverage        ?   93.21%           
=======================================
  Files           ?     2194           
  Lines           ?   188296           
  Branches        ?        0           
=======================================
  Hits            ?   175528           
  Misses          ?    12768           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alevenberg alevenberg marked this pull request as ready for review April 23, 2024 17:38
@alevenberg alevenberg requested a review from a team as a code owner April 23, 2024 17:38
Copy link
Member

@scotthart scotthart left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 9 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @alevenberg)

@alevenberg alevenberg merged commit 4dad770 into googleapis:main Apr 24, 2024
63 checks passed
@alevenberg alevenberg deleted the unary-tracing branch April 24, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pubsub unary pull does not correctly trace mod acks
2 participants