Skip to content

Conversation

@rite7sh
Copy link

@rite7sh rite7sh commented Dec 17, 2025

Refs #3475

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

This change updates a semantic convention attribute constant without modifying
runtime behavior. Local test execution on Windows is limited by environment
constraints; CI will validate the change.

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

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

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 17, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

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.

I think there are a lot more users of SpanAttributes in the asgi package

@xrmx xrmx moved this to Reviewed PRs that need fixes in @xrmx's Python PR digest Dec 17, 2025
rite7sh and others added 3 commits December 19, 2025 15:40
…emetry/instrumentation/asgi/__init__.py

Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
…emetry/instrumentation/asgi/__init__.py

Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
@rite7sh
Copy link
Author

rite7sh commented Dec 20, 2025

@xrmx kindly look into the changes as per convenience if the following PR is merge ready, thank you.

@xrmx
Copy link
Contributor

xrmx commented Dec 22, 2025

@xrmx kindly look into the changes as per convenience if the following PR is merge ready, thank you.

As I already mentioned there are a ton more of users of SpanAttributes in the asgi instrumentation, i.e. tests/test_asgi_middleware.py

"name": "GET / http send",
"kind": trace_api.SpanKind.INTERNAL,
"attributes": {
SpanAttributes.HTTP_STATUS_CODE: 200,
Copy link
Contributor

Choose a reason for hiding this comment

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

The task is to substitute the use of SpanAttributes with other modules inside opentelemetry.semconv, not to change what is asserted here. If we don't have a replacement with the same value then they should stay untouched

@rite7sh
Copy link
Author

rite7sh commented Jan 3, 2026

@xrmx
Thanks for the review and the clarification.
After taking a deeper look at the ASGI instrumentation and comparing it with the upstream implementation, I realized that the existing code already correctly handles semantic convention stability via the set_http* helper methods, including the cases where SpanAttributes.HTTP_SERVER_NAME is still required.
The changes I initially made (including tests) were broader than necessary and out of scope for this issue, so I’ve reverted them. At this point, it appears that no additional changes are required in init.py for this issue.
Please let me know if you’d like me to close this PR, or if there’s a more targeted change you’d prefer here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Reviewed PRs that need fixes

Development

Successfully merging this pull request may close these issues.

2 participants