Remove duplicate dimension validation

[carles-grafana]

What this PR does:

we have identified some valid use cases such as using multiple SDKs using different conventions,
such as {"deployment_environment", "deployment.environment"}

it still makes sense to keep it for dimension mappings as these can cause confusion

Which issue(s) this PR fixes:
Fixes #

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c0a0c00c7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve values when dimensions sanitize to same label

By dropping the collision check here, configs can now include dimensions that sanitize to the same label (e.g., deployment.environment + deployment_environment). In spanmetrics.aggregateMetricsForSpan, each dimension is added in order and LabelBuilder.Add ultimately calls labels.Builder.Set, which overwrites existing labels. If the later dimension is missing on a span, it will overwrite a real value from the earlier one with an empty string, so the resulting metric label becomes blank even though the span had a valid attribute. This creates silent data loss for the “multiple SDKs” use case; consider retaining collision validation or changing aggregation to prefer the first non-empty value when labels collide.

Useful? React with 👍 / 👎.

[mdisibio]

To make sure I understand, this config is no longer logged as a warning nor an error. Therefore we can remove the plumbing to return warn because there are no other warnings.

And then for this config, it sums the 2 inputs together. e.g. deployment_environment and deployment.environment are both counted into the final metric deployment_environment ?

[carles-grafana]

To make sure I understand, this config is no longer logged as a warning nor an error. Therefore we can remove the plumbing to return warn because there are no other warnings.

correct

And then for this config, it sums the 2 inputs together. e.g. deployment_environment and deployment.environment are both counted into the final metric deployment_environment ?

yes, but if a span contains both, one will overwrite the other. This matches the previous behavior. This would be an issue in the user's instrumentation. But having both attributes but in different spans can be a valid use case if instrumented differently.

[mdisibio]

+1 This is the approach we take for cost attribution, and I think it's good because it unlocks more use cases.

Good catch on the case when a span contains both - when this happens will it be deterministic which label is used? First or last both ok, as long as it is deterministic and explainable. Then it also allows for a tenant to change the behavior by swapping the order of the labels to whichever they prefer. But if it is random from a map and the data becomes unreliable and there is no fix except change their instrumentation, maybe let's think about if we can fix that.

Changelog?

[carles-grafana]

@mdisibio

it's deterministic: if there is a collision, for example deployment.environment and deployment_environment, both sanitized to deployment_environment, we just take the last one configured.
see https://github.com/carles-grafana/tempo/blob/remove-duplicate-dimension-validation/modules/generator/processor/spanmetrics/spanmetrics.go#L170

updated changelog