Add an option to limit length of values of attributes and metric values

[jtmalinowski]

Docs are based on existing Java implementation open-telemetry/opentelemetry-java#1484

Fixes #1133

Messaging about truncation was extracted to open-telemetry/semantic-conventions#1163

Metrics support extracted to #1830

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Jakub Malinowski (4507656)

[Oberon00]

Please create an issue for this PR.

[jtmalinowski]

PR extended to include metric label values.

[Oberon00]

PR extended to include metric label values.

👍

I was just reminded that resource values might also be relevant here. Sorry for the piece-wise comments 🙈

[Oberon00]

One more thing @jtmalinowski: Please either fill out the PR description or remove the part of the template that you don't use.

[jsuereth]

@jtmalinowski and I discussed this offline.

Action Items:

1. @jtmalinowski is going to update this PR so the metric-exemption is called out where the global attribute limit is defined, making it clear to implementers (and hopefully their documentation) that metrics are NOT impacted by this setting.
2. @jtmalinowski is going to open a bug/feature request to support metrics for Attribute limits. Specifically we need to deal with the issues of:

• Metric Identity overlap post-truncation
• Which attributes should be kept for metrics? (@reyang has this hint-api idea for preserving important attributes)
• Should attribute truncation happen before or after View attribute truncation?
• Should attribute truncation in metrics be considered user configuration error?

[jtmalinowski]

@arminru @jsuereth @yurishkuro All issues addressed!

[yurishkuro]

again, you cannot put MUST's into spec if the intention is to change the rules in the near future. It's not a backwards compatible change. Either leave the behavior unspecified or use the weak MAY.

[iNikem]

@jsuereth Do you approve or something else should be changed?

[jsuereth]

Sorry for delay @iNikem. Changes LGTM

[jtmalinowski]

@arminru Let's do it 🚀

[carlosalberto]

Fine to merge but I really want to have a follow up having what happens when you define both OTEL_ATTRIBUTE_COUNT_LIMIT and related variables, e.g. OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT. At this moment it's not clear which one has higher priority.

[jtmalinowski]

@carlosalberto https://github.com/open-telemetry/opentelemetry-specification/pull/1130/files#diff-500cb43b9725373c25ea03660c293e59681e6e2f6ba3eb588188ed3b83dcbdcdR78 describes it like so:

An SDK MAY implement model-specific limits, for example SpanAttributeCountLimit. If both a general and a model-specific limit are implemented, then the SDK MUST first attempt to use the model-specific limit [...]

I think that it can be inferred from the context that it extends to environment variables, ie. if OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT is set or has a default then for Span Attributes it takes priority.

[iNikem]

@open-telemetry/technical-committee can anybody press merge?

[carlosalberto]

I think that it can be inferred from the context that it extends to environment variables, ie. if OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT is set or has a default then for Span Attributes it takes priority.

I think we can do better than leave things inferred and we can put a link to this section from the env vars ;) But let's definitely do that as a minor amendment after this PR.