From 9a34c30ef0286fd95d28bf7c57da22e60a1d597f Mon Sep 17 00:00:00 2001
From: Henrique Graca <999396+hjgraca@users.noreply.github.com>
Date: Thu, 22 Jun 2023 15:38:43 +0100
Subject: [PATCH] Add lock to avoid race condition when adding metrics. Hold a
 lock for as short time as possible to reduce lock contention. Add tests.

---
 .../Model/MetricDirective.cs                  | 18 +++++--
 .../EMFValidationTests.cs                     | 47 +++++++++++++++++++
 2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricDirective.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricDirective.cs
index 06c0e8ac4..3db76295c 100644
--- a/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricDirective.cs
+++ b/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricDirective.cs
@@ -126,6 +126,11 @@ public List<List<string>> AllDimensionKeys
             return defaultKeys;
         }
     }
+    
+    /// <summary>
+    /// Shared synchronization object
+    /// </summary>
+    private readonly object _lockObj = new();
 
     /// <summary>
     ///     Adds metric to memory
@@ -139,11 +144,14 @@ public void AddMetric(string name, double value, MetricUnit unit, MetricResoluti
     {
         if (Metrics.Count < PowertoolsConfigurations.MaxMetrics)
         {
-            var metric = Metrics.FirstOrDefault(m => m.Name == name);
-            if (metric != null)
-                metric.AddValue(value);
-            else
-                Metrics.Add(new MetricDefinition(name, unit, value, metricResolution));
+            lock (_lockObj)
+            {
+                var metric = Metrics.FirstOrDefault(m => m.Name == name);
+                if (metric != null)
+                    metric.AddValue(value);
+                else
+                    Metrics.Add(new MetricDefinition(name, unit, value, metricResolution));
+            }
         }
         else
         {
diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs
index 79752e4bd..7a48603fe 100644
--- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs
+++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs
@@ -16,6 +16,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Threading.Tasks;
 using AWS.Lambda.Powertools.Common;
 using Moq;
 using Xunit;
@@ -642,5 +643,51 @@ private List<int> AllIndexesOf(string str, string value)
         }
 
         #endregion
+        
+        [Fact]
+        public async Task WhenMetricsAsyncRaceConditionItemSameKeyExists_ValidateLock()
+        {
+            // Arrange
+            var methodName = Guid.NewGuid().ToString();
+            var consoleOut = new StringWriter();
+            Console.SetOut(consoleOut);
+
+            var configurations = new Mock<IPowertoolsConfigurations>();
+
+            var metrics = new Metrics(configurations.Object,
+                nameSpace: "dotnet-powertools-test",
+                service: "testService");
+
+            var handler = new MetricsAspectHandler(metrics,
+                false);
+
+            var eventArgs = new AspectEventArgs { Name = methodName };
+
+            // Act
+            handler.OnEntry(eventArgs);
+
+            var tasks = new List<Task>();
+            for (var i = 0; i < 100; i++)
+            {
+                tasks.Add(Task.Run(() =>
+                {
+                    Metrics.AddMetric($"Metric Name", 0, MetricUnit.Count);
+                }));
+            }
+
+            await Task.WhenAll(tasks);
+
+
+            handler.OnExit(eventArgs);
+
+            var metricsOutput = consoleOut.ToString();
+
+            // Assert
+            Assert.Contains("{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"Metric Name\",\"Unit\":\"Count\"}],\"Dimensions\":[[\"Service\"]]",
+                metricsOutput);
+
+            // Reset
+            handler.ResetForTest();
+        }
     }
 }