From 989066bf0f190152a6f45fd98e8970bb731b2afd Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 21 Sep 2021 12:05:15 -0700 Subject: [PATCH] remove two unnecessary accessor methods; Controller implements MeterProvider and InstrumentationLibraryReader directly, no need to get these --- .../internal/otlpmetrictest/otlptest.go | 2 +- .../otlpmetric/otlpmetricgrpc/example_test.go | 6 ++--- exporters/prometheus/prometheus.go | 6 ++--- exporters/stdout/stdoutmetric/example_test.go | 2 +- exporters/stdout/stdoutmetric/metric_test.go | 4 +-- sdk/metric/controller/basic/controller.go | 13 +-------- .../controller/basic/controller_test.go | 27 +++++++------------ sdk/metric/controller/basic/pull_test.go | 14 +++++----- sdk/metric/controller/basic/push_test.go | 4 +-- 9 files changed, 29 insertions(+), 49 deletions(-) diff --git a/exporters/otlp/otlpmetric/internal/otlpmetrictest/otlptest.go b/exporters/otlp/otlpmetric/internal/otlpmetrictest/otlptest.go index cd84c0c564e..a1328312781 100644 --- a/exporters/otlp/otlpmetric/internal/otlpmetrictest/otlptest.go +++ b/exporters/otlp/otlpmetric/internal/otlpmetrictest/otlptest.go @@ -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 { diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/example_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/example_test.go index abbe0e981d4..76a6f1ee03a 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/example_test.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/example_test.go @@ -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) @@ -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) @@ -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) diff --git a/exporters/prometheus/prometheus.go b/exporters/prometheus/prometheus.go index aa9a42f8fa0..572feff5701 100644 --- a/exporters/prometheus/prometheus.go +++ b/exporters/prometheus/prometheus.go @@ -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. @@ -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) @@ -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() diff --git a/exporters/stdout/stdoutmetric/example_test.go b/exporters/stdout/stdoutmetric/example_test.go index bf19844b6ff..522877e73eb 100644 --- a/exporters/stdout/stdoutmetric/example_test.go +++ b/exporters/stdout/stdoutmetric/example_test.go @@ -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 { diff --git a/exporters/stdout/stdoutmetric/metric_test.go b/exporters/stdout/stdoutmetric/metric_test.go index ce27ab5bbc2..85ae1f3fb8e 100644 --- a/exporters/stdout/stdoutmetric/metric_test.go +++ b/exporters/stdout/stdoutmetric/metric_test.go @@ -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, @@ -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() diff --git a/sdk/metric/controller/basic/controller.go b/sdk/metric/controller/basic/controller.go index 4c59ee1ef26..7621f78e59c 100644 --- a/sdk/metric/controller/basic/controller.go +++ b/sdk/metric/controller/basic/controller.go @@ -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 @@ -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. diff --git a/sdk/metric/controller/basic/controller_test.go b/sdk/metric/controller/basic/controller_test.go index 29e25922f32..4cca2cc2442 100644 --- a/sdk/metric/controller/basic/controller_test.go +++ b/sdk/metric/controller/basic/controller_test.go @@ -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(), @@ -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 @@ -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) @@ -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++ @@ -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) @@ -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) @@ -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)) @@ -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) diff --git a/sdk/metric/controller/basic/pull_test.go b/sdk/metric/controller/basic/pull_test.go index c8d11cb2379..04c25c23571 100644 --- a/sdk/metric/controller/basic/pull_test.go +++ b/sdk/metric/controller/basic/pull_test.go @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, diff --git a/sdk/metric/controller/basic/push_test.go b/sdk/metric/controller/basic/push_test.go index e9e818de536..0b1b5474f3b 100644 --- a/sdk/metric/controller/basic/push_test.go +++ b/sdk/metric/controller/basic/push_test.go @@ -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) @@ -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")