Fix fromCache

open-telemetry · Apr 20, 2024 · 0269b93 · 0269b93
1 parent d930f97
commit 0269b93
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 40 deletions.
diff --git a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs
@@ -53,7 +53,7 @@ public async Task InvokeAsync(HttpContext httpContext)
         try
         {
             var openMetricsRequested = AcceptsOpenMetrics(httpContext.Request);
-            var collectionResponse = await this.exporter.CollectionManager.EnterCollect().ConfigureAwait(false);
+            var collectionResponse = await this.exporter.CollectionManager.EnterCollect().Response.ConfigureAwait(false);
 
             try
             {

diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs
@@ -38,9 +38,9 @@ public PrometheusCollectionManager(PrometheusExporter exporter)
     }
 
 #if NET6_0_OR_GREATER
-    public ValueTask<CollectionResponse> EnterCollect()
+    public (ValueTask<CollectionResponse> Response, bool FromCache) EnterCollect()
 #else
-    public Task<CollectionResponse> EnterCollect()
+    public (Task<CollectionResponse> Response, bool FromCache) EnterCollect()
 #endif
     {
         this.EnterGlobalLock();
@@ -54,9 +54,9 @@ public Task<CollectionResponse> EnterCollect()
                 Interlocked.Increment(ref this.readerCount);
                 this.ExitGlobalLock();
 #if NET6_0_OR_GREATER
-                return new ValueTask<CollectionResponse>(tcs.Task);
+                return (new ValueTask<CollectionResponse>(tcs.Task), true);
 #else
-                return tcs.Task;
+                return (tcs.Task, true);
 #endif
             }
         }
@@ -65,9 +65,9 @@ public Task<CollectionResponse> EnterCollect()
             Interlocked.Increment(ref this.readerCount);
             this.ExitGlobalLock();
 #if NET6_0_OR_GREATER
-            return new ValueTask<CollectionResponse>(tcs.Task);
+            return (new ValueTask<CollectionResponse>(tcs.Task), false);
 #else
-            return tcs.Task;
+            return (tcs.Task, false);
 #endif
         }
 
@@ -83,9 +83,9 @@ public Task<CollectionResponse> EnterCollect()
                 Task.Factory.StartNew(this.ExecuteCollect, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default);
                 this.ExitGlobalLock();
 #if NET6_0_OR_GREATER
-                return new ValueTask<CollectionResponse>(newTcs.Task);
+                return (new ValueTask<CollectionResponse>(newTcs.Task), false);
 #else
-                return newTcs.Task;
+                return (newTcs.Task, false);
 #endif
             }
 
@@ -147,7 +147,7 @@ private void ExecuteCollect()
 
         if (result)
         {
-            response = new CollectionResponse(this.previousOpenMetricsDataView, this.previousPlainTextDataView, DateTime.UtcNow, fromCache: false);
+            response = new CollectionResponse(this.previousOpenMetricsDataView, this.previousPlainTextDataView, DateTime.UtcNow);
         }
         else
         {
@@ -366,20 +366,17 @@ private PrometheusMetric GetPrometheusMetric(Metric metric)
 
     public readonly struct CollectionResponse
     {
-        public CollectionResponse(ArraySegment<byte> openMetricsView, ArraySegment<byte> plainTextView, DateTime generatedAtUtc, bool fromCache)
+        public CollectionResponse(ArraySegment<byte> openMetricsView, ArraySegment<byte> plainTextView, DateTime generatedAtUtc)
         {
             this.OpenMetricsView = openMetricsView;
             this.PlainTextView = plainTextView;
             this.GeneratedAtUtc = generatedAtUtc;
-            this.FromCache = fromCache;
         }
 
         public ArraySegment<byte> OpenMetricsView { get; }
 
         public ArraySegment<byte> PlainTextView { get; }
 
         public DateTime GeneratedAtUtc { get; }
-
-        public bool FromCache { get; }
     }
 }
