Skip to content

Commit 496424e

Browse files
authored
Merge pull request #1001 from aws-powertools/fix/metrics-concurrency
fix: concurrency issues in Metrics
2 parents 4c3c3ce + 6742670 commit 496424e

File tree

9 files changed

+1156
-76
lines changed

9 files changed

+1156
-76
lines changed

libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,7 @@ void IMetrics.AddMetric(string key, double value, MetricUnit unit, MetricResolut
199199

200200
if (metrics.Count > 0 &&
201201
(metrics.Count == PowertoolsConfigurations.MaxMetrics ||
202-
metrics.FirstOrDefault(x => x.Name == key)
203-
?.Values.Count == PowertoolsConfigurations.MaxMetrics))
202+
GetExistingMetric(metrics, key)?.Values.Count == PowertoolsConfigurations.MaxMetrics))
204203
{
205204
Instance.Flush(true);
206205
}
@@ -536,9 +535,17 @@ private Dictionary<string, string> ListToDictionary(List<DimensionSet> dimension
536535
var dictionary = new Dictionary<string, string>();
537536
try
538537
{
539-
return dimensions != null
540-
? new Dictionary<string, string>(dimensions.SelectMany(x => x.Dimensions))
541-
: dictionary;
538+
if (dimensions != null)
539+
{
540+
foreach (var dimensionSet in dimensions)
541+
{
542+
foreach (var kvp in dimensionSet.Dimensions)
543+
{
544+
dictionary[kvp.Key] = kvp.Value;
545+
}
546+
}
547+
}
548+
return dictionary;
542549
}
543550
catch (Exception e)
544551
{
@@ -624,6 +631,49 @@ public static void Flush(bool metricsOverflow = false)
624631
Instance.Flush(metricsOverflow);
625632
}
626633

634+
/// <summary>
635+
/// Safely searches for an existing metric by name without using LINQ enumeration
636+
/// </summary>
637+
/// <param name="metrics">The metrics collection to search</param>
638+
/// <param name="key">The metric name to search for</param>
639+
/// <returns>The found metric or null if not found</returns>
640+
private static MetricDefinition GetExistingMetric(List<MetricDefinition> metrics, string key)
641+
{
642+
// Use a traditional for loop instead of LINQ to avoid enumeration issues
643+
// when the collection is modified concurrently
644+
if (metrics == null || string.IsNullOrEmpty(key))
645+
return null;
646+
647+
// Create a snapshot of the count to avoid issues with concurrent modifications
648+
var count = metrics.Count;
649+
for (int i = 0; i < count; i++)
650+
{
651+
try
652+
{
653+
// Check bounds again in case collection was modified
654+
if (i >= metrics.Count)
655+
break;
656+
657+
var metric = metrics[i];
658+
if (metric != null && string.Equals(metric.Name, key, StringComparison.Ordinal))
659+
{
660+
return metric;
661+
}
662+
}
663+
catch (ArgumentOutOfRangeException)
664+
{
665+
// Collection was modified during iteration, return null to be safe
666+
break;
667+
}
668+
catch (IndexOutOfRangeException)
669+
{
670+
// Collection was modified during iteration, return null to be safe
671+
break;
672+
}
673+
}
674+
return null;
675+
}
676+
627677
/// <summary>
628678
/// Helper method for testing purposes. Clears static instance between test execution
629679
/// </summary>

libraries/src/AWS.Lambda.Powertools.Metrics/Model/DimensionSet.cs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,16 @@ public DimensionSet(string key, string value)
4343
/// Gets the dimension keys.
4444
/// </summary>
4545
/// <value>The dimension keys.</value>
46-
public List<string> DimensionKeys => Dimensions.Keys.ToList();
46+
public List<string> DimensionKeys
47+
{
48+
get
49+
{
50+
var keys = new List<string>();
51+
foreach (var key in Dimensions.Keys)
52+
{
53+
keys.Add(key);
54+
}
55+
return keys;
56+
}
57+
}
4758
}

libraries/src/AWS.Lambda.Powertools.Metrics/Model/Metadata.cs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Concurrent;
23
using System.Collections.Generic;
34
using System.Text.Json.Serialization;
45

@@ -15,7 +16,7 @@ public class Metadata
1516
public Metadata()
1617
{
1718
CloudWatchMetrics = new List<MetricDirective> { new() };
18-
CustomMetadata = new Dictionary<string, object>();
19+
CustomMetadata = new ConcurrentDictionary<string, object>();
1920
}
2021

2122
/// <summary>
@@ -43,14 +44,17 @@ public Metadata()
4344
/// </summary>
4445
/// <value>The custom metadata.</value>
4546
[JsonIgnore]
46-
public Dictionary<string, object> CustomMetadata { get; }
47+
public ConcurrentDictionary<string, object> CustomMetadata { get; }
4748

4849
/// <summary>
4950
/// Deletes all metrics from memory
5051
/// </summary>
5152
internal void ClearMetrics()
5253
{
53-
_metricDirective.Metrics.Clear();
54+
lock (_metricDirective._lockObj)
55+
{
56+
_metricDirective.Metrics.Clear();
57+
}
5458
CustomMetadata?.Clear();
5559
}
5660

@@ -59,7 +63,10 @@ internal void ClearMetrics()
5963
/// </summary>
6064
internal void ClearNonDefaultDimensions()
6165
{
62-
_metricDirective.Dimensions.Clear();
66+
lock (_metricDirective._lockObj)
67+
{
68+
_metricDirective.Dimensions.Clear();
69+
}
6370
}
6471

6572
/// <summary>
@@ -143,7 +150,11 @@ internal void SetDefaultDimensions(List<DimensionSet> defaultDimensionSets)
143150
/// <returns>List of metrics stored in memory</returns>
144151
internal List<MetricDefinition> GetMetrics()
145152
{
146-
return _metricDirective.Metrics;
153+
// Return a snapshot to avoid concurrent modification issues during serialization
154+
lock (_metricDirective._lockObj)
155+
{
156+
return new List<MetricDefinition>(_metricDirective.Metrics);
157+
}
147158
}
148159

149160
/// <summary>

libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricDefinition.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ public MetricDefinition(string name, MetricUnit unit, List<double> values, Metri
9393
/// <param name="value">Metric value to add to existing key</param>
9494
public void AddValue(double value)
9595
{
96-
Values.Add(value);
96+
lock (Values)
97+
{
98+
Values.Add(value);
99+
}
97100
}
98101
}

0 commit comments

Comments
 (0)