MetricPusher race condition makes pushing final state of metrics unreliable
jomadu opened this issue · 2 comments
Bug Report
What did you do?
MetricPusher race condition makes pushing final state of metrics unreliable
What did you expect to see?
I expected to see that the final state of my metrics were published reliably when I called MetricPusher.StopAsync()
.
What did you see instead? Under which circumstances?
- Main thread creates a metric
- Main thread creates a
MetricPusher
with a specificintervalMilliseconds
- Main thread calls
MetricPusher.Start()
- Main thread awaits a task that completes in the same duration as the specified
intervalMilliseconds
- Main thread updates the metric, and immediately attempts to stop the
MetricPusher
- The main thread calling
MetricPusher.StopAsync()
cancels theMetricPusher
's cancellation token while theMetricPusher
is publishing the previous state of metrics. - MetricPusher then checks the cancellation token, and seeing it cancelled, exits without publishing the updated metrics
Testing
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<ProduceReferenceAssembly>false</ProduceReferenceAssembly>
<IsPackable>false</IsPackable>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="FluentAssertions" Version="5.10.3" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />
<PackageReference Include="prometheus-net" Version="7.0.0" />
<PackageReference Include="xunit" Version="2.4.1" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="coverlet.collector" Version="3.1.0">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
</ItemGroup>
</Project>
using FluentAssertions;
using Prometheus;
using Xunit;
namespace Tests;
public class MetricPusherRaceConditionTests
{
private const string PushGatewayEndpoint = "http://localhost:9091";
private HttpClient _client;
/*
* Manual Setup:
*
* > docker run -d -p 9091:9091 prom/pushgateway --web.enable-admin-api
*
*/
public MetricPusherRaceConditionTests()
{
_client = new HttpClient() { BaseAddress = new Uri(PushGatewayEndpoint) };
// Wipe all stored metrics to avoid prior test state from interfering
using var resp = _client.PutAsync("api/v1/admin/wipe", null).GetAwaiter().GetResult();
resp.EnsureSuccessStatusCode();
Metrics.SuppressDefaultMetrics();
}
[Fact]
public async Task GivenTaskCompletesDuringPublishingWhenMetricPusherIsStoppedThenFinalMetricStateIsntPublished()
{
// arrange
using var resp = await _client.GetAsync("/metrics");
resp.EnsureSuccessStatusCode();
var content = await resp.Content.ReadAsStringAsync();
content.Should().NotContain("test_counter");
var counter = Metrics.CreateCounter("test_counter", "help");
var metricPusher = new MetricPusher($"{PushGatewayEndpoint}/metrics", "test_job", null, intervalMilliseconds: 500);
metricPusher.Start();
// act
await Task.Delay(500);
counter.Inc();
try
{
await metricPusher.StopAsync();
}
catch (OperationCanceledException) { }
// assert (last state of metric shouldn't be published)
using var resp2 = await _client.GetAsync("/metrics");
resp2.EnsureSuccessStatusCode();
content = await resp2.Content.ReadAsStringAsync();
content.Should().NotContain(@"test_counter{instance="""",job=""test_job""} 1");
}
[Fact]
public async Task GivenTaskCompletesNotDuringPublishingWhenMetricPusherIsStoppedThenFinalMetricStateIsPublished()
{
// arrange
using var resp = await _client.GetAsync("/metrics");
resp.EnsureSuccessStatusCode();
var content = await resp.Content.ReadAsStringAsync();
content.Should().NotContain("test_counter");
var counter = Metrics.CreateCounter("test_counter", "help");
var metricPusher = new MetricPusher($"{PushGatewayEndpoint}/metrics", "test_job", null, intervalMilliseconds: 500);
metricPusher.Start();
// act
await Task.Delay(250);
counter.Inc();
try
{
await metricPusher.StopAsync();
}
catch (OperationCanceledException) { }
// assert (last state of metric shouldn't be published)
using var resp2 = await _client.GetAsync("/metrics");
resp2.EnsureSuccessStatusCode();
content = await resp2.Content.ReadAsStringAsync();
content.Should().Contain(@"test_counter{instance="""",job=""test_job""} 1");
}
}
Results for GivenTaskCompletesDuringPublishingWhenMetricPusherIsStoppedThenFinalMzetricStateIsntPublished
:
Results for GivenTaskCompletesNotDuringPublishingWhenMetricPusherIsStoppedThenFinalMetricStateIsPublished
:
Environment
-
System information:
Windows
-
Pushgateway version:
7.0.0
I was also having some trouble building the test project on my own windows machine, targeting net6.0. Otherwise I would have added a set of tests in the netcore testing project to demonstrate more effectively.
Some output of me trying to build:
C:\dev\prometheus-net [master ≡]> git log -1
commit d8a4c95598683edc39138ce3451dc00f7d5d36b2 (HEAD -> master, origin/master, origin/HEAD)
Author: Sander Saares <sasaares@microsoft.com>
Date: Thu Oct 27 02:31:01 2022
Add .NET Standard sample project
C:\dev\prometheus-net [master ≡]> git status
On branch master
Your branch is up to date with 'origin/master'.
nothing to commit, working tree clean
C:\dev\prometheus-net [master ≡]> dotnet build
Microsoft (R) Build Engine version 17.1.1+a02f73656 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.
Determining projects to restore...
All projects are up-to-date for restore.
C:\dev\prometheus-net\Tester.NetFramework.AspNet\Tester.NetFramework.AspNet.csproj(113,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\6.0.203\Microsoft\VisualStudio\v17.0\WebApplications\Microsoft.WebApplication.targets" was not found. Confirm that the expression in the Import declaration "C:\Program Files\dotnet\sdk\6.0.203\Microsoft\VisualStudio\v17.0\WebApplications\Microsoft.WebApplication.targets" is correct, and that the file exists on disk.
C:\dev\prometheus-net\Sample.Web.NetFramework\Sample.Web.NetFramework.csproj(210,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\6.0.203\Microsoft\VisualStudio\v17.0\WebApplications\Microsoft.WebApplication.targets" was not found. Confirm that the expression in the Import declaration "C:\Program Files\dotnet\sdk\6.0.203\Microsoft\VisualStudio\v17.0\WebApplications\Microsoft.WebApplication.targets" is correct, and that the file exists on disk.
Sample.Grpc.Client -> C:\dev\prometheus-net\Sample.Grpc.Client\bin\Debug\net6.0\Sample.Grpc.Client.dll
C:\dev\prometheus-net\Prometheus\LabelSequence.cs(27,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(154,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0843: Auto-implemented property 'StringSequence.Length' must be fully assigned before control is returned to the caller. [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._values' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._inheritedValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._hashCode' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedInheritedArrays' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInCurrentArray' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\LabelSequence.cs(27,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(154,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0843: Auto-implemented property 'StringSequence.Length' must be fully assigned before control is returned to the caller. [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._values' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._inheritedValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._hashCode' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedInheritedArrays' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInCurrentArray' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\LabelSequence.cs(27,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedInheritedArrays' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInCurrentArray' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(154,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0843: Auto-implemented property 'StringSequence.Length' must be fully assigned before control is returned to the caller. [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._values' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._inheritedValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._hashCode' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
Build FAILED.
C:\dev\prometheus-net\Tester.NetFramework.AspNet\Tester.NetFramework.AspNet.csproj(113,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\6.0.203\Microsoft\VisualStudio\v17.0\WebApplications\Microsoft.WebApplication.targets" was not found. Confirm that the expression in the Import declaration "C:\Program Files\dotnet\sdk\6.0.203\Microsoft\VisualStudio\v17.0\WebApplications\Microsoft.WebApplication.targets" is correct, and that the file exists on disk.
C:\dev\prometheus-net\Sample.Web.NetFramework\Sample.Web.NetFramework.csproj(210,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\6.0.203\Microsoft\VisualStudio\v17.0\WebApplications\Microsoft.WebApplication.targets" was not found. Confirm that the expression in the Import declaration "C:\Program Files\dotnet\sdk\6.0.203\Microsoft\VisualStudio\v17.0\WebApplications\Microsoft.WebApplication.targets" is correct, and that the file exists on disk.
C:\dev\prometheus-net\Prometheus\LabelSequence.cs(27,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(154,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0843: Auto-implemented property 'StringSequence.Length' must be fully assigned before control is returned to the caller. [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._values' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._inheritedValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._hashCode' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedInheritedArrays' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInCurrentArray' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\LabelSequence.cs(27,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(154,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0843: Auto-implemented property 'StringSequence.Length' must be fully assigned before control is returned to the caller. [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._values' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._inheritedValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._hashCode' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedInheritedArrays' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInCurrentArray' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\LabelSequence.cs(27,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedInheritedArrays' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(35,16): error CS0171: Field 'StringSequence.Enumerator._completedItemsInCurrentArray' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(154,21): error CS0188: The 'this' object cannot be used before all of its fields have been assigned [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0843: Auto-implemented property 'StringSequence.Length' must be fully assigned before control is returned to the caller. [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._values' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._inheritedValues' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
C:\dev\prometheus-net\Prometheus\StringSequence.cs(123,13): error CS0171: Field 'StringSequence._hashCode' must be fully assigned before control is returned to the caller [C:\dev\prometheus-net\Prometheus\Prometheus.csproj]
0 Warning(s)
29 Error(s)
Time Elapsed 00:00:03.33
C:\dev\prometheus-net [master ≡]> dotnet --version
6.0.203
C:\dev\prometheus-net [master ≡]>
Whoops ... wrong project: prometheus-net/prometheus-net#383