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

Conflicting/confusing requirements around InstrumentationScope attributes #3773

Open
breedx-splk opened this issue Nov 17, 2023 · 6 comments
Assignees
Labels
spec:miscellaneous For issues that don't match any other spec label triage:accepted:ready Ready to be implemented. Small enough or uncontroversial enough to be implemented without sponsor

Comments

@breedx-splk
Copy link
Contributor

In this part of the mapping-to-non-otlp.md file, the section that contains otel.library.name is confusing.

I think the the intent was probably to require instrumentations to continue providing the old names for backwards compatibility. However, this section is really confusing to readers because it:

  • Has a heading that says "The following deprecated aliases MUST also be reported with exact same values"
  • but then has a description that says "Deprecated, use instead"
  • and then has a Requirement Level of "Recommended".

Why should attribute be Recommended, Deprecated, and MUST still contain a value?

@breedx-splk breedx-splk added the spec:miscellaneous For issues that don't match any other spec label label Nov 17, 2023
@pellared
Copy link
Member

pellared commented Nov 20, 2023

I see it as a similar issue to #3717 which I resolved via #3719.

Maybe we should consider defining a Deprecated Requirement Level in attribute-requirement-level.md? This could mean that if the attribute is already supported in a stable component then it MUST continue to be supported within the same major version releases.

@pellared
Copy link
Member

CC @open-telemetry/specs-semconv-approvers

@lmolkova
Copy link
Contributor

lmolkova commented Nov 20, 2023

@tigrannajaryan it seems we've deprecated otel.library.* in favor of otel.scope.* almost 2 years ago in #2276. Does it still make sense to require (or even mention) backward compatibility?

Given how much time has passed, I'd be in favor of removing mentions of otel.library.* deprecated attributes from MD files.

@tigrannajaryan
Copy link
Member

Given how much time has passed, I'd be in favor of removing mentions of otel.library.* deprecated attributes from MD files.

I agree. I suggest we do this carefully:

  1. Check existing implementations to make sure they don't always include otel.library.* and are actually treating it as a Recommended attribute.
  2. Provided that (1) is the case set a removal date/spec version and add a notice about upcoming removal. Something like 6 months from now is probably fine.

Given that the wording in the spec is confusing and conflicting I do not think we are bound by our stability guarantees and can treat it as a spec bug fix.

@jack-berg jack-berg added the [label deprecated] triaged-accepted [label deprecated] Issue triaged and accepted by OTel community, can proceed with creating a PR label Nov 29, 2023
@jack-berg
Copy link
Member

Given how much time has passed, I'd be in favor of removing mentions of otel.library.* deprecated attributes from MD files.

If that document was stable, we can't remove without breaking compatibility.

The problem seems to be that we have normative "MUST" language, which is stronger than the "RECOMMENDED" of the otel.library.name. We should probably update the language to say something to the effect of "if instrumentation previously included otel.library.name, it MUST continue to include it".

@jack-berg jack-berg assigned jack-berg and unassigned bogdandrutu Nov 29, 2023
@austinlparker austinlparker added triage:accepted:ready-with-sponsor Ready to be implemented and has a specification sponsor assigned and removed [label deprecated] triaged-accepted [label deprecated] Issue triaged and accepted by OTel community, can proceed with creating a PR labels Apr 30, 2024
@austinlparker austinlparker moved this to Spec - Accepted in 🔭 Main Backlog Jul 16, 2024
@austinlparker austinlparker moved this from Spec - Accepted to Spec - In Progress in 🔭 Main Backlog Jul 16, 2024
@austinlparker austinlparker moved this from Spec - In Progress to Spec - Accepted in 🔭 Main Backlog Jul 30, 2024
@austinlparker austinlparker added triage:accepted:ready Ready to be implemented. Small enough or uncontroversial enough to be implemented without sponsor and removed triage:accepted:ready-with-sponsor Ready to be implemented and has a specification sponsor assigned labels Jul 30, 2024
@austinlparker
Copy link
Member

This appears to not be under active work, please update it when that changes. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:miscellaneous For issues that don't match any other spec label triage:accepted:ready Ready to be implemented. Small enough or uncontroversial enough to be implemented without sponsor
Projects
Status: Spec - Accepted
Development

No branches or pull requests

7 participants