Skip to content

fix: Prevent a single metric have more than 100 data points #312

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.

Sign up for GitHub

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 5 commits into from
Jun 27, 2023
Merged

fix: Prevent a single metric have more than 100 data points #312

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5720c2e
Check if single metrics has more than 100 datapoints and flush. Add t…
hjgraca Jun 22, 2023
20af178
Merge branch 'develop' into metrics-prevent-exceed-100-datapoint
hjgraca Jun 26, 2023
aa7b447
Merge branch 'develop' into metrics-prevent-exceed-100-datapoint
hjgraca Jun 27, 2023
6c00e6e
Replace 100 with MaxMetrics. Refactor
hjgraca Jun 27, 2023
546caaa
Rename
hjgraca Jun 27, 2023
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
11 changes: 9 additions & 2 deletions libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using AWS.Lambda.Powertools.Common;

namespace AWS.Lambda.Powertools.Metrics;
Expand Down Expand Up @@ -96,8 +97,14 @@ void IMetrics.AddMetric(string key, double value, MetricUnit unit, MetricResolut
throw new ArgumentException(
"'AddMetric' method requires a valid metrics value. Value must be >= 0.");
}

if (_context.GetMetrics().Count == 100) _instance.Flush(true);

if (_context.GetMetrics().Count > 0 &&
Copy link
Contributor

Choose a reason for hiding this comment

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

use a variable var metrics = _context.GetMetrics(); instead of calling methods multiple times

(_context.GetMetrics().Count == PowertoolsConfigurations.MaxMetrics ||
_context.GetMetrics().FirstOrDefault(x => x.Name == key)
?.Values.Count == PowertoolsConfigurations.MaxMetrics))
{
_instance.Flush(true);
}

_context.AddMetric(key, value, unit, metricResolution);
}
Expand Down
8 changes: 7 additions & 1 deletion libraries/src/AWS.Lambda.Powertools.Metrics/Model/MetricDirective.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,13 @@ public void AddMetric(string name, double value, MetricUnit unit, MetricResoluti
{
var metric = Metrics.FirstOrDefault(m => m.Name == name);
if (metric != null)
metric.AddValue(value);
{
if (metric.Values.Count < PowertoolsConfigurations.MaxMetrics)
metric.AddValue(value);
else
throw new ArgumentOutOfRangeException(nameof(metric),
"Cannot add more than 100 metric data points at the same time.");
}
else
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe we will ever be in this situation (it only accepts one double not an array) but added the throwing exception for consistency with the bellow throw

Metrics.Add(new MetricDefinition(name, unit, value, metricResolution));
}
Expand Down
57 changes: 56 additions & 1 deletion libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,20 +140,75 @@ public void When100MetricsAreAdded_FlushAutomatically()
for (var i = 0; i <= 100; i++)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use the config value instead of hard code

for (var i = 0; i <= PowertoolsConfigurations.MaxMetrics; i++)

{
Metrics.AddMetric($"Metric Name {i + 1}", i, MetricUnit.Count);
}

if (i == 100)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to make sure and test that the first flush would happen and what are the values

Copy link
Contributor

Choose a reason for hiding this comment

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

We can use the config value instead of hard code

if (i == PowertoolsConfigurations.MaxMetrics)

{
// flush when it reaches 100 items
Assert.Contains("{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"Metric Name 1\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 2\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 3\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 4\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 5\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 6\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 7\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 8\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 9\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 10\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 11\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 12\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 13\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 14\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 15\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 16\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 17\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 18\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 19\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 20\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 21\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 22\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 23\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 24\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 25\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 26\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 27\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 28\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 29\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 30\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 31\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 32\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 33\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 34\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 35\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 36\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 37\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 38\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 39\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 40\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 41\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 42\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 43\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 44\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 45\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 46\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 47\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 48\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 49\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 50\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 51\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 52\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 53\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 54\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 55\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 56\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 57\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 58\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 59\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 60\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 61\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 62\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 63\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 64\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 65\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 66\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 67\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 68\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 69\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 70\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 71\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 72\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 73\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 74\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 75\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 76\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 77\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 78\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 79\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 80\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 81\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 82\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 83\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 84\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 85\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 86\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 87\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 88\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 89\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 90\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 91\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 92\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 93\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 94\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 95\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 96\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 97\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 98\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 99\",\"Unit\":\"Count\"},{\"Name\":\"Metric Name 100\",\"Unit\":\"Count\"}],\"Dimensions\":[[\"Service\"]]", consoleOut.ToString());
}
}
handler.OnExit(eventArgs);

var metricsOutput = consoleOut.ToString();

// Assert
// flush the 101 item only
Assert.Contains("{\"Namespace\":\"dotnet-powertools-test\",\"Metrics\":[{\"Name\":\"Metric Name 101\",\"Unit\":\"Count\"}],\"Dimensions\":[[\"Service\"]", metricsOutput);

// Reset
handler.ResetForTest();
}

[Trait("Category", "EMFLimits")]
[Fact]
public void When100DataPointsAreAddedToTheSameMetric_FlushAutomatically()
{
// Arrange
var methodName = Guid.NewGuid().ToString();
var consoleOut = new StringWriter();
Console.SetOut(consoleOut);

var configurations = new Mock<IPowertoolsConfigurations>();

var metrics = new Metrics(
configurations.Object,
nameSpace: "dotnet-powertools-test",
service: "testService"
);

var handler = new MetricsAspectHandler(
metrics,
false
);

var eventArgs = new AspectEventArgs { Name = methodName };

// Act
handler.OnEntry(eventArgs);

for (var i = 0; i <= 100; i++)
{
Metrics.AddMetric($"Metric Name", i, MetricUnit.Count);
if(i == 100)
{
// flush when it reaches 100 items
Assert.Contains(
"\"Service\":\"testService\",\"Metric Name\":[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99]}",
consoleOut.ToString());
}
}

handler.OnExit(eventArgs);

var metricsOutput = consoleOut.ToString();

// Assert
// flush the 101 item only
Assert.Contains("[[\"Service\"]]}]},\"Service\":\"testService\",\"Metric Name\":100}", metricsOutput);

// Reset
handler.ResetForTest();
}

[Trait("Category", "EMFLimits")]
[Fact]
Expand Down