Skip to content

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

Merged

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented May 21, 2025

Fixes #109388

This change introduces support for out-of-process histograms using base-2 exponential aggregation.

  • Aggregation is implemented according to the OpenTelemetry specification, with the calculation logic ported from the opentelemetry-dotnet project.
  • Negative values are currently ignored during recording, in line with the current specification. Support for negative values can be added in the future if needed.
  • A configuration option has been added to the event source, allowing base-2 exponential aggregation to be set as the default histogram aggregation method. In the future, per-meter or per-instrument aggregation settings can be introduced based on user feedback.

@Copilot Copilot AI review requested due to automatic review settings May 21, 2025 23:13
Copy link
Contributor

@Copilot Copilot AI left a 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.

@tarekgh tarekgh added this to the 10.0.0 milestone May 21, 2025
tarekgh and others added 2 commits May 21, 2025 16:22
…iagnostics/Metrics/Base2ExponentialHistogramAggregator.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@tarekgh
Copy link
Member Author

tarekgh commented May 21, 2025

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tarekgh
Copy link
Member Author

tarekgh commented May 21, 2025

CC @reyang as I ported some code originally written by him.

@tarekgh
Copy link
Member Author

tarekgh commented May 23, 2025

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@noahfalk noahfalk left a 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.

tarekgh and others added 3 commits May 23, 2025 09:41
Co-authored-by: Reiley Yang <reyang@microsoft.com>
…iagnostics/Metrics/CircularBufferBuckets.cs

Co-authored-by: Reiley Yang <reyang@microsoft.com>
@tarekgh
Copy link
Member Author

tarekgh commented May 23, 2025

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tarekgh
Copy link
Member Author

tarekgh commented May 24, 2025

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tarekgh
Copy link
Member Author

tarekgh commented May 25, 2025

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tarekgh tarekgh merged commit 2d5f1f0 into dotnet:main May 26, 2025
86 of 93 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.Diagnostics.Metrics out of proc monitoring improved histogram support
4 participants