open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.75k stars 889 forks source link

Conflicting/confusing requirements around InstrumentationScope attributes #3773

Open breedx-splk opened 11 months ago

breedx-splk commented 11 months ago

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:

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

pellared commented 11 months ago

I see it as a similar issue to https://github.com/open-telemetry/opentelemetry-specification/issues/3717 which I resolved via https://github.com/open-telemetry/opentelemetry-specification/pull/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 commented 11 months ago

CC @open-telemetry/specs-semconv-approvers

lmolkova commented 11 months ago

@tigrannajaryan it seems we've deprecated otel.library.* in favor of otel.scope.* almost 2 years ago in https://github.com/open-telemetry/opentelemetry-specification/pull/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 commented 11 months ago

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 commented 11 months ago

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".

austinlparker commented 3 months ago

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