Skip to content

fix(pika): pass destination instead of task_name to _enrich_span#4563

Merged
xrmx merged 11 commits into
open-telemetry:mainfrom
GitanshKapoor:fix/pika-update-destination-property
May 18, 2026
Merged

fix(pika): pass destination instead of task_name to _enrich_span#4563
xrmx merged 11 commits into
open-telemetry:mainfrom
GitanshKapoor:fix/pika-update-destination-property

Conversation

@GitanshKapoor
Copy link
Copy Markdown
Contributor

@GitanshKapoor GitanshKapoor commented May 10, 2026 •
edited
Loading

Description

In _get_span (utils.py), _enrich_span was called with task_name instead of destination, causing the messaging.destination span attribute to always show "(temporary)" on publish spans and the consumer tag on consume spans, instead of the actual queue or exchange name.

Fixes #1026

Type of change

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

How Has This Been Tested?

Updated test_get_span in test_utils.py to assert that _enrich_span is called with destination as the argument, not task_name. All existing tests in tests/test_utils.py pass.

Does This PR Require a Core Repo Change?

  • No.

Checklist:

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

@GitanshKapoor
Update span enrichment to use destination instead of task_name
42f8763
@GitanshKapoor
Refactor enrich_span assertion in test_utils.py
7f626ec 
- tightened the test_get_span assertion to verify _enrich_span
is called with destination, not task_name.
- changed enrich_span.assert_called_once() to enrich_span.assert_called_once_with(...) with destination as the argument.
@GitanshKapoor
Update CHANGELOG.md
52f8a6d
@GitanshKapoor GitanshKapoor requested a review from a team as a code owner May 10, 2026 09:42
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 10, 2026 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@xrmx xrmx moved this to Ready for review in Python PR digest May 11, 2026
xrmx
xrmx reviewed May 11, 2026
View reviewed changes
Comment thread CHANGELOG.md Outdated
@emdneto
Copy link
Copy Markdown
Member

emdneto commented May 12, 2026

Thanks for the PR!

Just a heads-up: we no longer update CHANGELOG.md directly. The changelog is now generated from changelog fragments using Towncrier.

Please add the appropriate changelog fragment for this change instead of editing CHANGELOG.md manually. You can find the instructions and expected format in CONTRIBUTING.md.

@GitanshKapoor GitanshKapoor requested a review from xrmx May 13, 2026 15:50
@GitanshKapoor
Copy link
Copy Markdown
Contributor Author

GitanshKapoor commented May 13, 2026

Thanks for the PR!

Just a heads-up: we no longer update CHANGELOG.md directly. The changelog is now generated from changelog fragments using Towncrier.

Please add the appropriate changelog fragment for this change instead of editing CHANGELOG.md manually. You can find the instructions and expected format in CONTRIBUTING.md.

Thanks for the heads-up!

I’ve added a changelog fragment in .changelog/4563.fixed following the Towncrier format, so CHANGELOG.md is no longer edited manually.

Please let me know if any adjustments are needed. 🙂

tammy-baylis-swi
tammy-baylis-swi reviewed May 13, 2026
View reviewed changes
Comment thread .changelog/4563.fixed Outdated
tammy-baylis-swi
tammy-baylis-swi approved these changes May 13, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@tammy-baylis-swi tammy-baylis-swi left a comment

Choose a reason for hiding this comment

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

Lgtm! Will also need a Maintainer's approval.

@GitanshKapoor
Copy link
Copy Markdown
Contributor Author

GitanshKapoor commented May 16, 2026

Lgtm! Will also need a Maintainer's approval.

Thank you for the feedback! Waiting on a Maintainer’s approval.

@xrmx @tammy-baylis-swi

xrmx and others added 2 commits May 18, 2026 10:42
@xrmx @tammy-baylis-swi
Update .changelog/4563.fixed
d5114bf 
Co-authored-by: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com>
@xrmx
Update 4563.fixed
cfe4898
@xrmx
Copy link
Copy Markdown
Contributor

xrmx commented May 18, 2026

@GitanshKapoor It you are interested in the instrumentation it would be nice to add some tests that test the exported telemetry instead of the utils in a followup PR

@github-project-automation github-project-automation Bot moved this from Ready for review to Approved PRs in Python PR digest May 18, 2026
@xrmx xrmx merged commit ee0b995 into open-telemetry:main May 18, 2026
753 checks passed
@github-project-automation github-project-automation Bot moved this from Approved PRs to Done in Python PR digest May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xrmx xrmx xrmx approved these changes

@tammy-baylis-swi tammy-baylis-swi tammy-baylis-swi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Python PR digest
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[autoinstrumentation-pika] Wrong property set as destination

4 participants

@GitanshKapoor @emdneto @xrmx @tammy-baylis-swi