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 embedded private method interfaces in metric API #3916

Merged
merged 23 commits into from
Apr 3, 2023

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Mar 21, 2023

Resolves #3865

This adds an embedded sub-package to otel/metric. It contains interfaces from otel/metric but only with a single unexported method. These interfaces are embedded in all their corresponding interfaces in otel/metric. This means that from the SDK developers perspective they need to ...

  1. Embed the appropriate embedded interface if they want their SDK to have a compile time error if an interface from otel/metric has a method added to it.
  2. Embed the otel/metric interfaces if they want to have runtime panics
  3. Embed another implementation, like the No-Op, if they want to have a default behavior other than panicking

Having the embedded interfaces in their own package removes the distraction for instrumentation authors who have no need to interact with the types in embedded.

@MrAlias MrAlias added pkg:API Related to an API package area:metrics Part of OpenTelemetry Metrics labels Mar 21, 2023
@MrAlias MrAlias added this to the v1.15.0 milestone Mar 21, 2023
@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Merging #3916 (ef80f84) into main (5f13db5) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #3916     +/-   ##
=======================================
- Coverage   81.7%   81.7%   -0.1%     
=======================================
  Files        171     171             
  Lines      12951   12951             
=======================================
- Hits       10590   10586      -4     
- Misses      2138    2142      +4     
  Partials     223     223             
Impacted Files Coverage Δ
internal/global/instruments.go 60.8% <ø> (ø)
internal/global/meter.go 93.8% <ø> (ø)
metric/instrument/asyncfloat64.go 100.0% <ø> (ø)
metric/instrument/asyncint64.go 100.0% <ø> (ø)
metric/instrument/syncfloat64.go 100.0% <ø> (ø)
metric/instrument/syncint64.go 100.0% <ø> (ø)
metric/noop/noop.go 100.0% <ø> (ø)
sdk/metric/instrument.go 96.0% <ø> (ø)
sdk/metric/provider.go 100.0% <ø> (ø)
sdk/metric/meter.go 89.5% <100.0%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

@MrAlias MrAlias changed the title PoC of embedded private method interfaces Add embedded private method interfaces in metric API Mar 27, 2023
@MrAlias MrAlias marked this pull request as ready for review March 27, 2023 23:03
@MrAlias MrAlias merged commit 65ebe5e into open-telemetry:main Apr 3, 2023
@MrAlias MrAlias deleted the embed-poc branch April 3, 2023 14:33
@MrAlias MrAlias modified the milestones: v1.15.0, Metric v0.38.0 Apr 18, 2023
@MrAlias MrAlias mentioned this pull request Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics pkg:API Related to an API package
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Expose a noop implementation for each interface in API that can be embedded
4 participants