-
Notifications
You must be signed in to change notification settings - Fork 54
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
Attribute requirement levels #92
Conversation
da12e8c
to
e592f39
Compare
@Oberon00 @arminru please review along with the new terminology I propose in open-telemetry/opentelemetry-specification#2469 specifically here: |
Hi! I think it would be great if the spec PR could be split into one that defines the new requirement levels and the actual HTTP-related changes. I think the build-tool repo won't attract many reviewers, and a PR (title) specifically for HTTP may give you different reviewers from the reviewers you would get on a PR that has a title indicating the more general, wider-reaching changes of introducing these new requirement levels. |
@Oberon00 thanks for the suggestion! Done in open-telemetry/opentelemetry-specification#2522 |
Would appreciate your review on this one. Requirement level definitions open-telemetry/opentelemetry-specification#2522 got merged open-telemetry/opentelemetry-specification#2542 shows what it'll mean for current semantic conventions. |
@thisthat @open-telemetry/specs-approvers can you please review? |
@lmolkova lots of maintainers/contributors are at Kubecon EU this week. Unfortunately my python skills are close to 0. |
semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py
Outdated
Show resolved
Hide resolved
semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py
Show resolved
Hide resolved
semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py
Outdated
Show resolved
Hide resolved
semantic-conventions/src/opentelemetry/semconv/templating/code.py
Outdated
Show resolved
Hide resolved
semantic-conventions/src/opentelemetry/semconv/templating/code.py
Outdated
Show resolved
Hide resolved
semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py
Outdated
Show resolved
Hide resolved
semantic-conventions/src/tests/semconv/model/test_error_detection.py
Outdated
Show resolved
Hide resolved
3b23c82
to
94ad5ea
Compare
@Oberon00 thank you! I believe I addressed your comments |
semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py
Show resolved
Hide resolved
@thisthat can you please take a look? |
1e048c8
to
2ca0765
Compare
@Oberon00 if you're happy with this change, could you please merge it? |
I'd be fine with merging, but don't have permission to do so. I think only @open-telemetry/technical-committee has maintainer access to this repository. |
@bogdandrutu or @jmacd can you please merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @lmolkova!
Tooling part of
open-telemetry/opentelemetry-specification#2469open-telemetry/opentelemetry-specification#2522Breaks down attribute requiredness info more levels: