From ef1ff92e0cd04feb01f00841260e10a841e6ff03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justinas=20=C4=8Cin=C4=8Dikas?= Date: Fri, 7 Jun 2024 12:19:36 +0300 Subject: [PATCH 1/6] Fix collection output buffer management when its resized When buffer is too small to fit its initial 85kB size, its resized to fit the output, however the resized buffer is not used in final output. This ultimately results in overflow exception as cursor is correct and follows resized buffer. I also changed to use the same buffer variable for WriteTargetInfo, it also can resize the buffer. --- .../CHANGELOG.md | 3 + .../Internal/PrometheusCollectionManager.cs | 14 +-- .../PrometheusExporterMiddlewareTests.cs | 24 ++++ .../PrometheusHttpListenerTests.cs | 117 +++++++++++++----- 4 files changed, 117 insertions(+), 41 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md index 07ac849c6d7..f444457e880 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* Fix collection output buffer management when its resized + ([#5661](https://github.com/open-telemetry/opentelemetry-dotnet/issues/5661)) + ## 1.9.0-alpha.2 Released 2024-May-29 diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs index b93812176fb..5c5efc69620 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs @@ -198,13 +198,13 @@ private bool ExecuteCollect(bool openMetricsRequested) private ExportResult OnCollect(Batch metrics) { var cursor = 0; - var buffer = this.exporter.OpenMetricsRequested ? this.openMetricsBuffer : this.plainTextBuffer; + ref byte[] buffer = ref (this.exporter.OpenMetricsRequested ? ref this.openMetricsBuffer : ref this.plainTextBuffer); try { if (this.exporter.OpenMetricsRequested) { - cursor = this.WriteTargetInfo(); + cursor = this.WriteTargetInfo(ref buffer); this.scopes.Clear(); @@ -291,11 +291,11 @@ private ExportResult OnCollect(Batch metrics) if (this.exporter.OpenMetricsRequested) { - this.previousOpenMetricsDataView = new ArraySegment(this.openMetricsBuffer, 0, cursor); + this.previousOpenMetricsDataView = new ArraySegment(buffer, 0, cursor); } else { - this.previousPlainTextDataView = new ArraySegment(this.plainTextBuffer, 0, cursor); + this.previousPlainTextDataView = new ArraySegment(buffer, 0, cursor); } return ExportResult.Success; @@ -315,7 +315,7 @@ private ExportResult OnCollect(Batch metrics) } } - private int WriteTargetInfo() + private int WriteTargetInfo(ref byte[] buffer) { if (this.targetInfoBufferLength < 0) { @@ -323,13 +323,13 @@ private int WriteTargetInfo() { try { - this.targetInfoBufferLength = PrometheusSerializer.WriteTargetInfo(this.openMetricsBuffer, 0, this.exporter.Resource); + this.targetInfoBufferLength = PrometheusSerializer.WriteTargetInfo(buffer, 0, this.exporter.Resource); break; } catch (IndexOutOfRangeException) { - if (!this.IncreaseBufferSize(ref this.openMetricsBuffer)) + if (!this.IncreaseBufferSize(ref buffer)) { throw; } diff --git a/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs b/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs index 19b22e47be5..42fc1b78a35 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs @@ -288,6 +288,30 @@ public async Task PrometheusExporterMiddlewareIntegration_CanServeOpenMetricsAnd await host.StopAsync(); } + [Fact] + public async Task PrometheusExporterMiddlewareIntegration_ALotOfMetrics() + { + using var host = await StartTestHostAsync( + app => app.UseOpenTelemetryPrometheusScrapingEndpoint()); + + using var meter = new Meter(MeterName, MeterVersion); + + for (var x = 0; x < 1000; x++) + { + var counter = meter.CreateCounter("counter_double_" + x, unit: "By"); + counter.Add(1); + } + + using var client = host.GetTestClient(); + + using var response = await client.GetAsync("/metrics"); + var text = await response.Content.ReadAsStringAsync(); + + Assert.NotEmpty(text); + + await host.StopAsync(); + } + private static async Task RunPrometheusExporterMiddlewareIntegrationTest( string path, Action configure, diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs index 9ef62de196f..1b425913f78 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs @@ -1,6 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System; using System.Diagnostics.Metrics; using System.Net; #if NETFRAMEWORK @@ -19,6 +20,48 @@ public class PrometheusHttpListenerTests private static readonly string MeterName = Utils.GetCurrentMethodName(); + private static MeterProvider BuildMeterProvider(Meter meter, IEnumerable> attributes, out string address) + { + Random random = new Random(); + int retryAttempts = 5; + int port = 0; + string generatedAddress = null; + MeterProvider provider = null; + + while (retryAttempts-- != 0) + { + port = random.Next(2000, 5000); + generatedAddress = $"http://localhost:{port}/"; + + try + { + provider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1").AddAttributes(attributes)) + .AddPrometheusHttpListener(options => + { + options.UriPrefixes = new string[] { generatedAddress }; + }) + .Build(); + + break; + } + catch + { + // ignored + } + } + + address = generatedAddress; + + if (provider == null) + { + throw new InvalidOperationException("HttpListener could not be started"); + } + + return provider; + } + [Theory] [InlineData("http://+:9464")] [InlineData("http://*:9464")] @@ -144,6 +187,45 @@ public void PrometheusHttpListenerThrowsOnStart() listener?.Dispose(); } + [Theory] + [InlineData("application/openmetrics-text")] + [InlineData("")] + public async Task PrometheusExporterHttpServerIntegration_LargePayload(string acceptHeader) + { + using var meter = new Meter(MeterName, MeterVersion); + + var attributes = new List>(); + var oneKb = new string('A', 1024); + for (var x = 0; x < 8500; x++) + { + attributes.Add(new KeyValuePair(x.ToString(), oneKb)); + } + + var provider = BuildMeterProvider(meter, attributes, out var address); + + for (var x = 0; x < 1000; x++) + { + var counter = meter.CreateCounter("counter_double_" + x, unit: "By"); + counter.Add(1); + } + + using HttpClient client = new HttpClient(); + + if (!string.IsNullOrEmpty(acceptHeader)) + { + client.DefaultRequestHeaders.Add("Accept", acceptHeader); + } + + using var response = await client.GetAsync($"{address}metrics"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("counter_double_999", content); + Assert.DoesNotContain('\0', content); + + provider.Dispose(); + } + private static void TestPrometheusHttpListenerUriPrefixOptions(string[] uriPrefixes) { using var exporter = new PrometheusExporter(new()); @@ -159,42 +241,9 @@ private async Task RunPrometheusExporterHttpServerIntegrationTest(bool skipMetri { var requestOpenMetrics = acceptHeader.StartsWith("application/openmetrics-text"); - Random random = new Random(); - int retryAttempts = 5; - int port = 0; - string address = null; - - MeterProvider provider = null; using var meter = new Meter(MeterName, MeterVersion); - while (retryAttempts-- != 0) - { - port = random.Next(2000, 5000); - address = $"http://localhost:{port}/"; - - try - { - provider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1")) - .AddPrometheusHttpListener(options => - { - options.UriPrefixes = new string[] { address }; - }) - .Build(); - - break; - } - catch - { - // ignored - } - } - - if (provider == null) - { - throw new InvalidOperationException("HttpListener could not be started"); - } + var provider = BuildMeterProvider(meter, [], out var address); var tags = new KeyValuePair[] { From defdb9191842fd08da64cb540e54d3e61b802407 Mon Sep 17 00:00:00 2001 From: jcin193 <59831245+jcin193@users.noreply.github.com> Date: Thu, 20 Jun 2024 08:10:57 +0300 Subject: [PATCH 2/6] Use pull request reference in CHANGELOG.md Co-authored-by: Reiley Yang --- src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md index dd437e3e48b..4917b9786cc 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md @@ -3,7 +3,7 @@ ## Unreleased * Fix collection output buffer management when its resized - ([#5661](https://github.com/open-telemetry/opentelemetry-dotnet/issues/5661)) + ([#5676](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5676)) ## 1.9.0-beta.1 From 47ad8411d363f900277b2ec7feba7238f536c167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justinas=20=C4=8Cin=C4=8Dikas?= Date: Thu, 20 Jun 2024 08:18:10 +0300 Subject: [PATCH 3/6] Style changes --- .../PrometheusHttpListenerTests.cs | 85 +++++++++---------- 1 file changed, 42 insertions(+), 43 deletions(-) diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs index 1b425913f78..a619c561040 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs @@ -1,7 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -using System; using System.Diagnostics.Metrics; using System.Net; #if NETFRAMEWORK @@ -20,48 +19,6 @@ public class PrometheusHttpListenerTests private static readonly string MeterName = Utils.GetCurrentMethodName(); - private static MeterProvider BuildMeterProvider(Meter meter, IEnumerable> attributes, out string address) - { - Random random = new Random(); - int retryAttempts = 5; - int port = 0; - string generatedAddress = null; - MeterProvider provider = null; - - while (retryAttempts-- != 0) - { - port = random.Next(2000, 5000); - generatedAddress = $"http://localhost:{port}/"; - - try - { - provider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1").AddAttributes(attributes)) - .AddPrometheusHttpListener(options => - { - options.UriPrefixes = new string[] { generatedAddress }; - }) - .Build(); - - break; - } - catch - { - // ignored - } - } - - address = generatedAddress; - - if (provider == null) - { - throw new InvalidOperationException("HttpListener could not be started"); - } - - return provider; - } - [Theory] [InlineData("http://+:9464")] [InlineData("http://*:9464")] @@ -308,4 +265,46 @@ private async Task RunPrometheusExporterHttpServerIntegrationTest(bool skipMetri provider.Dispose(); } + + private static MeterProvider BuildMeterProvider(Meter meter, IEnumerable> attributes, out string address) + { + Random random = new Random(); + int retryAttempts = 5; + int port = 0; + string generatedAddress = null; + MeterProvider provider = null; + + while (retryAttempts-- != 0) + { + port = random.Next(2000, 5000); + generatedAddress = $"http://localhost:{port}/"; + + try + { + provider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1").AddAttributes(attributes)) + .AddPrometheusHttpListener(options => + { + options.UriPrefixes = new string[] { generatedAddress }; + }) + .Build(); + + break; + } + catch + { + // ignored + } + } + + address = generatedAddress; + + if (provider == null) + { + throw new InvalidOperationException("HttpListener could not be started"); + } + + return provider; + } } From 152bd516df5770694bc0ea0640ac21513320b8aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Justinas=20=C4=8Cin=C4=8Dikas?= Date: Sun, 23 Jun 2024 14:54:07 +0300 Subject: [PATCH 4/6] Style changes --- .../PrometheusHttpListenerTests.cs | 84 +++++++++---------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs index a619c561040..cbaf619cbe9 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs @@ -194,6 +194,48 @@ private static void TestPrometheusHttpListenerUriPrefixOptions(string[] uriPrefi }); } + private static MeterProvider BuildMeterProvider(Meter meter, IEnumerable> attributes, out string address) + { + Random random = new Random(); + int retryAttempts = 5; + int port = 0; + string generatedAddress = null; + MeterProvider provider = null; + + while (retryAttempts-- != 0) + { + port = random.Next(2000, 5000); + generatedAddress = $"http://localhost:{port}/"; + + try + { + provider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1").AddAttributes(attributes)) + .AddPrometheusHttpListener(options => + { + options.UriPrefixes = new string[] { generatedAddress }; + }) + .Build(); + + break; + } + catch + { + // ignored + } + } + + address = generatedAddress; + + if (provider == null) + { + throw new InvalidOperationException("HttpListener could not be started"); + } + + return provider; + } + private async Task RunPrometheusExporterHttpServerIntegrationTest(bool skipMetrics = false, string acceptHeader = "application/openmetrics-text") { var requestOpenMetrics = acceptHeader.StartsWith("application/openmetrics-text"); @@ -265,46 +307,4 @@ private async Task RunPrometheusExporterHttpServerIntegrationTest(bool skipMetri provider.Dispose(); } - - private static MeterProvider BuildMeterProvider(Meter meter, IEnumerable> attributes, out string address) - { - Random random = new Random(); - int retryAttempts = 5; - int port = 0; - string generatedAddress = null; - MeterProvider provider = null; - - while (retryAttempts-- != 0) - { - port = random.Next(2000, 5000); - generatedAddress = $"http://localhost:{port}/"; - - try - { - provider = Sdk.CreateMeterProviderBuilder() - .AddMeter(meter.Name) - .ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1").AddAttributes(attributes)) - .AddPrometheusHttpListener(options => - { - options.UriPrefixes = new string[] { generatedAddress }; - }) - .Build(); - - break; - } - catch - { - // ignored - } - } - - address = generatedAddress; - - if (provider == null) - { - throw new InvalidOperationException("HttpListener could not be started"); - } - - return provider; - } } From 85c61c096cd035c6286b44571e7f29b9f180272b Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 24 Jun 2024 10:15:36 -0700 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Vishwesh Bankwar --- .../PrometheusExporterMiddlewareTests.cs | 2 +- .../PrometheusHttpListenerTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs b/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs index 42fc1b78a35..b63cabc36b3 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs @@ -289,7 +289,7 @@ public async Task PrometheusExporterMiddlewareIntegration_CanServeOpenMetricsAnd } [Fact] - public async Task PrometheusExporterMiddlewareIntegration_ALotOfMetrics() + public async Task PrometheusExporterMiddlewareIntegration_TestBufferSizeIncrease_With_LotOfMetrics() { using var host = await StartTestHostAsync( app => app.UseOpenTelemetryPrometheusScrapingEndpoint()); diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs index cbaf619cbe9..a132cf17753 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs @@ -147,7 +147,7 @@ public void PrometheusHttpListenerThrowsOnStart() [Theory] [InlineData("application/openmetrics-text")] [InlineData("")] - public async Task PrometheusExporterHttpServerIntegration_LargePayload(string acceptHeader) + public async Task PrometheusExporterHttpServerIntegration_TestBufferSizeIncrease_With_LargePayload(string acceptHeader) { using var meter = new Meter(MeterName, MeterVersion); From 518a42ff337a7ca0dcad9203c2ba7b1806f3abaa Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 24 Jun 2024 10:21:58 -0700 Subject: [PATCH 6/6] CHANGELOG tweaks. --- src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md | 4 ++++ .../CHANGELOG.md | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md index 9364cb454c1..17c6d731761 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Fixed a bug which lead to empty responses when the internal buffer is resized + processing a collection request + ([#5676](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5676)) + ## 1.9.0-beta.1 Released 2024-Jun-14 diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md index 4917b9786cc..4c8b6db142e 100644 --- a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md @@ -2,7 +2,8 @@ ## Unreleased -* Fix collection output buffer management when its resized +* Fixed a bug which lead to empty responses when the internal buffer is resized + processing a collection request ([#5676](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5676)) ## 1.9.0-beta.1