Separate InstrumentationLibrary from metric.Descriptor (#2197)

* factor instrumentation library out of the instrument descriptor

* SDK tests pass

* checkpoint work

* otlp and opencensus tests passing

* prometheus

* tests pass, working on lint

* lint applied: MetricReader->Reader

* comments

* Changelog

* Apply suggestions from code review

Co-authored-by: alrex <alrex.boten@gmail.com>

* remove an interdependency

* fix build

* re-indent one

* Apply suggestions from code review

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Lint&feedback

* update after rename

* comment fix

* style fix for meter options

* remove libraryReader, let Controller implement the reader API directly

* rename 'impl' field to 'provider'

* remove a type assertion

* move metric/registry into internal; move registry.MeterProvider into metric controller

* add test for controller registry function

* CheckpointSet->Reader everywhere

* lint

* remove two unnecessary accessor methods; Controller implements MeterProvider and InstrumentationLibraryReader directly, no need to get these

* use a sync.Map

* ensure the initOnce is always called; handle multiple errors

* Apply suggestions from code review

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>

* cleanup locking in metrictest

* Revert "ensure the initOnce is always called; handle multiple errors"

This reverts commit 3356eb5.

* Revert "use a sync.Map"

This reverts commit ea7bc59.

* restore the TODO about sync.Map

Co-authored-by: alrex <alrex.boten@gmail.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>

CHANGELOG.md

@@ -61,6 +61,9 @@ This release includes an API and SDK for the tracing signal that will comply wit
 - The Metrics SDK export record no longer contains a Resource pointer, the SDK `"go.opentelemetry.io/otel/sdk/trace/export/metric".Exporter.Export()` function for push-based exporters now takes a single Resource argument, pull-based exporters use `"go.opentelemetry.io/otel/sdk/metric/controller/basic".Controller.Resource()`. (#2120)
 - The JSON output of the `go.opentelemetry.io/otel/exporters/stdout/stdouttrace` is harmonized now such that the output is "plain" JSON objects after each other of the form `{ ... } { ... } { ... }`. Earlier the JSON objects describing a span were wrapped in a slice for each `Exporter.ExportSpans` call, like `[ { ... } ][ { ... } { ... } ]`. Outputting JSON object directly after each other is consistent with JSON loggers, and a bit easier to parse and read. (#2196)
 - Update the `NewTracerConfig`, `NewSpanStartConfig`, `NewSpanEndConfig`, and `NewEventConfig` function in the `go.opentelemetry.io/otel/trace` package to return their respective configurations as structs instead of pointers to the struct. (#2212)
+- The Metric SDK `Export()` function takes a new two-level reader interface for iterating over results one instrumentation library at a time. (#2197)
+  - The former `"go.opentelemetry.io/otel/sdk/export/metric".CheckpointSet` is renamed `Reader`.
+  - The new interface is named `"go.opentelemetry.io/otel/sdk/export/metric".InstrumentationLibraryReader`.
 
 ### Deprecated
 

bridge/opencensus/exporter.go

@@ -32,6 +32,7 @@ import (
 	"go.opentelemetry.io/otel/metric/unit"
 	export "go.opentelemetry.io/otel/sdk/export/metric"
 	"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
+	"go.opentelemetry.io/otel/sdk/instrumentation"
 	"go.opentelemetry.io/otel/sdk/resource"
 )
 
@@ -55,18 +56,30 @@ func (e *exporter) ExportMetrics(ctx context.Context, metrics []*metricdata.Metr
 	if len(metrics) != 0 {
 		res = convertResource(metrics[0].Resource)
 	}
-	return e.base.Export(ctx, res, &checkpointSet{metrics: metrics})
+	return e.base.Export(ctx, res, &censusLibraryReader{metrics: metrics})
 }
 
-type checkpointSet struct {
-	// RWMutex implements locking for the `CheckpointSet` interface.
+type censusLibraryReader struct {
+	metrics []*metricdata.Metric
+}
+
+func (r censusLibraryReader) ForEach(readerFunc func(instrumentation.Library, export.Reader) error) error {
+	return readerFunc(instrumentation.Library{
+		Name: "OpenCensus Bridge",
+	}, &metricReader{metrics: r.metrics})
+}
+
+type metricReader struct {
+	// RWMutex implements locking for the `Reader` interface.
 	sync.RWMutex
 	metrics []*metricdata.Metric
 }
 
-// ForEach iterates through the CheckpointSet, passing an
-// export.Record with the appropriate aggregation to an exporter.
-func (d *checkpointSet) ForEach(exporter export.ExportKindSelector, f func(export.Record) error) error {
+var _ export.Reader = &metricReader{}
+
+// ForEach iterates through the metrics data, synthesizing an
+// export.Record with the appropriate aggregation for the exporter.
+func (d *metricReader) ForEach(exporter export.ExportKindSelector, f func(export.Record) error) error {
 	for _, m := range d.metrics {
 		descriptor, err := convertDescriptor(m.Descriptor)
 		if err != nil {
@@ -158,7 +171,6 @@ func convertDescriptor(ocDescriptor metricdata.Descriptor) (metric.Descriptor, e
 	}
 	opts := []metric.InstrumentOption{
 		metric.WithDescription(ocDescriptor.Description),
-		metric.WithInstrumentationName("OpenCensus Bridge"),
 	}
 	switch ocDescriptor.Unit {
 	case metricdata.UnitDimensionless:
@@ -168,5 +180,6 @@ func convertDescriptor(ocDescriptor metricdata.Descriptor) (metric.Descriptor, e
 	case metricdata.UnitMilliseconds:
 		opts = append(opts, metric.WithUnit(unit.Milliseconds))
 	}
-	return metric.NewDescriptor(ocDescriptor.Name, ikind, nkind, opts...), nil
+	cfg := metric.NewInstrumentConfig(opts...)
+	return metric.NewDescriptor(ocDescriptor.Name, ikind, nkind, cfg.Description(), cfg.Unit()), nil
 }

bridge/opencensus/exporter_test.go

@@ -28,12 +28,15 @@ import (
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/metric"
+	"go.opentelemetry.io/otel/metric/metrictest"
 	"go.opentelemetry.io/otel/metric/number"
 	"go.opentelemetry.io/otel/metric/sdkapi"
 	"go.opentelemetry.io/otel/metric/unit"
 	export "go.opentelemetry.io/otel/sdk/export/metric"
 	exportmetric "go.opentelemetry.io/otel/sdk/export/metric"
 	"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
+	"go.opentelemetry.io/otel/sdk/instrumentation"
+	"go.opentelemetry.io/otel/sdk/metric/controller/controllertest"
 	"go.opentelemetry.io/otel/sdk/resource"
 )
 
@@ -44,12 +47,13 @@ type fakeExporter struct {
 	err      error
 }
 
-func (f *fakeExporter) Export(ctx context.Context, res *resource.Resource, cps exportmetric.CheckpointSet) error {
-	return cps.ForEach(f, func(record exportmetric.Record) error {
-		f.resource = res
-		f.records = append(f.records, record)
-		return f.err
-	})
+func (f *fakeExporter) Export(ctx context.Context, res *resource.Resource, ilr exportmetric.InstrumentationLibraryReader) error {
+	return controllertest.ReadAll(ilr, export.StatelessExportKindSelector(),
+		func(_ instrumentation.Library, record exportmetric.Record) error {
+			f.resource = res
+			f.records = append(f.records, record)
+			return f.err
+		})
 }
 
 type fakeErrorHandler struct {
@@ -71,11 +75,10 @@ func (f *fakeErrorHandler) matches(err error) error {
 
 func TestExportMetrics(t *testing.T) {
 	now := time.Now()
-	basicDesc := metric.NewDescriptor(
+	basicDesc := metrictest.NewDescriptor(
 		"",
 		sdkapi.GaugeObserverInstrumentKind,
 		number.Int64Kind,
-		metric.WithInstrumentationName("OpenCensus Bridge"),
 	)
 	fakeErrorHandler := &fakeErrorHandler{}
 	otel.SetErrorHandler(fakeErrorHandler)
@@ -393,11 +396,10 @@ func TestConvertDescriptor(t *testing.T) {
 	}{
 		{
 			desc: "empty descriptor",
-			expected: metric.NewDescriptor(
+			expected: metrictest.NewDescriptor(
 				"",
 				sdkapi.GaugeObserverInstrumentKind,
 				number.Int64Kind,
-				metric.WithInstrumentationName("OpenCensus Bridge"),
 			),
 		},
 		{
@@ -408,11 +410,10 @@ func TestConvertDescriptor(t *testing.T) {
 				Unit:        metricdata.UnitBytes,
 				Type:        metricdata.TypeGaugeInt64,
 			},
-			expected: metric.NewDescriptor(
+			expected: metrictest.NewDescriptor(
 				"foo",
 				sdkapi.GaugeObserverInstrumentKind,
 				number.Int64Kind,
-				metric.WithInstrumentationName("OpenCensus Bridge"),
 				metric.WithDescription("bar"),
 				metric.WithUnit(unit.Bytes),
 			),
@@ -425,11 +426,10 @@ func TestConvertDescriptor(t *testing.T) {
 				Unit:        metricdata.UnitMilliseconds,
 				Type:        metricdata.TypeGaugeFloat64,
 			},
-			expected: metric.NewDescriptor(
+			expected: metrictest.NewDescriptor(
 				"foo",
 				sdkapi.GaugeObserverInstrumentKind,
 				number.Float64Kind,
-				metric.WithInstrumentationName("OpenCensus Bridge"),
 				metric.WithDescription("bar"),
 				metric.WithUnit(unit.Milliseconds),
 			),
@@ -442,11 +442,10 @@ func TestConvertDescriptor(t *testing.T) {
 				Unit:        metricdata.UnitDimensionless,
 				Type:        metricdata.TypeCumulativeInt64,
 			},
-			expected: metric.NewDescriptor(
+			expected: metrictest.NewDescriptor(
 				"foo",
 				sdkapi.CounterObserverInstrumentKind,
 				number.Int64Kind,
-				metric.WithInstrumentationName("OpenCensus Bridge"),
 				metric.WithDescription("bar"),
 				metric.WithUnit(unit.Dimensionless),
 			),
@@ -459,11 +458,10 @@ func TestConvertDescriptor(t *testing.T) {
 				Unit:        metricdata.UnitDimensionless,
 				Type:        metricdata.TypeCumulativeFloat64,
 			},
-			expected: metric.NewDescriptor(
+			expected: metrictest.NewDescriptor(
 				"foo",
 				sdkapi.CounterObserverInstrumentKind,
 				number.Float64Kind,
-				metric.WithInstrumentationName("OpenCensus Bridge"),
 				metric.WithDescription("bar"),
 				metric.WithUnit(unit.Dimensionless),
 			),

bridge/opencensus/go.mod

@@ -8,6 +8,7 @@ require (
 	go.opentelemetry.io/otel/metric v0.23.0
 	go.opentelemetry.io/otel/sdk v1.0.0
 	go.opentelemetry.io/otel/sdk/export/metric v0.23.0
+	go.opentelemetry.io/otel/sdk/metric v0.23.0
 	go.opentelemetry.io/otel/trace v1.0.0
 )
 

bridge/opencensus/go.sum

@@ -1,5 +1,7 @@
 cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
+github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
+github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

bridge/opencensus/test/go.sum

@@ -1,5 +1,7 @@
 cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
+github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
+github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=

example/prometheus/main.go

@@ -40,7 +40,7 @@ var (
 func initMeter() {
 	config := prometheus.Config{}
 	c := controller.New(
-		processor.New(
+		processor.NewFactory(
 			selector.NewWithHistogramDistribution(
 				histogram.WithExplicitBoundaries(config.DefaultHistogramBoundaries),
 			),

exporters/otlp/otlpmetric/exporter.go

@@ -24,6 +24,7 @@ import (
 	metricsdk "go.opentelemetry.io/otel/sdk/export/metric"
 	"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
 	"go.opentelemetry.io/otel/sdk/resource"
+	metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
 )
 
 var (
@@ -43,16 +44,20 @@ type Exporter struct {
 }
 
 // Export exports a batch of metrics.
-func (e *Exporter) Export(ctx context.Context, res *resource.Resource, checkpointSet metricsdk.CheckpointSet) error {
-	rms, err := metrictransform.CheckpointSet(ctx, e, res, checkpointSet, 1)
+func (e *Exporter) Export(ctx context.Context, res *resource.Resource, ilr metricsdk.InstrumentationLibraryReader) error {
+	rm, err := metrictransform.InstrumentationLibraryReader(ctx, e, res, ilr, 1)
 	if err != nil {
 		return err
 	}
-	if len(rms) == 0 {
+	if rm == nil {
 		return nil
 	}
 
-	return e.client.UploadMetrics(ctx, rms)
+	// TODO: There is never more than one resource emitted by this
+	// call, as per the specification.  We can change the
+	// signature of UploadMetrics correspondingly. Here create a
+	// singleton list to reduce the size of the current PR:
+	return e.client.UploadMetrics(ctx, []*metricpb.ResourceMetrics{rm})
 }
 
 // Start establishes a connection to the receiving endpoint.