Skip to content

Commit 47227ad

Browse files
authored
Merge pull request #594 from hjgraca/metrics-thread-safety-bug
chore: Metrics - Add thread safety to AddMetric
2 parents 0cdab3f + ab95e85 commit 47227ad

File tree

3 files changed

+43
-9
lines changed

3 files changed

+43
-9
lines changed

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ public class Metrics : IMetrics, IDisposable
5252
/// </summary>
5353
private readonly bool _captureColdStartEnabled;
5454

55+
// <summary>
56+
// Shared synchronization object
57+
// </summary>
58+
private readonly object _lockObj = new();
59+
5560
/// <summary>
5661
/// Creates a Metrics object that provides features to send metrics to Amazon Cloudwatch using the Embedded metric
5762
/// format (EMF). See
@@ -98,17 +103,20 @@ void IMetrics.AddMetric(string key, double value, MetricUnit unit, MetricResolut
98103
"'AddMetric' method requires a valid metrics value. Value must be >= 0.", nameof(value));
99104
}
100105

101-
var metrics = _context.GetMetrics();
102-
103-
if (metrics.Count > 0 &&
104-
(metrics.Count == PowertoolsConfigurations.MaxMetrics ||
105-
metrics.FirstOrDefault(x => x.Name == key)
106-
?.Values.Count == PowertoolsConfigurations.MaxMetrics))
106+
lock (_lockObj)
107107
{
108-
_instance.Flush(true);
108+
var metrics = _context.GetMetrics();
109+
110+
if (metrics.Count > 0 &&
111+
(metrics.Count == PowertoolsConfigurations.MaxMetrics ||
112+
metrics.FirstOrDefault(x => x.Name == key)
113+
?.Values.Count == PowertoolsConfigurations.MaxMetrics))
114+
{
115+
_instance.Flush(true);
116+
}
117+
118+
_context.AddMetric(key, value, unit, metricResolution);
109119
}
110-
111-
_context.AddMetric(key, value, unit, metricResolution);
112120
}
113121

114122
/// <summary>

libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandler.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
* permissions and limitations under the License.
1414
*/
1515

16+
using System;
1617
using System.Globalization;
18+
using System.Linq;
1719
using System.Threading.Tasks;
1820

1921
namespace AWS.Lambda.Powertools.Metrics.Tests.Handlers;
@@ -41,4 +43,16 @@ public async Task<string> HandleTestSecondCall(string input)
4143

4244
return input.ToUpper(CultureInfo.InvariantCulture);
4345
}
46+
47+
[Metrics(Namespace = "ns", Service = "svc")]
48+
public async Task<string> HandleMultipleThreads(string input)
49+
{
50+
await Parallel.ForEachAsync(Enumerable.Range(0, Environment.ProcessorCount * 2), async (x, _) =>
51+
{
52+
Metrics.AddMetric("MyMetric", 1);
53+
await Task.Delay(1);
54+
});
55+
56+
return input.ToUpper(CultureInfo.InvariantCulture);
57+
}
4458
}

libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandlerTests.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,16 @@ public async Task When_Metrics_Add_Metadata_Second_Invocation_Should_Not_Throw_E
4949
exception = await Record.ExceptionAsync( () => handler.HandleTestSecondCall("whatever"));
5050
Assert.Null(exception);
5151
}
52+
53+
[Fact]
54+
public async Task When_Metrics_Add_Metadata_FromMultipleThread_Should_Not_Throw_Exception()
55+
{
56+
// Arrange
57+
Metrics.ResetForTest();
58+
var handler = new FunctionHandler();
59+
60+
// Act
61+
var exception = await Record.ExceptionAsync(() => handler.HandleMultipleThreads("whatever"));
62+
Assert.Null(exception);
63+
}
5264
}

0 commit comments

Comments
 (0)