-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Publish Aggregated Values For Counter and UpDownCounter For System.Diagnostics.Metrics
#84846
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
Publish Aggregated Values For Counter and UpDownCounter For System.Diagnostics.Metrics
#84846
Conversation
… just deltas). Also added tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kkeirstead! I think this is on the right track, but there is some fishy behavior in the tests I can't yet explain. Comments inline.
...braries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RateAggregator.cs
Outdated
Show resolved
Hide resolved
...braries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RateAggregator.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricEventSourceTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricEventSourceTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricEventSourceTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricEventSourceTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricEventSourceTests.cs
Outdated
Show resolved
Hide resolved
...braries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RateAggregator.cs
Outdated
Show resolved
Hide resolved
…ts and some naming
...ies/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kkeirstead, this looks good to me 👍 (one more suggestion above)
This amends the changes made in #81041 so that deltas and sums are both published for counters/updowncounters. Includes updated testing.
@noahfalk let me know if this is what you had in mind / you have any adjustments you'd like to see - once this is in I'll revisit integrating this into dotnet-monitor and dotnet-counters.