diff --git a/src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs b/src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs
@@ -148,7 +148,7 @@ private async Task ProcessRequestAsync(HttpListenerContext context)
         try
         {
             var openMetricsRequested = AcceptsOpenMetrics(context.Request);
-            var collectionResponse = await this.exporter.CollectionManager.EnterCollect().ConfigureAwait(false);
+            var collectionResponse = await this.exporter.CollectionManager.EnterCollect().Response.ConfigureAwait(false);
 
             try
             {

diff --git a/.../OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs b/.../OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs
@@ -49,19 +49,20 @@ public async Task EnterExitCollectTest(int scrapeResponseCacheDurationMillisecon
             var counter = meter.CreateCounter<int>("counter_int", description: "Prometheus help text goes here \n escaping.");
             counter.Add(100);
 
-            Task<Response>[] collectTasks = new Task<Response>[10];
+            Task<(Response Response, bool FromCache)>[] collectTasks = new Task<(Response, bool)>[10];
             for (int i = 0; i < collectTasks.Length; i++)
             {
                 collectTasks[i] = Task.Run(async () =>
                 {
-                    var response = await exporter.CollectionManager.EnterCollect();
+                    var result = exporter.CollectionManager.EnterCollect();
+                    var response = await result.Response;
                     try
                     {
-                        return new Response
+                        return (new Response
                         {
                             CollectionResponse = response,
                             ViewPayload = openMetricsRequested ? response.OpenMetricsView.ToArray() : response.PlainTextView.ToArray(),
-                        };
+                        }, result.FromCache);
                     }
                     finally
                     {
@@ -76,37 +77,34 @@ public async Task EnterExitCollectTest(int scrapeResponseCacheDurationMillisecon
 
             var firstResponse = await collectTasks[0];
 
-            Assert.False(firstResponse.CollectionResponse.FromCache);
+            Assert.False(firstResponse.FromCache);
 
             for (int i = 1; i < collectTasks.Length; i++)
             {
-                Assert.Equal(firstResponse.ViewPayload, (await collectTasks[i]).ViewPayload);
-                Assert.Equal(firstResponse.CollectionResponse.GeneratedAtUtc, (await collectTasks[i]).CollectionResponse.GeneratedAtUtc);
+                Assert.Equal(firstResponse.Response.ViewPayload, (await collectTasks[i]).Response.ViewPayload);
+                Assert.Equal(firstResponse.Response.CollectionResponse.GeneratedAtUtc, (await collectTasks[i]).Response.CollectionResponse.GeneratedAtUtc);
             }
 
             counter.Add(100);
 
             // This should use the cache and ignore the second counter update.
-            var task = exporter.CollectionManager.EnterCollect();
-            if (cacheEnabled)
-            {
-                Assert.True(task.IsCompleted);
-            }
+            var result = exporter.CollectionManager.EnterCollect();
+            Assert.Equal(cacheEnabled, result.Response.IsCompleted);
 
-            var response = await task;
+            var response = await result.Response;
             try
             {
                 if (cacheEnabled)
                 {
                     Assert.Equal(1, runningCollectCount);
-                    Assert.True(response.FromCache);
-                    Assert.Equal(firstResponse.CollectionResponse.GeneratedAtUtc, response.GeneratedAtUtc);
+                    Assert.True(result.FromCache);
+                    Assert.Equal(firstResponse.Response.CollectionResponse.GeneratedAtUtc, response.GeneratedAtUtc);
                 }
                 else
                 {
                     Assert.Equal(2, runningCollectCount);
-                    Assert.False(response.FromCache);
-                    Assert.True(firstResponse.CollectionResponse.GeneratedAtUtc < response.GeneratedAtUtc);
+                    Assert.False(result.FromCache);
+                    Assert.True(firstResponse.Response.CollectionResponse.GeneratedAtUtc < response.GeneratedAtUtc);
                 }
             }
             finally
@@ -122,14 +120,15 @@ public async Task EnterExitCollectTest(int scrapeResponseCacheDurationMillisecon
             {
                 collectTasks[i] = Task.Run(async () =>
                 {
-                    var response = await exporter.CollectionManager.EnterCollect();
+                    var result = exporter.CollectionManager.EnterCollect();
+                    var response = await result.Response;
                     try
                     {
-                        return new Response
+                        return (new Response
                         {
                             CollectionResponse = response,
                             ViewPayload = openMetricsRequested ? response.OpenMetricsView.ToArray() : response.PlainTextView.ToArray(),
-                        };
+                        }, result.FromCache);
                     }
                     finally
                     {
@@ -141,17 +140,17 @@ public async Task EnterExitCollectTest(int scrapeResponseCacheDurationMillisecon
             await Task.WhenAll(collectTasks);
 
             Assert.Equal(cacheEnabled ? 2 : 3, runningCollectCount);
-            Assert.NotEqual(firstResponse.ViewPayload, (await collectTasks[0]).ViewPayload);
-            Assert.NotEqual(firstResponse.CollectionResponse.GeneratedAtUtc, (await collectTasks[0]).CollectionResponse.GeneratedAtUtc);
+            Assert.NotEqual(firstResponse.Response.ViewPayload, (await collectTasks[0]).Response.ViewPayload);
+            Assert.NotEqual(firstResponse.Response.CollectionResponse.GeneratedAtUtc, (await collectTasks[0]).Response.CollectionResponse.GeneratedAtUtc);
 
             firstResponse = await collectTasks[0];
 
-            Assert.False(firstResponse.CollectionResponse.FromCache);
+            Assert.False(firstResponse.FromCache);
 
             for (int i = 1; i < collectTasks.Length; i++)
             {
-                Assert.Equal(firstResponse.ViewPayload, (await collectTasks[i]).ViewPayload);
-                Assert.Equal(firstResponse.CollectionResponse.GeneratedAtUtc, (await collectTasks[i]).CollectionResponse.GeneratedAtUtc);
+                Assert.Equal(firstResponse.Response.ViewPayload, (await collectTasks[i]).Response.ViewPayload);
+                Assert.Equal(firstResponse.Response.CollectionResponse.GeneratedAtUtc, (await collectTasks[i]).Response.CollectionResponse.GeneratedAtUtc);
             }
         }
     }