Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Mar 21, 2025

Description

This is a fix for #3380 that checks explicitly for a None result rather than evaluating truthiness. PubSub results with zero messages evaluate to False, causing span.end() to be called twice.

Fixes #3380

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Ran Tox
  • Ran locally against an empty PubSub queue and no longer received warning messages, but spans correctly started and ended.
  • Ran against queue with messages and behavior was as expected (spans correctly started and ended, as before)

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Copy link

linux-foundation-easycla bot commented Mar 21, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@ghost ghost self-requested a review as a code owner March 21, 2025 07:39
@xrmx
Copy link
Contributor

xrmx commented Mar 21, 2025

Thanks for the PR, could you please write a test for this to avoid regressions in the future?

@ghost ghost force-pushed the bugfix-3380 branch from 108c74d to f2e1d86 Compare March 25, 2025 08:25
@ghost
Copy link
Author

ghost commented Mar 25, 2025

Thanks for the PR, could you please write a test for this to avoid regressions in the future?

That was an adventure! Added a test that counts span.end() invocations.

@xrmx xrmx moved this to Reviewed PR that needs fixing in @xrmx's Python PR digest Apr 4, 2025
@ghost ghost force-pushed the bugfix-3380 branch from b28b222 to 86a9776 Compare April 18, 2025 07:10
@xrmx xrmx moved this from Reviewed PR that needs fixing to Easy to review / merge / close in @xrmx's Python PR digest Apr 18, 2025
@xrmx xrmx self-requested a review April 18, 2025 08:09
Copy link
Contributor

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

With the small nit fixed, LGTM

@ghost ghost force-pushed the bugfix-3380 branch from d237171 to 0282968 Compare April 18, 2025 20:10
@lzchen lzchen merged commit c54292f into open-telemetry:main Apr 21, 2025
720 checks passed
@github-project-automation github-project-automation bot moved this from Easy to review / merge / close to Done in @xrmx's Python PR digest Apr 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

GRPC traces called twice when result evaluates to false

2 participants