From 6a88ea9b02c76e7c6fedd606eaa3e737c713ebe8 Mon Sep 17 00:00:00 2001 From: Sander Saares Date: Wed, 3 Jan 2024 20:38:19 +0200 Subject: [PATCH 1/2] Fix occasional "Collection was modified" exception when serializing metrics. #464 --- History | 2 + Prometheus/CollectorRegistry.cs | 64 ++++++++++++++++++++++--------- Resources/SolutionAssemblyInfo.cs | 2 +- 3 files changed, 48 insertions(+), 20 deletions(-) diff --git a/History b/History index 7a957465..2c7d57cb 100644 --- a/History +++ b/History @@ -1,3 +1,5 @@ +* 8.2.1 +- Fix occasional "Collection was modified" exception when serializing metrics. #464 * 8.2.0 - .WithLabels() & similar now accept ReadOnlyMemory as alternative to string[]. Same behavior, just easier to use if you already have a ReadOnlyMemory. - .WithLabels() & similar now accept ReadOnlySpan as alternative to string[]. This enables allocation-free metric instance creation if a metric instance with these labels is already known. diff --git a/Prometheus/CollectorRegistry.cs b/Prometheus/CollectorRegistry.cs index 78d01e71..35f1971c 100644 --- a/Prometheus/CollectorRegistry.cs +++ b/Prometheus/CollectorRegistry.cs @@ -386,33 +386,59 @@ private void UpdateRegistryMetrics() if (_metricFamiliesPerType == null || _metricInstancesPerType == null || _metricTimeseriesPerType == null) return; // Debug metrics are not enabled. - foreach (MetricType type in Enum.GetValues(typeof(MetricType))) + // We copy references to the metric families to a temporary buffer to avoid having to hold locks to keep the collection consistent. + CollectorFamily[] familiesBuffer; + + _familiesLock.EnterReadLock(); + + var familiesCount = _families.Count; + familiesBuffer = ArrayPool.Shared.Rent(familiesCount); + + try { - long families = 0; - long instances = 0; - long timeseries = 0; + try + { + _families.Values.CopyTo(familiesBuffer, 0); + } + finally + { + _familiesLock.ExitReadLock(); + } - foreach (var family in _families.Values) + foreach (MetricType type in Enum.GetValues(typeof(MetricType))) { - bool hadMatchingType = false; + long families = 0; + long instances = 0; + long timeseries = 0; - family.ForEachCollector(collector => + for (var i = 0; i < familiesCount; i++) { - if (collector.Type != type) - return; + var family = familiesBuffer[i]; - hadMatchingType = true; - instances += collector.ChildCount; - timeseries += collector.TimeseriesCount; - }); + bool hadMatchingType = false; - if (hadMatchingType) - families++; - } + family.ForEachCollector(collector => + { + if (collector.Type != type) + return; + + hadMatchingType = true; + instances += collector.ChildCount; + timeseries += collector.TimeseriesCount; + }); - _metricFamiliesPerType[type].Set(families); - _metricInstancesPerType[type].Set(instances); - _metricTimeseriesPerType[type].Set(timeseries); + if (hadMatchingType) + families++; + } + + _metricFamiliesPerType[type].Set(families); + _metricInstancesPerType[type].Set(instances); + _metricTimeseriesPerType[type].Set(timeseries); + } + } + finally + { + ArrayPool.Shared.Return(familiesBuffer, clearArray: true); } } diff --git a/Resources/SolutionAssemblyInfo.cs b/Resources/SolutionAssemblyInfo.cs index 2ec83a8b..1f300ba8 100644 --- a/Resources/SolutionAssemblyInfo.cs +++ b/Resources/SolutionAssemblyInfo.cs @@ -2,7 +2,7 @@ using System.Runtime.CompilerServices; // This is the real version number, used in NuGet packages and for display purposes. -[assembly: AssemblyFileVersion("8.2.0")] +[assembly: AssemblyFileVersion("8.2.1")] // Only use major version here, with others kept at zero, for correct assembly binding logic. [assembly: AssemblyVersion("8.0.0")] From 60e9106a83ff1274fec0022c37366f04822b1d1b Mon Sep 17 00:00:00 2001 From: Sander Saares Date: Wed, 3 Jan 2024 20:57:43 +0200 Subject: [PATCH 2/2] De-confusify histogram1 in .NET Meters API sample It had implicit conflict with static label, which caused it to behave as unlabeled metric. Correct but potentially confusing because unlabeled metrics have special case publishing rules. Added a custom label to make it act like a labeled metric. --- Sample.Console.DotNetMeters/CustomDotNetMeters.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sample.Console.DotNetMeters/CustomDotNetMeters.cs b/Sample.Console.DotNetMeters/CustomDotNetMeters.cs index 9ec01552..296c9d36 100644 --- a/Sample.Console.DotNetMeters/CustomDotNetMeters.cs +++ b/Sample.Console.DotNetMeters/CustomDotNetMeters.cs @@ -65,7 +65,8 @@ int MeasureSandLevel() if (Random.Shared.Next(10) == 0) counter1.Add(1, new KeyValuePair("wing-type", "SlaxxWing 1.0"), new KeyValuePair("wing-version", "beta")); - histogram1.Record((byte)(Random.Shared.Next(256)), new KeyValuePair("is-faulted", true)); + // is-faulted here conflicts with the static label of the same name and gets overwritten by the static label. + histogram1.Record((byte)(Random.Shared.Next(256)), new KeyValuePair("is-faulted", true), new KeyValuePair("canbus_ver", "1.0")); // .NET 7 upDown1.Add(Random.Shared.Next(-1, 2));