From efb4f0fa563815516ca240369fb30426b88f25d3 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 13 Mar 2020 18:10:51 -0700 Subject: [PATCH 01/14] Add support for Resources in the SDK Add `Config` types for the push `Controller` and the `SDK`. Included with this are helper functions to configure the `ErrorHandler` and `Resource`. Add a `Resource` to the Meter `Descriptor`. The choice to add the `Resource` here (instead of say a `Record` or the `Instrument` itself) was motivated by the definition of the `Descriptor` as the way to uniquely describe a metric instrument. Update the push `Controller` and default `SDK` to pass down their configured `Resource` from instantiation to the metric instruments. --- exporters/metric/dogstatsd/dogstatsd_test.go | 3 +- exporters/metric/internal/statsd/conn_test.go | 11 ++-- .../metric/prometheus/prometheus_test.go | 7 +-- exporters/metric/stdout/stdout_test.go | 17 +++--- .../otlp/internal/transform/metric_test.go | 13 ++--- .../metric/aggregator/aggregator_test.go | 3 ++ sdk/export/metric/metric.go | 10 ++++ sdk/metric/aggregator/test/test.go | 3 +- sdk/metric/batcher/test/test.go | 9 ++-- sdk/metric/config.go | 50 +++++++++++++++++ sdk/metric/config_test.go | 47 ++++++++++++++++ sdk/metric/controller/push/config.go | 53 +++++++++++++++++++ sdk/metric/controller/push/config_test.go | 48 +++++++++++++++++ sdk/metric/controller/push/push.go | 11 ++-- sdk/metric/histogram_stress_test.go | 3 +- sdk/metric/minmaxsumcount_stress_test.go | 3 +- sdk/metric/sdk.go | 26 ++++++--- sdk/metric/selector/simple/simple_test.go | 7 +-- 18 files changed, 281 insertions(+), 43 deletions(-) create mode 100644 sdk/metric/config.go create mode 100644 sdk/metric/config_test.go create mode 100644 sdk/metric/controller/push/config.go create mode 100644 sdk/metric/controller/push/config_test.go diff --git a/exporters/metric/dogstatsd/dogstatsd_test.go b/exporters/metric/dogstatsd/dogstatsd_test.go index 892229ae5c4..bd8d2e82923 100644 --- a/exporters/metric/dogstatsd/dogstatsd_test.go +++ b/exporters/metric/dogstatsd/dogstatsd_test.go @@ -30,6 +30,7 @@ import ( export "go.opentelemetry.io/otel/sdk/export/metric" sdk "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" + "go.opentelemetry.io/otel/sdk/resource" ) // TestDogstatsLabels that labels are formatted in the correct style, @@ -44,7 +45,7 @@ func TestDogstatsLabels(t *testing.T) { ctx := context.Background() checkpointSet := test.NewCheckpointSet(encoder) - desc := export.NewDescriptor("test.name", export.CounterKind, nil, "", "", core.Int64NumberKind) + desc := export.NewDescriptor("test.name", export.CounterKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) cagg := sum.New() _ = cagg.Update(ctx, core.NewInt64Number(123), desc) cagg.Checkpoint(ctx, desc) diff --git a/exporters/metric/internal/statsd/conn_test.go b/exporters/metric/internal/statsd/conn_test.go index dc94c266daf..b0eaba85b74 100644 --- a/exporters/metric/internal/statsd/conn_test.go +++ b/exporters/metric/internal/statsd/conn_test.go @@ -31,6 +31,7 @@ import ( "go.opentelemetry.io/otel/exporters/metric/test" export "go.opentelemetry.io/otel/sdk/export/metric" sdk "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/resource" ) // withTagsAdapter tests a dogstatsd-style statsd exporter. @@ -124,13 +125,13 @@ timer.B.D:%s|ms checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) cdesc := export.NewDescriptor( - "counter", export.CounterKind, nil, "", "", nkind) + "counter", export.CounterKind, nil, "", "", nkind, resource.Resource{}) gdesc := export.NewDescriptor( - "observer", export.ObserverKind, nil, "", "", nkind) + "observer", export.ObserverKind, nil, "", "", nkind, resource.Resource{}) mdesc := export.NewDescriptor( - "measure", export.MeasureKind, nil, "", "", nkind) + "measure", export.MeasureKind, nil, "", "", nkind, resource.Resource{}) tdesc := export.NewDescriptor( - "timer", export.MeasureKind, nil, "", unit.Milliseconds, nkind) + "timer", export.MeasureKind, nil, "", unit.Milliseconds, nkind, resource.Resource{}) labels := []core.KeyValue{ key.New("A").String("B"), @@ -285,7 +286,7 @@ func TestPacketSplit(t *testing.T) { } checkpointSet := test.NewCheckpointSet(adapter.LabelEncoder) - desc := export.NewDescriptor("counter", export.CounterKind, nil, "", "", core.Int64NumberKind) + desc := export.NewDescriptor("counter", export.CounterKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) var expected []string diff --git a/exporters/metric/prometheus/prometheus_test.go b/exporters/metric/prometheus/prometheus_test.go index 72a7ae2339c..b142ef8b497 100644 --- a/exporters/metric/prometheus/prometheus_test.go +++ b/exporters/metric/prometheus/prometheus_test.go @@ -16,6 +16,7 @@ import ( "go.opentelemetry.io/otel/exporters/metric/test" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/resource" ) func TestPrometheusExporter(t *testing.T) { @@ -30,11 +31,11 @@ func TestPrometheusExporter(t *testing.T) { checkpointSet := test.NewCheckpointSet(metric.NewDefaultLabelEncoder()) counter := export.NewDescriptor( - "counter", export.CounterKind, nil, "", "", core.Float64NumberKind) + "counter", export.CounterKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) lastValue := export.NewDescriptor( - "lastvalue", export.ObserverKind, nil, "", "", core.Float64NumberKind) + "lastvalue", export.ObserverKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) measure := export.NewDescriptor( - "measure", export.MeasureKind, nil, "", "", core.Float64NumberKind) + "measure", export.MeasureKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) labels := []core.KeyValue{ key.New("A").String("B"), diff --git a/exporters/metric/stdout/stdout_test.go b/exporters/metric/stdout/stdout_test.go index d9c6544b0d4..c34a774834e 100644 --- a/exporters/metric/stdout/stdout_test.go +++ b/exporters/metric/stdout/stdout_test.go @@ -23,6 +23,7 @@ import ( "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" aggtest "go.opentelemetry.io/otel/sdk/metric/aggregator/test" + "go.opentelemetry.io/otel/sdk/resource" ) type testFixture struct { @@ -82,7 +83,7 @@ func TestStdoutTimestamp(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) ctx := context.Background() - desc := export.NewDescriptor("test.name", export.ObserverKind, nil, "", "", core.Int64NumberKind) + desc := export.NewDescriptor("test.name", export.ObserverKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) lvagg := lastvalue.New() aggtest.CheckedUpdate(t, lvagg, core.NewInt64Number(321), desc) lvagg.Checkpoint(ctx, desc) @@ -127,7 +128,7 @@ func TestStdoutCounterFormat(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.CounterKind, nil, "", "", core.Int64NumberKind) + desc := export.NewDescriptor("test.name", export.CounterKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) cagg := sum.New() aggtest.CheckedUpdate(fix.t, cagg, core.NewInt64Number(123), desc) cagg.Checkpoint(fix.ctx, desc) @@ -144,7 +145,7 @@ func TestStdoutLastValueFormat(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.ObserverKind, nil, "", "", core.Float64NumberKind) + desc := export.NewDescriptor("test.name", export.ObserverKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) lvagg := lastvalue.New() aggtest.CheckedUpdate(fix.t, lvagg, core.NewFloat64Number(123.456), desc) lvagg.Checkpoint(fix.ctx, desc) @@ -161,7 +162,7 @@ func TestStdoutMinMaxSumCount(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.MeasureKind, nil, "", "", core.Float64NumberKind) + desc := export.NewDescriptor("test.name", export.MeasureKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) magg := minmaxsumcount.New(desc) aggtest.CheckedUpdate(fix.t, magg, core.NewFloat64Number(123.456), desc) aggtest.CheckedUpdate(fix.t, magg, core.NewFloat64Number(876.543), desc) @@ -181,7 +182,7 @@ func TestStdoutMeasureFormat(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.MeasureKind, nil, "", "", core.Float64NumberKind) + desc := export.NewDescriptor("test.name", export.MeasureKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) magg := array.New() for i := 0; i < 1000; i++ { @@ -222,7 +223,7 @@ func TestStdoutMeasureFormat(t *testing.T) { } func TestStdoutEmptyDataSet(t *testing.T) { - desc := export.NewDescriptor("test.name", export.MeasureKind, nil, "", "", core.Float64NumberKind) + desc := export.NewDescriptor("test.name", export.MeasureKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) for name, tc := range map[string]export.Aggregator{ "ddsketch": ddsketch.New(ddsketch.NewDefaultConfig(), desc), "minmaxsumcount": minmaxsumcount.New(desc), @@ -252,7 +253,7 @@ func TestStdoutLastValueNotSet(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.ObserverKind, nil, "", "", core.Float64NumberKind) + desc := export.NewDescriptor("test.name", export.ObserverKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) lvagg := lastvalue.New() lvagg.Checkpoint(fix.ctx, desc) @@ -270,7 +271,7 @@ func TestStdoutCounterWithUnspecifiedKeys(t *testing.T) { keys := []core.Key{key.New("C"), key.New("D")} - desc := export.NewDescriptor("test.name", export.CounterKind, keys, "", "", core.Int64NumberKind) + desc := export.NewDescriptor("test.name", export.CounterKind, keys, "", "", core.Int64NumberKind, resource.Resource{}) cagg := sum.New() aggtest.CheckedUpdate(fix.t, cagg, core.NewInt64Number(10), desc) cagg.Checkpoint(fix.ctx, desc) diff --git a/exporters/otlp/internal/transform/metric_test.go b/exporters/otlp/internal/transform/metric_test.go index b8eb613363b..5617c671d3e 100644 --- a/exporters/otlp/internal/transform/metric_test.go +++ b/exporters/otlp/internal/transform/metric_test.go @@ -28,6 +28,7 @@ import ( "go.opentelemetry.io/otel/sdk/export/metric/aggregator" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" sumAgg "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" + "go.opentelemetry.io/otel/sdk/resource" ) func TestStringKeyValues(t *testing.T) { @@ -144,7 +145,7 @@ func TestMinMaxSumCountMetricDescriptor(t *testing.T) { } mmsc.Checkpoint(ctx, &metricsdk.Descriptor{}) for _, test := range tests { - desc := metricsdk.NewDescriptor(test.name, test.metricKind, test.keys, test.description, test.unit, test.numberKind) + desc := metricsdk.NewDescriptor(test.name, test.metricKind, test.keys, test.description, test.unit, test.numberKind, resource.Resource{}) labels := metricsdk.NewLabels(test.labels, "", nil) got, err := minMaxSumCount(desc, labels, mmsc) if assert.NoError(t, err) { @@ -154,7 +155,7 @@ func TestMinMaxSumCountMetricDescriptor(t *testing.T) { } func TestMinMaxSumCountDatapoints(t *testing.T) { - desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, []core.Key{}, "", unit.Dimensionless, core.Int64NumberKind) + desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, []core.Key{}, "", unit.Dimensionless, core.Int64NumberKind, resource.Resource{}) labels := metricsdk.NewLabels([]core.KeyValue{}, "", nil) mmsc := minmaxsumcount.New(desc) assert.NoError(t, mmsc.Update(context.Background(), 1, desc)) @@ -241,7 +242,7 @@ func TestSumMetricDescriptor(t *testing.T) { } for _, test := range tests { - desc := metricsdk.NewDescriptor(test.name, test.metricKind, test.keys, test.description, test.unit, test.numberKind) + desc := metricsdk.NewDescriptor(test.name, test.metricKind, test.keys, test.description, test.unit, test.numberKind, resource.Resource{}) labels := metricsdk.NewLabels(test.labels, "", nil) got, err := sum(desc, labels, sumAgg.New()) if assert.NoError(t, err) { @@ -250,8 +251,8 @@ func TestSumMetricDescriptor(t *testing.T) { } } -func TestSumInt64Datapoints(t *testing.T) { - desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, []core.Key{}, "", unit.Dimensionless, core.Int64NumberKind) +func TestSumInt64DataPoints(t *testing.T) { + desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, []core.Key{}, "", unit.Dimensionless, core.Int64NumberKind, resource.Resource{}) labels := metricsdk.NewLabels([]core.KeyValue{}, "", nil) s := sumAgg.New() assert.NoError(t, s.Update(context.Background(), core.Number(1), desc)) @@ -265,7 +266,7 @@ func TestSumInt64Datapoints(t *testing.T) { } func TestSumFloat64Datapoints(t *testing.T) { - desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, []core.Key{}, "", unit.Dimensionless, core.Float64NumberKind) + desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, []core.Key{}, "", unit.Dimensionless, core.Float64NumberKind, resource.Resource{}) labels := metricsdk.NewLabels([]core.KeyValue{}, "", nil) s := sumAgg.New() assert.NoError(t, s.Update(context.Background(), core.NewFloat64Number(1), desc)) diff --git a/sdk/export/metric/aggregator/aggregator_test.go b/sdk/export/metric/aggregator/aggregator_test.go index 2551106020d..fc3f7439cab 100644 --- a/sdk/export/metric/aggregator/aggregator_test.go +++ b/sdk/export/metric/aggregator/aggregator_test.go @@ -26,6 +26,7 @@ import ( "go.opentelemetry.io/otel/sdk/export/metric/aggregator" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" + "go.opentelemetry.io/otel/sdk/resource" ) func TestInconsistentMergeErr(t *testing.T) { @@ -79,6 +80,7 @@ func TestRangeTest(t *testing.T) { "", "", nkind, + resource.Resource{}, ) testRangeNegative(t, desc) }) @@ -100,6 +102,7 @@ func TestNaNTest(t *testing.T) { "", "", nkind, + resource.Resource{}, ) testRangeNaN(t, desc) } diff --git a/sdk/export/metric/metric.go b/sdk/export/metric/metric.go index dd5adae8d45..0ea4037ce5f 100644 --- a/sdk/export/metric/metric.go +++ b/sdk/export/metric/metric.go @@ -21,6 +21,7 @@ import ( "go.opentelemetry.io/otel/api/core" "go.opentelemetry.io/otel/api/unit" + "go.opentelemetry.io/otel/sdk/resource" ) // Batcher is responsible for deciding which kind of aggregation to @@ -309,6 +310,7 @@ type Descriptor struct { description string unit unit.Unit numberKind core.NumberKind + resource resource.Resource } // NewDescriptor builds a new descriptor, for use by `Meter` @@ -324,6 +326,7 @@ func NewDescriptor( description string, unit unit.Unit, numberKind core.NumberKind, + resource resource.Resource, ) *Descriptor { return &Descriptor{ name: name, @@ -332,6 +335,7 @@ func NewDescriptor( description: description, unit: unit, numberKind: numberKind, + resource: resource, } } @@ -370,3 +374,9 @@ func (d *Descriptor) Unit() unit.Unit { func (d *Descriptor) NumberKind() core.NumberKind { return d.numberKind } + +// Resource returns the Resource describing the entity for whom the metric +// instrument measures. +func (d *Descriptor) Resource() resource.Resource { + return d.resource +} diff --git a/sdk/metric/aggregator/test/test.go b/sdk/metric/aggregator/test/test.go index d39b8fe4d72..fa307812d86 100644 --- a/sdk/metric/aggregator/test/test.go +++ b/sdk/metric/aggregator/test/test.go @@ -26,6 +26,7 @@ import ( ottest "go.opentelemetry.io/otel/internal/testing" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/export/metric/aggregator" + "go.opentelemetry.io/otel/sdk/resource" ) const Magnitude = 1000 @@ -54,7 +55,7 @@ func newProfiles() []Profile { } func NewAggregatorTest(mkind export.Kind, nkind core.NumberKind) *export.Descriptor { - return export.NewDescriptor("test.name", mkind, nil, "", "", nkind) + return export.NewDescriptor("test.name", mkind, nil, "", "", nkind, resource.Resource{}) } func RunProfiles(t *testing.T, f func(*testing.T, Profile)) { diff --git a/sdk/metric/batcher/test/test.go b/sdk/metric/batcher/test/test.go index 4a791f1a8b6..e390bb6ac4a 100644 --- a/sdk/metric/batcher/test/test.go +++ b/sdk/metric/batcher/test/test.go @@ -25,6 +25,7 @@ import ( sdk "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" + "go.opentelemetry.io/otel/sdk/resource" ) type ( @@ -43,14 +44,14 @@ type ( var ( // LastValueADesc and LastValueBDesc group by "G" LastValueADesc = export.NewDescriptor( - "lastvalue.a", export.ObserverKind, []core.Key{key.New("G")}, "", "", core.Int64NumberKind) + "lastvalue.a", export.ObserverKind, []core.Key{key.New("G")}, "", "", core.Int64NumberKind, resource.Resource{}) LastValueBDesc = export.NewDescriptor( - "lastvalue.b", export.ObserverKind, []core.Key{key.New("G")}, "", "", core.Int64NumberKind) + "lastvalue.b", export.ObserverKind, []core.Key{key.New("G")}, "", "", core.Int64NumberKind, resource.Resource{}) // CounterADesc and CounterBDesc group by "C" CounterADesc = export.NewDescriptor( - "sum.a", export.CounterKind, []core.Key{key.New("C")}, "", "", core.Int64NumberKind) + "sum.a", export.CounterKind, []core.Key{key.New("C")}, "", "", core.Int64NumberKind, resource.Resource{}) CounterBDesc = export.NewDescriptor( - "sum.b", export.CounterKind, []core.Key{key.New("C")}, "", "", core.Int64NumberKind) + "sum.b", export.CounterKind, []core.Key{key.New("C")}, "", "", core.Int64NumberKind, resource.Resource{}) // SdkEncoder uses a non-standard encoder like K1~V1&K2~V2 SdkEncoder = &Encoder{} diff --git a/sdk/metric/config.go b/sdk/metric/config.go new file mode 100644 index 00000000000..3eb0bf21c93 --- /dev/null +++ b/sdk/metric/config.go @@ -0,0 +1,50 @@ +package metric + +import "go.opentelemetry.io/otel/sdk/resource" + +// Config contains configuration for an SDK. +type Config struct { + // ErrorHandler is the function called when the SDK encounters an error. + // + // This option can be overridden after instantiation of the SDK + // with the `SetErrorHandler` method. + ErrorHandler ErrorHandler + + // Resource is the OpenTelemetry resource associated with all Meters + // created by the SDK. + Resource resource.Resource +} + +// Option is the interface that applies the value to a configuration option. +type Option interface { + // Apply sets the Option value of a Config. + Apply(*Config) +} + +// WithErrorHandler sets the ErrorHandler configuration option of a Config. +func WithErrorHandler(fn ErrorHandler) Option { + return errorHandlerOption(fn) +} + +type errorHandlerOption ErrorHandler + +func (o errorHandlerOption) Apply(config *Config) { + if config == nil { + return + } + config.ErrorHandler = ErrorHandler(o) +} + +// WithResource sets the Resource configuration option of a Config. +func WithResource(r resource.Resource) Option { + return resourceOption(r) +} + +type resourceOption resource.Resource + +func (o resourceOption) Apply(config *Config) { + if config == nil { + return + } + config.Resource = resource.Resource(o) +} diff --git a/sdk/metric/config_test.go b/sdk/metric/config_test.go new file mode 100644 index 00000000000..2ab9324ccc7 --- /dev/null +++ b/sdk/metric/config_test.go @@ -0,0 +1,47 @@ +package metric + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/api/core" + "go.opentelemetry.io/otel/sdk/resource" +) + +func TestWithErrorHandler(t *testing.T) { + errH, reg := func() (ErrorHandler, *error) { + e := fmt.Errorf("default invalid") + reg := &e + return func(err error) { + *reg = err + }, reg + }() + + c := &Config{} + WithErrorHandler(errH).Apply(c) + err1 := fmt.Errorf("error 1") + c.ErrorHandler(err1) + assert.EqualError(t, *reg, err1.Error()) + + // Ensure overwriting works. + c = &Config{ErrorHandler: DefaultErrorHandler} + WithErrorHandler(errH).Apply(c) + err2 := fmt.Errorf("error 2") + c.ErrorHandler(err2) + assert.EqualError(t, *reg, err2.Error()) +} + +func TestWithResource(t *testing.T) { + r := resource.New(core.Key("A").String("a")) + + c := &Config{} + WithResource(*r).Apply(c) + assert.Equal(t, *r, c.Resource) + + // Ensure overwriting works. + c = &Config{Resource: resource.Resource{}} + WithResource(*r).Apply(c) + assert.Equal(t, *r, c.Resource) +} diff --git a/sdk/metric/controller/push/config.go b/sdk/metric/controller/push/config.go new file mode 100644 index 00000000000..a0639ce851b --- /dev/null +++ b/sdk/metric/controller/push/config.go @@ -0,0 +1,53 @@ +package push + +import ( + sdk "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/resource" +) + +// Config contains configuration for a push Controller. +type Config struct { + // ErrorHandler is the function called when the Controller encounters an error. + // + // This option can be overridden after instantiation of the Controller + // with the `SetErrorHandler` method. + ErrorHandler sdk.ErrorHandler + + // Resource is the OpenTelemetry resource associated with all Meters + // created by the Controller. + Resource resource.Resource +} + +// Option is the interface that applies the value to a configuration option. +type Option interface { + // Apply sets the Option value of a Config. + Apply(*Config) +} + +// WithErrorHandler sets the ErrorHandler configuration option of a Config. +func WithErrorHandler(fn sdk.ErrorHandler) Option { + return errorHandlerOption(fn) +} + +type errorHandlerOption sdk.ErrorHandler + +func (o errorHandlerOption) Apply(config *Config) { + if config == nil { + return + } + config.ErrorHandler = sdk.ErrorHandler(o) +} + +// WithResource sets the Resource configuration option of a Config. +func WithResource(r resource.Resource) Option { + return resourceOption(r) +} + +type resourceOption resource.Resource + +func (o resourceOption) Apply(config *Config) { + if config == nil { + return + } + config.Resource = resource.Resource(o) +} diff --git a/sdk/metric/controller/push/config_test.go b/sdk/metric/controller/push/config_test.go new file mode 100644 index 00000000000..90e38211cbd --- /dev/null +++ b/sdk/metric/controller/push/config_test.go @@ -0,0 +1,48 @@ +package push + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/api/core" + sdk "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/resource" +) + +func TestWithErrorHandler(t *testing.T) { + errH, reg := func() (sdk.ErrorHandler, *error) { + e := fmt.Errorf("default invalid") + reg := &e + return func(err error) { + *reg = err + }, reg + }() + + c := &Config{} + WithErrorHandler(errH).Apply(c) + err1 := fmt.Errorf("error 1") + c.ErrorHandler(err1) + assert.EqualError(t, *reg, err1.Error()) + + // Ensure overwriting works. + c = &Config{ErrorHandler: sdk.DefaultErrorHandler} + WithErrorHandler(errH).Apply(c) + err2 := fmt.Errorf("error 2") + c.ErrorHandler(err2) + assert.EqualError(t, *reg, err2.Error()) +} + +func TestWithResource(t *testing.T) { + r := resource.New(core.Key("A").String("a")) + + c := &Config{} + WithResource(*r).Apply(c) + assert.Equal(t, *r, c.Resource) + + // Ensure overwriting works. + c = &Config{Resource: resource.Resource{}} + WithResource(*r).Apply(c) + assert.Equal(t, *r, c.Resource) +} diff --git a/sdk/metric/controller/push/push.go b/sdk/metric/controller/push/push.go index cb2294a1c74..f2613c49eff 100644 --- a/sdk/metric/controller/push/push.go +++ b/sdk/metric/controller/push/push.go @@ -73,16 +73,21 @@ var _ Ticker = realTicker{} // exporter will be used as the label encoder for the SDK itself, // otherwise the SDK will be configured with the default label // encoder. -func New(batcher export.Batcher, exporter export.Exporter, period time.Duration) *Controller { +func New(batcher export.Batcher, exporter export.Exporter, period time.Duration, opts ...Option) *Controller { lencoder, _ := exporter.(export.LabelEncoder) if lencoder == nil { lencoder = sdk.NewDefaultLabelEncoder() } + c := &Config{ErrorHandler: sdk.DefaultErrorHandler} + for _, opt := range opts { + opt.Apply(c) + } + return &Controller{ - sdk: sdk.New(batcher, lencoder), - errorHandler: sdk.DefaultErrorHandler, + sdk: sdk.New(batcher, lencoder, sdk.WithResource(c.Resource)), + errorHandler: c.ErrorHandler, batcher: batcher, exporter: exporter, ch: make(chan struct{}), diff --git a/sdk/metric/histogram_stress_test.go b/sdk/metric/histogram_stress_test.go index 451c09b3bb2..0dd1e9cbc28 100644 --- a/sdk/metric/histogram_stress_test.go +++ b/sdk/metric/histogram_stress_test.go @@ -27,10 +27,11 @@ import ( "go.opentelemetry.io/otel/api/core" "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/histogram" + "go.opentelemetry.io/otel/sdk/resource" ) func TestStressInt64Histogram(t *testing.T) { - desc := metric.NewDescriptor("some_metric", metric.MeasureKind, nil, "", "", core.Int64NumberKind) + desc := metric.NewDescriptor("some_metric", metric.MeasureKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) h := histogram.New(desc, []core.Number{core.NewInt64Number(25), core.NewInt64Number(50), core.NewInt64Number(75)}) ctx, cancelFunc := context.WithCancel(context.Background()) diff --git a/sdk/metric/minmaxsumcount_stress_test.go b/sdk/metric/minmaxsumcount_stress_test.go index 47ab5aaf565..02d1206e858 100644 --- a/sdk/metric/minmaxsumcount_stress_test.go +++ b/sdk/metric/minmaxsumcount_stress_test.go @@ -27,10 +27,11 @@ import ( "go.opentelemetry.io/otel/api/core" "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" + "go.opentelemetry.io/otel/sdk/resource" ) func TestStressInt64MinMaxSumCount(t *testing.T) { - desc := metric.NewDescriptor("some_metric", metric.MeasureKind, nil, "", "", core.Int64NumberKind) + desc := metric.NewDescriptor("some_metric", metric.MeasureKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) mmsc := minmaxsumcount.New(desc) ctx, cancel := context.WithCancel(context.Background()) diff --git a/sdk/metric/sdk.go b/sdk/metric/sdk.go index d0e3606c1b4..2c1207addd9 100644 --- a/sdk/metric/sdk.go +++ b/sdk/metric/sdk.go @@ -29,6 +29,7 @@ import ( api "go.opentelemetry.io/otel/api/metric" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/export/metric/aggregator" + "go.opentelemetry.io/otel/sdk/resource" ) type ( @@ -66,6 +67,9 @@ type ( // errorHandler supports delivering errors to the user. errorHandler ErrorHandler + + // resource represents the entity producing telemetry. + resource resource.Resource } instrument struct { @@ -329,11 +333,17 @@ func (i *instrument) RecordOne(ctx context.Context, number core.Number, ls api.L // batcher will call Collect() when it receives a request to scrape // current metric values. A push-based batcher should configure its // own periodic collection. -func New(batcher export.Batcher, labelEncoder export.LabelEncoder) *SDK { +func New(batcher export.Batcher, labelEncoder export.LabelEncoder, opts ...Option) *SDK { + c := &Config{ErrorHandler: DefaultErrorHandler} + for _, opt := range opts { + opt.Apply(c) + } + m := &SDK{ batcher: batcher, labelEncoder: labelEncoder, - errorHandler: DefaultErrorHandler, + errorHandler: c.ErrorHandler, + resource: c.Resource, } m.empty.meter = m return m @@ -461,7 +471,7 @@ func (m *SDK) labsFor(ls api.LabelSet) *labels { return &m.empty } -func newDescriptor(name string, metricKind export.Kind, numberKind core.NumberKind, opts []api.Option) *export.Descriptor { +func (m *SDK) newDescriptor(name string, metricKind export.Kind, numberKind core.NumberKind, opts []api.Option) *export.Descriptor { config := api.Configure(opts) return export.NewDescriptor( name, @@ -469,11 +479,13 @@ func newDescriptor(name string, metricKind export.Kind, numberKind core.NumberKi config.Keys, config.Description, config.Unit, - numberKind) + numberKind, + m.resource, + ) } func (m *SDK) newInstrument(name string, metricKind export.Kind, numberKind core.NumberKind, opts []api.Option) *instrument { - descriptor := newDescriptor(name, metricKind, numberKind, opts) + descriptor := m.newDescriptor(name, metricKind, numberKind, opts) return &instrument{ descriptor: descriptor, meter: m, @@ -508,7 +520,7 @@ func (m *SDK) RegisterInt64Observer(name string, callback api.Int64ObserverCallb if callback == nil { return api.NoopMeter{}.RegisterInt64Observer("", nil) } - descriptor := newDescriptor(name, export.ObserverKind, core.Int64NumberKind, opts) + descriptor := m.newDescriptor(name, export.ObserverKind, core.Int64NumberKind, opts) cb := wrapInt64ObserverCallback(callback) obs := m.newObserver(descriptor, cb) return int64Observer{ @@ -529,7 +541,7 @@ func (m *SDK) RegisterFloat64Observer(name string, callback api.Float64ObserverC if callback == nil { return api.NoopMeter{}.RegisterFloat64Observer("", nil) } - descriptor := newDescriptor(name, export.ObserverKind, core.Float64NumberKind, opts) + descriptor := m.newDescriptor(name, export.ObserverKind, core.Float64NumberKind, opts) cb := wrapFloat64ObserverCallback(callback) obs := m.newObserver(descriptor, cb) return float64Observer{ diff --git a/sdk/metric/selector/simple/simple_test.go b/sdk/metric/selector/simple/simple_test.go index 595789ccc77..0d73bafb5e5 100644 --- a/sdk/metric/selector/simple/simple_test.go +++ b/sdk/metric/selector/simple/simple_test.go @@ -26,12 +26,13 @@ import ( "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" "go.opentelemetry.io/otel/sdk/metric/selector/simple" + "go.opentelemetry.io/otel/sdk/resource" ) var ( - testCounterDesc = export.NewDescriptor("counter", export.CounterKind, nil, "", "", core.Int64NumberKind) - testMeasureDesc = export.NewDescriptor("measure", export.MeasureKind, nil, "", "", core.Int64NumberKind) - testObserverDesc = export.NewDescriptor("observer", export.ObserverKind, nil, "", "", core.Int64NumberKind) + testCounterDesc = export.NewDescriptor("counter", export.CounterKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) + testMeasureDesc = export.NewDescriptor("measure", export.MeasureKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) + testObserverDesc = export.NewDescriptor("observer", export.ObserverKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) ) func TestInexpensiveMeasure(t *testing.T) { From 226343d68dc4842f3036d88314ff8c60910ae557 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 16 Mar 2020 08:54:02 -0700 Subject: [PATCH 02/14] Update New SDK constructor documentation --- sdk/metric/controller/push/push.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/metric/controller/push/push.go b/sdk/metric/controller/push/push.go index f2613c49eff..93bc085dbc4 100644 --- a/sdk/metric/controller/push/push.go +++ b/sdk/metric/controller/push/push.go @@ -65,9 +65,9 @@ var _ Clock = realClock{} var _ Ticker = realTicker{} // New constructs a Controller, an implementation of metric.Provider, -// using the provided batcher, exporter, and collection period to -// configure an SDK with periodic collection. The batcher itself is -// configured with the aggregation selector policy. +// using the provided batcher, exporter, collection period, and SDK +// configuration options to configure an SDK with periodic collection. +// The batcher itself is configured with the aggregation selector policy. // // If the Exporter implements the export.LabelEncoder interface, the // exporter will be used as the label encoder for the SDK itself, From 24796708c09083f2d9fd237bafedf19d673648ff Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 16 Mar 2020 10:07:05 -0700 Subject: [PATCH 03/14] Change NewDescriptor constructor to take opts Add DescriptorConfig and DescriptorOption to configure the metric Descriptor with the description, unit, keys, and resource. Update all function calls to NewDescriptor to use new function signature. --- exporters/metric/dogstatsd/dogstatsd_test.go | 3 +- exporters/metric/internal/statsd/conn_test.go | 15 +- .../metric/prometheus/prometheus_test.go | 10 +- exporters/metric/stdout/stdout_test.go | 17 +- .../otlp/internal/transform/metric_test.go | 21 ++- .../metric/aggregator/aggregator_test.go | 23 +-- sdk/export/metric/descriptor.go | 166 ++++++++++++++++++ sdk/export/metric/descriptor_test.go | 59 +++++++ sdk/export/metric/metric.go | 85 --------- sdk/metric/aggregator/test/test.go | 3 +- sdk/metric/batcher/test/test.go | 13 +- sdk/metric/histogram_stress_test.go | 3 +- sdk/metric/minmaxsumcount_stress_test.go | 3 +- sdk/metric/sdk.go | 8 +- sdk/metric/selector/simple/simple_test.go | 7 +- 15 files changed, 273 insertions(+), 163 deletions(-) create mode 100644 sdk/export/metric/descriptor.go create mode 100644 sdk/export/metric/descriptor_test.go diff --git a/exporters/metric/dogstatsd/dogstatsd_test.go b/exporters/metric/dogstatsd/dogstatsd_test.go index bd8d2e82923..08153a2ce24 100644 --- a/exporters/metric/dogstatsd/dogstatsd_test.go +++ b/exporters/metric/dogstatsd/dogstatsd_test.go @@ -30,7 +30,6 @@ import ( export "go.opentelemetry.io/otel/sdk/export/metric" sdk "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" - "go.opentelemetry.io/otel/sdk/resource" ) // TestDogstatsLabels that labels are formatted in the correct style, @@ -45,7 +44,7 @@ func TestDogstatsLabels(t *testing.T) { ctx := context.Background() checkpointSet := test.NewCheckpointSet(encoder) - desc := export.NewDescriptor("test.name", export.CounterKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("test.name", export.CounterKind, core.Int64NumberKind) cagg := sum.New() _ = cagg.Update(ctx, core.NewInt64Number(123), desc) cagg.Checkpoint(ctx, desc) diff --git a/exporters/metric/internal/statsd/conn_test.go b/exporters/metric/internal/statsd/conn_test.go index b0eaba85b74..c9af1a5016b 100644 --- a/exporters/metric/internal/statsd/conn_test.go +++ b/exporters/metric/internal/statsd/conn_test.go @@ -31,7 +31,6 @@ import ( "go.opentelemetry.io/otel/exporters/metric/test" export "go.opentelemetry.io/otel/sdk/export/metric" sdk "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/resource" ) // withTagsAdapter tests a dogstatsd-style statsd exporter. @@ -124,14 +123,10 @@ timer.B.D:%s|ms } checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - cdesc := export.NewDescriptor( - "counter", export.CounterKind, nil, "", "", nkind, resource.Resource{}) - gdesc := export.NewDescriptor( - "observer", export.ObserverKind, nil, "", "", nkind, resource.Resource{}) - mdesc := export.NewDescriptor( - "measure", export.MeasureKind, nil, "", "", nkind, resource.Resource{}) - tdesc := export.NewDescriptor( - "timer", export.MeasureKind, nil, "", unit.Milliseconds, nkind, resource.Resource{}) + cdesc := export.NewDescriptor("counter", export.CounterKind, nkind) + gdesc := export.NewDescriptor("observer", export.ObserverKind, nkind) + mdesc := export.NewDescriptor("measure", export.MeasureKind, nkind) + tdesc := export.NewDescriptor("timer", export.MeasureKind, nkind, export.WithUnit(unit.Milliseconds)) labels := []core.KeyValue{ key.New("A").String("B"), @@ -286,7 +281,7 @@ func TestPacketSplit(t *testing.T) { } checkpointSet := test.NewCheckpointSet(adapter.LabelEncoder) - desc := export.NewDescriptor("counter", export.CounterKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("counter", export.CounterKind, core.Int64NumberKind) var expected []string diff --git a/exporters/metric/prometheus/prometheus_test.go b/exporters/metric/prometheus/prometheus_test.go index b142ef8b497..617b2e3d9e9 100644 --- a/exporters/metric/prometheus/prometheus_test.go +++ b/exporters/metric/prometheus/prometheus_test.go @@ -16,7 +16,6 @@ import ( "go.opentelemetry.io/otel/exporters/metric/test" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/resource" ) func TestPrometheusExporter(t *testing.T) { @@ -30,12 +29,9 @@ func TestPrometheusExporter(t *testing.T) { var expected []string checkpointSet := test.NewCheckpointSet(metric.NewDefaultLabelEncoder()) - counter := export.NewDescriptor( - "counter", export.CounterKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) - lastValue := export.NewDescriptor( - "lastvalue", export.ObserverKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) - measure := export.NewDescriptor( - "measure", export.MeasureKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) + counter := export.NewDescriptor("counter", export.CounterKind, core.Float64NumberKind) + lastValue := export.NewDescriptor("lastvalue", export.ObserverKind, core.Float64NumberKind) + measure := export.NewDescriptor("measure", export.MeasureKind, core.Float64NumberKind) labels := []core.KeyValue{ key.New("A").String("B"), diff --git a/exporters/metric/stdout/stdout_test.go b/exporters/metric/stdout/stdout_test.go index c34a774834e..ebb972443ef 100644 --- a/exporters/metric/stdout/stdout_test.go +++ b/exporters/metric/stdout/stdout_test.go @@ -23,7 +23,6 @@ import ( "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" aggtest "go.opentelemetry.io/otel/sdk/metric/aggregator/test" - "go.opentelemetry.io/otel/sdk/resource" ) type testFixture struct { @@ -83,7 +82,7 @@ func TestStdoutTimestamp(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) ctx := context.Background() - desc := export.NewDescriptor("test.name", export.ObserverKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("test.name", export.ObserverKind, core.Int64NumberKind) lvagg := lastvalue.New() aggtest.CheckedUpdate(t, lvagg, core.NewInt64Number(321), desc) lvagg.Checkpoint(ctx, desc) @@ -128,7 +127,7 @@ func TestStdoutCounterFormat(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.CounterKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("test.name", export.CounterKind, core.Int64NumberKind) cagg := sum.New() aggtest.CheckedUpdate(fix.t, cagg, core.NewInt64Number(123), desc) cagg.Checkpoint(fix.ctx, desc) @@ -145,7 +144,7 @@ func TestStdoutLastValueFormat(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.ObserverKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("test.name", export.ObserverKind, core.Float64NumberKind) lvagg := lastvalue.New() aggtest.CheckedUpdate(fix.t, lvagg, core.NewFloat64Number(123.456), desc) lvagg.Checkpoint(fix.ctx, desc) @@ -162,7 +161,7 @@ func TestStdoutMinMaxSumCount(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.MeasureKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("test.name", export.MeasureKind, core.Float64NumberKind) magg := minmaxsumcount.New(desc) aggtest.CheckedUpdate(fix.t, magg, core.NewFloat64Number(123.456), desc) aggtest.CheckedUpdate(fix.t, magg, core.NewFloat64Number(876.543), desc) @@ -182,7 +181,7 @@ func TestStdoutMeasureFormat(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.MeasureKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("test.name", export.MeasureKind, core.Float64NumberKind) magg := array.New() for i := 0; i < 1000; i++ { @@ -223,7 +222,7 @@ func TestStdoutMeasureFormat(t *testing.T) { } func TestStdoutEmptyDataSet(t *testing.T) { - desc := export.NewDescriptor("test.name", export.MeasureKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("test.name", export.MeasureKind, core.Float64NumberKind) for name, tc := range map[string]export.Aggregator{ "ddsketch": ddsketch.New(ddsketch.NewDefaultConfig(), desc), "minmaxsumcount": minmaxsumcount.New(desc), @@ -253,7 +252,7 @@ func TestStdoutLastValueNotSet(t *testing.T) { checkpointSet := test.NewCheckpointSet(sdk.NewDefaultLabelEncoder()) - desc := export.NewDescriptor("test.name", export.ObserverKind, nil, "", "", core.Float64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("test.name", export.ObserverKind, core.Float64NumberKind) lvagg := lastvalue.New() lvagg.Checkpoint(fix.ctx, desc) @@ -271,7 +270,7 @@ func TestStdoutCounterWithUnspecifiedKeys(t *testing.T) { keys := []core.Key{key.New("C"), key.New("D")} - desc := export.NewDescriptor("test.name", export.CounterKind, keys, "", "", core.Int64NumberKind, resource.Resource{}) + desc := export.NewDescriptor("test.name", export.CounterKind, core.Int64NumberKind, export.WithKeys(keys...)) cagg := sum.New() aggtest.CheckedUpdate(fix.t, cagg, core.NewInt64Number(10), desc) cagg.Checkpoint(fix.ctx, desc) diff --git a/exporters/otlp/internal/transform/metric_test.go b/exporters/otlp/internal/transform/metric_test.go index 5617c671d3e..592134a4718 100644 --- a/exporters/otlp/internal/transform/metric_test.go +++ b/exporters/otlp/internal/transform/metric_test.go @@ -28,7 +28,6 @@ import ( "go.opentelemetry.io/otel/sdk/export/metric/aggregator" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" sumAgg "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" - "go.opentelemetry.io/otel/sdk/resource" ) func TestStringKeyValues(t *testing.T) { @@ -145,7 +144,12 @@ func TestMinMaxSumCountMetricDescriptor(t *testing.T) { } mmsc.Checkpoint(ctx, &metricsdk.Descriptor{}) for _, test := range tests { - desc := metricsdk.NewDescriptor(test.name, test.metricKind, test.keys, test.description, test.unit, test.numberKind, resource.Resource{}) + opts := []metricsdk.DescriptorOption{ + metricsdk.WithUnit(test.unit), + metricsdk.WithDescription(test.description), + metricsdk.WithKeys(test.keys...), + } + desc := metricsdk.NewDescriptor(test.name, test.metricKind, test.numberKind, opts...) labels := metricsdk.NewLabels(test.labels, "", nil) got, err := minMaxSumCount(desc, labels, mmsc) if assert.NoError(t, err) { @@ -155,7 +159,7 @@ func TestMinMaxSumCountMetricDescriptor(t *testing.T) { } func TestMinMaxSumCountDatapoints(t *testing.T) { - desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, []core.Key{}, "", unit.Dimensionless, core.Int64NumberKind, resource.Resource{}) + desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, core.Int64NumberKind) labels := metricsdk.NewLabels([]core.KeyValue{}, "", nil) mmsc := minmaxsumcount.New(desc) assert.NoError(t, mmsc.Update(context.Background(), 1, desc)) @@ -242,7 +246,12 @@ func TestSumMetricDescriptor(t *testing.T) { } for _, test := range tests { - desc := metricsdk.NewDescriptor(test.name, test.metricKind, test.keys, test.description, test.unit, test.numberKind, resource.Resource{}) + opts := []metricsdk.DescriptorOption{ + metricsdk.WithUnit(test.unit), + metricsdk.WithDescription(test.description), + metricsdk.WithKeys(test.keys...), + } + desc := metricsdk.NewDescriptor(test.name, test.metricKind, test.numberKind, opts...) labels := metricsdk.NewLabels(test.labels, "", nil) got, err := sum(desc, labels, sumAgg.New()) if assert.NoError(t, err) { @@ -252,7 +261,7 @@ func TestSumMetricDescriptor(t *testing.T) { } func TestSumInt64DataPoints(t *testing.T) { - desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, []core.Key{}, "", unit.Dimensionless, core.Int64NumberKind, resource.Resource{}) + desc := metricsdk.NewDescriptor("", metricsdk.CounterKind, core.Int64NumberKind) labels := metricsdk.NewLabels([]core.KeyValue{}, "", nil) s := sumAgg.New() assert.NoError(t, s.Update(context.Background(), core.Number(1), desc)) @@ -266,7 +275,7 @@ func TestSumInt64DataPoints(t *testing.T) { } func TestSumFloat64Datapoints(t *testing.T) { - desc := metricsdk.NewDescriptor("", metricsdk.MeasureKind, []core.Key{}, "", unit.Dimensionless, core.Float64NumberKind, resource.Resource{}) + desc := metricsdk.NewDescriptor("", metricsdk.CounterKind, core.Float64NumberKind) labels := metricsdk.NewLabels([]core.KeyValue{}, "", nil) s := sumAgg.New() assert.NoError(t, s.Update(context.Background(), core.NewFloat64Number(1), desc)) diff --git a/sdk/export/metric/aggregator/aggregator_test.go b/sdk/export/metric/aggregator/aggregator_test.go index fc3f7439cab..a06efbdc330 100644 --- a/sdk/export/metric/aggregator/aggregator_test.go +++ b/sdk/export/metric/aggregator/aggregator_test.go @@ -26,7 +26,6 @@ import ( "go.opentelemetry.io/otel/sdk/export/metric/aggregator" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" - "go.opentelemetry.io/otel/sdk/resource" ) func TestInconsistentMergeErr(t *testing.T) { @@ -73,16 +72,7 @@ func TestRangeTest(t *testing.T) { // Only Counters implement a range test. for _, nkind := range []core.NumberKind{core.Float64NumberKind, core.Int64NumberKind} { t.Run(nkind.String(), func(t *testing.T) { - desc := export.NewDescriptor( - "name", - export.CounterKind, - nil, - "", - "", - nkind, - resource.Resource{}, - ) - testRangeNegative(t, desc) + testRangeNegative(t, export.NewDescriptor("name", export.CounterKind, nkind)) }) } } @@ -95,16 +85,7 @@ func TestNaNTest(t *testing.T) { export.MeasureKind, export.ObserverKind, } { - desc := export.NewDescriptor( - "name", - mkind, - nil, - "", - "", - nkind, - resource.Resource{}, - ) - testRangeNaN(t, desc) + testRangeNaN(t, export.NewDescriptor("name", mkind, nkind)) } }) } diff --git a/sdk/export/metric/descriptor.go b/sdk/export/metric/descriptor.go new file mode 100644 index 00000000000..590306d9c2c --- /dev/null +++ b/sdk/export/metric/descriptor.go @@ -0,0 +1,166 @@ +package metric + +import ( + "go.opentelemetry.io/otel/api/core" + "go.opentelemetry.io/otel/api/unit" + "go.opentelemetry.io/otel/sdk/resource" +) + +// DescriptorConfig contains configuration for a metric Descriptor +type DescriptorConfig struct { + // keys are the common keys related to all measurements. + Keys []core.Key + + // description describes the metric in human readable terms. + Description string + + // unit is the determinate quantity used as a standard of all + // measurements for the metric. + Unit unit.Unit + + // resource descrbes the entity for which this metric makes measurements. + Resource resource.Resource +} + +// DescriptorOption is the interface that applies the value to a DescriptorConfig option. +type DescriptorOption interface { + // Apply sets the option value of a DescriptorConfig. + Apply(*DescriptorConfig) +} + +// WithKeys applies common label keys. +// Multiple `WithKeys` options accumulate. +func WithKeys(keys ...core.Key) DescriptorOption { + return keysOption(keys) +} + +type keysOption []core.Key + +func (k keysOption) Apply(config *DescriptorConfig) { + if config == nil { + return + } + config.Keys = append(config.Keys, k...) +} + +// WithDescription applies provided description. +func WithDescription(d string) DescriptorOption { + return descriptionOption(d) +} + +type descriptionOption string + +func (d descriptionOption) Apply(config *DescriptorConfig) { + if config == nil { + return + } + config.Description = string(d) +} + +// WithUnit applies provided unit. +func WithUnit(u unit.Unit) DescriptorOption { + return unitOption(u) +} + +type unitOption unit.Unit + +func (u unitOption) Apply(config *DescriptorConfig) { + if config == nil { + return + } + config.Unit = unit.Unit(u) +} + +// WithResource applies provided Resource. +func WithResource(r resource.Resource) DescriptorOption { + return resourceOption(r) +} + +type resourceOption resource.Resource + +func (r resourceOption) Apply(config *DescriptorConfig) { + if config == nil { + return + } + config.Resource = resource.Resource(r) +} + +// Descriptor describes a metric instrument to the exporter. +// +// Descriptors are created once per instrument and a pointer to the +// descriptor may be used to uniquely identify the instrument in an +// exporter. +type Descriptor struct { + name string + metricKind Kind + keys []core.Key + description string + unit unit.Unit + numberKind core.NumberKind + resource resource.Resource +} + +// NewDescriptor builds a new descriptor, for use by `Meter` +// implementations in constructing new metric instruments. +// +// Descriptors are created once per instrument and a pointer to the +// descriptor may be used to uniquely identify the instrument in an +// exporter. +func NewDescriptor(name string, metricKind Kind, numberKind core.NumberKind, opts ...DescriptorOption) *Descriptor { + c := &DescriptorConfig{} + for _, opt := range opts { + opt.Apply(c) + } + + return &Descriptor{ + name: name, + metricKind: metricKind, + keys: c.Keys, + description: c.Description, + unit: c.Unit, + numberKind: numberKind, + resource: c.Resource, + } +} + +// Name returns the metric instrument's name. +func (d *Descriptor) Name() string { + return d.name +} + +// MetricKind returns the kind of instrument: counter, measure, or +// observer. +func (d *Descriptor) MetricKind() Kind { + return d.metricKind +} + +// Keys returns the recommended keys included in the metric +// definition. These keys may be used by a Batcher as a default set +// of grouping keys for the metric instrument. +func (d *Descriptor) Keys() []core.Key { + return d.keys +} + +// Description provides a human-readable description of the metric +// instrument. +func (d *Descriptor) Description() string { + return d.description +} + +// Unit describes the units of the metric instrument. Unitless +// metrics return the empty string. +func (d *Descriptor) Unit() unit.Unit { + return d.unit +} + +// NumberKind returns whether this instrument is declared over int64 +// or a float64 values. +func (d *Descriptor) NumberKind() core.NumberKind { + return d.numberKind +} + +// Resource returns the Resource describing the entity for whom the metric +// instrument measures. +func (d *Descriptor) Resource() resource.Resource { + return d.resource +} diff --git a/sdk/export/metric/descriptor_test.go b/sdk/export/metric/descriptor_test.go new file mode 100644 index 00000000000..f2a329ecf53 --- /dev/null +++ b/sdk/export/metric/descriptor_test.go @@ -0,0 +1,59 @@ +package metric + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/api/core" + "go.opentelemetry.io/otel/api/unit" + "go.opentelemetry.io/otel/sdk/resource" +) + +func TestDescriptorNoOpts(t *testing.T) { + tests := []Descriptor{ + {}, // Empty values for everything + { + name: "empty value for options", + metricKind: CounterKind, + numberKind: core.Float64NumberKind, + }, + } + + for _, test := range tests { + got := NewDescriptor(test.name, test.metricKind, test.numberKind) + assert.Equal(t, test, *got) + } +} + +func TestDescriptorConfiguration(t *testing.T) { + tests := []Descriptor{ + {}, // Empty values for everything + { + name: "empty value for options", + metricKind: CounterKind, + numberKind: core.Float64NumberKind, + }, + { + name: "with options", + metricKind: CounterKind, + numberKind: core.Float64NumberKind, + description: "test description", + unit: unit.Bytes, + keys: []core.Key{"keys key"}, + resource: *(resource.New(core.Key("resource key").Bool(true))), + }, + } + + for _, test := range tests { + opts := []DescriptorOption{ + WithKeys(test.keys...), + WithDescription(test.description), + WithUnit(test.unit), + WithResource(test.resource), + } + + got := NewDescriptor(test.name, test.metricKind, test.numberKind, opts...) + assert.Equal(t, test, *got) + } +} diff --git a/sdk/export/metric/metric.go b/sdk/export/metric/metric.go index 0ea4037ce5f..bc88f3c46dc 100644 --- a/sdk/export/metric/metric.go +++ b/sdk/export/metric/metric.go @@ -20,8 +20,6 @@ import ( "context" "go.opentelemetry.io/otel/api/core" - "go.opentelemetry.io/otel/api/unit" - "go.opentelemetry.io/otel/sdk/resource" ) // Batcher is responsible for deciding which kind of aggregation to @@ -297,86 +295,3 @@ const ( // Observer kind indicates an observer instrument ObserverKind ) - -// Descriptor describes a metric instrument to the exporter. -// -// Descriptors are created once per instrument and a pointer to the -// descriptor may be used to uniquely identify the instrument in an -// exporter. -type Descriptor struct { - name string - metricKind Kind - keys []core.Key - description string - unit unit.Unit - numberKind core.NumberKind - resource resource.Resource -} - -// NewDescriptor builds a new descriptor, for use by `Meter` -// implementations in constructing new metric instruments. -// -// Descriptors are created once per instrument and a pointer to the -// descriptor may be used to uniquely identify the instrument in an -// exporter. -func NewDescriptor( - name string, - metricKind Kind, - keys []core.Key, - description string, - unit unit.Unit, - numberKind core.NumberKind, - resource resource.Resource, -) *Descriptor { - return &Descriptor{ - name: name, - metricKind: metricKind, - keys: keys, - description: description, - unit: unit, - numberKind: numberKind, - resource: resource, - } -} - -// Name returns the metric instrument's name. -func (d *Descriptor) Name() string { - return d.name -} - -// MetricKind returns the kind of instrument: counter, measure, or -// observer. -func (d *Descriptor) MetricKind() Kind { - return d.metricKind -} - -// Keys returns the recommended keys included in the metric -// definition. These keys may be used by a Batcher as a default set -// of grouping keys for the metric instrument. -func (d *Descriptor) Keys() []core.Key { - return d.keys -} - -// Description provides a human-readable description of the metric -// instrument. -func (d *Descriptor) Description() string { - return d.description -} - -// Unit describes the units of the metric instrument. Unitless -// metrics return the empty string. -func (d *Descriptor) Unit() unit.Unit { - return d.unit -} - -// NumberKind returns whether this instrument is declared over int64 -// or a float64 values. -func (d *Descriptor) NumberKind() core.NumberKind { - return d.numberKind -} - -// Resource returns the Resource describing the entity for whom the metric -// instrument measures. -func (d *Descriptor) Resource() resource.Resource { - return d.resource -} diff --git a/sdk/metric/aggregator/test/test.go b/sdk/metric/aggregator/test/test.go index fa307812d86..81215b0b1e3 100644 --- a/sdk/metric/aggregator/test/test.go +++ b/sdk/metric/aggregator/test/test.go @@ -26,7 +26,6 @@ import ( ottest "go.opentelemetry.io/otel/internal/testing" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/export/metric/aggregator" - "go.opentelemetry.io/otel/sdk/resource" ) const Magnitude = 1000 @@ -55,7 +54,7 @@ func newProfiles() []Profile { } func NewAggregatorTest(mkind export.Kind, nkind core.NumberKind) *export.Descriptor { - return export.NewDescriptor("test.name", mkind, nil, "", "", nkind, resource.Resource{}) + return export.NewDescriptor("test.name", mkind, nkind) } func RunProfiles(t *testing.T, f func(*testing.T, Profile)) { diff --git a/sdk/metric/batcher/test/test.go b/sdk/metric/batcher/test/test.go index e390bb6ac4a..763ab65111a 100644 --- a/sdk/metric/batcher/test/test.go +++ b/sdk/metric/batcher/test/test.go @@ -25,7 +25,6 @@ import ( sdk "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" - "go.opentelemetry.io/otel/sdk/resource" ) type ( @@ -43,15 +42,11 @@ type ( var ( // LastValueADesc and LastValueBDesc group by "G" - LastValueADesc = export.NewDescriptor( - "lastvalue.a", export.ObserverKind, []core.Key{key.New("G")}, "", "", core.Int64NumberKind, resource.Resource{}) - LastValueBDesc = export.NewDescriptor( - "lastvalue.b", export.ObserverKind, []core.Key{key.New("G")}, "", "", core.Int64NumberKind, resource.Resource{}) + LastValueADesc = export.NewDescriptor("lastvalue.a", export.ObserverKind, core.Int64NumberKind, export.WithKeys(key.New("G"))) + LastValueBDesc = export.NewDescriptor("lastvalue.b", export.ObserverKind, core.Int64NumberKind, export.WithKeys(key.New("G"))) // CounterADesc and CounterBDesc group by "C" - CounterADesc = export.NewDescriptor( - "sum.a", export.CounterKind, []core.Key{key.New("C")}, "", "", core.Int64NumberKind, resource.Resource{}) - CounterBDesc = export.NewDescriptor( - "sum.b", export.CounterKind, []core.Key{key.New("C")}, "", "", core.Int64NumberKind, resource.Resource{}) + CounterADesc = export.NewDescriptor("sum.a", export.CounterKind, core.Int64NumberKind, export.WithKeys(key.New("C"))) + CounterBDesc = export.NewDescriptor("sum.b", export.CounterKind, core.Int64NumberKind, export.WithKeys(key.New("C"))) // SdkEncoder uses a non-standard encoder like K1~V1&K2~V2 SdkEncoder = &Encoder{} diff --git a/sdk/metric/histogram_stress_test.go b/sdk/metric/histogram_stress_test.go index 0dd1e9cbc28..a18d4333c43 100644 --- a/sdk/metric/histogram_stress_test.go +++ b/sdk/metric/histogram_stress_test.go @@ -27,11 +27,10 @@ import ( "go.opentelemetry.io/otel/api/core" "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/histogram" - "go.opentelemetry.io/otel/sdk/resource" ) func TestStressInt64Histogram(t *testing.T) { - desc := metric.NewDescriptor("some_metric", metric.MeasureKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) + desc := metric.NewDescriptor("some_metric", metric.MeasureKind, core.Int64NumberKind) h := histogram.New(desc, []core.Number{core.NewInt64Number(25), core.NewInt64Number(50), core.NewInt64Number(75)}) ctx, cancelFunc := context.WithCancel(context.Background()) diff --git a/sdk/metric/minmaxsumcount_stress_test.go b/sdk/metric/minmaxsumcount_stress_test.go index 02d1206e858..f23438ac876 100644 --- a/sdk/metric/minmaxsumcount_stress_test.go +++ b/sdk/metric/minmaxsumcount_stress_test.go @@ -27,11 +27,10 @@ import ( "go.opentelemetry.io/otel/api/core" "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" - "go.opentelemetry.io/otel/sdk/resource" ) func TestStressInt64MinMaxSumCount(t *testing.T) { - desc := metric.NewDescriptor("some_metric", metric.MeasureKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) + desc := metric.NewDescriptor("some_metric", metric.MeasureKind, core.Int64NumberKind) mmsc := minmaxsumcount.New(desc) ctx, cancel := context.WithCancel(context.Background()) diff --git a/sdk/metric/sdk.go b/sdk/metric/sdk.go index 2c1207addd9..40d1c4d88da 100644 --- a/sdk/metric/sdk.go +++ b/sdk/metric/sdk.go @@ -476,11 +476,11 @@ func (m *SDK) newDescriptor(name string, metricKind export.Kind, numberKind core return export.NewDescriptor( name, metricKind, - config.Keys, - config.Description, - config.Unit, numberKind, - m.resource, + export.WithKeys(config.Keys...), + export.WithDescription(config.Description), + export.WithUnit(config.Unit), + export.WithResource(m.resource), ) } diff --git a/sdk/metric/selector/simple/simple_test.go b/sdk/metric/selector/simple/simple_test.go index 0d73bafb5e5..46255e2d0d8 100644 --- a/sdk/metric/selector/simple/simple_test.go +++ b/sdk/metric/selector/simple/simple_test.go @@ -26,13 +26,12 @@ import ( "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" "go.opentelemetry.io/otel/sdk/metric/selector/simple" - "go.opentelemetry.io/otel/sdk/resource" ) var ( - testCounterDesc = export.NewDescriptor("counter", export.CounterKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) - testMeasureDesc = export.NewDescriptor("measure", export.MeasureKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) - testObserverDesc = export.NewDescriptor("observer", export.ObserverKind, nil, "", "", core.Int64NumberKind, resource.Resource{}) + testCounterDesc = export.NewDescriptor("counter", export.CounterKind, core.Int64NumberKind) + testMeasureDesc = export.NewDescriptor("measure", export.MeasureKind, core.Int64NumberKind) + testObserverDesc = export.NewDescriptor("observer", export.ObserverKind, core.Int64NumberKind) ) func TestInexpensiveMeasure(t *testing.T) { From df0cf90fdc23b2eb162accd3d7c4b1f4e86df317 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 16 Mar 2020 10:40:23 -0700 Subject: [PATCH 04/14] Apply suggestions from code review Co-Authored-By: Rahul Patel --- sdk/export/metric/descriptor.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/export/metric/descriptor.go b/sdk/export/metric/descriptor.go index 590306d9c2c..d6a9ab6fafa 100644 --- a/sdk/export/metric/descriptor.go +++ b/sdk/export/metric/descriptor.go @@ -8,17 +8,17 @@ import ( // DescriptorConfig contains configuration for a metric Descriptor type DescriptorConfig struct { - // keys are the common keys related to all measurements. + // Keys are the common keys related to all measurements. Keys []core.Key - // description describes the metric in human readable terms. + // Description describes the metric in human readable terms. Description string - // unit is the determinate quantity used as a standard of all + // Unit is the determinate quantity used as a standard of all // measurements for the metric. Unit unit.Unit - // resource descrbes the entity for which this metric makes measurements. + // Resource describes the entity for which this metric makes measurements. Resource resource.Resource } From 4e69ae60c1cfeb0fc8b8fd4178ad8a741d1f963e Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 16 Mar 2020 10:45:54 -0700 Subject: [PATCH 05/14] Update and add copyright notices --- sdk/export/metric/descriptor.go | 14 ++++++++++++++ sdk/export/metric/descriptor_test.go | 14 ++++++++++++++ sdk/export/metric/metric.go | 2 +- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/sdk/export/metric/descriptor.go b/sdk/export/metric/descriptor.go index d6a9ab6fafa..b2be47acd8e 100644 --- a/sdk/export/metric/descriptor.go +++ b/sdk/export/metric/descriptor.go @@ -1,3 +1,17 @@ +// Copyright 2020, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package metric import ( diff --git a/sdk/export/metric/descriptor_test.go b/sdk/export/metric/descriptor_test.go index f2a329ecf53..ba5e9bab87f 100644 --- a/sdk/export/metric/descriptor_test.go +++ b/sdk/export/metric/descriptor_test.go @@ -1,3 +1,17 @@ +// Copyright 2020, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package metric import ( diff --git a/sdk/export/metric/metric.go b/sdk/export/metric/metric.go index bc88f3c46dc..657ddb28d47 100644 --- a/sdk/export/metric/metric.go +++ b/sdk/export/metric/metric.go @@ -1,4 +1,4 @@ -// Copyright 2019, OpenTelemetry Authors +// Copyright 2020, OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 437afe1f5de78307d4ed44b841623e92395d58a2 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 19 Mar 2020 20:21:30 -0700 Subject: [PATCH 06/14] Update push controller creator func Pass the configured ErrorHandler for the controller to the SDK. --- sdk/metric/controller/push/push.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metric/controller/push/push.go b/sdk/metric/controller/push/push.go index fa68f99e53f..641a1691406 100644 --- a/sdk/metric/controller/push/push.go +++ b/sdk/metric/controller/push/push.go @@ -86,7 +86,7 @@ func New(batcher export.Batcher, exporter export.Exporter, period time.Duration, opt.Apply(c) } - impl := sdk.New(batcher, lencoder, sdk.WithResource(c.Resource)) + impl := sdk.New(batcher, lencoder, sdk.WithResource(c.Resource), sdk.WithErrorHandler(c.ErrorHandler)) return &Controller{ sdk: impl, meter: metric.WrapMeterImpl(impl), From cf2c6d03c3eabedc3d39f96e3942b240ae4ac60a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 19 Mar 2020 20:22:28 -0700 Subject: [PATCH 07/14] Update Resource integration with the SDK Add back the Resource field to the Descriptor that was moved in the last merge with master. Add a resource.Provider interface. Have the default SDK implement the new resource.Provider interface and integrate the new interface into the newSync/newAsync workflows. Now, if the SDK has a Resource defined it will be passed to all Descriptors created for the instruments it creates. --- api/metric/api.go | 22 ++++++++++++++++++++++ api/metric/sdkhelpers.go | 18 ++++++++++++++++++ sdk/metric/sdk.go | 11 +++++++++++ sdk/resource/provider.go | 21 +++++++++++++++++++++ 4 files changed, 72 insertions(+) create mode 100644 sdk/resource/provider.go diff --git a/api/metric/api.go b/api/metric/api.go index 94940169411..63dd3f28635 100644 --- a/api/metric/api.go +++ b/api/metric/api.go @@ -21,6 +21,7 @@ import ( "go.opentelemetry.io/otel/api/core" "go.opentelemetry.io/otel/api/unit" + "go.opentelemetry.io/otel/sdk/resource" ) // Provider supports named Meter instances. @@ -45,6 +46,8 @@ type Config struct { // Keys are recommended keys determined in the handles // obtained for the metric. Keys []core.Key + // Resource describes the entity for which measurements are made. + Resource resource.Resource } // Option is an interface for applying metric options. @@ -141,6 +144,12 @@ func (d Descriptor) NumberKind() core.NumberKind { return d.numberKind } +// Resource returns the Resource describing the entity for which the metric +// instrument measures. +func (d Descriptor) Resource() resource.Resource { + return d.config.Resource +} + // Meter is an interface to the metrics portion of the OpenTelemetry SDK. type Meter interface { // Labels returns a reference to a set of labels that cannot @@ -212,3 +221,16 @@ type keysOption []core.Key func (k keysOption) Apply(config *Config) { config.Keys = append(config.Keys, k...) } + +// WithResource applies provided Resource. +// +// This will override any existing Resource. +func WithResource(r resource.Resource) Option { + return resourceOption(r) +} + +type resourceOption resource.Resource + +func (r resourceOption) Apply(config *Config) { + config.Resource = resource.Resource(r) +} diff --git a/api/metric/sdkhelpers.go b/api/metric/sdkhelpers.go index 72167830d1e..029f46863ee 100644 --- a/api/metric/sdkhelpers.go +++ b/api/metric/sdkhelpers.go @@ -18,6 +18,7 @@ import ( "context" "go.opentelemetry.io/otel/api/core" + "go.opentelemetry.io/otel/sdk/resource" ) // MeterImpl is a convenient interface for SDK and test @@ -133,6 +134,21 @@ func Configure(opts []Option) Config { return config } +// insertResource inserts a WithResource option at the beginning of opts +// using the resource defined by impl if impl implements resource.Provider. +// +// If opts contains a WithResource option already, that Option will take +// precidence and overwrite the Resource set from impl. +// +// The returned []Option may uses the same underlying array as opts. +func insertResource(impl MeterImpl, opts []Option) []Option { + if r, ok := impl.(resource.Provider); ok { + // default to the impl resrouce and override if passed in opts. + return append([]Option{WithResource(r.Resource())}, opts...) + } + return opts +} + // WrapMeterImpl constructs a `Meter` implementation from a // `MeterImpl` implementation. func WrapMeterImpl(impl MeterImpl) Meter { @@ -159,6 +175,7 @@ func (m *wrappedMeterImpl) RecordBatch(ctx context.Context, ls LabelSet, ms ...M } func (m *wrappedMeterImpl) newSync(name string, metricKind Kind, numberKind core.NumberKind, opts []Option) (SyncImpl, error) { + opts = insertResource(m.impl, opts) return m.impl.NewSyncInstrument(NewDescriptor(name, metricKind, numberKind, opts...)) } @@ -219,6 +236,7 @@ func WrapFloat64MeasureInstrument(syncInst SyncImpl, err error) (Float64Measure, } func (m *wrappedMeterImpl) newAsync(name string, mkind Kind, nkind core.NumberKind, opts []Option, callback func(func(core.Number, LabelSet))) (AsyncImpl, error) { + opts = insertResource(m.impl, opts) return m.impl.NewAsyncInstrument( NewDescriptor(name, mkind, nkind, opts...), callback) diff --git a/sdk/metric/sdk.go b/sdk/metric/sdk.go index f8fc79b75d4..1e67cc2175a 100644 --- a/sdk/metric/sdk.go +++ b/sdk/metric/sdk.go @@ -159,6 +159,7 @@ var ( _ api.AsyncImpl = &asyncInstrument{} _ api.SyncImpl = &syncInstrument{} _ api.BoundSyncImpl = &record{} + _ resource.Provider = &SDK{} kvType = reflect.TypeOf(core.KeyValue{}) ) @@ -558,6 +559,16 @@ func (m *SDK) checkpoint(ctx context.Context, descriptor *metric.Descriptor, rec return 1 } +// Resource returns the Resource this SDK was created with describing the +// entity for which it creates instruments for. +// +// Resource means that the SDK implements the resource.Provider iterface, +// which means that all metric instruments it creates will inherited its +// Resource by default unles explicitly overwriten. +func (m *SDK) Resource() resource.Resource { + return m.resource +} + // RecordBatch enters a batch of metric events. func (m *SDK) RecordBatch(ctx context.Context, ls api.LabelSet, measurements ...api.Measurement) { for _, meas := range measurements { diff --git a/sdk/resource/provider.go b/sdk/resource/provider.go new file mode 100644 index 00000000000..05d5da6a35b --- /dev/null +++ b/sdk/resource/provider.go @@ -0,0 +1,21 @@ +// Copyright 2020, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package resource + +// Provider is implemented by any value that has a Resource method, +// which returns the Resource associated with the value. +type Provider interface { + Resource() Resource +} From c4f89c60ff7cf172123587d436eb10e409dc9f74 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 19 Mar 2020 21:02:16 -0700 Subject: [PATCH 08/14] Remove nil check for metric SDK config --- sdk/metric/config.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sdk/metric/config.go b/sdk/metric/config.go index 3eb0bf21c93..d91a5ab4563 100644 --- a/sdk/metric/config.go +++ b/sdk/metric/config.go @@ -29,9 +29,6 @@ func WithErrorHandler(fn ErrorHandler) Option { type errorHandlerOption ErrorHandler func (o errorHandlerOption) Apply(config *Config) { - if config == nil { - return - } config.ErrorHandler = ErrorHandler(o) } @@ -43,8 +40,5 @@ func WithResource(r resource.Resource) Option { type resourceOption resource.Resource func (o resourceOption) Apply(config *Config) { - if config == nil { - return - } config.Resource = resource.Resource(o) } From 51ea24469dd96384a6246076bdaa7d7ca099ae15 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 19 Mar 2020 21:02:51 -0700 Subject: [PATCH 09/14] Fix and add test for API Options Add an `Equal` method to the Resource so it can be compared with github.com/google/go-cmp/cmp. Add additional test of the API Option unit tests to ensure WithResource correctly sets a new resource. --- api/metric/api_test.go | 67 ++++++++++++++++++++++++++-------------- sdk/resource/resource.go | 7 +++++ 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/api/metric/api_test.go b/api/metric/api_test.go index 4f0e44451ba..aab513e1d3f 100644 --- a/api/metric/api_test.go +++ b/api/metric/api_test.go @@ -25,6 +25,7 @@ import ( "go.opentelemetry.io/otel/api/metric" "go.opentelemetry.io/otel/api/unit" mockTest "go.opentelemetry.io/otel/internal/metric" + "go.opentelemetry.io/otel/sdk/resource" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" @@ -35,19 +36,21 @@ var Must = metric.Must func TestOptions(t *testing.T) { type testcase struct { - name string - opts []metric.Option - keys []core.Key - desc string - unit unit.Unit + name string + opts []metric.Option + keys []core.Key + desc string + unit unit.Unit + resource resource.Resource } testcases := []testcase{ { - name: "no opts", - opts: nil, - keys: nil, - desc: "", - unit: "", + name: "no opts", + opts: nil, + keys: nil, + desc: "", + unit: "", + resource: resource.Resource{}, }, { name: "keys keys keys", @@ -61,17 +64,19 @@ func TestOptions(t *testing.T) { key.New("bar"), key.New("bar2"), key.New("baz"), key.New("baz2"), }, - desc: "", - unit: "", + desc: "", + unit: "", + resource: resource.Resource{}, }, { name: "description", opts: []metric.Option{ metric.WithDescription("stuff"), }, - keys: nil, - desc: "stuff", - unit: "", + keys: nil, + desc: "stuff", + unit: "", + resource: resource.Resource{}, }, { name: "description override", @@ -79,18 +84,20 @@ func TestOptions(t *testing.T) { metric.WithDescription("stuff"), metric.WithDescription("things"), }, - keys: nil, - desc: "things", - unit: "", + keys: nil, + desc: "things", + unit: "", + resource: resource.Resource{}, }, { name: "unit", opts: []metric.Option{ metric.WithUnit("s"), }, - keys: nil, - desc: "", - unit: "s", + keys: nil, + desc: "", + unit: "s", + resource: resource.Resource{}, }, { name: "unit override", @@ -98,9 +105,20 @@ func TestOptions(t *testing.T) { metric.WithUnit("s"), metric.WithUnit("h"), }, - keys: nil, - desc: "", - unit: "h", + keys: nil, + desc: "", + unit: "h", + resource: resource.Resource{}, + }, + { + name: "resource override", + opts: []metric.Option{ + metric.WithResource(*resource.New(key.New("name").String("test-name"))), + }, + keys: nil, + desc: "", + unit: "", + resource: *resource.New(key.New("name").String("test-name")), }, } for idx, tt := range testcases { @@ -109,6 +127,7 @@ func TestOptions(t *testing.T) { Description: tt.desc, Unit: tt.unit, Keys: tt.keys, + Resource: tt.resource, }); diff != "" { t.Errorf("Compare options: -got +want %s", diff) } diff --git a/sdk/resource/resource.go b/sdk/resource/resource.go index 8e78ed80358..296856ae496 100644 --- a/sdk/resource/resource.go +++ b/sdk/resource/resource.go @@ -17,6 +17,8 @@ package resource import ( + "reflect" + "go.opentelemetry.io/otel/api/core" ) @@ -70,3 +72,8 @@ func (r Resource) Attributes() []core.KeyValue { } return attrs } + +// Equal returns if other Resource is the equal to r. +func (r Resource) Equal(other Resource) bool { + return reflect.DeepEqual(r.labels, other.labels) +} From 1d14009ddd265a23b2a3e822b616b60aba0cb8f3 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 19 Mar 2020 21:20:39 -0700 Subject: [PATCH 10/14] Move the resource.Provider interface to the API package Move the interface to where it is used. Fix spelling. --- api/metric/sdkhelpers.go | 18 ++++++++++++++---- sdk/metric/sdk.go | 6 +++--- sdk/resource/provider.go | 21 --------------------- 3 files changed, 17 insertions(+), 28 deletions(-) delete mode 100644 sdk/resource/provider.go diff --git a/api/metric/sdkhelpers.go b/api/metric/sdkhelpers.go index 029f46863ee..9c3786af36b 100644 --- a/api/metric/sdkhelpers.go +++ b/api/metric/sdkhelpers.go @@ -16,6 +16,7 @@ package metric import ( "context" + "fmt" "go.opentelemetry.io/otel/api/core" "go.opentelemetry.io/otel/sdk/resource" @@ -134,16 +135,25 @@ func Configure(opts []Option) Config { return config } +// Resourcer is implemented by any value that has a Resource method, +// which returns the Resource associated with the value. +// The Resource method is used to set the Resource for Descriptors of new +// metric instruments. +type Resourcer interface { + Resource() resource.Resource +} + // insertResource inserts a WithResource option at the beginning of opts -// using the resource defined by impl if impl implements resource.Provider. +// using the resource defined by impl if impl implements Resourcer. // // If opts contains a WithResource option already, that Option will take -// precidence and overwrite the Resource set from impl. +// precedence and overwrite the Resource set from impl. // // The returned []Option may uses the same underlying array as opts. func insertResource(impl MeterImpl, opts []Option) []Option { - if r, ok := impl.(resource.Provider); ok { - // default to the impl resrouce and override if passed in opts. + fmt.Stringer + if r, ok := impl.(Resourcer); ok { + // default to the impl resource and override if passed in opts. return append([]Option{WithResource(r.Resource())}, opts...) } return opts diff --git a/sdk/metric/sdk.go b/sdk/metric/sdk.go index 1e67cc2175a..9b9eaca7f64 100644 --- a/sdk/metric/sdk.go +++ b/sdk/metric/sdk.go @@ -562,9 +562,9 @@ func (m *SDK) checkpoint(ctx context.Context, descriptor *metric.Descriptor, rec // Resource returns the Resource this SDK was created with describing the // entity for which it creates instruments for. // -// Resource means that the SDK implements the resource.Provider iterface, -// which means that all metric instruments it creates will inherited its -// Resource by default unles explicitly overwriten. +// Resource means that the SDK implements the Resourcer interface and +// therefore all metric instruments it creates will inherited its +// Resource by default unless explicitly overwritten. func (m *SDK) Resource() resource.Resource { return m.resource } diff --git a/sdk/resource/provider.go b/sdk/resource/provider.go deleted file mode 100644 index 05d5da6a35b..00000000000 --- a/sdk/resource/provider.go +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2020, OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package resource - -// Provider is implemented by any value that has a Resource method, -// which returns the Resource associated with the value. -type Provider interface { - Resource() Resource -} From 48e8f00ed5293641b4c40635b39c5220ed903ced Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 19 Mar 2020 21:25:15 -0700 Subject: [PATCH 11/14] Remove errant line --- api/metric/sdkhelpers.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/metric/sdkhelpers.go b/api/metric/sdkhelpers.go index 9c3786af36b..201860f2fc3 100644 --- a/api/metric/sdkhelpers.go +++ b/api/metric/sdkhelpers.go @@ -16,7 +16,6 @@ package metric import ( "context" - "fmt" "go.opentelemetry.io/otel/api/core" "go.opentelemetry.io/otel/sdk/resource" @@ -151,7 +150,6 @@ type Resourcer interface { // // The returned []Option may uses the same underlying array as opts. func insertResource(impl MeterImpl, opts []Option) []Option { - fmt.Stringer if r, ok := impl.(Resourcer); ok { // default to the impl resource and override if passed in opts. return append([]Option{WithResource(r.Resource())}, opts...) From ad619860e4b4c8242c799cbb1c0bb67bf7d4a5af Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 19 Mar 2020 21:25:34 -0700 Subject: [PATCH 12/14] Remove nil checks for the push controller config --- sdk/metric/controller/push/config.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sdk/metric/controller/push/config.go b/sdk/metric/controller/push/config.go index a0639ce851b..e3a254fca9c 100644 --- a/sdk/metric/controller/push/config.go +++ b/sdk/metric/controller/push/config.go @@ -32,9 +32,6 @@ func WithErrorHandler(fn sdk.ErrorHandler) Option { type errorHandlerOption sdk.ErrorHandler func (o errorHandlerOption) Apply(config *Config) { - if config == nil { - return - } config.ErrorHandler = sdk.ErrorHandler(o) } @@ -46,8 +43,5 @@ func WithResource(r resource.Resource) Option { type resourceOption resource.Resource func (o resourceOption) Apply(config *Config) { - if config == nil { - return - } config.Resource = resource.Resource(o) } From dfec047a4059ff8c93863ff2a712a5cfa89acdba Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 19 Mar 2020 21:30:56 -0700 Subject: [PATCH 13/14] Fix check SDK implements Resourcer --- sdk/metric/sdk.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metric/sdk.go b/sdk/metric/sdk.go index 9b9eaca7f64..1b4185e09d6 100644 --- a/sdk/metric/sdk.go +++ b/sdk/metric/sdk.go @@ -159,7 +159,7 @@ var ( _ api.AsyncImpl = &asyncInstrument{} _ api.SyncImpl = &syncInstrument{} _ api.BoundSyncImpl = &record{} - _ resource.Provider = &SDK{} + _ api.Resourcer = &SDK{} kvType = reflect.TypeOf(core.KeyValue{}) ) From cedc235249bd3fbe541a4263da4d8cac10ac1ae2 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 20 Mar 2020 08:02:18 -0700 Subject: [PATCH 14/14] Apply suggestions from code review Co-Authored-By: Rahul Patel --- sdk/metric/sdk.go | 2 +- sdk/resource/resource.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/metric/sdk.go b/sdk/metric/sdk.go index 5dd3e306db2..bfab7247223 100644 --- a/sdk/metric/sdk.go +++ b/sdk/metric/sdk.go @@ -574,7 +574,7 @@ func (m *SDK) checkpoint(ctx context.Context, descriptor *metric.Descriptor, rec // entity for which it creates instruments for. // // Resource means that the SDK implements the Resourcer interface and -// therefore all metric instruments it creates will inherited its +// therefore all metric instruments it creates will inherit its // Resource by default unless explicitly overwritten. func (m *SDK) Resource() resource.Resource { return m.resource diff --git a/sdk/resource/resource.go b/sdk/resource/resource.go index 296856ae496..cc45075c4ba 100644 --- a/sdk/resource/resource.go +++ b/sdk/resource/resource.go @@ -73,7 +73,7 @@ func (r Resource) Attributes() []core.KeyValue { return attrs } -// Equal returns if other Resource is the equal to r. +// Equal returns true if other Resource is the equal to r. func (r Resource) Equal(other Resource) bool { return reflect.DeepEqual(r.labels, other.labels) }