From 01f61d829b83bdb0261bff7ccd5b380b068f87fc Mon Sep 17 00:00:00 2001 From: Henrique <999396+hjgraca@users.noreply.github.com> Date: Thu, 11 Jul 2024 18:13:26 +0100 Subject: [PATCH 1/7] add dependency to ILambdaContext, Powertools Lambda Context fails serialization due to reflection (AOT). Create new converter and use the default one on strings only. --- .../AWS.Lambda.Powertools.Metrics.csproj | 8 ++ .../Internal/MetricsAspect.cs | 35 +++--- .../Model/MetricResolution.cs | 2 +- .../Model/MetricUnit.cs | 2 +- .../MetricResolutionJsonConverter.cs | 52 +++++++++ .../Serializer/StringEnumConverter.cs | 102 ------------------ 6 files changed, 84 insertions(+), 117 deletions(-) create mode 100644 libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/MetricResolutionJsonConverter.cs delete mode 100644 libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/StringEnumConverter.cs diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/AWS.Lambda.Powertools.Metrics.csproj b/libraries/src/AWS.Lambda.Powertools.Metrics/AWS.Lambda.Powertools.Metrics.csproj index 0e90e99b..87db1bca 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/AWS.Lambda.Powertools.Metrics.csproj +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/AWS.Lambda.Powertools.Metrics.csproj @@ -10,5 +10,13 @@ + + + + + + + + diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Internal/MetricsAspect.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Internal/MetricsAspect.cs index abbefc5f..a72e205e 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/Internal/MetricsAspect.cs +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/Internal/MetricsAspect.cs @@ -17,6 +17,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; +using Amazon.Lambda.Core; using AspectInjector.Broker; using AWS.Lambda.Powertools.Common; @@ -34,11 +35,6 @@ public class MetricsAspect /// private static bool _isColdStart; - /// - /// Specify to clear Lambda Context on exit - /// - private bool _clearLambdaContext; - /// /// Gets the metrics instance. /// @@ -100,14 +96,14 @@ public void Before( var nameSpace = _metricsInstance.GetNamespace(); var service = _metricsInstance.GetService(); Dictionary dimensions = null; - - _clearLambdaContext = PowertoolsLambdaContext.Extract(eventArgs); - - if (PowertoolsLambdaContext.Instance is not null) + + var context = GetContext(eventArgs); + + if (context is not null) { dimensions = new Dictionary { - { "FunctionName", PowertoolsLambdaContext.Instance.FunctionName } + { "FunctionName", context.FunctionName } }; } @@ -129,8 +125,6 @@ public void Before( public void Exit() { _metricsInstance.Flush(); - if (_clearLambdaContext) - PowertoolsLambdaContext.Clear(); } @@ -142,6 +136,21 @@ internal static void ResetForTest() _metricsInstance = null; _isColdStart = true; Metrics.ResetForTest(); - PowertoolsLambdaContext.Clear(); + } + + /// + /// Gets the Lambda context + /// + /// + /// + private static ILambdaContext GetContext(AspectEventArgs args) + { + var index = Array.FindIndex(args.Method.GetParameters(), p => p.ParameterType == typeof(ILambdaContext)); + if (index >= 0) + { + return (ILambdaContext)args.Args[index]; + } + + return null; } } \ No newline at end of file diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricResolution.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricResolution.cs index 2112c1c5..3929beec 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricResolution.cs +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricResolution.cs @@ -1,4 +1,3 @@ -using System.Runtime.Serialization; using System.Text.Json.Serialization; namespace AWS.Lambda.Powertools.Metrics; @@ -6,6 +5,7 @@ namespace AWS.Lambda.Powertools.Metrics; /// /// Enum MetricResolution /// +[JsonConverter(typeof(MetricResolutionJsonConverter))] public enum MetricResolution { /// diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricUnit.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricUnit.cs index 132b10f3..2fbb389d 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricUnit.cs +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricUnit.cs @@ -24,7 +24,7 @@ namespace AWS.Lambda.Powertools.Metrics; #if NET8_0_OR_GREATER [JsonConverter(typeof(JsonStringEnumConverter))] #else -[JsonConverter(typeof(StringEnumConverter))] +[JsonConverter(typeof(JsonStringEnumConverter))] #endif public enum MetricUnit { diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/MetricResolutionJsonConverter.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/MetricResolutionJsonConverter.cs new file mode 100644 index 00000000..164f2dd9 --- /dev/null +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/MetricResolutionJsonConverter.cs @@ -0,0 +1,52 @@ +using System; +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace AWS.Lambda.Powertools.Metrics; + +/// +/// Class MetricResolutionJsonConverter. +/// Implements the +/// +/// +public class MetricResolutionJsonConverter : JsonConverter +{ + /// + /// Reads the JSON representation of the object. + /// + /// The to read from. + /// The being converted. + /// The being used. + /// The object value. + public override MetricResolution Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + switch (reader.TokenType) + { + case JsonTokenType.String: + { + string stringValue = reader.GetString(); + if (int.TryParse(stringValue, out int value)) + { + return (MetricResolution)value; + } + + break; + } + case JsonTokenType.Number: + return (MetricResolution)reader.GetInt32(); + } + + throw new JsonException(); + } + + /// + /// Writes the JSON representation of the object. + /// + /// The to write to. + /// The value to convert. + /// The being used. + public override void Write(Utf8JsonWriter writer, MetricResolution value, JsonSerializerOptions options) + { + writer.WriteNumberValue((int)value); + } +} \ No newline at end of file diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/StringEnumConverter.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/StringEnumConverter.cs deleted file mode 100644 index 242ee90b..00000000 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/StringEnumConverter.cs +++ /dev/null @@ -1,102 +0,0 @@ -/* - * 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; -using System.Linq; -using System.Reflection; -using System.Runtime.Serialization; -using System.Text.Json; -using System.Text.Json.Serialization; - -namespace AWS.Lambda.Powertools.Metrics; - -#if NET6_0 - -/// -/// Class StringEnumConverter. -/// Implements the -/// .NET 6 only -/// -/// -public class StringEnumConverter : JsonConverterFactory -{ - /// - /// The allow integer values - /// - private readonly bool _allowIntegerValues; - - /// - /// The base converter - /// - private readonly JsonStringEnumConverter _baseConverter; - - /// - /// The naming policy - /// - private readonly JsonNamingPolicy _namingPolicy; - - /// - /// Initializes a new instance of the class. - /// - public StringEnumConverter() : this(null) - { - } - - /// - /// Initializes a new instance of the class. - /// - /// The naming policy. - /// if set to true [allow integer values]. - private StringEnumConverter(JsonNamingPolicy namingPolicy = null, bool allowIntegerValues = true) - { - _namingPolicy = namingPolicy; - _allowIntegerValues = allowIntegerValues; - _baseConverter = new JsonStringEnumConverter(namingPolicy, allowIntegerValues); - } - - /// - /// When overridden in a derived class, determines whether the converter instance can convert the specified object - /// type. - /// - /// The type of the object to check whether it can be converted by this converter instance. - /// - /// if the instance can convert the specified object type; otherwise, - /// . - /// - public override bool CanConvert(Type typeToConvert) - { - return _baseConverter.CanConvert(typeToConvert); - } - - /// - /// Creates a converter for a specified type. - /// - /// The type handled by the converter. - /// The serialization options to use. - /// A converter for which type is compatible with . - public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options) - { - var query = from field in typeToConvert.GetFields(BindingFlags.Public | BindingFlags.Static) - let attr = field.GetCustomAttribute() - where attr != null - select (field.Name, attr.Value); - var dictionary = query.ToDictionary(p => p.Item1, p => p.Item2); - return dictionary.Count > 0 - ? new JsonStringEnumConverter(new DictionaryLookupNamingPolicy(dictionary, _namingPolicy), - _allowIntegerValues).CreateConverter(typeToConvert, options) - : _baseConverter.CreateConverter(typeToConvert, options); - } -} -#endif \ No newline at end of file From 623f8069e3a033e7be50192fe8c9329f30bdbb22 Mon Sep 17 00:00:00 2001 From: Henrique <999396+hjgraca@users.noreply.github.com> Date: Thu, 11 Jul 2024 18:25:20 +0100 Subject: [PATCH 2/7] add tests for context --- .../Handlers/FunctionHandler.cs | 8 +++- .../Handlers/FunctionHandlerTests.cs | 40 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandler.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandler.cs index 8ae59b9f..8447575d 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandler.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandler.cs @@ -169,8 +169,14 @@ public void HandleWithLambdaContext(ILambdaContext context) } [Metrics(Namespace = "ns", Service = "svc")] - public void HandleWithLambdaContextAndMetrics(TestLambdaContext context) + public void HandleColdStartNoContext() { Metrics.AddMetric("MyMetric", 1); } + + [Metrics(Namespace = "ns", Service = "svc", CaptureColdStart = true)] + public void HandleWithParamAndLambdaContext(string input, ILambdaContext context) + { + + } } \ No newline at end of file diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandlerTests.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandlerTests.cs index 556d6cce..075da0a0 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandlerTests.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/FunctionHandlerTests.cs @@ -88,6 +88,46 @@ public void When_LambdaContext_Should_Add_FunctioName_Dimension_CaptureColdStart "\"Metrics\":[{\"Name\":\"ColdStart\",\"Unit\":\"Count\"}],\"Dimensions\":[[\"FunctionName\"],[\"Service\"]]}]}", metricsOutput); } + + [Fact] + public void When_LambdaContext_And_Parameter_Should_Add_FunctioName_Dimension_CaptureColdStart() + { + // Arrange + var context = new TestLambdaContext + { + FunctionName = "My Function with context" + }; + + // Act + _handler.HandleWithParamAndLambdaContext("Hello",context); + var metricsOutput = _consoleOut.ToString(); + + // Assert + Assert.Contains( + "\"FunctionName\":\"My Function with context\"", + metricsOutput); + + Assert.Contains( + "\"Metrics\":[{\"Name\":\"ColdStart\",\"Unit\":\"Count\"}],\"Dimensions\":[[\"FunctionName\"],[\"Service\"]]}]}", + metricsOutput); + } + + [Fact] + public void When_No_LambdaContext_Should_Not_Add_FunctioName_Dimension_CaptureColdStart() + { + // Act + _handler.HandleColdStartNoContext(); + var metricsOutput = _consoleOut.ToString(); + + // Assert + Assert.DoesNotContain( + "\"FunctionName\"", + metricsOutput); + + Assert.Contains( + "\"Metrics\":[{\"Name\":\"MyMetric\",\"Unit\":\"None\"}],\"Dimensions\":[[\"Service\"]]}]},\"Service\":\"svc\",\"MyMetric\":1}", + metricsOutput); + } public void Dispose() { From 4bd4329f1ddfd7e9e7dda2bc1fe5514d11c45f60 Mon Sep 17 00:00:00 2001 From: Henrique <999396+hjgraca@users.noreply.github.com> Date: Thu, 11 Jul 2024 18:39:44 +0100 Subject: [PATCH 3/7] update version for release --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index ec7a6148..9fb16922 100644 --- a/version.json +++ b/version.json @@ -1,7 +1,7 @@ { "Core": { "Logging": "1.5.1", - "Metrics": "1.7.0", + "Metrics": "1.7.1", "Tracing": "1.4.2" }, "Utilities": { From e799f2a22d95a8e224f86e252c0213ec9dbae799 Mon Sep 17 00:00:00 2001 From: Henrique <999396+hjgraca@users.noreply.github.com> Date: Thu, 11 Jul 2024 18:41:50 +0100 Subject: [PATCH 4/7] remove file removal from csproj --- .../AWS.Lambda.Powertools.Metrics.csproj | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/AWS.Lambda.Powertools.Metrics.csproj b/libraries/src/AWS.Lambda.Powertools.Metrics/AWS.Lambda.Powertools.Metrics.csproj index 87db1bca..e20deb7e 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/AWS.Lambda.Powertools.Metrics.csproj +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/AWS.Lambda.Powertools.Metrics.csproj @@ -10,11 +10,7 @@ - - - - - + From 9b42b344bc16c489ddbfe2b65ed7c3853f715fcb Mon Sep 17 00:00:00 2001 From: Henrique <999396+hjgraca@users.noreply.github.com> Date: Thu, 11 Jul 2024 19:11:57 +0100 Subject: [PATCH 5/7] add unit tests for MetricResolutionJsonConverter --- .../SerializationTests.cs | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/SerializationTests.cs diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/SerializationTests.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/SerializationTests.cs new file mode 100644 index 00000000..85446da2 --- /dev/null +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/SerializationTests.cs @@ -0,0 +1,65 @@ +using System; +using System.Text.Json; +using Xunit; + +namespace AWS.Lambda.Powertools.Metrics.Tests; + +[Collection("Sequential")] +public class SerializationTests +{ + [Fact] + public void Metrics_Resolution_JsonConverter() + { + // Arrange + var options = new JsonSerializerOptions(); + options.Converters.Add(new MetricResolutionJsonConverter()); + + { + var myInt = JsonSerializer.Deserialize("1", options); + Assert.Equal(MetricResolution.High, myInt); + Assert.Equal("1", JsonSerializer.Serialize(myInt, options)); + } + + { + var myInt = JsonSerializer.Deserialize("60", options); + Assert.Equal(MetricResolution.Standard, myInt); + Assert.Equal("60", JsonSerializer.Serialize(myInt, options)); + } + + { + var myInt = JsonSerializer.Deserialize("0", options); + Assert.Equal(MetricResolution.Default, myInt); + Assert.Equal("0", JsonSerializer.Serialize(myInt, options)); + } + + { + var myInt = JsonSerializer.Deserialize(@"""1""", options); + Assert.Equal(MetricResolution.High, myInt); + Assert.Equal("1", JsonSerializer.Serialize(myInt, options)); + } + { + var myInt = JsonSerializer.Deserialize(@"""60""", options); + Assert.Equal(MetricResolution.Standard, myInt); + Assert.Equal("60", JsonSerializer.Serialize(myInt, options)); + } + { + var myInt = JsonSerializer.Deserialize(@"""0""", options); + Assert.Equal(MetricResolution.Default, myInt); + Assert.Equal("0", JsonSerializer.Serialize(myInt, options)); + } + } + + [Fact] + public void Metrics_Resolution_JsonConverter_Exception() + { + // Arrange + var options = new JsonSerializerOptions(); + options.Converters.Add(new MetricResolutionJsonConverter()); + + // Act + Action act = () => JsonSerializer.Deserialize("abd", options); + + // Assert + Assert.Throws(act); + } +} \ No newline at end of file From e34c38972dffafac14c54525941caabb467cf730 Mon Sep 17 00:00:00 2001 From: Henrique <999396+hjgraca@users.noreply.github.com> Date: Fri, 12 Jul 2024 09:34:07 +0100 Subject: [PATCH 6/7] refactor for codecov --- .../MetricResolutionJsonConverter.cs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/MetricResolutionJsonConverter.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/MetricResolutionJsonConverter.cs index 164f2dd9..710b1a2a 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/MetricResolutionJsonConverter.cs +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/Serializer/MetricResolutionJsonConverter.cs @@ -20,20 +20,17 @@ public class MetricResolutionJsonConverter : JsonConverter /// The object value. public override MetricResolution Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - switch (reader.TokenType) + if (reader.TokenType == JsonTokenType.String) { - case JsonTokenType.String: + var stringValue = reader.GetString(); + if (int.TryParse(stringValue, out int value)) { - string stringValue = reader.GetString(); - if (int.TryParse(stringValue, out int value)) - { - return (MetricResolution)value; - } - - break; + return (MetricResolution)value; } - case JsonTokenType.Number: - return (MetricResolution)reader.GetInt32(); + } + else if (reader.TokenType == JsonTokenType.Number) + { + return (MetricResolution)reader.GetInt32(); } throw new JsonException(); From 7f4d676ba7d07addb52cf9659bf3795fef47f1c8 Mon Sep 17 00:00:00 2001 From: Henrique <999396+hjgraca@users.noreply.github.com> Date: Fri, 12 Jul 2024 09:59:27 +0100 Subject: [PATCH 7/7] more test coverage --- .../SerializationTests.cs | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/SerializationTests.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/SerializationTests.cs index 85446da2..2433c5a2 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/SerializationTests.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/SerializationTests.cs @@ -13,19 +13,25 @@ public void Metrics_Resolution_JsonConverter() // Arrange var options = new JsonSerializerOptions(); options.Converters.Add(new MetricResolutionJsonConverter()); - + + { + var myInt = JsonSerializer.Deserialize(1, options); + Assert.Equal(MetricResolution.High, myInt); + Assert.Equal("1", JsonSerializer.Serialize(myInt, options)); + } + { var myInt = JsonSerializer.Deserialize("1", options); Assert.Equal(MetricResolution.High, myInt); Assert.Equal("1", JsonSerializer.Serialize(myInt, options)); } - + { var myInt = JsonSerializer.Deserialize("60", options); Assert.Equal(MetricResolution.Standard, myInt); Assert.Equal("60", JsonSerializer.Serialize(myInt, options)); } - + { var myInt = JsonSerializer.Deserialize("0", options); Assert.Equal(MetricResolution.Default, myInt); @@ -48,18 +54,27 @@ public void Metrics_Resolution_JsonConverter() Assert.Equal("0", JsonSerializer.Serialize(myInt, options)); } } - + [Fact] public void Metrics_Resolution_JsonConverter_Exception() { // Arrange var options = new JsonSerializerOptions(); options.Converters.Add(new MetricResolutionJsonConverter()); + + { + Action act = () => JsonSerializer.Deserialize("1.3", options); + Assert.Throws(act); + } - // Act - Action act = () => JsonSerializer.Deserialize("abd", options); + { + Action act = () => JsonSerializer.Deserialize(@"""1.3""", options); + Assert.Throws(act); + } - // Assert - Assert.Throws(act); + { + Action act = () => JsonSerializer.Deserialize(@"""abc""", options); + Assert.Throws(act); + } } } \ No newline at end of file