Skip to content
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

Add the sdk/metric/internal/exemplar package #4835

Merged
merged 6 commits into from
Jan 24, 2024

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Jan 19, 2024

Part of #559

Add the implementation package of of OpenTelemetry metric exemplars reservoirs skeleton.

This PR is split from #4455 to reduce the size of changes being evaluated. It is meant to be followed up with changes included in that PR:

  • add the drop reservoir
  • add the fixed size reservoir
  • add the histogram reservoir
  • add the trace-based sampler reservoir/filter
  • integrate this new package into the metric pipeline as an experimental feature:

Lack of ExemplarFilter

The ExemplarFilter interface currently defined in the specification is not included in this package. This is because open-telemetry/opentelemetry-specification#3820 removes this type and is expected to be merged.

If open-telemetry/opentelemetry-specification#3820 fails to merge, or is modified to not remove the ExemplarFilter, it can be added back into this package (i.e. revert c4d8381).

Offer dropped attributes

The Offer method of the ExemplarReservoir is defined to accept the full set of attributes a measurement is made with. However, there is an optional provision below this definition stating that it can also accept just the dropped attributes. This package defines the method to accept just the dropped attributes.

The reason for this decision is based on the fact that attribute.Set's Filter method returns these values at no additional cost when the set is filtered. See this comment describing the performance improvement numbers.

@MrAlias MrAlias added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jan 19, 2024
@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 19, 2024

cc @jsuereth

MrAlias and others added 4 commits January 19, 2024 11:08
This is a test helper function that is not exported outside of testing
for the package. Use the exported name so the CI system doesn't complain
about it not being used. It will be used in follow-up PRs.
Copy link

codecov bot commented Jan 24, 2024

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (cef39a1) 82.3% compared to head (b4f8613) 82.3%.
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #4835   +/-   ##
=====================================
  Coverage   82.3%   82.3%           
=====================================
  Files        226     226           
  Lines      18481   18557   +76     
=====================================
+ Hits       15222   15286   +64     
- Misses      2973    2983   +10     
- Partials     286     288    +2     
Files Coverage Δ
sdk/metric/cache.go 100.0% <100.0%> (ø)
sdk/metric/meter.go 90.4% <81.6%> (-2.2%) ⬇️

... and 1 file with indirect coverage changes

@MrAlias MrAlias merged commit c573785 into open-telemetry:main Jan 24, 2024
23 checks passed
@MrAlias MrAlias deleted the exemplar-pkg branch January 24, 2024 16:05
@MrAlias MrAlias added this to the v1.23.0 milestone Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants