Merge pull request #505 from hjgraca/fix-exception-addmetadata

fix: Metrics throws exception when using AddMetadata

Author: hjgraca

docs/core/metrics.md

@@ -311,6 +311,9 @@ You can add high-cardinality data as part of your Metrics log with `AddMetadata`
 !!! info
     **This will not be available during metrics visualization** - Use **dimensions** for this purpose
 
+!!! info
+    Adding metadata with a key that is the same as an existing metric will be ignored
+
 === "Function.cs"
 
     ```csharp hl_lines="9"

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

@@ -1,12 +1,12 @@
 /*
  * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License").
  * You may not use this file except in compliance with the License.
  * A copy of the License is located at
- * 
+ *
  *  http://aws.amazon.com/apache2.0
- * 
+ *
  * or in the "license" file accompanying this file. This file is distributed
  * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
  * express or implied. See the License for the specific language governing
@@ -29,7 +29,7 @@ public class Metadata
     /// </summary>
     public Metadata()
     {
-        CloudWatchMetrics = new List<MetricDirective> {new()};
+        CloudWatchMetrics = new List<MetricDirective> { new() };
         CustomMetadata = new Dictionary<string, object>();
     }
 
@@ -66,6 +66,7 @@ public Metadata()
     internal void ClearMetrics()
     {
         _metricDirective.Metrics.Clear();
+        CustomMetadata?.Clear();
     }
 
     /// <summary>
@@ -158,7 +159,7 @@ internal List<MetricDefinition> GetMetrics()
     /// <param name="value">Metadata value</param>
     internal void AddMetadata(string key, object value)
     {
-        CustomMetadata.Add(key, value);
+        CustomMetadata.TryAdd(key, value);
     }
 
     /// <summary>

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

@@ -43,14 +43,14 @@ public Dictionary<string, object> MetricData
             var targetMembers = new Dictionary<string, object>();
 
             foreach (var dimension in AWS.ExpandAllDimensionSets()) targetMembers.Add(dimension.Key, dimension.Value);
-
-            foreach (var metadata in AWS.CustomMetadata) targetMembers.Add(metadata.Key, metadata.Value);
-
+            
             foreach (var metricDefinition in AWS.GetMetrics())
             {
                 var values = metricDefinition.Values;
                 targetMembers.Add(metricDefinition.Name, values.Count == 1 ? values[0] : values);
             }
+            
+            foreach (var metadata in AWS.CustomMetadata) targetMembers.TryAdd(metadata.Key, metadata.Value);
 
             return targetMembers;
         }

libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs

@@ -546,7 +546,47 @@ public void WhenMetricsAndMetadataAdded_ValidateOutput()
             var result = consoleOut.ToString();
 
             // Assert
-            Assert.Contains("CloudWatchMetrics\":[{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"Time\",\"Unit\":\"Milliseconds\"}],\"Dimensions\":[[\"Service\"],[\"functionVersion\"]]}]},\"Service\":\"testService\",\"functionVersion\":\"$LATEST\",\"env\":\"dev\",\"Time\":100.7}"
+            Assert.Contains("CloudWatchMetrics\":[{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"Time\",\"Unit\":\"Milliseconds\"}],\"Dimensions\":[[\"Service\"],[\"functionVersion\"]]}]},\"Service\":\"testService\",\"functionVersion\":\"$LATEST\",\"Time\":100.7,\"env\":\"dev\"}"
+                , result);
+
+            // Reset
+            handler.ResetForTest();
+        }
+        
+        [Fact]
+        public void When_Metrics_And_Metadata_Added_With_Same_Key_Should_Only_Write_Metrics()
+        {
+            // Arrange
+            var methodName = Guid.NewGuid().ToString();
+            var consoleOut = new StringWriter();
+            Console.SetOut(consoleOut);
+            var configurations = Substitute.For<IPowertoolsConfigurations>();
+
+            var metrics = new Metrics(
+                configurations,
+                nameSpace: "dotnet-powertools-test",
+                service: "testService"
+            );
+
+            var handler = new MetricsAspectHandler(
+                metrics,
+                false
+            );
+
+            var eventArgs = new AspectEventArgs { Name = methodName };
+
+            // Act 
+            handler.OnEntry(eventArgs);
+            Metrics.AddDimension("functionVersion", "$LATEST");
+            Metrics.AddMetric("Time", 100.7, MetricUnit.Milliseconds);
+            Metrics.AddMetadata("Time", "dev");
+            handler.OnExit(eventArgs);
+
+            var result = consoleOut.ToString();
+
+            // Assert
+            // No Metadata key was added
+            Assert.Contains("CloudWatchMetrics\":[{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"Time\",\"Unit\":\"Milliseconds\"}],\"Dimensions\":[[\"Service\"],[\"functionVersion\"]]}]},\"Service\":\"testService\",\"functionVersion\":\"$LATEST\",\"Time\":100.7}"
                 , result);
 
             // Reset

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

@@ -0,0 +1,44 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+using System.Globalization;
+using System.Threading.Tasks;
+
+namespace AWS.Lambda.Powertools.Metrics.Tests.Handlers;
+
+public class FunctionHandler
+{
+    [Metrics(Namespace = "ns", Service = "svc")]
+    public async Task<string> HandleSameKey(string input)
+    {
+        Metrics.AddMetric("MyMetric", 1);
+        Metrics.AddMetadata("MyMetric", "meta");
+
+        await Task.Delay(1);
+
+        return input.ToUpper(CultureInfo.InvariantCulture);
+    }
+    
+    [Metrics(Namespace = "ns", Service = "svc")]
+    public async Task<string> HandleTestSecondCall(string input)
+    {
+        Metrics.AddMetric("MyMetric", 1);
+        Metrics.AddMetadata("MyMetadata", "meta");
+        
+        await Task.Delay(1);
+
+        return input.ToUpper(CultureInfo.InvariantCulture);
+    }
+}

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

@@ -0,0 +1,52 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+using System.Threading.Tasks;
+using Xunit;
+
+namespace AWS.Lambda.Powertools.Metrics.Tests.Handlers;
+
+[Collection("Sequential")]
+public class FunctionHandlerTests
+{
+    [Fact]
+    public async Task When_Metrics_Add_Metadata_Same_Key_Should_Ignore_Metadata()
+    {
+        // Arrange
+        Metrics.ResetForTest();
+        var handler = new FunctionHandler();
+        
+        // Act
+        var exception = await Record.ExceptionAsync( () => handler.HandleSameKey("whatever"));
+        
+        // Assert
+        Assert.Null(exception);
+    }
+    
+    [Fact]
+    public async Task When_Metrics_Add_Metadata_Second_Invocation_Should_Not_Throw_Exception()
+    {
+        // Arrange
+        Metrics.ResetForTest();
+        var handler = new FunctionHandler();
+
+        // Act
+        var exception = await Record.ExceptionAsync( () => handler.HandleTestSecondCall("whatever"));
+        Assert.Null(exception);
+        
+        exception = await Record.ExceptionAsync( () => handler.HandleTestSecondCall("whatever"));
+        Assert.Null(exception);
+    }
+}