Skip to content

Commit

Permalink
remove two unnecessary accessor methods; Controller implements MeterP…
Browse files Browse the repository at this point in the history
…rovider and InstrumentationLibraryReader directly, no need to get these
  • Loading branch information
Joshua MacDonald committed Sep 21, 2021
1 parent 9b990d2 commit 989066b
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func RunEndToEndTest(ctx context.Context, t *testing.T, exp *otlpmetric.Exporter
cont := controller.New(proc, controller.WithExporter(exp))
require.NoError(t, cont.Start(ctx))

meter := cont.MeterProvider().Meter("test-meter")
meter := cont.Meter("test-meter")
labels := []attribute.KeyValue{attribute.Bool("test", true)}

type data struct {
Expand Down
6 changes: 3 additions & 3 deletions exporters/otlp/otlpmetric/otlpmetricgrpc/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func Example_insecure() {
controller.WithExporter(exp),
controller.WithCollectPeriod(2*time.Second),
)
global.SetMeterProvider(pusher.MeterProvider())
global.SetMeterProvider(pusher)

if err := pusher.Start(ctx); err != nil {
log.Fatalf("could not start metric controoler: %v", err)
Expand Down Expand Up @@ -114,7 +114,7 @@ func Example_withTLS() {
controller.WithExporter(exp),
controller.WithCollectPeriod(2*time.Second),
)
global.SetMeterProvider(pusher.MeterProvider())
global.SetMeterProvider(pusher)

if err := pusher.Start(ctx); err != nil {
log.Fatalf("could not start metric controoler: %v", err)
Expand Down Expand Up @@ -171,7 +171,7 @@ func Example_withDifferentSignalCollectors() {
controller.WithExporter(exp),
controller.WithCollectPeriod(2*time.Second),
)
global.SetMeterProvider(pusher.MeterProvider())
global.SetMeterProvider(pusher)

if err := pusher.Start(ctx); err != nil {
log.Fatalf("could not start metric controoler: %v", err)
Expand Down
6 changes: 3 additions & 3 deletions exporters/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func New(config Config, controller *controller.Controller) (*Exporter, error) {

// MeterProvider returns the MeterProvider of this exporter.
func (e *Exporter) MeterProvider() metric.MeterProvider {
return e.controller.MeterProvider()
return e.controller
}

// Controller returns the controller object that coordinates collection for the SDK.
Expand Down Expand Up @@ -153,7 +153,7 @@ func (c *collector) Describe(ch chan<- *prometheus.Desc) {
c.exp.lock.RLock()
defer c.exp.lock.RUnlock()

_ = c.exp.Controller().InstrumentationLibraryReader().ForEach(func(_ instrumentation.Library, reader export.Reader) error {
_ = c.exp.Controller().ForEach(func(_ instrumentation.Library, reader export.Reader) error {
return reader.ForEach(c.exp, func(record export.Record) error {
var labelKeys []string
mergeLabels(record, c.exp.controller.Resource(), &labelKeys, nil)
Expand All @@ -176,7 +176,7 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {
otel.Handle(err)
}

err := ctrl.InstrumentationLibraryReader().ForEach(func(_ instrumentation.Library, reader export.Reader) error {
err := ctrl.ForEach(func(_ instrumentation.Library, reader export.Reader) error {
return reader.ForEach(c.exp, func(record export.Record) error {

agg := record.Aggregation()
Expand Down
2 changes: 1 addition & 1 deletion exporters/stdout/stdoutmetric/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func InstallExportPipeline(ctx context.Context) func() {
if err = pusher.Start(ctx); err != nil {
log.Fatalf("starting push controller: %v", err)
}
global.SetMeterProvider(pusher.MeterProvider())
global.SetMeterProvider(pusher)

return func() {
if err := pusher.Stop(ctx); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions exporters/stdout/stdoutmetric/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func newFixtureWithResource(t *testing.T, res *resource.Resource, opts ...stdout
)
ctx := context.Background()
require.NoError(t, cont.Start(ctx))
meter := cont.MeterProvider().Meter("test")
meter := cont.Meter("test")

return testFixture{
t: t,
Expand Down Expand Up @@ -101,7 +101,7 @@ func TestStdoutTimestamp(t *testing.T) {
ctx := context.Background()

require.NoError(t, cont.Start(ctx))
meter := cont.MeterProvider().Meter("test")
meter := cont.Meter("test")
counter := metric.Must(meter).NewInt64Counter("name.lastvalue")

before := time.Now()
Expand Down
13 changes: 1 addition & 12 deletions sdk/metric/controller/basic/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,23 +153,12 @@ func (c *Controller) SetClock(clock controllerTime.Clock) {
c.clock = clock
}

// MeterProvider returns a MeterProvider instance for this controller.
func (c *Controller) MeterProvider() metric.MeterProvider {
return c
}

// Resource returns the *resource.Resource associated with this
// controller.
func (c *Controller) Resource() *resource.Resource {
return c.resource
}

// InstrumentationLibraryReader returns an InstrumentationLibraryReader for iterating
// through the metrics of each registered library, one at a time.
func (c *Controller) InstrumentationLibraryReader() export.InstrumentationLibraryReader {
return c
}

// Start begins a ticker that periodically collects and exports
// metrics with the configured interval. This is required for calling
// a configured Exporter (see WithExporter) and is otherwise optional
Expand Down Expand Up @@ -330,7 +319,7 @@ func (c *Controller) export(ctx context.Context) error {
defer cancel()
}

return c.exporter.Export(ctx, c.resource, c.InstrumentationLibraryReader())
return c.exporter.Export(ctx, c.resource, c)
}

// ForEach implements export.InstrumentationLibraryReader.
Expand Down
27 changes: 9 additions & 18 deletions sdk/metric/controller/basic/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const envVar = "OTEL_RESOURCE_ATTRIBUTES"
func getMap(t *testing.T, cont *controller.Controller) map[string]float64 {
out := processortest.NewOutput(attribute.DefaultEncoder())

require.NoError(t, cont.InstrumentationLibraryReader().ForEach(
require.NoError(t, cont.ForEach(
func(_ instrumentation.Library, reader export.Reader) error {
return reader.ForEach(
export.CumulativeExportKindSelector(),
Expand Down Expand Up @@ -125,9 +125,8 @@ func TestControllerUsesResource(t *testing.T) {
)
ctx := context.Background()
require.NoError(t, cont.Start(ctx))
prov := cont.MeterProvider()

ctr := metric.Must(prov.Meter("named")).NewFloat64Counter("calls.sum")
ctr := metric.Must(cont.Meter("named")).NewFloat64Counter("calls.sum")
ctr.Add(context.Background(), 1.)

// Collect once
Expand All @@ -153,10 +152,9 @@ func TestStartNoExporter(t *testing.T) {
mock := controllertest.NewMockClock()
cont.SetClock(mock)

prov := cont.MeterProvider()
calls := int64(0)

_ = metric.Must(prov.Meter("named")).NewInt64CounterObserver("calls.lastvalue",
_ = metric.Must(cont.Meter("named")).NewInt64CounterObserver("calls.lastvalue",
func(ctx context.Context, result metric.Int64ObserverResult) {
calls++
checkTestContext(t, ctx)
Expand Down Expand Up @@ -222,10 +220,9 @@ func TestObserverCanceled(t *testing.T) {
controller.WithResource(resource.Empty()),
)

prov := cont.MeterProvider()
calls := int64(0)

_ = metric.Must(prov.Meter("named")).NewInt64CounterObserver("done.lastvalue",
_ = metric.Must(cont.Meter("named")).NewInt64CounterObserver("done.lastvalue",
func(ctx context.Context, result metric.Int64ObserverResult) {
<-ctx.Done()
calls++
Expand Down Expand Up @@ -254,9 +251,7 @@ func TestObserverContext(t *testing.T) {
controller.WithResource(resource.Empty()),
)

prov := cont.MeterProvider()

_ = metric.Must(prov.Meter("named")).NewInt64CounterObserver("done.lastvalue",
_ = metric.Must(cont.Meter("named")).NewInt64CounterObserver("done.lastvalue",
func(ctx context.Context, result metric.Int64ObserverResult) {
time.Sleep(10 * time.Millisecond)
checkTestContext(t, ctx)
Expand Down Expand Up @@ -322,10 +317,8 @@ func TestExportTimeout(t *testing.T) {
mock := controllertest.NewMockClock()
cont.SetClock(mock)

prov := cont.MeterProvider()

calls := int64(0)
_ = metric.Must(prov.Meter("named")).NewInt64CounterObserver("one.lastvalue",
_ = metric.Must(cont.Meter("named")).NewInt64CounterObserver("one.lastvalue",
func(ctx context.Context, result metric.Int64ObserverResult) {
calls++
result.Observe(calls)
Expand Down Expand Up @@ -378,10 +371,8 @@ func TestCollectAfterStopThenStartAgain(t *testing.T) {
mock := controllertest.NewMockClock()
cont.SetClock(mock)

prov := cont.MeterProvider()

calls := 0
_ = metric.Must(prov.Meter("named")).NewInt64CounterObserver("one.lastvalue",
_ = metric.Must(cont.Meter("named")).NewInt64CounterObserver("one.lastvalue",
func(ctx context.Context, result metric.Int64ObserverResult) {
calls++
result.Observe(int64(calls))
Expand Down Expand Up @@ -457,8 +448,8 @@ func TestRegistryFunction(t *testing.T) {
controller.WithResource(resource.Empty()),
)

m1 := cont.MeterProvider().Meter("test")
m2 := cont.MeterProvider().Meter("test")
m1 := cont.Meter("test")
m2 := cont.Meter("test")

require.NotNil(t, m1)
require.Equal(t, m1, m2)
Expand Down
14 changes: 7 additions & 7 deletions sdk/metric/controller/basic/pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ func TestPullNoCollect(t *testing.T) {
)

ctx := context.Background()
meter := puller.MeterProvider().Meter("nocache")
meter := puller.Meter("nocache")
counter := metric.Must(meter).NewInt64Counter("counter.sum")

counter.Add(ctx, 10, attribute.String("A", "B"))

require.NoError(t, puller.Collect(ctx))
records := processortest.NewOutput(attribute.DefaultEncoder())
require.NoError(t, controllertest.ReadAll(puller.InstrumentationLibraryReader(), export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))
require.NoError(t, controllertest.ReadAll(puller, export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))

require.EqualValues(t, map[string]float64{
"counter.sum/A=B/": 10,
Expand All @@ -61,7 +61,7 @@ func TestPullNoCollect(t *testing.T) {

require.NoError(t, puller.Collect(ctx))
records = processortest.NewOutput(attribute.DefaultEncoder())
require.NoError(t, controllertest.ReadAll(puller.InstrumentationLibraryReader(), export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))
require.NoError(t, controllertest.ReadAll(puller, export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))

require.EqualValues(t, map[string]float64{
"counter.sum/A=B/": 20,
Expand All @@ -82,14 +82,14 @@ func TestPullWithCollect(t *testing.T) {
puller.SetClock(mock)

ctx := context.Background()
meter := puller.MeterProvider().Meter("nocache")
meter := puller.Meter("nocache")
counter := metric.Must(meter).NewInt64Counter("counter.sum")

counter.Add(ctx, 10, attribute.String("A", "B"))

require.NoError(t, puller.Collect(ctx))
records := processortest.NewOutput(attribute.DefaultEncoder())
require.NoError(t, controllertest.ReadAll(puller.InstrumentationLibraryReader(), export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))
require.NoError(t, controllertest.ReadAll(puller, export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))

require.EqualValues(t, map[string]float64{
"counter.sum/A=B/": 10,
Expand All @@ -100,7 +100,7 @@ func TestPullWithCollect(t *testing.T) {
// Cached value!
require.NoError(t, puller.Collect(ctx))
records = processortest.NewOutput(attribute.DefaultEncoder())
require.NoError(t, controllertest.ReadAll(puller.InstrumentationLibraryReader(), export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))
require.NoError(t, controllertest.ReadAll(puller, export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))

require.EqualValues(t, map[string]float64{
"counter.sum/A=B/": 10,
Expand All @@ -112,7 +112,7 @@ func TestPullWithCollect(t *testing.T) {
// Re-computed value!
require.NoError(t, puller.Collect(ctx))
records = processortest.NewOutput(attribute.DefaultEncoder())
require.NoError(t, controllertest.ReadAll(puller.InstrumentationLibraryReader(), export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))
require.NoError(t, controllertest.ReadAll(puller, export.CumulativeExportKindSelector(), records.AddInstrumentationLibraryRecord))

require.EqualValues(t, map[string]float64{
"counter.sum/A=B/": 20,
Expand Down
4 changes: 2 additions & 2 deletions sdk/metric/controller/basic/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func TestPushTicker(t *testing.T) {
controller.WithCollectPeriod(time.Second),
controller.WithResource(testResource),
)
meter := p.MeterProvider().Meter("name")
meter := p.Meter("name")

mock := controllertest.NewMockClock()
p.SetClock(mock)
Expand Down Expand Up @@ -196,7 +196,7 @@ func TestPushExportError(t *testing.T) {

ctx := context.Background()

meter := p.MeterProvider().Meter("name")
meter := p.Meter("name")
counter1 := metric.Must(meter).NewInt64Counter("counter1.sum")
counter2 := metric.Must(meter).NewInt64Counter("counter2.sum")

Expand Down

0 comments on commit 989066b

Please sign in to comment.