[prometheus] Fixed a race condition happening in simultaneous OpenMetrics and PlainText requests

src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Fix a race condition in Prometheus exporter where a non-OpenMetrics request following by an OpenMetrics request would cause malformed response
+  ([#5517](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5517))
+
 ## 1.8.0-rc.1
 
 Released 2024-Mar-27

src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs

@@ -21,6 +21,7 @@ internal sealed class PrometheusCollectionManager
     private int globalLockState;
     private ArraySegment<byte> previousDataView;
     private DateTime? previousDataViewGeneratedAtUtc;
+    private bool previousIsOpenMetricsRequested;
     private int readerCount;
     private bool collectionRunning;
     private TaskCompletionSource<CollectionResponse> collectionTcs;
@@ -46,7 +47,8 @@ public Task<CollectionResponse> EnterCollect(bool openMetricsRequested)
         // last successful collect, return the previous view.
         if (this.previousDataViewGeneratedAtUtc.HasValue
             && this.scrapeResponseCacheDurationMilliseconds > 0
-            && this.previousDataViewGeneratedAtUtc.Value.AddMilliseconds(this.scrapeResponseCacheDurationMilliseconds) >= DateTime.UtcNow)
+            && this.previousDataViewGeneratedAtUtc.Value.AddMilliseconds(this.scrapeResponseCacheDurationMilliseconds) >= DateTime.UtcNow
+            && openMetricsRequested == this.previousIsOpenMetricsRequested)
         {
             Interlocked.Increment(ref this.readerCount);
             this.ExitGlobalLock();
@@ -58,7 +60,7 @@ public Task<CollectionResponse> EnterCollect(bool openMetricsRequested)
         }
 
         // If a collection is already running, return a task to wait on the result.
-        if (this.collectionRunning)
+        if (this.collectionRunning && openMetricsRequested == this.previousIsOpenMetricsRequested)
         {
             if (this.collectionTcs == null)
             {
@@ -79,6 +81,7 @@ public Task<CollectionResponse> EnterCollect(bool openMetricsRequested)
         // Start a collection on the current thread.
         this.collectionRunning = true;
         this.previousDataViewGeneratedAtUtc = null;
+        this.previousIsOpenMetricsRequested = openMetricsRequested;
         Interlocked.Increment(ref this.readerCount);
         this.ExitGlobalLock();
 
@@ -213,6 +216,10 @@ private ExportResult OnCollect(Batch<Metric> metrics)
                     }
                 }
             }
+            else
+            {
+                this.targetInfoBufferLength = -1;
+            }
 
             foreach (var metric in metrics)
             {

test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs

@@ -248,6 +248,83 @@ public Task PrometheusExporterMiddlewareIntegration_UseOpenMetricsVersionHeader(
             acceptHeader: "application/openmetrics-text; version=1.0.0");
     }
 
+    [Fact]
+    public async Task PrometheusExporterMiddlewareIntegration_MultipleRequests()
+    {
+        using var host = await StartTestHostAsync(
+            app => app.UseOpenTelemetryPrometheusScrapingEndpoint());
+
+        var tags = new KeyValuePair<string, object>[]
+        {
+            new KeyValuePair<string, object>("key1", "value1"),
+            new KeyValuePair<string, object>("key2", "value2"),
+        };
+
+        using var meter = new Meter(MeterName, MeterVersion);
+
+        var beginTimestamp = DateTimeOffset.Now.ToUnixTimeMilliseconds();
+
+        var counter = meter.CreateCounter<double>("counter_double");
+        counter.Add(100.18D, tags);
+        counter.Add(0.99D, tags);
+
+        bool[] requestOpenMetricsTestCases = [true, false, true, false];
+
+        foreach (var requestOpenMetrics in requestOpenMetricsTestCases)
+        {
+            using var client = host.GetTestClient();
+            client.DefaultRequestHeaders.Add("Accept", requestOpenMetrics ? "application/openmetrics-text" : "text/plain");
+
+            using var response = await client.GetAsync("/metrics");
+
+            var endTimestamp = DateTimeOffset.Now.ToUnixTimeMilliseconds();
+
+            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()).ReplaceLineEndings();
+
+            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.Single(matches);
+
+            var timestamp = long.Parse(matches[0].Groups[1].Value.Replace(".", string.Empty));
+
+            Assert.True(beginTimestamp <= timestamp && timestamp <= endTimestamp);
+        }
+
+        await host.StopAsync();
+    }
+
     private static async Task RunPrometheusExporterMiddlewareIntegrationTest(
         string path,
         Action<IApplicationBuilder> configure,
@@ -260,26 +337,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<string, object>[]
         {
@@ -323,21 +381,27 @@ private static async Task RunPrometheusExporterMiddlewareIntegrationTest(
                 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";
+            string content = (await response.Content.ReadAsStringAsync()).ReplaceLineEndings();
+
+            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 + "$").Replace('\'', '"'));
 
@@ -356,5 +420,33 @@ private static async Task RunPrometheusExporterMiddlewareIntegrationTest(
 
         await host.StopAsync();
     }
+
+    private static Task<IHost> StartTestHostAsync(
+        Action<IApplicationBuilder> configure,
+        Action<IServiceCollection> configureServices = null,
+        bool registerMeterProvider = true,
+        Action<PrometheusAspNetCoreOptions> 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