Attributes namespacing clarification

[bertysentry]

Fixes #3131

Changes

Specification for attributes naming has been clarified with regards to namespaces, when to use namespaces or not:

• Namespaces for attribute names are mandatory for Resource, Trace, and Log.
• Namespaces are optional for Metric.

Related issues: #3129 open-telemetry/semantic-conventions#713 #2513 #585

[bertysentry]

@jmacd @jack-berg @sebastien-rosset Namespaces for metric attributes are discussed in multiple issues, a clarification is needed. Please review and discuss, thank you!

[Oberon00]

Isn't there an implicit requirement to have metric and other attributes use the same names. Otherwise, metrics would need to use completely separate semantic conventions.

[bertysentry]

@Oberon00 The requirement is actually explicit: when attributes apply to metrics and traces, they must be identical and thus use namespacing as required for trace attributes (like: http.method, etc.)

See "Recommendations for OpenTelemetry authors":

Semantic conventions exist for four areas: for Resource, Span, Log, and Metric attribute names. In addition, for spans we have two more areas: Event and Link attribute names. Identical namespaces or names in all these areas MUST have identical meanings. For example the http.method span attribute name denotes exactly the same concept as the http.method metric attribute, has the same data type and the same set of possible values (in both cases it records the value of the HTTP protocol's request method as a string).

[sebastien-rosset]

This may be a nitpick: the word "MUST" has a specific compliance meaning, it does not seem right to use the word "MUST" twice under a section named "Recommendations":

Recommendation: a suggestion or proposal as to the best course of action, especially one put forward by an authoritative body.

Maybe the word "recommendations" should be changed to something more normative? For example:

• Conventions and Compliance for OpenTelemetry authors
• Rules for OpenTelemetry authors
• Normative rules for OpenTelemetry authors

[bertysentry]

@sebastien-rosset I pushed some changes after your suggestions. Thank you for the review, mulch appreciated! 😊

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[bertysentry]

@jsuereth @sebastien-rosset Any chance we can discuss the clarification on the requirement of namespaces for metric attributes?

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[jsuereth]

I think this PR needs a lot more justification. As others have pointed out:

• We want to share attributes between metrics + other signals. This is to aide in telemetry correlation, one of OTEL's core missions. Why the disparity?
• There's a lot of title changes in this PR that are extraneous the actual meat of what you want to do. Please pull that into a separate PR.
• Have you verified that all semconv attributes are part of a namespace before making this restriction?
• It would be a breaking change to go from SHOULD -> MUST for general attributes being part of a namespace. Specifically, as worded this PR would require a major version bump of the specification if I'm reading it right (usage of MUST).

Overall, I'm really not sure what we gain here.

[bertysentry]

Thank you for the review, @jsuereth!

This PR aligns the specification with the current state of semantic conventions for metrics, where attributes often don't use namespaces, because they don't need to. Several issues were registered on semantic conventions, listing metric attributes that are not using namespaces, because this specification is currently unclear.

The justification for not needing namespaces in metric attributes is that metrics don't have the same hierarchical nature like traces, so their attributes have a low probability of conflict. This may explain why current semantic conventions rarely use namespaces in their attributes.

However, as you noted, and as recommended in this PR, when metrics in semantic conventions are related to trace spans in other semantic conventions, attributes for metrics must align with those for spans, and thus use namespaces.

It would be a breaking change to go from SHOULD -> MUST for general attributes being part of a namespace. Specifically, as worded this PR would require a major version bump of the specification if I'm reading it right (usage of MUST).

Oh! It's a mistake, I did not intend this change, sorry about that! And thank you for pointing that out!

There's a lot of title changes in this PR that are extraneous the actual meat of what you want to do. Please pull that into a separate PR.

The changes in the titles are caused by the necessary reshuffling in the various requirements and recommendations. It was impossible to clarify the specification for attributes without changing the titles and the order of the sections.

The only extraneous change is about using sentence case in the headings instead of title case (which should be used only for document titles).

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[bertysentry]

@jack-berg Do you have any feedback on this proposed change on the use of namespaces for metrics attributes? Thanks!

[sebastien-rosset]

jsuereth wrote:

Overall, I'm really not sure what we gain here.

One crucial difference is this change:

- All names that are part of OpenTelemetry semantic conventions SHOULD be part
  of a namespace.
+ All attribute names that are part of OpenTelemetry semantic conventions for
  Resources, Traces, and Logs SHOULD be part of a namespace.

The spec states metric/attribute names are hierarchical:

Metric names and attributes exist within a single universe and a single hierarchy. Metric names and attributes MUST be considered within the universe of all existing metric names. When defining new metric names and attributes, consider the prior art of existing standard metrics and metrics from frameworks/libraries.

There are other parts of the spec that elaborate on the namespace hierarchy, in particular:

Names SHOULD NOT coincide with namespaces. For example if service.instance.id is an attribute name then it is no longer valid to have an attribute named service.instance because service.instance is already a namespace. Because of this rule be careful when choosing names: every existing name prohibits existence of an equally named namespace in the future, and vice versa: any existing namespace prohibits existence of an equally named attribute key in the future.

However, many of the per-metric attributes in the semantic guidelines do not use a hierarchical structure, so it seems the spec is not following its own rules. Related issues/PRs are #3129, #3131, #3431. @jsuereth, I see you have approved #3431. Does this mean the spec is actually going in the direction of using hierarchical names?

The proposed change by @bertysentry is technically resolving the contradiction, but only because it omits "metric attribute" from the sentence. I'd like to see something more explicit, i.e. a sentence that states and explains why metric attribute are not subject to the hierarchical naming rule.

[bertysentry]

@sebastien-rosset See @tigrannajaryan's comment on PR open-telemetry/opentelemetry-specification#3431 on why metrics attributes don't need namespaces.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[reyang]

@bertysentry heads up - most likely this PR will be closed, and we'll ask you to resubmit the PR in a new repo, please refer to #3474 (comment).

[jsuereth]

However, many of the per-metric attributes in the semantic guidelines do not use a hierarchical structure, so it seems the spec is not following its own rules. Related issues/PRs are #3129, #3131, #3431. @jsuereth, I see you have approved #3431. Does this mean the spec is actually going in the direction of using hierarchical names?

Not necessarily. We're discussing the direction of this issue in the Technical Comittee (notes). It's a "hot topic". That is my viewpoint, but we're likely to reach a bit of a compromise/middle ground here.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[trask]

hey all, the https://github.com/orgs/open-telemetry/teams/technical-committee is reviewing this issue, and asked to create a summary of pros/cons. I have created open-telemetry/semantic-conventions#51 based on the comments here, please comment further over there if I missed anything, thx!

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.