fix: Metrics throws exception when using AddMetadata #505
Conversation
Clear Metadata dictionary when clearing metrics after flush. Move metadata node serialization to bottom, and try add, if it fails ignore. Add tests
pull-request-size
bot
added
the
size/L
| @@ -66,6 +66,7 @@ public Metadata() | |||
| internal void ClearMetrics() | |||
| { | |||
| _metricDirective.Metrics.Clear(); | |||
| CustomMetadata?.Clear(); | |||
There was a problem hiding this comment.
Clear Metadata dictionary when clearing Metrics. Null check for safety
github-actions
bot
added
the
bug
| @@ -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); | |||
There was a problem hiding this comment.
Just to be safe and avoid exceptions
| 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); |
There was a problem hiding this comment.
Move to the bottom. If Metrics has key. Metadata will be ignored
boring-cyborg
bot
added
the
documentation
|
Kudos, SonarCloud Quality Gate passed!
|
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #505 +/- ##
===========================================
- Coverage 72.08% 71.72% -0.37%
===========================================
Files 101 101
Lines 4063 4064 +1
Branches 412 413 +1
===========================================
- Hits 2929 2915 -14
- Misses 1020 1035 +15
Partials 114 114
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Issue number: #504
Summary
There are two issues when calling Metrics.AddMetadata
Changes
User experience
Checklist
Please leave checklist items unchecked if they do not apply to your change.
Is this a breaking change?
RFC issue number:
Checklist:
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.