Skip to content

Remove metric instrument provider API - #3530

Merged
MrAlias merged 15 commits into
open-telemetry:mainfrom
MrAlias:rm-inst-provider
Jan 3, 2023
Merged

Remove metric instrument provider API#3530
MrAlias merged 15 commits into
open-telemetry:mainfrom
MrAlias:rm-inst-provider

Conversation

@MrAlias

@MrAlias MrAlias commented Dec 8, 2022

Copy link
Copy Markdown
Contributor

Fix #3454

Move all instrument provider instrument creation functionality to the Meter as required by the OpenTelemetry specification.

@MrAlias MrAlias added pkg:API Related to an API package pkg:SDK Related to an SDK package area:metrics Part of OpenTelemetry Metrics labels Dec 8, 2022
@MrAlias MrAlias added this to the Metric v0.35.0 milestone Dec 8, 2022
@codecov

codecov Bot commented Dec 8, 2022

Copy link
Copy Markdown

Codecov Report

Merging #3530 (36a37c9) into main (aac35a8) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #3530     +/-   ##
=======================================
- Coverage   78.1%   77.9%   -0.2%     
=======================================
  Files        164     163      -1     
  Lines      11826   11850     +24     
=======================================
- Hits        9237    9233      -4     
- Misses      2391    2419     +28     
  Partials     198     198             
Impacted Files Coverage Δ
metric/internal/global/instruments.go 54.3% <100.0%> (ø)
metric/internal/global/meter.go 95.2% <100.0%> (+0.3%) ⬆️
metric/noop.go 62.1% <100.0%> (-35.9%) ⬇️
sdk/metric/meter.go 92.1% <100.0%> (+4.8%) ⬆️
sdk/trace/batch_span_processor.go 80.2% <0.0%> (-1.8%) ⬇️

@MadVikingGod

Copy link
Copy Markdown
Contributor

I have posted my objections to this in the primary issue: #3454 (comment)

Assuming that we resolve that, I think this is an acceptable way of closing the issue.

@MrAlias

MrAlias commented Dec 16, 2022

Copy link
Copy Markdown
Contributor Author

@MadVikingGod based on the SIG meeting discussion it seemed your issue was addressed. Are you okay with merging this?

@MrAlias
MrAlias merged commit a54167d into open-telemetry:main Jan 3, 2023
@MrAlias
MrAlias deleted the rm-inst-provider branch January 3, 2023 18:34
MrAlias added a commit to MrAlias/opentelemetry-go that referenced this pull request Jan 3, 2023
MrAlias added a commit that referenced this pull request Jan 4, 2023
* Rename metric SDK instrument kind to match API

Follow up to #3530.

* Update CHANGELOG

Fix trailing spaces and update PR number.
@MrAlias MrAlias mentioned this pull request Jan 28, 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 pkg:SDK Related to an SDK package

Projects

No open projects
Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

There MUST NOT be any API for creating an instrument other than with a Meter

5 participants