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 schema_url constants in semconv #4069

Merged
merged 8 commits into from
Jul 22, 2024
Merged

Conversation

emdneto
Copy link
Member

@emdneto emdneto commented Jul 22, 2024

Description

Right now, we have an inaccessible constant for SCHEMA_URL; fix this. I don't know the impact of this in practice for users, but I can imagine that using the wrong schema_url, will lead to issues in a telemetry consumer which does schema translations when needed to compare the schema url's (i.e., schemaprocessor) . Also, this can affect the schema_url we are setting in instrumentation scope for instrumentation libraries that are being migrated to new semconv in contrib repo, see https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py#L396

  • PR to update tests in contrib after this is merged

@emdneto emdneto requested review from a team and lmolkova and removed request for a team July 22, 2024 13:04
@xrmx
Copy link
Contributor

xrmx commented Jul 22, 2024

Good catch!

emdneto and others added 5 commits July 22, 2024 10:12
…chemas.py

Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
…chemas.py

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

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

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

Thank you!

@lzchen lzchen merged commit 8749168 into open-telemetry:main Jul 22, 2024
284 checks passed
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.

4 participants