aws-powertools / powertools-lambda-dotnet

Powertools is a developer toolkit to implement Serverless best practices and increase developer velocity.
https://docs.powertools.aws.dev/lambda/dotnet/
MIT No Attribution
152 stars 24 forks source link

chore: Metrics - Add thread safety to AddMetric #594

Closed hjgraca closed 3 months ago

hjgraca commented 3 months ago

Please provide the issue number

Issue number: #593

Summary

When PowertoolsConfigurations.MaxMetrics is set to a lower number and using multi threaded operations AddMetric will throw the following exception.

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.Collections.Generic.List`1.Enumerator.MoveNext()
   at System.Linq.Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Func`2 predicate, Boolean& found)
   at System.Linq.Enumerable.FirstOrDefault[TSource](IEnumerable`1 source, Func`2 predicate)
   at AWS.Lambda.Powertools.Metrics.Metrics.AWS.Lambda.Powertools.Metrics.IMetrics.AddMetric(String key, Double value, MetricUnit unit, MetricResolution metricResolution) in C:\Users\jesse.poulton\repos\powertools-lambda-dotnet\libraries\src\AWS.Lambda.Powertools.Metrics\Metrics.cs:line 103
   at AWS.Lambda.Powertools.Metrics.Metrics.AddMetric(String key, Double value, MetricUnit unit, MetricResolution metricResolution) in C:\Users\jesse.poulton\repos\powertools-lambda-dotnet\libraries\src\AWS.Lambda.Powertools.Metrics\Metrics.cs:line 302
   at AWS.Lambda.Powertools.Metrics.Tests.Handlers.FunctionHandler.<>c.<<HandleMultipleThreads>b__2_0>d.MoveNext() in C:\Users\jesse.poulton\repos\powertools-lambda-dotnet\libraries\tests\AWS.Lambda.Powertools.Metrics.Tests\Handlers\FunctionHandler.cs:line 51
--- End of stack trace from previous location ---
   at System.Threading.Tasks.Parallel.<>c__53`1.<<ForEachAsync>b__53_0>d.MoveNext()
--- End of stack trace from previous location ---
   at AWS.Lambda.Powertools.Metrics.Tests.Handlers.FunctionHandler.HandleMultipleThreads(String input) in C:\Users\jesse.poulton\repos\powertools-lambda-dotnet\libraries\tests\AWS.Lambda.Powertools.Metrics.Tests\Handlers\FunctionHandler.cs:line 49
   at AWS.Lambda.Powertools.Common.MethodAspectAttribute.WrapAsync[T](Func`2 target, Object[] args, AspectEventArgs eventArgs) in C:\Users\jesse.poulton\repos\powertools-lambda-dotnet\libraries\src\AWS.Lambda.Powertools.Common\Aspects\MethodAspectAttribute.cs:line 90
   at AWS.Lambda.Powertools.Metrics.MetricsAspectHandler.OnException(AspectEventArgs eventArgs, Exception exception) in C:\Users\jesse.poulton\repos\powertools-lambda-dotnet\libraries\src\AWS.Lambda.Powertools.Metrics\Internal\MetricsAspectHandler.cs:line 133
   at AWS.Lambda.Powertools.Common.MethodAspectAttribute.WrapAsync[T](Func`2 target, Object[] args, AspectEventArgs eventArgs) in C:\Users\jesse.poulton\repos\powertools-lambda-dotnet\libraries\src\AWS.Lambda.Powertools.Common\Aspects\MethodAspectAttribute.cs:line 96
   at Xunit.Record.ExceptionAsync(Func`1 testCode) in /_/src/xunit.core/Record.cs:line 76

Changes

Add thread safety lock to AddMetric method. The lock statement ensures that at maximum only one thread executes its body at any time moment.

User experience

Please share what the user experience looks like before and after this change

Checklist

Please leave checklist items unchecked if they do not apply to your change.

Is this a breaking change? **RFC issue number**: Checklist: * [ ] Migration process documented * [ ] Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

sonarcloud[bot] commented 3 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 72.61%. Comparing base (0cdab3f) to head (ab95e85).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #594 +/- ## =========================================== - Coverage 72.62% 72.61% -0.02% =========================================== Files 190 190 Lines 7901 7905 +4 Branches 851 851 =========================================== + Hits 5738 5740 +2 - Misses 1872 1874 +2 Partials 291 291 ``` | [Flag](https://app.codecov.io/gh/aws-powertools/powertools-lambda-dotnet/pull/594/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aws-powertools) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/aws-powertools/powertools-lambda-dotnet/pull/594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aws-powertools) | `72.61% <100.00%> (-0.02%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aws-powertools#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.