Global meter forwarding implementation #392
Conversation
jmacd
requested review from
iredelmeier,
lizthegrey,
paivagustavo,
rghetia and
tedsuo
as code owners
api/global/global.go
Outdated
| // If none is registered then an instance of metric.NoopProvider is returned. | ||
| // Use the trace provider to create a named meter. E.g. | ||
| // MeterProvider returns the registered global meter provider. If | ||
| // none is registered then a dewfault meter provider is returned that |
api/global/internal/state.go
Outdated
| if current == mp { | ||
| // Setting the provider to the prior default | ||
| // is nonsense, set it to a noop. | ||
| mp = metric.NoopProvider{} |
There was a problem hiding this comment.
panic would be better than replacing it with NoopProvider{}.
| @@ -71,13 +77,19 @@ const ( | |||
| ) | |||
|
|
|||
| func (i *Instrument) AcquireHandle(labels apimetric.LabelSet) apimetric.HandleImpl { | |||
| if ld, ok := labels.(apimetric.LabelSetDelegate); ok { | |||
| labels = ld.Delegate() | |||
| } | |||
There was a problem hiding this comment.
This check should be done for sdk as well.
There was a problem hiding this comment.
Yes! This crossed my mind as I was walking out the office. Suggests more tests, too :)
| mock.MeasurementBatches[2].Measurements[0].Number) | ||
| require.Equal(t, "test.measure", | ||
| mock.MeasurementBatches[2].Measurements[0].Instrument.Name) | ||
| } |
There was a problem hiding this comment.
Perhaps add test for multiple Meters.
|
FYI I discovered that named Tracers will need similar support as discussed in open-telemetry/oteps#74 but I will leave that out of this PR. |
api/global/internal/meter.go
Outdated
| // provider. Mutexes in the Provider and Meters ensure that each | ||
| // instrument has a delegate before the global provider is set. | ||
| // | ||
| // LabelSets are implemented using the by delegating to the Meter |
There was a problem hiding this comment.
It seems that there is an extra using the here
LabelSets are implemented
using theby delegating
api/global/internal/meter.go
Outdated
| // LabelSets are implemented using the by delegating to the Meter | ||
| // instance using the metric.LabelSetDelegator interface. | ||
| // | ||
| // Bound instrument operations are implementing by delegating to the |
There was a problem hiding this comment.
Should be are implemented to be consistent with the LabelSets doc?
Implements deferred initialization for metric instruments registered before the first Meter SDK is installed.
This implements the behavior specified in open-telemetry/oteps#45.
Resolves #388.