diff --git a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md index 8168dd9e043..25396307f1d 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Fixed an issue with corrupted buffers when reading both OpenMetrics and + plain text formats from Prometheus exporters. + ([#5623](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5623)) + ## 1.8.0-rc.1 Released 2024-Mar-27 diff --git a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs index 7e815c3e18c..fdfcd2b7113 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs @@ -57,7 +57,9 @@ public async Task InvokeAsync(HttpContext httpContext) try { - if (collectionResponse.View.Count > 0) + var dataView = openMetricsRequested ? collectionResponse.OpenMetricsView : collectionResponse.PlainTextView; + + if (dataView.Count > 0) { response.StatusCode = 200; #if NET8_0_OR_GREATER @@ -69,7 +71,7 @@ public async Task InvokeAsync(HttpContext httpContext) ? "application/openmetrics-text; version=1.0.0; charset=utf-8" : "text/plain; charset=utf-8; version=0.0.4"; - await response.Body.WriteAsync(collectionResponse.View.Array, 0, collectionResponse.View.Count).ConfigureAwait(false); + await response.Body.WriteAsync(dataView.Array, 0, dataView.Count).ConfigureAwait(false); } else { diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md index ba522fc60a4..c9944512d32 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Fixed an issue with corrupted buffers when reading both OpenMetrics and + plain text formats from Prometheus exporters. + ([#5623](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5623)) + ## 1.8.0-rc.1 Released 2024-Mar-27 diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs index 27ab845164b..b93812176fb 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs @@ -16,11 +16,14 @@ internal sealed class PrometheusCollectionManager private readonly Dictionary metricsCache; private readonly HashSet scopes; private int metricsCacheCount; - private byte[] buffer = new byte[85000]; // encourage the object to live in LOH (large object heap) + private byte[] plainTextBuffer = new byte[85000]; // encourage the object to live in LOH (large object heap) + private byte[] openMetricsBuffer = new byte[85000]; // encourage the object to live in LOH (large object heap) private int targetInfoBufferLength = -1; // zero or positive when target_info has been written for the first time + private ArraySegment previousPlainTextDataView; + private ArraySegment previousOpenMetricsDataView; private int globalLockState; - private ArraySegment previousDataView; - private DateTime? previousDataViewGeneratedAtUtc; + private DateTime? previousPlainTextDataViewGeneratedAtUtc; + private DateTime? previousOpenMetricsDataViewGeneratedAtUtc; private int readerCount; private bool collectionRunning; private TaskCompletionSource collectionTcs; @@ -44,16 +47,20 @@ public Task EnterCollect(bool openMetricsRequested) // If we are within {ScrapeResponseCacheDurationMilliseconds} of the // last successful collect, return the previous view. - if (this.previousDataViewGeneratedAtUtc.HasValue + var previousDataViewGeneratedAtUtc = openMetricsRequested + ? this.previousOpenMetricsDataViewGeneratedAtUtc + : this.previousPlainTextDataViewGeneratedAtUtc; + + if (previousDataViewGeneratedAtUtc.HasValue && this.scrapeResponseCacheDurationMilliseconds > 0 - && this.previousDataViewGeneratedAtUtc.Value.AddMilliseconds(this.scrapeResponseCacheDurationMilliseconds) >= DateTime.UtcNow) + && previousDataViewGeneratedAtUtc.Value.AddMilliseconds(this.scrapeResponseCacheDurationMilliseconds) >= DateTime.UtcNow) { Interlocked.Increment(ref this.readerCount); this.ExitGlobalLock(); #if NET6_0_OR_GREATER - return new ValueTask(new CollectionResponse(this.previousDataView, this.previousDataViewGeneratedAtUtc.Value, fromCache: true)); + return new ValueTask(new CollectionResponse(this.previousOpenMetricsDataView, this.previousPlainTextDataView, previousDataViewGeneratedAtUtc.Value, fromCache: true)); #else - return Task.FromResult(new CollectionResponse(this.previousDataView, this.previousDataViewGeneratedAtUtc.Value, fromCache: true)); + return Task.FromResult(new CollectionResponse(this.previousOpenMetricsDataView, this.previousPlainTextDataView, previousDataViewGeneratedAtUtc.Value, fromCache: true)); #endif } @@ -78,7 +85,16 @@ public Task EnterCollect(bool openMetricsRequested) // Start a collection on the current thread. this.collectionRunning = true; - this.previousDataViewGeneratedAtUtc = null; + + if (openMetricsRequested) + { + this.previousOpenMetricsDataViewGeneratedAtUtc = null; + } + else + { + this.previousPlainTextDataViewGeneratedAtUtc = null; + } + Interlocked.Increment(ref this.readerCount); this.ExitGlobalLock(); @@ -86,8 +102,20 @@ public Task EnterCollect(bool openMetricsRequested) var result = this.ExecuteCollect(openMetricsRequested); if (result) { - this.previousDataViewGeneratedAtUtc = DateTime.UtcNow; - response = new CollectionResponse(this.previousDataView, this.previousDataViewGeneratedAtUtc.Value, fromCache: false); + if (openMetricsRequested) + { + this.previousOpenMetricsDataViewGeneratedAtUtc = DateTime.UtcNow; + } + else + { + this.previousPlainTextDataViewGeneratedAtUtc = DateTime.UtcNow; + } + + previousDataViewGeneratedAtUtc = openMetricsRequested + ? this.previousOpenMetricsDataViewGeneratedAtUtc + : this.previousPlainTextDataViewGeneratedAtUtc; + + response = new CollectionResponse(this.previousOpenMetricsDataView, this.previousPlainTextDataView, previousDataViewGeneratedAtUtc.Value, fromCache: false); } else { @@ -170,6 +198,7 @@ private bool ExecuteCollect(bool openMetricsRequested) private ExportResult OnCollect(Batch metrics) { var cursor = 0; + var buffer = this.exporter.OpenMetricsRequested ? this.openMetricsBuffer : this.plainTextBuffer; try { @@ -192,13 +221,13 @@ private ExportResult OnCollect(Batch metrics) { try { - cursor = PrometheusSerializer.WriteScopeInfo(this.buffer, cursor, metric.MeterName); + cursor = PrometheusSerializer.WriteScopeInfo(buffer, cursor, metric.MeterName); break; } catch (IndexOutOfRangeException) { - if (!this.IncreaseBufferSize()) + if (!this.IncreaseBufferSize(ref buffer)) { // there are two cases we might run into the following condition: // 1. we have many metrics to be exported - in this case we probably want @@ -226,7 +255,7 @@ private ExportResult OnCollect(Batch metrics) try { cursor = PrometheusSerializer.WriteMetric( - this.buffer, + buffer, cursor, metric, this.GetPrometheusMetric(metric), @@ -236,7 +265,7 @@ private ExportResult OnCollect(Batch metrics) } catch (IndexOutOfRangeException) { - if (!this.IncreaseBufferSize()) + if (!this.IncreaseBufferSize(ref buffer)) { throw; } @@ -248,24 +277,40 @@ private ExportResult OnCollect(Batch metrics) { try { - cursor = PrometheusSerializer.WriteEof(this.buffer, cursor); + cursor = PrometheusSerializer.WriteEof(buffer, cursor); break; } catch (IndexOutOfRangeException) { - if (!this.IncreaseBufferSize()) + if (!this.IncreaseBufferSize(ref buffer)) { throw; } } } - this.previousDataView = new ArraySegment(this.buffer, 0, cursor); + if (this.exporter.OpenMetricsRequested) + { + this.previousOpenMetricsDataView = new ArraySegment(this.openMetricsBuffer, 0, cursor); + } + else + { + this.previousPlainTextDataView = new ArraySegment(this.plainTextBuffer, 0, cursor); + } + return ExportResult.Success; } catch (Exception) { - this.previousDataView = new ArraySegment(Array.Empty(), 0, 0); + if (this.exporter.OpenMetricsRequested) + { + this.previousOpenMetricsDataView = new ArraySegment(Array.Empty(), 0, 0); + } + else + { + this.previousPlainTextDataView = new ArraySegment(Array.Empty(), 0, 0); + } + return ExportResult.Failure; } } @@ -278,13 +323,13 @@ private int WriteTargetInfo() { try { - this.targetInfoBufferLength = PrometheusSerializer.WriteTargetInfo(this.buffer, 0, this.exporter.Resource); + this.targetInfoBufferLength = PrometheusSerializer.WriteTargetInfo(this.openMetricsBuffer, 0, this.exporter.Resource); break; } catch (IndexOutOfRangeException) { - if (!this.IncreaseBufferSize()) + if (!this.IncreaseBufferSize(ref this.openMetricsBuffer)) { throw; } @@ -295,9 +340,9 @@ private int WriteTargetInfo() return this.targetInfoBufferLength; } - private bool IncreaseBufferSize() + private bool IncreaseBufferSize(ref byte[] buffer) { - var newBufferSize = this.buffer.Length * 2; + var newBufferSize = buffer.Length * 2; if (newBufferSize > 100 * 1024 * 1024) { @@ -305,8 +350,8 @@ private bool IncreaseBufferSize() } var newBuffer = new byte[newBufferSize]; - this.buffer.CopyTo(newBuffer, 0); - this.buffer = newBuffer; + buffer.CopyTo(newBuffer, 0); + buffer = newBuffer; return true; } @@ -331,14 +376,17 @@ private PrometheusMetric GetPrometheusMetric(Metric metric) public readonly struct CollectionResponse { - public CollectionResponse(ArraySegment view, DateTime generatedAtUtc, bool fromCache) + public CollectionResponse(ArraySegment openMetricsView, ArraySegment plainTextView, DateTime generatedAtUtc, bool fromCache) { - this.View = view; + this.OpenMetricsView = openMetricsView; + this.PlainTextView = plainTextView; this.GeneratedAtUtc = generatedAtUtc; this.FromCache = fromCache; } - public ArraySegment View { get; } + public ArraySegment OpenMetricsView { get; } + + public ArraySegment PlainTextView { get; } public DateTime GeneratedAtUtc { get; } diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs index 6f9663ba429..8576873f520 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs @@ -153,7 +153,10 @@ private async Task ProcessRequestAsync(HttpListenerContext context) try { context.Response.Headers.Add("Server", string.Empty); - if (collectionResponse.View.Count > 0) + + var dataView = openMetricsRequested ? collectionResponse.OpenMetricsView : collectionResponse.PlainTextView; + + if (dataView.Count > 0) { context.Response.StatusCode = 200; context.Response.Headers.Add("Last-Modified", collectionResponse.GeneratedAtUtc.ToString("R")); @@ -161,7 +164,7 @@ private async Task ProcessRequestAsync(HttpListenerContext context) ? "application/openmetrics-text; version=1.0.0; charset=utf-8" : "text/plain; charset=utf-8; version=0.0.4"; - await context.Response.OutputStream.WriteAsync(collectionResponse.View.Array, 0, collectionResponse.View.Count).ConfigureAwait(false); + await context.Response.OutputStream.WriteAsync(dataView.Array, 0, dataView.Count).ConfigureAwait(false); } else { diff --git a/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs b/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs index 46da01e6417..7386a5a9729 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs @@ -248,6 +248,46 @@ public Task PrometheusExporterMiddlewareIntegration_UseOpenMetricsVersionHeader( acceptHeader: "application/openmetrics-text; version=1.0.0"); } + [Fact] + public async Task PrometheusExporterMiddlewareIntegration_CanServeOpenMetricsAndPlainFormats() + { + using var host = await StartTestHostAsync( + app => app.UseOpenTelemetryPrometheusScrapingEndpoint()); + + var tags = new KeyValuePair[] + { + new KeyValuePair("key1", "value1"), + new KeyValuePair("key2", "value2"), + }; + + using var meter = new Meter(MeterName, MeterVersion); + + var beginTimestamp = DateTimeOffset.Now.ToUnixTimeMilliseconds(); + + var counter = meter.CreateCounter("counter_double"); + counter.Add(100.18D, tags); + counter.Add(0.99D, tags); + + var testCases = new bool[] { true, false, true, true, false }; + + using var client = host.GetTestClient(); + + foreach (var testCase in testCases) + { + using var request = new HttpRequestMessage + { + Headers = { { "Accept", testCase ? "application/openmetrics-text" : "text/plain" } }, + RequestUri = new Uri("/metrics", UriKind.Relative), + Method = HttpMethod.Get, + }; + using var response = await client.SendAsync(request); + var endTimestamp = DateTimeOffset.Now.ToUnixTimeMilliseconds(); + await VerifyAsync(beginTimestamp, endTimestamp, response, testCase); + } + + await host.StopAsync(); + } + private static async Task RunPrometheusExporterMiddlewareIntegrationTest( string path, Action configure, @@ -260,26 +300,7 @@ private static async Task RunPrometheusExporterMiddlewareIntegrationTest( { var requestOpenMetrics = acceptHeader.StartsWith("application/openmetrics-text"); - using var host = await new HostBuilder() - .ConfigureWebHost(webBuilder => webBuilder - .UseTestServer() - .ConfigureServices(services => - { - if (registerMeterProvider) - { - services.AddOpenTelemetry().WithMetrics(builder => builder - .ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1")) - .AddMeter(MeterName) - .AddPrometheusExporter(o => - { - configureOptions?.Invoke(o); - })); - } - - configureServices?.Invoke(services); - }) - .Configure(configure)) - .StartAsync(); + using var host = await StartTestHostAsync(configure, configureServices, registerMeterProvider, configureOptions); var tags = new KeyValuePair[] { @@ -310,51 +331,90 @@ private static async Task RunPrometheusExporterMiddlewareIntegrationTest( var endTimestamp = DateTimeOffset.Now.ToUnixTimeMilliseconds(); if (!skipMetrics) + { + await VerifyAsync(beginTimestamp, endTimestamp, response, requestOpenMetrics); + } + else { Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.True(response.Content.Headers.Contains("Last-Modified")); - - if (requestOpenMetrics) - { - Assert.Equal("application/openmetrics-text; version=1.0.0; charset=utf-8", response.Content.Headers.ContentType.ToString()); - } - else - { - Assert.Equal("text/plain; charset=utf-8; version=0.0.4", response.Content.Headers.ContentType.ToString()); - } - - string content = await response.Content.ReadAsStringAsync(); - - string expected = requestOpenMetrics - ? "# TYPE target info\n" - + "# HELP target Target metadata\n" - + "target_info{service_name='my_service',service_instance_id='id1'} 1\n" - + "# 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" - + "# 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" - + "# EOF\n"; + } - var matches = Regex.Matches(content, ("^" + expected + "$").Replace('\'', '"')); + validateResponse?.Invoke(response); - Assert.Single(matches); + await host.StopAsync(); + } - var timestamp = long.Parse(matches[0].Groups[1].Value.Replace(".", string.Empty)); + private static async Task VerifyAsync(long beginTimestamp, long endTimestamp, HttpResponseMessage response, bool requestOpenMetrics) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.True(response.Content.Headers.Contains("Last-Modified")); - Assert.True(beginTimestamp <= timestamp && timestamp <= endTimestamp); + if (requestOpenMetrics) + { + Assert.Equal("application/openmetrics-text; version=1.0.0; charset=utf-8", response.Content.Headers.ContentType.ToString()); } else { - Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("text/plain; charset=utf-8; version=0.0.4", response.Content.Headers.ContentType.ToString()); } - validateResponse?.Invoke(response); + string content = (await response.Content.ReadAsStringAsync()).ReplaceLineEndings(); - await host.StopAsync(); + string expected = requestOpenMetrics + ? $$""" + # TYPE target info + # HELP target Target metadata + target_info{service_name="my_service",service_instance_id="id1"} 1 + # 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}) + # 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+) + # EOF + + """.ReplaceLineEndings(); + + var matches = Regex.Matches(content, "^" + expected + "$"); + + Assert.True(matches.Count == 1, content); + + var timestamp = long.Parse(matches[0].Groups[1].Value.Replace(".", string.Empty)); + + Assert.True(beginTimestamp <= timestamp && timestamp <= endTimestamp, $"{beginTimestamp} {timestamp} {endTimestamp}"); + } + + private static Task StartTestHostAsync( + Action configure, + Action configureServices = null, + bool registerMeterProvider = true, + Action configureOptions = null) + { + return new HostBuilder() + .ConfigureWebHost(webBuilder => webBuilder + .UseTestServer() + .ConfigureServices(services => + { + if (registerMeterProvider) + { + services.AddOpenTelemetry().WithMetrics(builder => builder + .ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1")) + .AddMeter(MeterName) + .AddPrometheusExporter(o => + { + configureOptions?.Invoke(o); + })); + } + + configureServices?.Invoke(services); + }) + .Configure(configure)) + .StartAsync(); } } #endif diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs index f4cadff53b5..32ee87e3de5 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs @@ -60,7 +60,7 @@ public async Task EnterExitCollectTest(int scrapeResponseCacheDurationMillisecon return new Response { CollectionResponse = response, - ViewPayload = response.View.ToArray(), + ViewPayload = openMetricsRequested ? response.OpenMetricsView.ToArray() : response.PlainTextView.ToArray(), }; } finally @@ -124,7 +124,7 @@ public async Task EnterExitCollectTest(int scrapeResponseCacheDurationMillisecon return new Response { CollectionResponse = response, - ViewPayload = response.View.ToArray(), + ViewPayload = openMetricsRequested ? response.OpenMetricsView.ToArray() : response.PlainTextView.ToArray(), }; } finally