Metrics-generator can generate metrics with invalid labels

[yvrhdn]

Describe the bug

If spans contain attributes that start with a number, the metrics-generator might create metrics with invalid labels. These metrics are rejected by the Prometheus backend when they are being remote written.

Example response from Mimir (with data trimmed):

received a series with an invalid label: '1cloud' series: 'traces_target_info{1cloud=\"...\", ...' (err-mimir-label-invalid)"

Prometheus says the following about metric labels:

Metric labels:

• Enable Prometheus's dimensional data model to identify any given combination of labels for the same metric name. It identifies a particular dimensional instantiation of that metric (for example: all HTTP requests that used the method POST to the /api/tracks handler). The query language allows filtering and aggregation based on these dimensions.
• The change of any label's value, including adding or removing labels, will create a new time series.
• Labels may contain ASCII letters, numbers, as well as underscores. They must match the regex [a-zA-Z_][a-zA-Z0-9_]*.
• Label names beginning with __ (two "_") are reserved for internal use.
• Label values may contain any Unicode characters.
• Labels with an empty label value are considered equivalent to labels that do not exist.

To Reproduce
Steps to reproduce the behavior:

1. Start Tempo (SHA or version)
2. Configure metrics-generator to generate span and/or service graphs metrics
3. Send spans containing attributes with invalid Prometheus label names

Expected behavior
We have a few options, I'm not sure which one is best:

• try to 'correct' the label, would it make sense to add a prefix in this case?
• drop the label when the metric is created and report this
• drop the entire sample when the metric is created and report this

Environment:

• Infrastructure: [e.g., Kubernetes, bare-metal, laptop]
• Deployment tool: [e.g., helm, jsonnet]

Additional Context

Similar issue: #4293

[github-actions]

This issue has been automatically marked as stale because it has not had any activity in the past 60 days.
The next time this stale check runs, the stale label will be removed if there is new activity. The issue will be closed after 15 days if there is no new activity.
Please apply keepalive label to exempt this Issue.

[KyriosGN0]

@yvrhdn i think that the second options is the best, has some other prefernece been decided by the Team ?
if not, i would like to try and implement this

[javiermolinar]

Dropping the label is consistent with current behavior. We already do that for invalid utf8 encoded runes

tempo/modules/generator/processor/spanmetrics/spanmetrics.go

Line 212 in 6457388

err := validateLabelValues(labelValues)