From a9936ccfd8fb6a4f4e56552d9deb96b2d08b17b7 Mon Sep 17 00:00:00 2001 From: Julian Tescher Date: Tue, 7 Nov 2023 11:56:48 -0500 Subject: [PATCH] prometheus: Ignore unknown instrument units ## Motivation The [metric unit semantic conventions] suggest that integer counts should use annotations (e.g. `{packet}`), which breaks the current unit appending logic as they are not properly escaped. [metric unit semantic conventions]: https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/general/metrics.md#instrument-units ## Solution Ignore unknown units (including annotations) as other language implementations currently do. This change also removes the `$` mapping as it is not UCUM. --- opentelemetry-prometheus/CHANGELOG.md | 4 ++++ opentelemetry-prometheus/src/utils.rs | 13 ++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/opentelemetry-prometheus/CHANGELOG.md b/opentelemetry-prometheus/CHANGELOG.md index e7ced72311..908f3e075a 100644 --- a/opentelemetry-prometheus/CHANGELOG.md +++ b/opentelemetry-prometheus/CHANGELOG.md @@ -2,6 +2,10 @@ ## vNext +### Fixed + +- Fix UCUM annotation escaping by ignoring unknown instrument units and annotations + ## v0.14.0 ### Changed diff --git a/opentelemetry-prometheus/src/utils.rs b/opentelemetry-prometheus/src/utils.rs index 5243c22aa3..d0a66a6e5a 100644 --- a/opentelemetry-prometheus/src/utils.rs +++ b/opentelemetry-prometheus/src/utils.rs @@ -36,7 +36,9 @@ pub(crate) fn get_unit_suffixes(unit: &Unit) -> Option> { }; } - Some(Cow::Owned(unit.as_str().to_string())) + // Unmatched units and annotations are ignored + // e.g. "{request}" + None } fn get_prom_units(unit: &str) -> Option<&'static str> { @@ -49,9 +51,9 @@ fn get_prom_units(unit: &str) -> Option<&'static str> { "ms" => Some("milliseconds"), "us" => Some("microseconds"), "ns" => Some("nanoseconds"), - "By" => Some("bytes"), // Bytes + "By" => Some("bytes"), "KiBy" => Some("kibibytes"), "MiBy" => Some("mebibytes"), "GiBy" => Some("gibibytes"), @@ -79,7 +81,6 @@ fn get_prom_units(unit: &str) -> Option<&'static str> { "Hz" => Some("hertz"), "1" => Some("ratio"), "%" => Some("percent"), - "$" => Some("dollars"), _ => None, } } @@ -185,10 +186,12 @@ mod tests { ("1/y", Some(Cow::Owned("per_year".to_owned()))), ("m/s", Some(Cow::Owned("meters_per_second".to_owned()))), // No match - ("invalid", Some(Cow::Owned("invalid".to_string()))), + ("invalid", None), ("invalid/invalid", None), - ("seconds", Some(Cow::Owned("seconds".to_string()))), + ("seconds", None), ("", None), + // annotations + ("{request}", None), ]; for (unit_str, expected_suffix) in test_cases { let unit = Unit::new(unit_str);