-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Support out of proc Histogram Base 2 Exponential aggregation #115852
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
Support out of proc Histogram Base 2 Exponential aggregation #115852
Conversation
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.
Pull Request Overview
This PR introduces support for out‑of‑process histograms using Base‑2 exponential aggregation, in accordance with the OpenTelemetry spec. Key changes include adding the new Base2ExponentialHistogramAggregator and CircularBufferBuckets classes, extending configuration options in the event source and aggregation manager, and adding tests to validate the new histogram behavior.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
System.Diagnostics.DiagnosticSource.Tests.csproj | Added compile links for new aggregator and buckets test files. |
tests/Resources/Strings.resx | Added error messages for invalid histogram configuration values. |
MetricEventSourceTests.cs | Introduced new tests verifying Base‑2 histogram aggregation with various recordings. |
MetricsEventSource.cs | Added event and parsing logic for Base‑2 exponential histogram aggregation. |
CircularBufferBuckets.cs | New circular buffer implementation for bucket counting in histograms. |
Base2ExponentialHistogramAggregator.cs | New aggregator implementation for Base‑2 exponential histograms; updates measurement and supports scale reduction. |
AggregationManager.cs | Updated to select the appropriate aggregator based on histogram aggregation configuration. |
Other files | Updated project files, resource strings, and third‑party notices accordingly. |
...stics.DiagnosticSource/src/System/Diagnostics/Metrics/Base2ExponentialHistogramAggregator.cs
Outdated
Show resolved
Hide resolved
...stics.DiagnosticSource/src/System/Diagnostics/Metrics/Base2ExponentialHistogramAggregator.cs
Show resolved
Hide resolved
…iagnostics/Metrics/Base2ExponentialHistogramAggregator.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
CC @reyang as I ported some code originally written by him. |
...stics.DiagnosticSource/src/System/Diagnostics/Metrics/Base2ExponentialHistogramAggregator.cs
Show resolved
Hide resolved
...ies/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/AggregationManager.cs
Outdated
Show resolved
Hide resolved
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
...stics.DiagnosticSource/src/System/Diagnostics/Metrics/Base2ExponentialHistogramAggregator.cs
Outdated
Show resolved
Hide resolved
.../System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/CircularBufferBuckets.cs
Outdated
Show resolved
Hide resolved
...ies/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/AggregationManager.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.
A few suggestions/questions inline. LGTM overall. I didn't scrutinize the algorithm for the histogram itself as I assume that is copied code and appears well tested.
...ies/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs
Show resolved
Hide resolved
...ies/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs
Outdated
Show resolved
Hide resolved
...ies/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs
Show resolved
Hide resolved
...ies/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/AggregationManager.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Reiley Yang <reyang@microsoft.com>
…iagnostics/Metrics/CircularBufferBuckets.cs Co-authored-by: Reiley Yang <reyang@microsoft.com>
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes #109388
This change introduces support for out-of-process histograms using base-2 exponential aggregation.