From fb7b83b623e30fe4e93a23abd9a2bcecbd4d010c Mon Sep 17 00:00:00 2001 From: Robert Coltheart Date: Thu, 23 May 2024 06:03:46 +1000 Subject: [PATCH 1/6] Fix OpenMetrics format suffixes for Prometheus --- .../CHANGELOG.md | 3 + .../CHANGELOG.md | 3 + .../Internal/PrometheusMetric.cs | 47 +++++++++-- .../Internal/PrometheusSerializer.cs | 35 +++++--- .../Internal/PrometheusSerializerExt.cs | 14 ++-- .../PrometheusExporterMiddlewareTests.cs | 14 ++-- .../PrometheusHttpListenerTests.cs | 12 +-- .../PrometheusMetricTests.cs | 80 ++++++++++++++++++- 8 files changed, 173 insertions(+), 35 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md index 703074b0be6..c4d7b083aba 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* Fixed issue with OpenMetrics counter suffixes for Prometheus + ([#5623](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5623)) + ## 1.9.0-alpha.1 Released 2024-May-20 diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md index abb4d7287fc..19b77b88a2d 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* Fixed issue with OpenMetrics counter suffixes for Prometheus + ([#5623](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5623)) + ## 1.9.0-alpha.1 Released 2024-May-20 diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs index a39a426136a..688c2e81b16 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs @@ -27,6 +27,7 @@ public PrometheusMetric(string name, string unit, PrometheusType type, bool disa // consecutive `_` characters MUST be replaced with a single `_` character. // https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#L230-L233 var sanitizedName = SanitizeMetricName(name); + var openMetricsName = SanitizeOpenMetricsName(sanitizedName); string sanitizedUnit = null; if (!string.IsNullOrEmpty(unit)) @@ -41,10 +42,31 @@ public PrometheusMetric(string name, string unit, PrometheusType type, bool disa // https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#L242-L246 if (!sanitizedName.Contains(sanitizedUnit)) { - sanitizedName = sanitizedName + "_" + sanitizedUnit; + sanitizedName += $"_{sanitizedUnit}"; } + + // OpenMetrics name MUST be suffixed with '_{unit}', regardless of whether the unit name appears within the text. + // Note that this may change in the future, however for the moment Prometheus will fail to read the metric using + // OpenMetrics format unless the suffix matches the unit. + // https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#unit + if (!openMetricsName.EndsWith(sanitizedUnit)) + { + openMetricsName += $"_{sanitizedUnit}"; + } + } + + // Special case: Converting "1" to "ratio". + // https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#L239 + if (type == PrometheusType.Gauge && unit == "1" && !sanitizedName.Contains("ratio")) + { + sanitizedName += "_ratio"; + openMetricsName += "_ratio"; } + // For TYPE, HELP and UNIT declarations for counters, the suffix '_total' is omitted in OpenMetrics format. + // https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#counter-1 + this.OpenMetricsMetadataName = openMetricsName; + // If the metric name for monotonic Sum metric points does not end in a suffix of `_total` a suffix of `_total` MUST be added by default, otherwise the name MUST remain unchanged. // Exporters SHOULD provide a configuration option to disable the addition of `_total` suffixes. // https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#L286 @@ -53,20 +75,25 @@ public PrometheusMetric(string name, string unit, PrometheusType type, bool disa sanitizedName += "_total"; } - // Special case: Converting "1" to "ratio". - // https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#L239 - if (type == PrometheusType.Gauge && unit == "1" && !sanitizedName.Contains("ratio")) + // For counters requested using OpenMetrics format, the MetricFamily name MUST be suffixed with '_total', regardless of the setting to disable the 'total' suffix. + // https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#counter-1 + if (type == PrometheusType.Counter && !openMetricsName.EndsWith("_total")) { - sanitizedName += "_ratio"; + openMetricsName += "_total"; } this.Name = sanitizedName; + this.OpenMetricsName = openMetricsName; this.Unit = sanitizedUnit; this.Type = type; } public string Name { get; } + public string OpenMetricsName { get; } + + public string OpenMetricsMetadataName { get; } + public string Unit { get; } public PrometheusType Type { get; } @@ -159,6 +186,16 @@ internal static string RemoveAnnotations(string unit) return sb.ToString(); } + private static string SanitizeOpenMetricsName(string metricName) + { + if (metricName.EndsWith("_total")) + { + return metricName.Substring(0, metricName.Length - 6); + } + + return metricName; + } + private static string GetUnit(string unit) { // Dropping the portions of the Unit within brackets (e.g. {packet}). Brackets MUST NOT be included in the resulting unit. A "count of foo" is considered unitless in Prometheus. diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs index 719f21a0c61..3c142d91233 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs @@ -230,12 +230,29 @@ static string GetLabelValueString(object labelValue) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int WriteMetricName(byte[] buffer, int cursor, PrometheusMetric metric) + public static int WriteMetricName(byte[] buffer, int cursor, PrometheusMetric metric, bool openMetricsRequested) { // Metric name has already been escaped. - for (int i = 0; i < metric.Name.Length; i++) + var name = openMetricsRequested ? metric.OpenMetricsName : metric.Name; + + for (int i = 0; i < name.Length; i++) + { + var ordinal = (ushort)name[i]; + buffer[cursor++] = unchecked((byte)ordinal); + } + + return cursor; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static int WriteMetricMetadataName(byte[] buffer, int cursor, PrometheusMetric metric, bool openMetricsRequested) + { + // Metric name has already been escaped. + var name = openMetricsRequested ? metric.OpenMetricsMetadataName : metric.Name; + + for (int i = 0; i < name.Length; i++) { - var ordinal = (ushort)metric.Name[i]; + var ordinal = (ushort)name[i]; buffer[cursor++] = unchecked((byte)ordinal); } @@ -252,7 +269,7 @@ public static int WriteEof(byte[] buffer, int cursor) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int WriteHelpMetadata(byte[] buffer, int cursor, PrometheusMetric metric, string metricDescription) + public static int WriteHelpMetadata(byte[] buffer, int cursor, PrometheusMetric metric, string metricDescription, bool openMetricsRequested) { if (string.IsNullOrEmpty(metricDescription)) { @@ -260,7 +277,7 @@ public static int WriteHelpMetadata(byte[] buffer, int cursor, PrometheusMetric } cursor = WriteAsciiStringNoEscape(buffer, cursor, "# HELP "); - cursor = WriteMetricName(buffer, cursor, metric); + cursor = WriteMetricMetadataName(buffer, cursor, metric, openMetricsRequested); if (!string.IsNullOrEmpty(metricDescription)) { @@ -274,14 +291,14 @@ public static int WriteHelpMetadata(byte[] buffer, int cursor, PrometheusMetric } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int WriteTypeMetadata(byte[] buffer, int cursor, PrometheusMetric metric) + public static int WriteTypeMetadata(byte[] buffer, int cursor, PrometheusMetric metric, bool openMetricsRequested) { var metricType = MapPrometheusType(metric.Type); Debug.Assert(!string.IsNullOrEmpty(metricType), $"{nameof(metricType)} should not be null or empty."); cursor = WriteAsciiStringNoEscape(buffer, cursor, "# TYPE "); - cursor = WriteMetricName(buffer, cursor, metric); + cursor = WriteMetricMetadataName(buffer, cursor, metric, openMetricsRequested); buffer[cursor++] = unchecked((byte)' '); cursor = WriteAsciiStringNoEscape(buffer, cursor, metricType); @@ -291,7 +308,7 @@ public static int WriteTypeMetadata(byte[] buffer, int cursor, PrometheusMetric } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int WriteUnitMetadata(byte[] buffer, int cursor, PrometheusMetric metric) + public static int WriteUnitMetadata(byte[] buffer, int cursor, PrometheusMetric metric, bool openMetricsRequested) { if (string.IsNullOrEmpty(metric.Unit)) { @@ -299,7 +316,7 @@ public static int WriteUnitMetadata(byte[] buffer, int cursor, PrometheusMetric } cursor = WriteAsciiStringNoEscape(buffer, cursor, "# UNIT "); - cursor = WriteMetricName(buffer, cursor, metric); + cursor = WriteMetricMetadataName(buffer, cursor, metric, openMetricsRequested); buffer[cursor++] = unchecked((byte)' '); diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializerExt.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializerExt.cs index 1523ef7c160..70d67f468bf 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializerExt.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializerExt.cs @@ -24,9 +24,9 @@ public static bool CanWriteMetric(Metric metric) public static int WriteMetric(byte[] buffer, int cursor, Metric metric, PrometheusMetric prometheusMetric, bool openMetricsRequested = false) { - cursor = WriteTypeMetadata(buffer, cursor, prometheusMetric); - cursor = WriteUnitMetadata(buffer, cursor, prometheusMetric); - cursor = WriteHelpMetadata(buffer, cursor, prometheusMetric, metric.Description); + cursor = WriteTypeMetadata(buffer, cursor, prometheusMetric, openMetricsRequested); + cursor = WriteUnitMetadata(buffer, cursor, prometheusMetric, openMetricsRequested); + cursor = WriteHelpMetadata(buffer, cursor, prometheusMetric, metric.Description, openMetricsRequested); if (!metric.MetricType.IsHistogram()) { @@ -35,7 +35,7 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric, Promethe var timestamp = metricPoint.EndTime.ToUnixTimeMilliseconds(); // Counter and Gauge - cursor = WriteMetricName(buffer, cursor, prometheusMetric); + cursor = WriteMetricName(buffer, cursor, prometheusMetric, openMetricsRequested); cursor = WriteTags(buffer, cursor, metric, metricPoint.Tags); buffer[cursor++] = unchecked((byte)' '); @@ -85,7 +85,7 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric, Promethe { totalCount += histogramMeasurement.BucketCount; - cursor = WriteMetricName(buffer, cursor, prometheusMetric); + cursor = WriteMetricName(buffer, cursor, prometheusMetric, openMetricsRequested); cursor = WriteAsciiStringNoEscape(buffer, cursor, "_bucket{"); cursor = WriteTags(buffer, cursor, metric, tags, writeEnclosingBraces: false); @@ -111,7 +111,7 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric, Promethe } // Histogram sum - cursor = WriteMetricName(buffer, cursor, prometheusMetric); + cursor = WriteMetricName(buffer, cursor, prometheusMetric, openMetricsRequested); cursor = WriteAsciiStringNoEscape(buffer, cursor, "_sum"); cursor = WriteTags(buffer, cursor, metric, metricPoint.Tags); @@ -125,7 +125,7 @@ public static int WriteMetric(byte[] buffer, int cursor, Metric metric, Promethe buffer[cursor++] = ASCII_LINEFEED; // Histogram count - cursor = WriteMetricName(buffer, cursor, prometheusMetric); + cursor = WriteMetricName(buffer, cursor, prometheusMetric, openMetricsRequested); cursor = WriteAsciiStringNoEscape(buffer, cursor, "_count"); cursor = WriteTags(buffer, cursor, metric, metricPoint.Tags); diff --git a/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs b/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs index 7386a5a9729..19b22e47be5 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs @@ -264,7 +264,7 @@ public async Task PrometheusExporterMiddlewareIntegration_CanServeOpenMetricsAnd var beginTimestamp = DateTimeOffset.Now.ToUnixTimeMilliseconds(); - var counter = meter.CreateCounter("counter_double"); + var counter = meter.CreateCounter("counter_double", unit: "By"); counter.Add(100.18D, tags); counter.Add(0.99D, tags); @@ -312,7 +312,7 @@ private static async Task RunPrometheusExporterMiddlewareIntegrationTest( var beginTimestamp = DateTimeOffset.Now.ToUnixTimeMilliseconds(); - var counter = meter.CreateCounter("counter_double"); + var counter = meter.CreateCounter("counter_double", unit: "By"); if (!skipMetrics) { counter.Add(100.18D, tags); @@ -368,14 +368,16 @@ private static async Task VerifyAsync(long beginTimestamp, long endTimestamp, Ht # TYPE otel_scope_info info # HELP otel_scope_info Scope metadata otel_scope_info{otel_scope_name="{{MeterName}}"} 1 - # TYPE counter_double_total counter - counter_double_total{otel_scope_name="{{MeterName}}",otel_scope_version="{{MeterVersion}}",key1="value1",key2="value2"} 101.17 (\d+\.\d{3}) + # TYPE counter_double_bytes counter + # UNIT counter_double_bytes bytes + counter_double_bytes_total{otel_scope_name="{{MeterName}}",otel_scope_version="{{MeterVersion}}",key1="value1",key2="value2"} 101.17 (\d+\.\d{3}) # EOF """.ReplaceLineEndings() : $$""" - # TYPE counter_double_total counter - counter_double_total{otel_scope_name="{{MeterName}}",otel_scope_version="{{MeterVersion}}",key1="value1",key2="value2"} 101.17 (\d+) + # TYPE counter_double_bytes_total counter + # UNIT counter_double_bytes_total bytes + counter_double_bytes_total{otel_scope_name="{{MeterName}}",otel_scope_version="{{MeterVersion}}",key1="value1",key2="value2"} 101.17 (\d+) # EOF """.ReplaceLineEndings(); diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs index 3a4ca1c4153..9ef62de196f 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs @@ -202,7 +202,7 @@ private async Task RunPrometheusExporterHttpServerIntegrationTest(bool skipMetri new KeyValuePair("key2", "value2"), }; - var counter = meter.CreateCounter("counter_double"); + var counter = meter.CreateCounter("counter_double", unit: "By"); if (!skipMetrics) { counter.Add(100.18D, tags); @@ -241,11 +241,13 @@ private async Task RunPrometheusExporterHttpServerIntegrationTest(bool skipMetri + "# TYPE otel_scope_info info\n" + "# HELP otel_scope_info Scope metadata\n" + $"otel_scope_info{{otel_scope_name='{MeterName}'}} 1\n" - + "# TYPE counter_double_total counter\n" - + $"counter_double_total{{otel_scope_name='{MeterName}',otel_scope_version='{MeterVersion}',key1='value1',key2='value2'}} 101.17 (\\d+\\.\\d{{3}})\n" + + "# TYPE counter_double_bytes counter\n" + + "# UNIT counter_double_bytes bytes\n" + + $"counter_double_bytes_total{{otel_scope_name='{MeterName}',otel_scope_version='{MeterVersion}',key1='value1',key2='value2'}} 101.17 (\\d+\\.\\d{{3}})\n" + "# EOF\n" - : "# TYPE counter_double_total counter\n" - + $"counter_double_total{{otel_scope_name='{MeterName}',otel_scope_version='{MeterVersion}',key1='value1',key2='value2'}} 101.17 (\\d+)\n" + : "# TYPE counter_double_bytes_total counter\n" + + "# UNIT counter_double_bytes_total bytes\n" + + $"counter_double_bytes_total{{otel_scope_name='{MeterName}',otel_scope_version='{MeterVersion}',key1='value1',key2='value2'}} 101.17 (\\d+)\n" + "# EOF\n"; Assert.Matches(("^" + expected + "$").Replace('\'', '"'), content); diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs index 81ce3e1f7da..d2990ceca70 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs @@ -26,7 +26,7 @@ public void SanitizeMetricName_SupportLeadingAndTrailingUnderscores() } [Fact] - public void SanitizeMetricName_RemoveUnsupportedChracters() + public void SanitizeMetricName_RemoveUnsupportedCharacters() { AssertSanitizeMetricName("metric_unit_$1000", "metric_unit_1000"); } @@ -116,13 +116,13 @@ public void Unit_AnnotationMismatch_Close() } [Fact] - public void Name_SpecialCaseGuage_AppendRatio() + public void Name_SpecialCaseGauge_AppendRatio() { AssertName("sample", "1", PrometheusType.Gauge, false, "sample_ratio"); } [Fact] - public void Name_GuageWithUnit_NoAppendRatio() + public void Name_GaugeWithUnit_NoAppendRatio() { AssertName("sample", "unit", PrometheusType.Gauge, false, "sample_unit"); } @@ -205,6 +205,66 @@ public void Name_StartWithNumber_UnderscoreStart() AssertName("2_metric_name", "By", PrometheusType.Summary, false, "_metric_name_bytes"); } + [Fact] + public void OpenMetricsName_UnitAlreadyPresentInName_Appended() + { + AssertOpenMetricsName("db_bytes_written", "By", PrometheusType.Gauge, false, "db_bytes_written_bytes"); + } + + [Fact] + public void OpenMetricsName_SuffixedWithUnit_NotAppended() + { + AssertOpenMetricsName("db_written_bytes", "By", PrometheusType.Gauge, false, "db_written_bytes"); + } + + [Fact] + public void OpenMetricsName_Counter_AppendTotal() + { + AssertOpenMetricsName("db_bytes_written", "By", PrometheusType.Counter, false, "db_bytes_written_bytes_total"); + } + + [Fact] + public void OpenMetricsName_Counter_DisableSuffixTotal_AppendTotal() + { + AssertOpenMetricsName("db_bytes_written", "By", PrometheusType.Counter, true, "db_bytes_written_bytes_total"); + } + + [Fact] + public void OpenMetricsName_CounterSuffixedWithTotal_AppendUnitAndTotal() + { + AssertOpenMetricsName("db_bytes_written_total", "By", PrometheusType.Counter, false, "db_bytes_written_bytes_total"); + } + + [Fact] + public void OpenMetricsName_CounterSuffixedWithTotal_DisableSuffixTotal_AppendTotal() + { + AssertOpenMetricsName("db_bytes_written_total", "By", PrometheusType.Counter, false, "db_bytes_written_bytes_total"); + } + + [Fact] + public void OpenMetricsName_SpecialCaseGauge_AppendRatio() + { + AssertOpenMetricsName("sample", "1", PrometheusType.Gauge, false, "sample_ratio"); + } + + [Fact] + public void OpenMetricsMetadataName_Counter_NotAppendTotal() + { + AssertOpenMetricsMetadataName("db_bytes_written", "By", PrometheusType.Counter, false, "db_bytes_written_bytes"); + } + + [Fact] + public void OpenMetricsMetadataName_Counter_DisableSuffixTotal_NotAppendTotal() + { + AssertOpenMetricsMetadataName("db_bytes_written", "By", PrometheusType.Counter, true, "db_bytes_written_bytes"); + } + + [Fact] + public void OpenMetricsMetadataName_SpecialCaseGauge_AppendRatio() + { + AssertOpenMetricsMetadataName("sample", "1", PrometheusType.Gauge, false, "sample_ratio"); + } + private static void AssertName( string name, string unit, PrometheusType type, bool disableTotalNameSuffixForCounters, string expected) { @@ -217,4 +277,18 @@ private static void AssertSanitizeMetricName(string name, string expected) var sanatizedName = PrometheusMetric.SanitizeMetricName(name); Assert.Equal(expected, sanatizedName); } + + private static void AssertOpenMetricsName( + string name, string unit, PrometheusType type, bool disableTotalNameSuffixForCounters, string expected) + { + var prometheusMetric = new PrometheusMetric(name, unit, type, disableTotalNameSuffixForCounters); + Assert.Equal(expected, prometheusMetric.OpenMetricsName); + } + + private static void AssertOpenMetricsMetadataName( + string name, string unit, PrometheusType type, bool disableTotalNameSuffixForCounters, string expected) + { + var prometheusMetric = new PrometheusMetric(name, unit, type, disableTotalNameSuffixForCounters); + Assert.Equal(expected, prometheusMetric.OpenMetricsMetadataName); + } } From 1a1ad5472050a9f5930903c54eee9389e717d06c Mon Sep 17 00:00:00 2001 From: Robert Coltheart Date: Thu, 23 May 2024 06:10:02 +1000 Subject: [PATCH 2/6] Update changelog --- src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md | 2 +- src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md index c4d7b083aba..ffac72e9362 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md @@ -3,7 +3,7 @@ ## Unreleased * Fixed issue with OpenMetrics counter suffixes for Prometheus - ([#5623](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5623)) + ([#5646](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5646)) ## 1.9.0-alpha.1 diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md index 19b77b88a2d..cf01c0556de 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md @@ -3,7 +3,7 @@ ## Unreleased * Fixed issue with OpenMetrics counter suffixes for Prometheus - ([#5623](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5623)) + ([#5646](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5646)) ## 1.9.0-alpha.1 From 62df371fc72f59d0b33c7856e78955fcb6cf2b2d Mon Sep 17 00:00:00 2001 From: Robert Coltheart Date: Thu, 23 May 2024 06:10:33 +1000 Subject: [PATCH 3/6] Update changelog --- src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md | 2 +- src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md index ffac72e9362..e673dbfb822 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -* Fixed issue with OpenMetrics counter suffixes for Prometheus +* Fixed issue with OpenMetrics suffixes for Prometheus ([#5646](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5646)) ## 1.9.0-alpha.1 diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md index cf01c0556de..717e61290b8 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -* Fixed issue with OpenMetrics counter suffixes for Prometheus +* Fixed issue with OpenMetrics suffixes for Prometheus ([#5646](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5646)) ## 1.9.0-alpha.1 From 2f1eef7aaff6ba303958ac487429950226fc1604 Mon Sep 17 00:00:00 2001 From: Robert Coltheart Date: Fri, 24 May 2024 10:06:37 +1000 Subject: [PATCH 4/6] Remove ratio special case, combine _total in names --- .../Internal/PrometheusMetric.cs | 56 +++++++------------ .../PrometheusMetricTests.cs | 18 ------ 2 files changed, 21 insertions(+), 53 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs index 688c2e81b16..c4a1fad9e7f 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs @@ -36,40 +36,19 @@ public PrometheusMetric(string name, string unit, PrometheusType type, bool disa // The resulting unit SHOULD be added to the metric as // [OpenMetrics UNIT metadata](https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metricfamily) - // and as a suffix to the metric name unless the metric name already contains the - // unit, or the unit MUST be omitted. The unit suffix comes before any - // type-specific suffixes. - // https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#L242-L246 - if (!sanitizedName.Contains(sanitizedUnit)) + // and as a suffix to the metric name. The unit suffix comes before any type-specific suffixes. + // https://github.com/open-telemetry/opentelemetry-specification/blob/3dfb383fe583e3b74a2365c5a1d90256b273ee76/specification/compatibility/prometheus_and_openmetrics.md#metric-metadata-1 + if (!sanitizedName.EndsWith(sanitizedUnit)) { sanitizedName += $"_{sanitizedUnit}"; - } - - // OpenMetrics name MUST be suffixed with '_{unit}', regardless of whether the unit name appears within the text. - // Note that this may change in the future, however for the moment Prometheus will fail to read the metric using - // OpenMetrics format unless the suffix matches the unit. - // https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#unit - if (!openMetricsName.EndsWith(sanitizedUnit)) - { openMetricsName += $"_{sanitizedUnit}"; } } - // Special case: Converting "1" to "ratio". - // https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#L239 - if (type == PrometheusType.Gauge && unit == "1" && !sanitizedName.Contains("ratio")) - { - sanitizedName += "_ratio"; - openMetricsName += "_ratio"; - } - - // For TYPE, HELP and UNIT declarations for counters, the suffix '_total' is omitted in OpenMetrics format. - // https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#counter-1 - this.OpenMetricsMetadataName = openMetricsName; - // If the metric name for monotonic Sum metric points does not end in a suffix of `_total` a suffix of `_total` MUST be added by default, otherwise the name MUST remain unchanged. // Exporters SHOULD provide a configuration option to disable the addition of `_total` suffixes. // https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#L286 + // Note that we no longer append '_ratio' for units that are '1', see: https://github.com/open-telemetry/opentelemetry-specification/issues/4058 if (type == PrometheusType.Counter && !sanitizedName.EndsWith("_total") && !disableTotalNameSuffixForCounters) { sanitizedName += "_total"; @@ -82,8 +61,15 @@ public PrometheusMetric(string name, string unit, PrometheusType type, bool disa openMetricsName += "_total"; } + // In OpenMetrics format, the UNIT, TYPE and HELP metadata must be suffixed with the unit (handled above), and not the '_total' suffix, as in the case for counters. + // https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#unit + var openMetricsMetadataName = type == PrometheusType.Counter + ? SanitizeOpenMetricsName(openMetricsName) + : sanitizedName; + this.Name = sanitizedName; this.OpenMetricsName = openMetricsName; + this.OpenMetricsMetadataName = openMetricsMetadataName; this.Unit = sanitizedUnit; this.Type = type; } @@ -142,6 +128,16 @@ internal static string SanitizeMetricName(string metricName) static StringBuilder CreateStringBuilder(string name) => new StringBuilder(name.Length); } + private static string SanitizeOpenMetricsName(string metricName) + { + if (metricName.EndsWith("_total")) + { + return metricName.Substring(0, metricName.Length - 6); + } + + return metricName; + } + internal static string RemoveAnnotations(string unit) { // UCUM standard says the curly braces shouldn't be nested: @@ -186,16 +182,6 @@ internal static string RemoveAnnotations(string unit) return sb.ToString(); } - private static string SanitizeOpenMetricsName(string metricName) - { - if (metricName.EndsWith("_total")) - { - return metricName.Substring(0, metricName.Length - 6); - } - - return metricName; - } - private static string GetUnit(string unit) { // Dropping the portions of the Unit within brackets (e.g. {packet}). Brackets MUST NOT be included in the resulting unit. A "count of foo" is considered unitless in Prometheus. diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs index d2990ceca70..a1350f03ec9 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs @@ -115,12 +115,6 @@ public void Unit_AnnotationMismatch_Close() Assert.Equal(string.Empty, PrometheusMetric.RemoveAnnotations("{{one}")); } - [Fact] - public void Name_SpecialCaseGauge_AppendRatio() - { - AssertName("sample", "1", PrometheusType.Gauge, false, "sample_ratio"); - } - [Fact] public void Name_GaugeWithUnit_NoAppendRatio() { @@ -241,12 +235,6 @@ public void OpenMetricsName_CounterSuffixedWithTotal_DisableSuffixTotal_AppendTo AssertOpenMetricsName("db_bytes_written_total", "By", PrometheusType.Counter, false, "db_bytes_written_bytes_total"); } - [Fact] - public void OpenMetricsName_SpecialCaseGauge_AppendRatio() - { - AssertOpenMetricsName("sample", "1", PrometheusType.Gauge, false, "sample_ratio"); - } - [Fact] public void OpenMetricsMetadataName_Counter_NotAppendTotal() { @@ -259,12 +247,6 @@ public void OpenMetricsMetadataName_Counter_DisableSuffixTotal_NotAppendTotal() AssertOpenMetricsMetadataName("db_bytes_written", "By", PrometheusType.Counter, true, "db_bytes_written_bytes"); } - [Fact] - public void OpenMetricsMetadataName_SpecialCaseGauge_AppendRatio() - { - AssertOpenMetricsMetadataName("sample", "1", PrometheusType.Gauge, false, "sample_ratio"); - } - private static void AssertName( string name, string unit, PrometheusType type, bool disableTotalNameSuffixForCounters, string expected) { From 0eab0649208b75b5947cd41335a059ed8d6c5cc2 Mon Sep 17 00:00:00 2001 From: Robert Coltheart Date: Fri, 24 May 2024 10:20:22 +1000 Subject: [PATCH 5/6] Fix linting --- .../Internal/PrometheusMetric.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs index c4a1fad9e7f..6fb5ccb80a0 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs @@ -128,16 +128,6 @@ internal static string SanitizeMetricName(string metricName) static StringBuilder CreateStringBuilder(string name) => new StringBuilder(name.Length); } - private static string SanitizeOpenMetricsName(string metricName) - { - if (metricName.EndsWith("_total")) - { - return metricName.Substring(0, metricName.Length - 6); - } - - return metricName; - } - internal static string RemoveAnnotations(string unit) { // UCUM standard says the curly braces shouldn't be nested: @@ -182,6 +172,16 @@ internal static string RemoveAnnotations(string unit) return sb.ToString(); } + private static string SanitizeOpenMetricsName(string metricName) + { + if (metricName.EndsWith("_total")) + { + return metricName.Substring(0, metricName.Length - 6); + } + + return metricName; + } + private static string GetUnit(string unit) { // Dropping the portions of the Unit within brackets (e.g. {packet}). Brackets MUST NOT be included in the resulting unit. A "count of foo" is considered unitless in Prometheus. From 28787835f9b29851b172e172ec6cf64d58c22ae0 Mon Sep 17 00:00:00 2001 From: Robert Coltheart Date: Fri, 24 May 2024 11:28:52 +1000 Subject: [PATCH 6/6] Fix linting again --- .../Internal/PrometheusMetric.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs index 6fb5ccb80a0..6f18f06a536 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs @@ -61,7 +61,7 @@ public PrometheusMetric(string name, string unit, PrometheusType type, bool disa openMetricsName += "_total"; } - // In OpenMetrics format, the UNIT, TYPE and HELP metadata must be suffixed with the unit (handled above), and not the '_total' suffix, as in the case for counters. + // In OpenMetrics format, the UNIT, TYPE and HELP metadata must be suffixed with the unit (handled above), and not the '_total' suffix, as in the case for counters. // https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#unit var openMetricsMetadataName = type == PrometheusType.Counter ? SanitizeOpenMetricsName(openMetricsName)