Skip to content

Commit

Permalink
Merge pull request #499 from hjgraca/metrics-decorator-exception
Browse files Browse the repository at this point in the history 
fix: Metrics throws exception when decorating non handler methods
  • Loading branch information
@hjgraca
hjgraca authored Oct 20, 2023
2 parents 654de19 + 95d300b commit 21aff1f
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 24 deletions.
2 changes: 1 addition & 1 deletion libraries/src/AWS.Lambda.Powertools.Metrics/IMetrics.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace AWS.Lambda.Powertools.Metrics;
/// Implements the <see cref="System.IDisposable" />
/// </summary>
/// <seealso cref="System.IDisposable" />
public interface IMetrics : IDisposable
public interface IMetrics
{
/// <summary>
/// Adds metric
Expand Down
52 changes: 33 additions & 19 deletions libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ namespace AWS.Lambda.Powertools.Metrics;
/// Implements the <see cref="IMetrics" />
/// </summary>
/// <seealso cref="IMetrics" />
public class Metrics : IMetrics
public class Metrics : IMetrics, IDisposable
{
/// <summary>
/// The instance
/// </summary>
private static IMetrics _instance;

/// <summary>
/// The context
/// </summary>
Expand Down Expand Up @@ -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);

}

/// <summary>
Expand All @@ -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();
Expand Down Expand Up @@ -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);
}
Expand All @@ -168,28 +168,28 @@ 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);
}

/// <summary>
/// Implements interface that sets default dimension list
/// </summary>
/// <param name="defaultDimensions">Default Dimension List</param>
/// <param name="defaultDimension">Default Dimension List</param>
/// <exception cref="System.ArgumentNullException">
/// 'SetDefaultDimensions' method requires a valid key pair. 'Null' or empty
/// values are not allowed.
/// </exception>
void IMetrics.SetDefaultDimensions(Dictionary<string, string> defaultDimensions)
void IMetrics.SetDefaultDimensions(Dictionary<string, string> 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.");
throw new ArgumentNullException(nameof(item.Key),
"'SetDefaultDimensions' method requires a valid key pair. 'Null' or empty values are not allowed.");

_context.SetDefaultDimensions(DictionaryToList(defaultDimensions));
_context.SetDefaultDimensions(DictionaryToList(defaultDimension));
}

/// <summary>
Expand Down Expand Up @@ -258,8 +258,8 @@ void IMetrics.PushSingleMetric(string metricName, double value, MetricUnit unit,
Dictionary<string, string> 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);
Expand All @@ -272,7 +272,21 @@ void IMetrics.PushSingleMetric(string metricName, double value, MetricUnit unit,
/// </summary>
public void Dispose()
{
_instance.Flush();
Dispose(true);
GC.SuppressFinalize(this);
}

/// <summary>
///
/// </summary>
/// <param name="disposing"></param>
protected virtual void Dispose(bool disposing)
{
// Cleanup
if (disposing)
{
_instance.Flush();
}
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ public void WhenMetricIsNegativeValue_ThrowException()

// Assert
var exception = Assert.Throws<ArgumentException>(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();
Expand Down
38 changes: 35 additions & 3 deletions libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,49 @@ namespace AWS.Lambda.Powertools.Metrics.Tests.Handlers;
public class ExceptionFunctionHandler
{
[Metrics(Namespace = "ns", Service = "svc")]
public async Task<string> Handle(string input)
public Task<string> Handle(string input)
{
ThisThrows();
return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture));
}

[Metrics(Namespace = "ns", Service = "svc")]
public Task<string> HandleDecoratorOutsideHandler(string input)
{
MethodDecorated();

Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count);

await Task.Delay(1);
return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture));
}

[Metrics(Namespace = "ns", Service = "svc")]
public Task<string> HandleDecoratorOutsideHandlerException(string input)
{
MethodDecorated();

Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count);

ThisThrowsDecorated();

return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture));
}

return input.ToUpper(CultureInfo.InvariantCulture);
[Metrics(Namespace = "ns", Service = "svc")]
private void MethodDecorated()
{
Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count);
Metrics.AddMetric($"Metric Name Decorated", 1, MetricUnit.Count);
}

private void ThisThrows()
{
throw new NullReferenceException();
}

[Metrics(Namespace = "ns", Service = "svc")]
private void ThisThrowsDecorated()
{
throw new NullReferenceException();
}
}
29 changes: 29 additions & 0 deletions ...aries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,35 @@ public async Task Stack_Trace_Included_When_Decorator_Present()
// Assert
var tracedException = await Assert.ThrowsAsync<NullReferenceException>(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<NullReferenceException>(Handle);
Assert.StartsWith("at AWS.Lambda.Powertools.Metrics.Tests.Handlers.ExceptionFunctionHandler.__a$_around_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);
}
}

0 comments on commit 21aff1f

Please sign in to comment.