Introduce metric constructor errors, MeterMust wrapper#529
Introduce metric constructor errors, MeterMust wrapper#529jmacd merged 12 commits intoopen-telemetry:masterfrom
MeterMust wrapper#529Conversation
jmacd
requested review from
MrAlias,
krnowak,
lizthegrey,
paivagustavo,
rghetia and
tedsuo
as code owners
|
This is now a WIP having received feedback on Gitter, see https://gitter.im/open-telemetry/opentelemetry-go?at=5e66b07cff8bf14a54470408 |
jmacd
changed the title
Must variationsMust variations
jmacd
force-pushed
the
jmacd/must_metric
branch
from
216bc8b to
3fc1244
Compare
jmacd
changed the title
Must variationsMust wrapper
|
@MrAlias I moved the PTAL |
krnowak
left a comment
krnowak
left a comment
There was a problem hiding this comment.
Looks good, but I have some questions.
MrAlias
left a comment
MrAlias
left a comment
There was a problem hiding this comment.
I think this is a positive approach in general, I have a few questions about the implementation though.
The breaking apart of the Meter interface seems a bit like it is bloating the API with interfaces that will not be used and seems a bit concerning. I'm interested in the answer to @krnowak's question about whether the *Must interfaces could just be concret type or not.
As for the changes to include the Must function, I think that looks like a positive move.
jmacd
changed the title
Must wrapperMeterMust wrapper
jmacd
dismissed
krnowak’s stale review
Removed a sentence that was no longer true.
MrAlias
left a comment
MrAlias
left a comment
There was a problem hiding this comment.
Awesome work, and thanks for putting up with my pestering! 😄
|
Thanks for putting up with my rushing. 😀 |
…try#529) * Update api for Must constructors, with SDK helpers * Update for Must constructors, leaving TODOs about global errors * Add tests * Move Must methods into metric.Must * Apply the feedback * Remove interfaces * Remove more interfaces * Again... * Remove a sentence about a dead inteface
4 participants
This is a partial implementation for #514. It will be easier if we introduce the API change in one PR and implement error checking in a second PR.
This introduces
metric.Must()to wrap a Meter so that its instrument constructors return an error.Also, #174 is mentioned because there is a new requirement for the global package to handle errors that might occur during deferred constructors.
Note this retains some refactoring of the
MeterAPI into Measure and Observer constructor sub-interfaces. This adopts the terminology that has been developed in open-telemetry/oteps#88, calling the new constructors eitherMeasureorObservertype.