From c924443dbc4527930fa5ceff9dc1efe6f141807a Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Tue, 3 Oct 2023 12:34:01 +0100 Subject: [PATCH 1/6] Update Metrics constructor to not return and set all values. Add tests --- .../AWS.Lambda.Powertools.Metrics/Metrics.cs | 4 ++-- .../Handlers/ExceptionFunctionHandler.cs | 21 ++++++++++++++++--- .../Handlers/ExceptionFunctionHandlerTests.cs | 17 +++++++++++++++ 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs index 9d55b06ae..cb3259a68 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs @@ -65,15 +65,15 @@ public class Metrics : IMetrics internal Metrics(IPowertoolsConfigurations powertoolsConfigurations, string nameSpace = null, string service = null, bool raiseOnEmptyMetrics = false, bool captureColdStartEnabled = false) { - if (_instance != null) return; + _instance ??= this; - _instance = this; _powertoolsConfigurations = powertoolsConfigurations; _raiseOnEmptyMetrics = raiseOnEmptyMetrics; _captureColdStartEnabled = captureColdStartEnabled; _context = InitializeContext(nameSpace, service, null); _powertoolsConfigurations.SetExecutionEnvironment(this); + } /// diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs index 61ba60c88..d3fb48be4 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs @@ -7,13 +7,28 @@ namespace AWS.Lambda.Powertools.Metrics.Tests.Handlers; public class ExceptionFunctionHandler { [Metrics(Namespace = "ns", Service = "svc")] - public async Task Handle(string input) + public Task Handle(string input) { ThisThrows(); + return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture)); + } + + [Metrics(Namespace = "ns", Service = "svc")] + public Task HandleDecoratorOutsideHandler(string input) + { + MethodDecorated(); + + Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count); - await Task.Delay(1); + return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture)); + } - return input.ToUpper(CultureInfo.InvariantCulture); + [Metrics(Namespace = "ns", Service = "svc")] + private void MethodDecorated() + { + // NOOP + Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count); + Metrics.AddMetric($"Metric Name Decorated", 1, MetricUnit.Count); } private void ThisThrows() diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs index 011cd1e5c..77bfd0a2b 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs @@ -22,4 +22,21 @@ public async Task Stack_Trace_Included_When_Decorator_Present() Assert.StartsWith("at AWS.Lambda.Powertools.Metrics.Tests.Handlers.ExceptionFunctionHandler.ThisThrows()", tracedException.StackTrace?.TrimStart()); } + + [Fact] + public async Task Decorator_In_Non_Handler_Method_Does_Not_Throw_Exception() + { + // Arrange + Metrics.ResetForTest(); + var handler = new ExceptionFunctionHandler(); + + // Act + Task Handle() => handler.HandleDecoratorOutsideHandler("whatever"); + + // Assert + var tracedException = await Record.ExceptionAsync(Handle); + Assert.Null(tracedException); + //Assert.StartsWith("at AWS.Lambda.Powertools.Metrics.Tests.Handlers.ExceptionFunctionHandler.ThisThrows()", tracedException.StackTrace?.TrimStart()); + + } } \ No newline at end of file From 1ae3c0ccf2efe62c7ee69912e7e302a45a145de1 Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Tue, 3 Oct 2023 12:37:17 +0100 Subject: [PATCH 2/6] remove test comments --- .../Handlers/ExceptionFunctionHandlerTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs index 77bfd0a2b..d4d8d4fcd 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs @@ -36,7 +36,5 @@ public async Task Decorator_In_Non_Handler_Method_Does_Not_Throw_Exception() // Assert var tracedException = await Record.ExceptionAsync(Handle); Assert.Null(tracedException); - //Assert.StartsWith("at AWS.Lambda.Powertools.Metrics.Tests.Handlers.ExceptionFunctionHandler.ThisThrows()", tracedException.StackTrace?.TrimStart()); - } } \ No newline at end of file From 073da9c96b6d090dd8e1c1dcafb1daecf9fb3356 Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Wed, 4 Oct 2023 11:29:27 +0100 Subject: [PATCH 3/6] test stacktrace with decorated method --- .../Handlers/ExceptionFunctionHandler.cs | 19 ++++++++++++++++++- .../Handlers/ExceptionFunctionHandlerTests.cs | 14 ++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs index d3fb48be4..8a9e9204c 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs @@ -22,11 +22,22 @@ public Task HandleDecoratorOutsideHandler(string input) return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture)); } + + [Metrics(Namespace = "ns", Service = "svc")] + public Task HandleDecoratorOutsideHandlerException(string input) + { + MethodDecorated(); + + Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count); + + ThisThrowsDecorated(); + + return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture)); + } [Metrics(Namespace = "ns", Service = "svc")] private void MethodDecorated() { - // NOOP Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count); Metrics.AddMetric($"Metric Name Decorated", 1, MetricUnit.Count); } @@ -35,4 +46,10 @@ private void ThisThrows() { throw new NullReferenceException(); } + + [Metrics(Namespace = "ns", Service = "svc")] + private void ThisThrowsDecorated() + { + throw new NullReferenceException(); + } } \ No newline at end of file diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs index d4d8d4fcd..0a1aad173 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs @@ -20,7 +20,21 @@ public async Task Stack_Trace_Included_When_Decorator_Present() // Assert var tracedException = await Assert.ThrowsAsync(Handle); Assert.StartsWith("at AWS.Lambda.Powertools.Metrics.Tests.Handlers.ExceptionFunctionHandler.ThisThrows()", tracedException.StackTrace?.TrimStart()); + } + + [Fact] + public async Task Stack_Trace_Included_When_Decorator_Present_In_Method() + { + // Arrange + Metrics.ResetForTest(); + var handler = new ExceptionFunctionHandler(); + // Act + Task Handle() => handler.HandleDecoratorOutsideHandlerException("whatever"); + + // Assert + var tracedException = await Assert.ThrowsAsync(Handle); + Assert.StartsWith("at AWS.Lambda.Powertools.Metrics.Tests.Handlers.ExceptionFunctionHandler.__a$_around_ThisThrows", tracedException.StackTrace?.TrimStart()); } [Fact] From b40af90e18ce7314f724b3b677b11131c02a879a Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Mon, 9 Oct 2023 21:44:02 +0100 Subject: [PATCH 4/6] fix sonar issues --- .../AWS.Lambda.Powertools.Metrics/IMetrics.cs | 2 +- .../AWS.Lambda.Powertools.Metrics/Metrics.cs | 28 ++++++++++++++----- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/IMetrics.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/IMetrics.cs index 5d33f484b..6f0b868a0 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/IMetrics.cs +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/IMetrics.cs @@ -23,7 +23,7 @@ namespace AWS.Lambda.Powertools.Metrics; /// Implements the /// /// -public interface IMetrics : IDisposable +public interface IMetrics { /// /// Adds metric diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs index cb3259a68..9a2fe575a 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs @@ -25,13 +25,13 @@ namespace AWS.Lambda.Powertools.Metrics; /// Implements the /// /// -public class Metrics : IMetrics +public class Metrics : IMetrics, IDisposable { /// /// The instance /// private static IMetrics _instance; - + /// /// The context /// @@ -177,19 +177,19 @@ void IMetrics.AddMetadata(string key, object value) /// /// Implements interface that sets default dimension list /// - /// Default Dimension List + /// Default Dimension List /// /// 'SetDefaultDimensions' method requires a valid key pair. 'Null' or empty /// values are not allowed. /// - void IMetrics.SetDefaultDimensions(Dictionary defaultDimensions) + void IMetrics.SetDefaultDimensions(Dictionary defaultDimension) { - foreach (var item in defaultDimensions) + foreach (var item in defaultDimension) if (string.IsNullOrWhiteSpace(item.Key) || string.IsNullOrWhiteSpace(item.Value)) throw new ArgumentNullException( $"'SetDefaultDimensions' method requires a valid key pair. 'Null' or empty values are not allowed."); - _context.SetDefaultDimensions(DictionaryToList(defaultDimensions)); + _context.SetDefaultDimensions(DictionaryToList(defaultDimension)); } /// @@ -272,7 +272,21 @@ void IMetrics.PushSingleMetric(string metricName, double value, MetricUnit unit, /// public void Dispose() { - _instance.Flush(); + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// + /// + /// + protected virtual void Dispose(bool disposing) + { + // Cleanup + if (disposing) + { + _instance.Flush(); + } } /// From ff808865994cf98fa9ce0450cb14259becfaae5c Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Mon, 9 Oct 2023 21:53:56 +0100 Subject: [PATCH 5/6] more sonar updates --- .../AWS.Lambda.Powertools.Metrics/Metrics.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs b/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs index 9a2fe575a..af72bd895 100644 --- a/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs +++ b/libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs @@ -91,11 +91,11 @@ void IMetrics.AddMetric(string key, double value, MetricUnit unit, MetricResolut { if (string.IsNullOrWhiteSpace(key)) throw new ArgumentNullException( - $"'AddMetric' method requires a valid metrics key. 'Null' or empty values are not allowed."); + nameof(key), "'AddMetric' method requires a valid metrics key. 'Null' or empty values are not allowed."); if (value < 0) { throw new ArgumentException( - "'AddMetric' method requires a valid metrics value. Value must be >= 0."); + "'AddMetric' method requires a valid metrics value. Value must be >= 0.", nameof(value)); } var metrics = _context.GetMetrics(); @@ -150,8 +150,8 @@ string IMetrics.GetService() void IMetrics.AddDimension(string key, string value) { if (string.IsNullOrWhiteSpace(key)) - throw new ArgumentNullException( - $"'AddDimension' method requires a valid dimension key. 'Null' or empty values are not allowed."); + throw new ArgumentNullException(nameof(key), + "'AddDimension' method requires a valid dimension key. 'Null' or empty values are not allowed."); _context.AddDimension(key, value); } @@ -168,8 +168,8 @@ void IMetrics.AddDimension(string key, string value) void IMetrics.AddMetadata(string key, object value) { if (string.IsNullOrWhiteSpace(key)) - throw new ArgumentNullException( - $"'AddMetadata' method requires a valid metadata key. 'Null' or empty values are not allowed."); + throw new ArgumentNullException(nameof(key), + "'AddMetadata' method requires a valid metadata key. 'Null' or empty values are not allowed."); _context.AddMetadata(key, value); } @@ -186,8 +186,8 @@ void IMetrics.SetDefaultDimensions(Dictionary defaultDimension) { foreach (var item in defaultDimension) if (string.IsNullOrWhiteSpace(item.Key) || string.IsNullOrWhiteSpace(item.Value)) - throw new ArgumentNullException( - $"'SetDefaultDimensions' method requires a valid key pair. 'Null' or empty values are not allowed."); + throw new ArgumentNullException(nameof(item.Key), + "'SetDefaultDimensions' method requires a valid key pair. 'Null' or empty values are not allowed."); _context.SetDefaultDimensions(DictionaryToList(defaultDimension)); } @@ -258,8 +258,8 @@ void IMetrics.PushSingleMetric(string metricName, double value, MetricUnit unit, Dictionary defaultDimensions, MetricResolution metricResolution) { if (string.IsNullOrWhiteSpace(metricName)) - throw new ArgumentNullException( - $"'PushSingleMetric' method requires a valid metrics key. 'Null' or empty values are not allowed."); + throw new ArgumentNullException(nameof(metricName), + "'PushSingleMetric' method requires a valid metrics key. 'Null' or empty values are not allowed."); using var context = InitializeContext(nameSpace, service, defaultDimensions); context.AddMetric(metricName, value, unit, metricResolution); From 95d300ba9fc7b4d66e278d29a9f15d2e71bb50c5 Mon Sep 17 00:00:00 2001 From: Henrique Graca <999396+hjgraca@users.noreply.github.com> Date: Mon, 9 Oct 2023 21:59:06 +0100 Subject: [PATCH 6/6] exception now has parameter --- .../AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs index 4072a1d74..d08b598d4 100644 --- a/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs +++ b/libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs @@ -467,7 +467,7 @@ public void WhenMetricIsNegativeValue_ThrowException() // Assert var exception = Assert.Throws(act); - Assert.Equal("'AddMetric' method requires a valid metrics value. Value must be >= 0.", exception.Message); + Assert.Equal("'AddMetric' method requires a valid metrics value. Value must be >= 0. (Parameter 'value')", exception.Message); // RESET handler.ResetForTest();