Skip to content

Conversation

@CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Apr 17, 2024

Changes

  • Expose ExemplarReservoir in pre-release builds.
  • Expose MetricStreamConfiguration.ExemplarReservoirFactory in pre-release builds.

Details

This is being done to be complaint with the view configuration spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#stream-configuration

Note: At the moment everything on Exemplar has a private setter and the Update method is internal so there isn't really a good practical way to make a custom ExemplarReservoir. I'm going to tackle that as a follow-up because there are different approaches we could take.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@CodeBlanch CodeBlanch added pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package metrics Metrics signal related labels Apr 17, 2024
@CodeBlanch CodeBlanch requested a review from a team April 17, 2024 17:12
@cijothomas
Copy link
Member

Is Exemplar stable release planned for 1.9.0 ? if yes, can we just expose this unconditionally? Or are you anticipating that 1.9.0 stable won't have stable exemplars, and hence this PR?

@codecov
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

Attention: Patch coverage is 54.54545% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 85.56%. Comparing base (6250307) to head (15e1cea).
Report is 182 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5542      +/-   ##
==========================================
+ Coverage   83.38%   85.56%   +2.18%     
==========================================
  Files         297      289       -8     
  Lines       12531    12484      -47     
==========================================
+ Hits        10449    10682     +233     
+ Misses       2082     1802     -280     
Flag Coverage Δ
unittests ?
unittests-Solution-Experimental 85.39% <54.54%> (?)
unittests-Solution-Stable 85.52% <50.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...penTelemetry/Metrics/Exemplar/ExemplarReservoir.cs 100.00% <ø> (ø)
...OpenTelemetry/Metrics/MetricStreamConfiguration.cs 88.88% <100.00%> (+13.88%) ⬆️
src/OpenTelemetry/Metrics/MetricPoint.cs 94.24% <60.00%> (+25.76%) ⬆️
...nTelemetry/Internal/OpenTelemetrySdkEventSource.cs 79.31% <40.00%> (-2.60%) ⬇️

... and 76 files with indirect coverage changes

@CodeBlanch
Copy link
Member Author

@cijothomas

Is Exemplar stable release planned for 1.9.0 ? if yes, can we just expose this unconditionally? Or are you anticipating that 1.9.0 stable won't have stable exemplars, and hence this PR?

Yes it is planned for 1.9.0. I don't think we're ready to remove experimental status yet though. I want to finish implementing the spec and wait for it go stable before we do that. Probably just a final PR at the end of the effort which makes the switch for everything.

@CodeBlanch CodeBlanch merged commit 89aa7a4 into open-telemetry:main Apr 17, 2024
@CodeBlanch CodeBlanch deleted the sdk-metrics-expose-exemplarreservoir branch April 17, 2024 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

metrics Metrics signal related pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants