Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to not add _total suffix to counter metrics in Prometheus exporter #5305

Merged
merged 9 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Microsoft.AspNetCore.Builder.PrometheusExporterApplicationBuilderExtensions
Microsoft.AspNetCore.Builder.PrometheusExporterEndpointRouteBuilderExtensions
OpenTelemetry.Exporter.PrometheusAspNetCoreOptions
OpenTelemetry.Exporter.PrometheusAspNetCoreOptions.DisableTotalNameSuffixForCounters.get -> bool
OpenTelemetry.Exporter.PrometheusAspNetCoreOptions.DisableTotalNameSuffixForCounters.set -> void
OpenTelemetry.Exporter.PrometheusAspNetCoreOptions.PrometheusAspNetCoreOptions() -> void
OpenTelemetry.Exporter.PrometheusAspNetCoreOptions.ScrapeEndpointPath.get -> string
OpenTelemetry.Exporter.PrometheusAspNetCoreOptions.ScrapeEndpointPath.set -> void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Added option to disable _total suffix addition to counter metrics in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to mention "in Prometheus exporter". This CHANGELOG file is specific to Prometheus exporter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Prometheus exporter
([#5305](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5305))

* Export OpenMetrics format from Prometheus exporters ([#5107](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5107))
* For requests with OpenMetrics format, scope info is automatically added
([#5086](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5086)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ public class PrometheusAspNetCoreOptions
/// </summary>
public string ScrapeEndpointPath { get; set; } = DefaultScrapeEndpointPath;

/// <summary>
/// Gets or sets a value indicating whether addition of _total suffix for counter metric names is disabled. Default value: <see langword="false"/>.
/// </summary>
public bool DisableTotalNameSuffixForCounters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public bool EnableCounterTotalSuffix = true - alternate suggestion.

The default should be true as per https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/compatibility/prometheus_and_openmetrics.md#sums

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exporters SHOULD provide a configuration option to disable..

So I think DisableTotalNameSuffixForCounters will be better

Bassardes marked this conversation as resolved.
Show resolved Hide resolved
{
get => this.ExporterOptions.DisableTotalNameSuffixForCounters;
set => this.ExporterOptions.DisableTotalNameSuffixForCounters = value;
}

/// <summary>
/// Gets or sets the cache duration in milliseconds for scrape responses. Default value: 300.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
OpenTelemetry.Exporter.PrometheusHttpListenerOptions
OpenTelemetry.Exporter.PrometheusHttpListenerOptions.DisableTotalNameSuffixForCounters.get -> bool
OpenTelemetry.Exporter.PrometheusHttpListenerOptions.DisableTotalNameSuffixForCounters.set -> void
OpenTelemetry.Exporter.PrometheusHttpListenerOptions.UriPrefixes.get -> System.Collections.Generic.IReadOnlyCollection<string>
OpenTelemetry.Exporter.PrometheusHttpListenerOptions.UriPrefixes.set -> void
OpenTelemetry.Exporter.PrometheusHttpListenerOptions.PrometheusHttpListenerOptions() -> void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Added option to disable _total suffix addition to counter metrics in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Prometheus exporter
([#5305](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5305))

* Export OpenMetrics format from Prometheus exporters
([#5107](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5107))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ private PrometheusMetric GetPrometheusMetric(Metric metric)
// Optimize writing metrics with bounded cache that has pre-calculated Prometheus names.
if (!this.metricsCache.TryGetValue(metric, out var prometheusMetric))
{
prometheusMetric = PrometheusMetric.Create(metric);
prometheusMetric = PrometheusMetric.Create(metric, this.exporter.DisableTotalNameSuffixForCounters);

// Add to the cache if there is space.
if (this.metricsCacheCount < MaxCachedMetrics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public PrometheusExporter(PrometheusExporterOptions options)
Guard.ThrowIfNull(options);

this.ScrapeResponseCacheDurationMilliseconds = options.ScrapeResponseCacheDurationMilliseconds;
this.DisableTotalNameSuffixForCounters = options.DisableTotalNameSuffixForCounters;

this.CollectionManager = new PrometheusCollectionManager(this);
}
Expand All @@ -50,6 +51,8 @@ internal Func<Batch<Metric>, ExportResult> OnExport

internal int ScrapeResponseCacheDurationMilliseconds { get; }

internal bool DisableTotalNameSuffixForCounters { get; }

internal bool OpenMetricsRequested { get; set; }

/// <inheritdoc/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ public int ScrapeResponseCacheDurationMilliseconds
this.scrapeResponseCacheDurationMilliseconds = value;
}
}

/// <summary>
/// Gets or sets a value indicating whether addition of _total suffix for counter metric names is disabled. Default value: <see langword="false"/>.
/// </summary>
public bool DisableTotalNameSuffixForCounters { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ UpDownCounter becomes gauge
PrometheusType.Untyped, PrometheusType.Counter, PrometheusType.Gauge, PrometheusType.Summary, PrometheusType.Histogram, PrometheusType.Histogram, PrometheusType.Histogram, PrometheusType.Histogram, PrometheusType.Gauge,
};

public PrometheusMetric(string name, string unit, PrometheusType type)
public PrometheusMetric(string name, string unit, PrometheusType type, bool disableTotalNameSuffixForCounters)
{
// The metric name is
// required to match the regex: `[a-zA-Z_:]([a-zA-Z0-9_:])*`. Invalid characters
Expand Down Expand Up @@ -48,7 +48,7 @@ public PrometheusMetric(string name, string unit, PrometheusType type)
// 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
if (type == PrometheusType.Counter && !sanitizedName.EndsWith("_total"))
if (type == PrometheusType.Counter && !sanitizedName.EndsWith("_total") && !disableTotalNameSuffixForCounters)
{
sanitizedName += "_total";
}
Expand All @@ -71,9 +71,9 @@ public PrometheusMetric(string name, string unit, PrometheusType type)

public PrometheusType Type { get; }

public static PrometheusMetric Create(Metric metric)
public static PrometheusMetric Create(Metric metric, bool disableTotalNameSuffixForCounters)
{
return new PrometheusMetric(metric.Name, metric.Unit, GetPrometheusType(metric));
return new PrometheusMetric(metric.Name, metric.Unit, GetPrometheusType(metric), disableTotalNameSuffixForCounters);
}

internal static string SanitizeMetricName(string metricName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ public static MeterProviderBuilder AddPrometheusHttpListener(
private static MetricReader BuildPrometheusHttpListenerMetricReader(
PrometheusHttpListenerOptions options)
{
var exporter = new PrometheusExporter(new PrometheusExporterOptions { ScrapeResponseCacheDurationMilliseconds = 0 });
var exporter = new PrometheusExporter(new PrometheusExporterOptions
{
ScrapeResponseCacheDurationMilliseconds = 0,
DisableTotalNameSuffixForCounters = options.DisableTotalNameSuffixForCounters,
});

var reader = new BaseExportingMetricReader(exporter)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ public class PrometheusHttpListenerOptions
/// </summary>
public string ScrapeEndpointPath { get; set; } = "/metrics";

/// <summary>
/// Gets or sets a value indicating whether addition of _total suffix for counter metric names is disabled. Default value: <see langword="false"/>.
/// </summary>
public bool DisableTotalNameSuffixForCounters { get; set; }

/// <summary>
/// Gets or sets the URI (Uniform Resource Identifier) prefixes to use for the http listener.
/// Default value: <c>["http://localhost:9464/"]</c>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private PrometheusMetric GetPrometheusMetric(Metric metric)
{
if (!this.cache.TryGetValue(metric, out var prometheusMetric))
{
prometheusMetric = PrometheusMetric.Create(metric);
prometheusMetric = PrometheusMetric.Create(metric, false);
this.cache[metric] = prometheusMetric;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,84 +118,97 @@ public void Unit_AnnotationMismatch_Close()
[Fact]
public void Name_SpecialCaseGuage_AppendRatio()
{
AssertName("sample", "1", PrometheusType.Gauge, "sample_ratio");
AssertName("sample", "1", PrometheusType.Gauge, false, "sample_ratio");
}

[Fact]
public void Name_GuageWithUnit_NoAppendRatio()
{
AssertName("sample", "unit", PrometheusType.Gauge, "sample_unit");
AssertName("sample", "unit", PrometheusType.Gauge, false, "sample_unit");
}

[Fact]
public void Name_SpecialCaseCounter_AppendTotal()
{
AssertName("sample", "unit", PrometheusType.Counter, "sample_unit_total");
AssertName("sample", "unit", PrometheusType.Counter, false, "sample_unit_total");
}

[Fact]
public void Name_SpecialCaseCounterWithoutUnit_DropUnitAppendTotal()
{
AssertName("sample", "1", PrometheusType.Counter, "sample_total");
AssertName("sample", "1", PrometheusType.Counter, false, "sample_total");
}

[Fact]
public void Name_DisableTotalSuffixAddition_TotalNotAppended()
{
AssertName("sample", "1", PrometheusType.Counter, true, "sample");
}

[Fact]
public void Name_TotalSuffixAlreadyPresent_DisableTotalSuffixAddition_TotalNotRemoved()
{
AssertName("sample_total", "1", PrometheusType.Counter, true, "sample_total");
}

[Fact]
public void Name_SpecialCaseCounterWithNumber_AppendTotal()
{
AssertName("sample", "2", PrometheusType.Counter, "sample_2_total");
AssertName("sample", "2", PrometheusType.Counter, false, "sample_2_total");
}

[Fact]
public void Name_UnsupportedMetricNameChars_Drop()
{
AssertName("s%%ple", "%/m", PrometheusType.Summary, "s_ple_percent_per_minute");
AssertName("s%%ple", "%/m", PrometheusType.Summary, false, "s_ple_percent_per_minute");
}

[Fact]
public void Name_UnitOtherThanOne_Normal()
{
AssertName("metric_name", "2", PrometheusType.Summary, "metric_name_2");
AssertName("metric_name", "2", PrometheusType.Summary, false, "metric_name_2");
}

[Fact]
public void Name_UnitAlreadyPresentInName_NotAppended()
{
AssertName("metric_name_total", "total", PrometheusType.Counter, "metric_name_total");
AssertName("metric_name_total", "total", PrometheusType.Counter, false, "metric_name_total");
Bassardes marked this conversation as resolved.
Show resolved Hide resolved
}

[Fact]
public void Name_UnitAlreadyPresentInName_TotalNonCounterType_NotAppended()
{
AssertName("metric_name_total", "total", PrometheusType.Summary, "metric_name_total");
AssertName("metric_name_total", "total", PrometheusType.Summary, false, "metric_name_total");
}

[Fact]
public void Name_UnitAlreadyPresentInName_CustomGauge_NotAppended()
{
AssertName("metric_hertz", "hertz", PrometheusType.Gauge, "metric_hertz");
AssertName("metric_hertz", "hertz", PrometheusType.Gauge, false, "metric_hertz");
}

[Fact]
public void Name_UnitAlreadyPresentInName_CustomCounter_NotAppended()
{
AssertName("metric_hertz_total", "hertz_total", PrometheusType.Counter, "metric_hertz_total");
AssertName("metric_hertz_total", "hertz_total", PrometheusType.Counter, false, "metric_hertz_total");
}

[Fact]
public void Name_UnitAlreadyPresentInName_OrderMatters_Appended()
{
AssertName("metric_total_hertz", "hertz_total", PrometheusType.Counter, "metric_total_hertz_hertz_total");
AssertName("metric_total_hertz", "hertz_total", PrometheusType.Counter, false, "metric_total_hertz_hertz_total");
}

[Fact]
public void Name_StartWithNumber_UnderscoreStart()
{
AssertName("2_metric_name", "By", PrometheusType.Summary, "_metric_name_bytes");
AssertName("2_metric_name", "By", PrometheusType.Summary, false, "_metric_name_bytes");
}

private static void AssertName(string name, string unit, PrometheusType type, string expected)
private static void AssertName(
string name, string unit, PrometheusType type, bool disableTotalNameSuffixForCounters, string expected)
{
var prometheusMetric = new PrometheusMetric(name, unit, type);
var prometheusMetric = new PrometheusMetric(name, unit, type, disableTotalNameSuffixForCounters);
Assert.Equal(expected, prometheusMetric.Name);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,6 @@ public void HistogramOneDimensionWithScopeVersion()

private static int WriteMetric(byte[] buffer, int cursor, Metric metric, bool useOpenMetrics = false)
{
return PrometheusSerializer.WriteMetric(buffer, cursor, metric, PrometheusMetric.Create(metric), useOpenMetrics);
return PrometheusSerializer.WriteMetric(buffer, cursor, metric, PrometheusMetric.Create(metric, false), useOpenMetrics);
}
}