open-telemetry / opentelemetry-python

OpenTelemetry Python API and SDK
https://opentelemetry.io
Apache License 2.0
1.67k stars 569 forks source link

Prometheus exporter should handle metric label key collisions #3928

Open aabmass opened 1 month ago

aabmass commented 1 month ago

As described in the spec

Metrics from OpenTelemetry with unsupported Attribute names MUST replace invalid characters with the _ character. Multiple consecutive _ characters MUST be replaced with a single _ character. This may cause ambiguity in scenarios where multiple similar-named attributes share invalid characters at the same location. In such unlikely cases, if multiple key-value pairs are converted to have the same Prometheus key, the values MUST be concatenated together, separated by ;, and ordered by the lexicographical order of the original keys.

Right now this is not happening. The following code

counter = meter_provider.get_meter("scope").create_counter(
    "test_with_collision"
)
counter.add(1, {"hello_world": "hello_world", "hello.world": "hello.world"})

expected

# HELP test_with_collision_total 
# TYPE test_with_collision_total counter
test_with_collision_total{hello_world="hello.world;hello_world"} 1.0

actual

# HELP test_with_collision_total My description
# TYPE test_with_collision_total counter
test_with_collision_total{hello_world="hello_world"} 1.0
aabmass commented 1 month ago

~If I understand the spec correctly, the colliding stream values should be aggregated together like shown above. This seems really tricky and like a lot of work for an exporter to implement.~

Nvm i was misunderstanding. This applies to metric label keys in the same labelset. The original issue is not covered in the spec.

aabmass commented 1 month ago

We should probably not work on this since Prometheus is hopefully adding UTF8 support and we will no longer need sanitization