From 0269b936971cd724abe406dc035a91c352f1ad14 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Apr 2024 13:37:24 +0900 Subject: [PATCH] Fix fromCache --- .../PrometheusExporterMiddleware.cs | 2 +- .../Internal/PrometheusCollectionManager.cs | 23 ++++----- .../PrometheusHttpListener.cs | 2 +- .../PrometheusCollectionManagerTests.cs | 49 +++++++++---------- 4 files changed, 36 insertions(+), 40 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs b/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs index df9ee1084d..da9db13b19 100644 --- 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 index e39c848676..6efbe2894b 100644 --- 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 EnterCollect() + public (ValueTask Response, bool FromCache) EnterCollect() #else - public Task EnterCollect() + public (Task Response, bool FromCache) EnterCollect() #endif { this.EnterGlobalLock(); @@ -54,9 +54,9 @@ public Task EnterCollect() Interlocked.Increment(ref this.readerCount); this.ExitGlobalLock(); #if NET6_0_OR_GREATER - return new ValueTask(tcs.Task); + return (new ValueTask(tcs.Task), true); #else - return tcs.Task; + return (tcs.Task, true); #endif } } @@ -65,9 +65,9 @@ public Task EnterCollect() Interlocked.Increment(ref this.readerCount); this.ExitGlobalLock(); #if NET6_0_OR_GREATER - return new ValueTask(tcs.Task); + return (new ValueTask(tcs.Task), false); #else - return tcs.Task; + return (tcs.Task, false); #endif } @@ -83,9 +83,9 @@ public Task EnterCollect() Task.Factory.StartNew(this.ExecuteCollect, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); this.ExitGlobalLock(); #if NET6_0_OR_GREATER - return new ValueTask(newTcs.Task); + return (new ValueTask(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,12 +366,11 @@ private PrometheusMetric GetPrometheusMetric(Metric metric) public readonly struct CollectionResponse { - public CollectionResponse(ArraySegment openMetricsView, ArraySegment plainTextView, DateTime generatedAtUtc, bool fromCache) + public CollectionResponse(ArraySegment openMetricsView, ArraySegment plainTextView, DateTime generatedAtUtc) { this.OpenMetricsView = openMetricsView; this.PlainTextView = plainTextView; this.GeneratedAtUtc = generatedAtUtc; - this.FromCache = fromCache; } public ArraySegment OpenMetricsView { get; } @@ -379,7 +378,5 @@ public CollectionResponse(ArraySegment openMetricsView, ArraySegment public ArraySegment 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 index 46a0813d3a..ed68a640c3 100644 --- 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/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs index b64779eebf..5c3be9c586 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusCollectionManagerTests.cs @@ -49,19 +49,20 @@ public async Task EnterExitCollectTest(int scrapeResponseCacheDurationMillisecon var counter = meter.CreateCounter("counter_int", description: "Prometheus help text goes here \n escaping."); counter.Add(100); - Task[] collectTasks = new Task[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); } } }