Skip to content

fix: Metrics throws exception when using AddMetadata #505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/core/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
11 changes: 6 additions & 5 deletions libraries/src/AWS.Lambda.Powertools.Metrics/Model/Metadata.cs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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>();
}

Expand Down Expand Up @@ -66,6 +66,7 @@ public Metadata()
internal void ClearMetrics()
{
_metricDirective.Metrics.Clear();
CustomMetadata?.Clear();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear Metadata dictionary when clearing Metrics. Null check for safety

}

/// <summary>
Expand Down Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be safe and avoid exceptions

}

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions libraries/src/AWS.Lambda.Powertools.Metrics/Model/RootNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to the bottom. If Metrics has key. Metadata will be ignored


return targetMembers;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}