diff --git a/CHANGELOG.md b/CHANGELOG.md index c93742eb56a..11549cd77a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Accept 201 to 299 HTTP status as success in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#4365) - Document the `Temporality` and `Aggregation` methods of the `"go.opentelemetry.io/otel/sdk/metric".Exporter"` need to be concurrent safe. (#4381) - Expand the set of units supported by the prometheus exporter, and don't add unit suffixes if they are already present in `go.opentelemetry.op/otel/exporters/prometheus` (#4374) +- Move the `Aggregation` interface and its implementations from `go.opentelemetry.io/otel/sdk/metric/aggregation` to `go.opentelemetry.io/otel/sdk/metric`. (#4435) - The exporters in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` support the `OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION` environment variable. (#4437) ### Changed @@ -89,6 +90,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig` package is deprecated. (#4425) - The `go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlptracetest` package is deprecated. (#4425) - The `go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/retry` package is deprecated. (#4425) +- The `go.opentelemetry.io/otel/sdk/metric/aggregation` package is deprecated. + Use the aggregation types added to `go.opentelemetry.io/otel/sdk/metric` instead. (#4435) ## [1.16.0/0.39.0] 2023-05-18 diff --git a/example/view/main.go b/example/view/main.go index 0e0175aa5ee..712e325301e 100644 --- a/example/view/main.go +++ b/example/view/main.go @@ -29,7 +29,6 @@ import ( api "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" ) const meterName = "github.com/open-telemetry/opentelemetry-go/example/view" @@ -53,7 +52,7 @@ func main() { }, metric.Stream{ Name: "bar", - Aggregation: aggregation.ExplicitBucketHistogram{ + Aggregation: metric.AggregationExplicitBucketHistogram{ Boundaries: []float64{64, 128, 256, 512, 1024, 2048, 4096}, }, }, diff --git a/exporters/otlp/otlpmetric/internal/client.go b/exporters/otlp/otlpmetric/internal/client.go index 7950a4170cd..6c6bf67c1c7 100644 --- a/exporters/otlp/otlpmetric/internal/client.go +++ b/exporters/otlp/otlpmetric/internal/client.go @@ -18,7 +18,6 @@ import ( "context" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" mpb "go.opentelemetry.io/proto/otlp/metrics/v1" ) @@ -29,7 +28,7 @@ type Client interface { Temporality(metric.InstrumentKind) metricdata.Temporality // Aggregation returns the Aggregation to use for an instrument kind. - Aggregation(metric.InstrumentKind) aggregation.Aggregation + Aggregation(metric.InstrumentKind) metric.Aggregation // UploadMetrics transmits metric data to an OTLP receiver. // diff --git a/exporters/otlp/otlpmetric/internal/exporter.go b/exporters/otlp/otlpmetric/internal/exporter.go index 73c51320ec2..508fecd6bb1 100644 --- a/exporters/otlp/otlpmetric/internal/exporter.go +++ b/exporters/otlp/otlpmetric/internal/exporter.go @@ -25,7 +25,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/internal/transform" // nolint: staticcheck // Atomic deprecation. "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" mpb "go.opentelemetry.io/proto/otlp/metrics/v1" ) @@ -47,7 +46,7 @@ func (e *Exporter) Temporality(k metric.InstrumentKind) metricdata.Temporality { } // Aggregation returns the Aggregation to use for an instrument kind. -func (e *Exporter) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (e *Exporter) Aggregation(k metric.InstrumentKind) metric.Aggregation { e.clientMu.Lock() defer e.clientMu.Unlock() return e.client.Aggregation(k) @@ -120,7 +119,7 @@ func (c shutdownClient) Temporality(k metric.InstrumentKind) metricdata.Temporal return c.temporalitySelector(k) } -func (c shutdownClient) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (c shutdownClient) Aggregation(k metric.InstrumentKind) metric.Aggregation { return c.aggregationSelector(k) } diff --git a/exporters/otlp/otlpmetric/internal/exporter_test.go b/exporters/otlp/otlpmetric/internal/exporter_test.go index bdbe3513840..6814c9dce3d 100644 --- a/exporters/otlp/otlpmetric/internal/exporter_test.go +++ b/exporters/otlp/otlpmetric/internal/exporter_test.go @@ -22,7 +22,6 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" mpb "go.opentelemetry.io/proto/otlp/metrics/v1" ) @@ -37,7 +36,7 @@ func (c *client) Temporality(k metric.InstrumentKind) metricdata.Temporality { return metric.DefaultTemporalitySelector(k) } -func (c *client) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (c *client) Aggregation(k metric.InstrumentKind) metric.Aggregation { return metric.DefaultAggregationSelector(k) } diff --git a/exporters/otlp/otlpmetric/internal/oconf/options.go b/exporters/otlp/otlpmetric/internal/oconf/options.go index a5e71a86cfb..5d8f1d25b77 100644 --- a/exporters/otlp/otlpmetric/internal/oconf/options.go +++ b/exporters/otlp/otlpmetric/internal/oconf/options.go @@ -33,9 +33,7 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/internal" // nolint: staticcheck // Synchronous deprecation. "go.opentelemetry.io/otel/exporters/otlp/internal/retry" // nolint: staticcheck // Synchronous deprecation. ominternal "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/internal" // nolint: staticcheck // Atomic deprecation. - "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" ) const ( @@ -340,23 +338,8 @@ func WithTemporalitySelector(selector metric.TemporalitySelector) GenericOption } func WithAggregationSelector(selector metric.AggregationSelector) GenericOption { - // Deep copy and validate before using. - wrapped := func(ik metric.InstrumentKind) aggregation.Aggregation { - a := selector(ik) - cpA := a.Copy() - if err := cpA.Err(); err != nil { - cpA = metric.DefaultAggregationSelector(ik) - global.Error( - err, "using default aggregation instead", - "aggregation", a, - "replacement", cpA, - ) - } - return cpA - } - return newGenericOption(func(cfg Config) Config { - cfg.Metrics.AggregationSelector = wrapped + cfg.Metrics.AggregationSelector = selector return cfg }) } diff --git a/exporters/otlp/otlpmetric/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/internal/oconf/options_test.go index 77cd9b09e07..80102da7d01 100644 --- a/exporters/otlp/otlpmetric/internal/oconf/options_test.go +++ b/exporters/otlp/otlpmetric/internal/oconf/options_test.go @@ -24,7 +24,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/internal/envconfig" // nolint: staticcheck // Synchronous deprecation. "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/internal/oconf" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -415,7 +414,7 @@ func TestConfigs(t *testing.T) { // all" was set. var undefinedKind metric.InstrumentKind got := c.Metrics.AggregationSelector - assert.Equal(t, aggregation.Drop{}, got(undefinedKind)) + assert.Equal(t, metric.AggregationDrop{}, got(undefinedKind)) }, }, } @@ -441,8 +440,8 @@ func TestConfigs(t *testing.T) { } } -func dropSelector(metric.InstrumentKind) aggregation.Aggregation { - return aggregation.Drop{} +func dropSelector(metric.InstrumentKind) metric.Aggregation { + return metric.AggregationDrop{} } func deltaSelector(metric.InstrumentKind) metricdata.Temporality { diff --git a/exporters/otlp/otlpmetric/internal/otest/client_test.go b/exporters/otlp/otlpmetric/internal/otest/client_test.go index 2c2ddcc9c01..3f7bb848e2c 100644 --- a/exporters/otlp/otlpmetric/internal/otest/client_test.go +++ b/exporters/otlp/otlpmetric/internal/otest/client_test.go @@ -22,7 +22,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/internal" // nolint: staticcheck // Synchronous deprecation. ominternal "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/internal" // nolint: staticcheck // Atomic deprecation. "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" cpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1" mpb "go.opentelemetry.io/proto/otlp/metrics/v1" @@ -37,7 +36,7 @@ func (c *client) Temporality(k metric.InstrumentKind) metricdata.Temporality { return metric.DefaultTemporalitySelector(k) } -func (c *client) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (c *client) Aggregation(k metric.InstrumentKind) metric.Aggregation { return metric.DefaultAggregationSelector(k) } diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go index 9908b61a379..1de64a77268 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go @@ -30,7 +30,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -137,7 +136,7 @@ type clientShim struct { func (clientShim) Temporality(metric.InstrumentKind) metricdata.Temporality { return metricdata.CumulativeTemporality } -func (clientShim) Aggregation(metric.InstrumentKind) aggregation.Aggregation { +func (clientShim) Aggregation(metric.InstrumentKind) metric.Aggregation { return nil } func (clientShim) ForceFlush(ctx context.Context) error { diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/exporter.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/exporter.go index f5d8b7f9148..826276ba392 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/exporter.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/exporter.go @@ -23,7 +23,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/transform" "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" metricpb "go.opentelemetry.io/proto/otlp/metrics/v1" ) @@ -70,7 +69,7 @@ func (e *Exporter) Temporality(k metric.InstrumentKind) metricdata.Temporality { } // Aggregation returns the Aggregation to use for an instrument kind. -func (e *Exporter) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (e *Exporter) Aggregation(k metric.InstrumentKind) metric.Aggregation { return e.aggregationSelector(k) } diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/envconfig.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/envconfig.go index c08b0b6d483..ae100513bad 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/envconfig.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/envconfig.go @@ -29,7 +29,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/envconfig" "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -204,9 +203,9 @@ func withEnvAggPreference(n string, fn func(metric.AggregationSelector)) func(e case "explicit_bucket_histogram": fn(metric.DefaultAggregationSelector) case "base2_exponential_bucket_histogram": - fn(func(kind metric.InstrumentKind) aggregation.Aggregation { + fn(func(kind metric.InstrumentKind) metric.Aggregation { if kind == metric.InstrumentKindHistogram { - return aggregation.Base2ExponentialHistogram{ + return metric.AggregationBase2ExponentialHistogram{ MaxSize: 160, MaxScale: 20, NoMinMax: false, diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/envconfig_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/envconfig_test.go index 559d9f3bc23..b3f9f9c3714 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/envconfig_test.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/envconfig_test.go @@ -24,7 +24,6 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -111,7 +110,7 @@ func TestWithEnvAggPreference(t *testing.T) { tests := []struct { name string envValue string - want map[metric.InstrumentKind]aggregation.Aggregation + want map[metric.InstrumentKind]metric.Aggregation }{ { name: "default do not set the selector", @@ -124,7 +123,7 @@ func TestWithEnvAggPreference(t *testing.T) { { name: "explicit_bucket_histogram", envValue: "explicit_bucket_histogram", - want: map[metric.InstrumentKind]aggregation.Aggregation{ + want: map[metric.InstrumentKind]metric.Aggregation{ metric.InstrumentKindCounter: metric.DefaultAggregationSelector(metric.InstrumentKindCounter), metric.InstrumentKindHistogram: metric.DefaultAggregationSelector(metric.InstrumentKindHistogram), metric.InstrumentKindUpDownCounter: metric.DefaultAggregationSelector(metric.InstrumentKindUpDownCounter), @@ -136,9 +135,9 @@ func TestWithEnvAggPreference(t *testing.T) { { name: "base2_exponential_bucket_histogram", envValue: "base2_exponential_bucket_histogram", - want: map[metric.InstrumentKind]aggregation.Aggregation{ + want: map[metric.InstrumentKind]metric.Aggregation{ metric.InstrumentKindCounter: metric.DefaultAggregationSelector(metric.InstrumentKindCounter), - metric.InstrumentKindHistogram: aggregation.Base2ExponentialHistogram{ + metric.InstrumentKindHistogram: metric.AggregationBase2ExponentialHistogram{ MaxSize: 160, MaxScale: 20, NoMinMax: false, diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go index 36d03a5b398..40a4469f77a 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options.go @@ -32,9 +32,7 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/retry" - "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" ) const ( @@ -354,23 +352,8 @@ func WithTemporalitySelector(selector metric.TemporalitySelector) GenericOption } func WithAggregationSelector(selector metric.AggregationSelector) GenericOption { - // Deep copy and validate before using. - wrapped := func(ik metric.InstrumentKind) aggregation.Aggregation { - a := selector(ik) - cpA := a.Copy() - if err := cpA.Err(); err != nil { - cpA = metric.DefaultAggregationSelector(ik) - global.Error( - err, "using default aggregation instead", - "aggregation", a, - "replacement", cpA, - ) - } - return cpA - } - return newGenericOption(func(cfg Config) Config { - cfg.Metrics.AggregationSelector = wrapped + cfg.Metrics.AggregationSelector = selector return cfg }) } diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go index 1b5c32e5f94..8687acabcfa 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/oconf/options_test.go @@ -26,7 +26,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/envconfig" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -417,7 +416,7 @@ func TestConfigs(t *testing.T) { // all" was set. var undefinedKind metric.InstrumentKind got := c.Metrics.AggregationSelector - assert.Equal(t, aggregation.Drop{}, got(undefinedKind)) + assert.Equal(t, metric.AggregationDrop{}, got(undefinedKind)) }, }, } @@ -443,8 +442,8 @@ func TestConfigs(t *testing.T) { } } -func dropSelector(metric.InstrumentKind) aggregation.Aggregation { - return aggregation.Drop{} +func dropSelector(metric.InstrumentKind) metric.Aggregation { + return metric.AggregationDrop{} } func deltaSelector(metric.InstrumentKind) metricdata.Temporality { diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/client_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/client_test.go index 9a6f8fe61f0..e325f16b97e 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/client_test.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/client_test.go @@ -24,7 +24,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc/internal" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" cpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1" mpb "go.opentelemetry.io/proto/otlp/metrics/v1" @@ -39,7 +38,7 @@ func (c *client) Temporality(k metric.InstrumentKind) metricdata.Temporality { return metric.DefaultTemporalitySelector(k) } -func (c *client) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (c *client) Aggregation(k metric.InstrumentKind) metric.Aggregation { return metric.DefaultAggregationSelector(k) } diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go index b5af1d61ba1..36075c19ee5 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go @@ -30,7 +30,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -41,7 +40,7 @@ type clientShim struct { func (clientShim) Temporality(metric.InstrumentKind) metricdata.Temporality { return metricdata.CumulativeTemporality } -func (clientShim) Aggregation(metric.InstrumentKind) aggregation.Aggregation { +func (clientShim) Aggregation(metric.InstrumentKind) metric.Aggregation { return nil } func (clientShim) ForceFlush(ctx context.Context) error { diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/exporter.go b/exporters/otlp/otlpmetric/otlpmetrichttp/exporter.go index dd6f80b82ed..96991ede5c7 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/exporter.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/exporter.go @@ -23,7 +23,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/transform" "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" metricpb "go.opentelemetry.io/proto/otlp/metrics/v1" ) @@ -70,7 +69,7 @@ func (e *Exporter) Temporality(k metric.InstrumentKind) metricdata.Temporality { } // Aggregation returns the Aggregation to use for an instrument kind. -func (e *Exporter) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (e *Exporter) Aggregation(k metric.InstrumentKind) metric.Aggregation { return e.aggregationSelector(k) } diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/envconfig.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/envconfig.go index 67f3b32f618..2bab35be6c4 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/envconfig.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/envconfig.go @@ -29,7 +29,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/envconfig" "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -204,9 +203,9 @@ func withEnvAggPreference(n string, fn func(metric.AggregationSelector)) func(e case "explicit_bucket_histogram": fn(metric.DefaultAggregationSelector) case "base2_exponential_bucket_histogram": - fn(func(kind metric.InstrumentKind) aggregation.Aggregation { + fn(func(kind metric.InstrumentKind) metric.Aggregation { if kind == metric.InstrumentKindHistogram { - return aggregation.Base2ExponentialHistogram{ + return metric.AggregationBase2ExponentialHistogram{ MaxSize: 160, MaxScale: 20, NoMinMax: false, diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/envconfig_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/envconfig_test.go index 559d9f3bc23..b3f9f9c3714 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/envconfig_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/envconfig_test.go @@ -24,7 +24,6 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -111,7 +110,7 @@ func TestWithEnvAggPreference(t *testing.T) { tests := []struct { name string envValue string - want map[metric.InstrumentKind]aggregation.Aggregation + want map[metric.InstrumentKind]metric.Aggregation }{ { name: "default do not set the selector", @@ -124,7 +123,7 @@ func TestWithEnvAggPreference(t *testing.T) { { name: "explicit_bucket_histogram", envValue: "explicit_bucket_histogram", - want: map[metric.InstrumentKind]aggregation.Aggregation{ + want: map[metric.InstrumentKind]metric.Aggregation{ metric.InstrumentKindCounter: metric.DefaultAggregationSelector(metric.InstrumentKindCounter), metric.InstrumentKindHistogram: metric.DefaultAggregationSelector(metric.InstrumentKindHistogram), metric.InstrumentKindUpDownCounter: metric.DefaultAggregationSelector(metric.InstrumentKindUpDownCounter), @@ -136,9 +135,9 @@ func TestWithEnvAggPreference(t *testing.T) { { name: "base2_exponential_bucket_histogram", envValue: "base2_exponential_bucket_histogram", - want: map[metric.InstrumentKind]aggregation.Aggregation{ + want: map[metric.InstrumentKind]metric.Aggregation{ metric.InstrumentKindCounter: metric.DefaultAggregationSelector(metric.InstrumentKindCounter), - metric.InstrumentKindHistogram: aggregation.Base2ExponentialHistogram{ + metric.InstrumentKindHistogram: metric.AggregationBase2ExponentialHistogram{ MaxSize: 160, MaxScale: 20, NoMinMax: false, diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go index de0916313d7..c1ec5ed210a 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options.go @@ -32,9 +32,7 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/retry" - "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" ) const ( @@ -354,23 +352,8 @@ func WithTemporalitySelector(selector metric.TemporalitySelector) GenericOption } func WithAggregationSelector(selector metric.AggregationSelector) GenericOption { - // Deep copy and validate before using. - wrapped := func(ik metric.InstrumentKind) aggregation.Aggregation { - a := selector(ik) - cpA := a.Copy() - if err := cpA.Err(); err != nil { - cpA = metric.DefaultAggregationSelector(ik) - global.Error( - err, "using default aggregation instead", - "aggregation", a, - "replacement", cpA, - ) - } - return cpA - } - return newGenericOption(func(cfg Config) Config { - cfg.Metrics.AggregationSelector = wrapped + cfg.Metrics.AggregationSelector = selector return cfg }) } diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go index 5b4ee0358b7..bb34337a1f2 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf/options_test.go @@ -26,7 +26,6 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/envconfig" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -417,7 +416,7 @@ func TestConfigs(t *testing.T) { // all" was set. var undefinedKind metric.InstrumentKind got := c.Metrics.AggregationSelector - assert.Equal(t, aggregation.Drop{}, got(undefinedKind)) + assert.Equal(t, metric.AggregationDrop{}, got(undefinedKind)) }, }, } @@ -443,8 +442,8 @@ func TestConfigs(t *testing.T) { } } -func dropSelector(metric.InstrumentKind) aggregation.Aggregation { - return aggregation.Drop{} +func dropSelector(metric.InstrumentKind) metric.Aggregation { + return metric.AggregationDrop{} } func deltaSelector(metric.InstrumentKind) metricdata.Temporality { diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/client_test.go index e51c6dfd0fb..72f4f40b116 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/client_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/client_test.go @@ -24,7 +24,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" cpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1" mpb "go.opentelemetry.io/proto/otlp/metrics/v1" @@ -39,7 +38,7 @@ func (c *client) Temporality(k metric.InstrumentKind) metricdata.Temporality { return metric.DefaultTemporalitySelector(k) } -func (c *client) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (c *client) Aggregation(k metric.InstrumentKind) metric.Aggregation { return metric.DefaultAggregationSelector(k) } diff --git a/exporters/prometheus/config_test.go b/exporters/prometheus/config_test.go index 8fc88819b94..3e3ba9c1cb0 100644 --- a/exporters/prometheus/config_test.go +++ b/exporters/prometheus/config_test.go @@ -21,13 +21,12 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" ) func TestNewConfig(t *testing.T) { registry := prometheus.NewRegistry() - aggregationSelector := func(metric.InstrumentKind) aggregation.Aggregation { return nil } + aggregationSelector := func(metric.InstrumentKind) metric.Aggregation { return nil } testCases := []struct { name string @@ -142,7 +141,7 @@ func TestNewConfig(t *testing.T) { } func TestConfigManualReaderOptions(t *testing.T) { - aggregationSelector := func(metric.InstrumentKind) aggregation.Aggregation { return nil } + aggregationSelector := func(metric.InstrumentKind) metric.Aggregation { return nil } testCases := []struct { name string diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go index 3c96983a918..86cc8dc0b20 100644 --- a/exporters/prometheus/exporter_test.go +++ b/exporters/prometheus/exporter_test.go @@ -28,7 +28,6 @@ import ( "go.opentelemetry.io/otel/attribute" otelmetric "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/resource" semconv "go.opentelemetry.io/otel/semconv/v1.21.0" ) @@ -397,7 +396,7 @@ func TestPrometheusExporter(t *testing.T) { metric.WithReader(exporter), metric.WithView(metric.NewView( metric.Instrument{Name: "histogram_*"}, - metric.Stream{Aggregation: aggregation.ExplicitBucketHistogram{ + metric.Stream{Aggregation: metric.AggregationExplicitBucketHistogram{ Boundaries: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000}, }}, )), diff --git a/exporters/stdout/stdoutmetric/config.go b/exporters/stdout/stdoutmetric/config.go index 9b62bde5083..6189c019f37 100644 --- a/exporters/stdout/stdoutmetric/config.go +++ b/exporters/stdout/stdoutmetric/config.go @@ -17,9 +17,7 @@ import ( "encoding/json" "os" - "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" ) // config contains options for the exporter. @@ -100,22 +98,7 @@ func (t temporalitySelectorOption) apply(c config) config { // package or the aggregation explicitly passed for a view matching an // instrument. func WithAggregationSelector(selector metric.AggregationSelector) Option { - // Deep copy and validate before using. - wrapped := func(ik metric.InstrumentKind) aggregation.Aggregation { - a := selector(ik) - cpA := a.Copy() - if err := cpA.Err(); err != nil { - cpA = metric.DefaultAggregationSelector(ik) - global.Error( - err, "using default aggregation instead", - "aggregation", a, - "replacement", cpA, - ) - } - return cpA - } - - return aggregationSelectorOption{selector: wrapped} + return aggregationSelectorOption{selector: selector} } type aggregationSelectorOption struct { diff --git a/exporters/stdout/stdoutmetric/exporter.go b/exporters/stdout/stdoutmetric/exporter.go index e3d867e00dc..c223a84da59 100644 --- a/exporters/stdout/stdoutmetric/exporter.go +++ b/exporters/stdout/stdoutmetric/exporter.go @@ -23,7 +23,6 @@ import ( "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -58,7 +57,7 @@ func (e *exporter) Temporality(k metric.InstrumentKind) metricdata.Temporality { return e.temporalitySelector(k) } -func (e *exporter) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (e *exporter) Aggregation(k metric.InstrumentKind) metric.Aggregation { return e.aggregationSelector(k) } diff --git a/exporters/stdout/stdoutmetric/exporter_test.go b/exporters/stdout/stdoutmetric/exporter_test.go index 72feb08c2fb..71679d623a1 100644 --- a/exporters/stdout/stdoutmetric/exporter_test.go +++ b/exporters/stdout/stdoutmetric/exporter_test.go @@ -26,7 +26,6 @@ import ( "go.opentelemetry.io/otel/exporters/stdout/stdoutmetric" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -115,8 +114,8 @@ func TestTemporalitySelector(t *testing.T) { assert.Equal(t, metricdata.DeltaTemporality, exp.Temporality(unknownKind)) } -func dropSelector(metric.InstrumentKind) aggregation.Aggregation { - return aggregation.Drop{} +func dropSelector(metric.InstrumentKind) metric.Aggregation { + return metric.AggregationDrop{} } func TestAggregationSelector(t *testing.T) { @@ -127,5 +126,5 @@ func TestAggregationSelector(t *testing.T) { require.NoError(t, err) var unknownKind metric.InstrumentKind - assert.Equal(t, aggregation.Drop{}, exp.Aggregation(unknownKind)) + assert.Equal(t, metric.AggregationDrop{}, exp.Aggregation(unknownKind)) } diff --git a/internal/shared/otlp/otlpmetric/oconf/envconfig.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/envconfig.go.tmpl index e1dc855b631..a76a4835ddb 100644 --- a/internal/shared/otlp/otlpmetric/oconf/envconfig.go.tmpl +++ b/internal/shared/otlp/otlpmetric/oconf/envconfig.go.tmpl @@ -29,7 +29,6 @@ import ( "{{ .envconfigImportPath }}" "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -204,9 +203,9 @@ func withEnvAggPreference(n string, fn func(metric.AggregationSelector)) func(e case "explicit_bucket_histogram": fn(metric.DefaultAggregationSelector) case "base2_exponential_bucket_histogram": - fn(func(kind metric.InstrumentKind) aggregation.Aggregation { + fn(func(kind metric.InstrumentKind) metric.Aggregation { if kind == metric.InstrumentKindHistogram { - return aggregation.Base2ExponentialHistogram{ + return metric.AggregationBase2ExponentialHistogram{ MaxSize: 160, MaxScale: 20, NoMinMax: false, diff --git a/internal/shared/otlp/otlpmetric/oconf/envconfig_test.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/envconfig_test.go.tmpl index 559d9f3bc23..b3f9f9c3714 100644 --- a/internal/shared/otlp/otlpmetric/oconf/envconfig_test.go.tmpl +++ b/internal/shared/otlp/otlpmetric/oconf/envconfig_test.go.tmpl @@ -24,7 +24,6 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -111,7 +110,7 @@ func TestWithEnvAggPreference(t *testing.T) { tests := []struct { name string envValue string - want map[metric.InstrumentKind]aggregation.Aggregation + want map[metric.InstrumentKind]metric.Aggregation }{ { name: "default do not set the selector", @@ -124,7 +123,7 @@ func TestWithEnvAggPreference(t *testing.T) { { name: "explicit_bucket_histogram", envValue: "explicit_bucket_histogram", - want: map[metric.InstrumentKind]aggregation.Aggregation{ + want: map[metric.InstrumentKind]metric.Aggregation{ metric.InstrumentKindCounter: metric.DefaultAggregationSelector(metric.InstrumentKindCounter), metric.InstrumentKindHistogram: metric.DefaultAggregationSelector(metric.InstrumentKindHistogram), metric.InstrumentKindUpDownCounter: metric.DefaultAggregationSelector(metric.InstrumentKindUpDownCounter), @@ -136,9 +135,9 @@ func TestWithEnvAggPreference(t *testing.T) { { name: "base2_exponential_bucket_histogram", envValue: "base2_exponential_bucket_histogram", - want: map[metric.InstrumentKind]aggregation.Aggregation{ + want: map[metric.InstrumentKind]metric.Aggregation{ metric.InstrumentKindCounter: metric.DefaultAggregationSelector(metric.InstrumentKindCounter), - metric.InstrumentKindHistogram: aggregation.Base2ExponentialHistogram{ + metric.InstrumentKindHistogram: metric.AggregationBase2ExponentialHistogram{ MaxSize: 160, MaxScale: 20, NoMinMax: false, diff --git a/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl index fb373a634bf..9518b23e8bc 100644 --- a/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl +++ b/internal/shared/otlp/otlpmetric/oconf/options.go.tmpl @@ -32,9 +32,7 @@ import ( "go.opentelemetry.io/otel/exporters/otlp/otlpmetric" "{{ .retryImportPath }}" - "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" ) const ( @@ -354,23 +352,8 @@ func WithTemporalitySelector(selector metric.TemporalitySelector) GenericOption } func WithAggregationSelector(selector metric.AggregationSelector) GenericOption { - // Deep copy and validate before using. - wrapped := func(ik metric.InstrumentKind) aggregation.Aggregation { - a := selector(ik) - cpA := a.Copy() - if err := cpA.Err(); err != nil { - cpA = metric.DefaultAggregationSelector(ik) - global.Error( - err, "using default aggregation instead", - "aggregation", a, - "replacement", cpA, - ) - } - return cpA - } - return newGenericOption(func(cfg Config) Config { - cfg.Metrics.AggregationSelector = wrapped + cfg.Metrics.AggregationSelector = selector return cfg }) } diff --git a/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl b/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl index 3b0a4f1f0c8..16ddfdb7b53 100644 --- a/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl +++ b/internal/shared/otlp/otlpmetric/oconf/options_test.go.tmpl @@ -26,7 +26,6 @@ import ( "{{ .envconfigImportPath }}" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -417,7 +416,7 @@ func TestConfigs(t *testing.T) { // all" was set. var undefinedKind metric.InstrumentKind got := c.Metrics.AggregationSelector - assert.Equal(t, aggregation.Drop{}, got(undefinedKind)) + assert.Equal(t, metric.AggregationDrop{}, got(undefinedKind)) }, }, } @@ -443,8 +442,8 @@ func TestConfigs(t *testing.T) { } } -func dropSelector(metric.InstrumentKind) aggregation.Aggregation { - return aggregation.Drop{} +func dropSelector(metric.InstrumentKind) metric.Aggregation { + return metric.AggregationDrop{} } func deltaSelector(metric.InstrumentKind) metricdata.Temporality { diff --git a/internal/shared/otlp/otlpmetric/otest/client_test.go.tmpl b/internal/shared/otlp/otlpmetric/otest/client_test.go.tmpl index f02512e966c..b7cfa019fb5 100644 --- a/internal/shared/otlp/otlpmetric/otest/client_test.go.tmpl +++ b/internal/shared/otlp/otlpmetric/otest/client_test.go.tmpl @@ -24,7 +24,6 @@ import ( "go.opentelemetry.io/otel" "{{ .internalImportPath }}" "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" cpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1" mpb "go.opentelemetry.io/proto/otlp/metrics/v1" @@ -39,7 +38,7 @@ func (c *client) Temporality(k metric.InstrumentKind) metricdata.Temporality { return metric.DefaultTemporalitySelector(k) } -func (c *client) Aggregation(k metric.InstrumentKind) aggregation.Aggregation { +func (c *client) Aggregation(k metric.InstrumentKind) metric.Aggregation { return metric.DefaultAggregationSelector(k) } diff --git a/sdk/metric/aggregation.go b/sdk/metric/aggregation.go new file mode 100644 index 00000000000..08ff6cc3dd8 --- /dev/null +++ b/sdk/metric/aggregation.go @@ -0,0 +1,201 @@ +// Copyright The 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 "go.opentelemetry.io/otel/sdk/metric" + +import ( + "errors" + "fmt" +) + +// errAgg is wrapped by misconfigured aggregations. +var errAgg = errors.New("aggregation") + +// Aggregation is the aggregation used to summarize recorded measurements. +type Aggregation interface { + // copy returns a deep copy of the Aggregation. + copy() Aggregation + + // err returns an error for any misconfigured Aggregation. + err() error +} + +// AggregationDrop is an Aggregation that drops all recorded data. +type AggregationDrop struct{} // AggregationDrop has no parameters. + +var _ Aggregation = AggregationDrop{} + +// copy returns a deep copy of d. +func (d AggregationDrop) copy() Aggregation { return d } + +// err returns an error for any misconfiguration. A drop aggregation has no +// parameters and cannot be misconfigured, therefore this always returns nil. +func (AggregationDrop) err() error { return nil } + +// AggregationDefault is an Aggregation that uses the default instrument kind selection +// mapping to select another Aggregation. A metric reader can be configured to +// make an aggregation selection based on instrument kind that differs from +// the default. This Aggregation ensures the default is used. +// +// See the "go.opentelemetry.io/otel/sdk/metric".DefaultAggregationSelector +// for information about the default instrument kind selection mapping. +type AggregationDefault struct{} // AggregationDefault has no parameters. + +var _ Aggregation = AggregationDefault{} + +// copy returns a deep copy of d. +func (d AggregationDefault) copy() Aggregation { return d } + +// err returns an error for any misconfiguration. A default aggregation has no +// parameters and cannot be misconfigured, therefore this always returns nil. +func (AggregationDefault) err() error { return nil } + +// AggregationSum is an Aggregation that summarizes a set of measurements as their +// arithmetic sum. +type AggregationSum struct{} // AggregationSum has no parameters. + +var _ Aggregation = AggregationSum{} + +// copy returns a deep copy of s. +func (s AggregationSum) copy() Aggregation { return s } + +// err returns an error for any misconfiguration. A sum aggregation has no +// parameters and cannot be misconfigured, therefore this always returns nil. +func (AggregationSum) err() error { return nil } + +// AggregationLastValue is an Aggregation that summarizes a set of measurements as the +// last one made. +type AggregationLastValue struct{} // AggregationLastValue has no parameters. + +var _ Aggregation = AggregationLastValue{} + +// copy returns a deep copy of l. +func (l AggregationLastValue) copy() Aggregation { return l } + +// err returns an error for any misconfiguration. A last-value aggregation has +// no parameters and cannot be misconfigured, therefore this always returns +// nil. +func (AggregationLastValue) err() error { return nil } + +// AggregationExplicitBucketHistogram is an Aggregation that summarizes a set of +// measurements as an histogram with explicitly defined buckets. +type AggregationExplicitBucketHistogram struct { + // Boundaries are the increasing bucket boundary values. Boundary values + // define bucket upper bounds. Buckets are exclusive of their lower + // boundary and inclusive of their upper bound (except at positive + // infinity). A measurement is defined to fall into the greatest-numbered + // bucket with a boundary that is greater than or equal to the + // measurement. As an example, boundaries defined as: + // + // []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000} + // + // Will define these buckets: + // + // (-∞, 0], (0, 5.0], (5.0, 10.0], (10.0, 25.0], (25.0, 50.0], + // (50.0, 75.0], (75.0, 100.0], (100.0, 250.0], (250.0, 500.0], + // (500.0, 1000.0], (1000.0, +∞) + Boundaries []float64 + // NoMinMax indicates whether to not record the min and max of the + // distribution. By default, these extrema are recorded. + // + // Recording these extrema for cumulative data is expected to have little + // value, they will represent the entire life of the instrument instead of + // just the current collection cycle. It is recommended to set this to true + // for that type of data to avoid computing the low-value extrema. + NoMinMax bool +} + +var _ Aggregation = AggregationExplicitBucketHistogram{} + +// errHist is returned by misconfigured ExplicitBucketHistograms. +var errHist = fmt.Errorf("%w: explicit bucket histogram", errAgg) + +// err returns an error for any misconfiguration. +func (h AggregationExplicitBucketHistogram) err() error { + if len(h.Boundaries) <= 1 { + return nil + } + + // Check boundaries are monotonic. + i := h.Boundaries[0] + for _, j := range h.Boundaries[1:] { + if i >= j { + return fmt.Errorf("%w: non-monotonic boundaries: %v", errHist, h.Boundaries) + } + i = j + } + + return nil +} + +// copy returns a deep copy of h. +func (h AggregationExplicitBucketHistogram) copy() Aggregation { + b := make([]float64, len(h.Boundaries)) + copy(b, h.Boundaries) + return AggregationExplicitBucketHistogram{ + Boundaries: b, + NoMinMax: h.NoMinMax, + } +} + +// AggregationBase2ExponentialHistogram is an Aggregation that summarizes a set of +// measurements as an histogram with bucket widths that grow exponentially. +type AggregationBase2ExponentialHistogram struct { + // MaxSize is the maximum number of buckets to use for the histogram. + MaxSize int32 + // MaxScale is the maximum resolution scale to use for the histogram. + // + // MaxScale has a maximum value of 20. Using a value of 20 means the + // maximum number of buckets that can fit within the range of a + // signed 32-bit integer index could be used. + // + // MaxScale has a minimum value of -10. Using a value of -10 means only + // two buckets will be use. + MaxScale int32 + + // NoMinMax indicates whether to not record the min and max of the + // distribution. By default, these extrema are recorded. + // + // Recording these extrema for cumulative data is expected to have little + // value, they will represent the entire life of the instrument instead of + // just the current collection cycle. It is recommended to set this to true + // for that type of data to avoid computing the low-value extrema. + NoMinMax bool +} + +var _ Aggregation = AggregationBase2ExponentialHistogram{} + +// copy returns a deep copy of the Aggregation. +func (e AggregationBase2ExponentialHistogram) copy() Aggregation { + return e +} + +const ( + expoMaxScale = 20 + expoMinScale = -10 +) + +// errExpoHist is returned by misconfigured Base2ExponentialBucketHistograms. +var errExpoHist = fmt.Errorf("%w: exponential histogram", errAgg) + +// err returns an error for any misconfigured Aggregation. +func (e AggregationBase2ExponentialHistogram) err() error { + if e.MaxScale > expoMaxScale { + return fmt.Errorf("%w: max size %d is greater than maximum scale %d", errExpoHist, e.MaxSize, expoMaxScale) + } + if e.MaxSize <= 0 { + return fmt.Errorf("%w: max size %d is less than or equal to zero", errExpoHist, e.MaxSize) + } + return nil +} diff --git a/sdk/metric/aggregation/aggregation.go b/sdk/metric/aggregation/aggregation.go index 850e309ea37..5d5643eb294 100644 --- a/sdk/metric/aggregation/aggregation.go +++ b/sdk/metric/aggregation/aggregation.go @@ -14,6 +14,9 @@ // Package aggregation contains configuration types that define the // aggregation operation used to summarizes recorded measurements. +// +// Deprecated: Use the aggregation types in go.opentelemetry.io/otel/sdk/metric +// instead. package aggregation // import "go.opentelemetry.io/otel/sdk/metric/aggregation" import ( diff --git a/sdk/metric/aggregation_test.go b/sdk/metric/aggregation_test.go new file mode 100644 index 00000000000..640fd7747e0 --- /dev/null +++ b/sdk/metric/aggregation_test.go @@ -0,0 +1,95 @@ +// Copyright The 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 ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAggregationErr(t *testing.T) { + t.Run("DropOperation", func(t *testing.T) { + assert.NoError(t, AggregationDrop{}.err()) + }) + + t.Run("SumOperation", func(t *testing.T) { + assert.NoError(t, AggregationSum{}.err()) + }) + + t.Run("LastValueOperation", func(t *testing.T) { + assert.NoError(t, AggregationLastValue{}.err()) + }) + + t.Run("ExplicitBucketHistogramOperation", func(t *testing.T) { + assert.NoError(t, AggregationExplicitBucketHistogram{}.err()) + + assert.NoError(t, AggregationExplicitBucketHistogram{ + Boundaries: []float64{0}, + NoMinMax: true, + }.err()) + + assert.NoError(t, AggregationExplicitBucketHistogram{ + Boundaries: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000}, + }.err()) + }) + + t.Run("NonmonotonicHistogramBoundaries", func(t *testing.T) { + assert.ErrorIs(t, AggregationExplicitBucketHistogram{ + Boundaries: []float64{2, 1}, + }.err(), errAgg) + + assert.ErrorIs(t, AggregationExplicitBucketHistogram{ + Boundaries: []float64{0, 1, 2, 1, 3, 4}, + }.err(), errAgg) + }) + + t.Run("ExponentialHistogramOperation", func(t *testing.T) { + assert.NoError(t, AggregationBase2ExponentialHistogram{ + MaxSize: 160, + MaxScale: 20, + }.err()) + + assert.NoError(t, AggregationBase2ExponentialHistogram{ + MaxSize: 1, + NoMinMax: true, + }.err()) + + assert.NoError(t, AggregationBase2ExponentialHistogram{ + MaxSize: 1024, + MaxScale: -3, + }.err()) + }) + + t.Run("InvalidExponentialHistogramOperation", func(t *testing.T) { + // MazSize must be greater than 0 + assert.ErrorIs(t, AggregationBase2ExponentialHistogram{}.err(), errAgg) + + // MaxScale Must be <=20 + assert.ErrorIs(t, AggregationBase2ExponentialHistogram{ + MaxSize: 1, + MaxScale: 30, + }.err(), errAgg) + }) +} + +func TestExplicitBucketHistogramDeepCopy(t *testing.T) { + const orig = 0.0 + b := []float64{orig} + h := AggregationExplicitBucketHistogram{Boundaries: b} + cpH := h.copy().(AggregationExplicitBucketHistogram) + b[0] = orig + 1 + assert.Equal(t, orig, cpH.Boundaries[0], "changing the underlying slice data should not affect the copy") +} diff --git a/sdk/metric/benchmark_test.go b/sdk/metric/benchmark_test.go index a243738cfb7..dd75de3cd63 100644 --- a/sdk/metric/benchmark_test.go +++ b/sdk/metric/benchmark_test.go @@ -23,7 +23,6 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -36,7 +35,7 @@ var viewBenchmarks = []struct { "DropView", []View{NewView( Instrument{Name: "*"}, - Stream{Aggregation: aggregation.Drop{}}, + Stream{Aggregation: AggregationDrop{}}, )}, }, { diff --git a/sdk/metric/config_test.go b/sdk/metric/config_test.go index 6e48a4599ca..ae7159f2d2e 100644 --- a/sdk/metric/config_test.go +++ b/sdk/metric/config_test.go @@ -22,7 +22,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" "go.opentelemetry.io/otel/sdk/resource" ) @@ -39,7 +38,7 @@ type reader struct { var _ Reader = (*reader)(nil) -func (r *reader) aggregation(kind InstrumentKind) aggregation.Aggregation { // nolint:revive // import-shadow for method scoped by type. +func (r *reader) aggregation(kind InstrumentKind) Aggregation { // nolint:revive // import-shadow for method scoped by type. return r.aggregationFunc(kind) } diff --git a/sdk/metric/exporter.go b/sdk/metric/exporter.go index 7efb8bf2fbe..695cf466c0e 100644 --- a/sdk/metric/exporter.go +++ b/sdk/metric/exporter.go @@ -18,7 +18,6 @@ import ( "context" "fmt" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -39,7 +38,7 @@ type Exporter interface { // // This method needs to be concurrent safe with itself and all the other // Exporter methods. - Aggregation(InstrumentKind) aggregation.Aggregation + Aggregation(InstrumentKind) Aggregation // Export serializes and transmits metric data to a receiver. // diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index 67300d489f6..c09c89361c6 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -26,7 +26,6 @@ import ( "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/embedded" "go.opentelemetry.io/otel/sdk/instrumentation" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/internal/aggregate" ) @@ -146,7 +145,7 @@ type Stream struct { // Unit is the unit of measurement recorded. Unit string // Aggregation the stream uses for an instrument. - Aggregation aggregation.Aggregation + Aggregation Aggregation // AllowAttributeKeys are an allow-list of attribute keys that will be // preserved for the stream. Any attribute recorded for the stream with a // key not in this slice will be dropped. diff --git a/sdk/metric/internal/aggregate/aggregate.go b/sdk/metric/internal/aggregate/aggregate.go index 6dd531d1cbb..8dec14237b9 100644 --- a/sdk/metric/internal/aggregate/aggregate.go +++ b/sdk/metric/internal/aggregate/aggregate.go @@ -19,7 +19,6 @@ import ( "time" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -102,8 +101,8 @@ func (b Builder[N]) Sum(monotonic bool) (Measure[N], ComputeAggregation) { // ExplicitBucketHistogram returns a histogram aggregate function input and // output. -func (b Builder[N]) ExplicitBucketHistogram(cfg aggregation.ExplicitBucketHistogram, noSum bool) (Measure[N], ComputeAggregation) { - h := newHistogram[N](cfg, noSum) +func (b Builder[N]) ExplicitBucketHistogram(boundaries []float64, noMinMax, noSum bool) (Measure[N], ComputeAggregation) { + h := newHistogram[N](boundaries, noMinMax, noSum) switch b.Temporality { case metricdata.DeltaTemporality: return b.filter(h.measure), h.delta @@ -114,8 +113,8 @@ func (b Builder[N]) ExplicitBucketHistogram(cfg aggregation.ExplicitBucketHistog // ExponentialBucketHistogram returns a histogram aggregate function input and // output. -func (b Builder[N]) ExponentialBucketHistogram(cfg aggregation.Base2ExponentialHistogram, noSum bool) (Measure[N], ComputeAggregation) { - h := newExponentialHistogram[N](cfg, noSum) +func (b Builder[N]) ExponentialBucketHistogram(maxSize, maxScale int32, noMinMax, noSum bool) (Measure[N], ComputeAggregation) { + h := newExponentialHistogram[N](maxSize, maxScale, noMinMax, noSum) switch b.Temporality { case metricdata.DeltaTemporality: return b.filter(h.measure), h.delta diff --git a/sdk/metric/internal/aggregate/exponential_histogram.go b/sdk/metric/internal/aggregate/exponential_histogram.go index c684c87e600..b46c100de4b 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram.go +++ b/sdk/metric/internal/aggregate/exponential_histogram.go @@ -23,7 +23,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -326,12 +325,12 @@ func (b *expoBuckets) downscale(delta int) { // newExponentialHistogram returns an Aggregator that summarizes a set of // measurements as an exponential histogram. Each histogram is scoped by attributes // and the aggregation cycle the measurements were made in. -func newExponentialHistogram[N int64 | float64](cfg aggregation.Base2ExponentialHistogram, noSum bool) *expoHistogram[N] { +func newExponentialHistogram[N int64 | float64](maxSize, maxScale int32, noMinMax, noSum bool) *expoHistogram[N] { return &expoHistogram[N]{ expoHistogramValues: newExpoHistValues[N]( - int(cfg.MaxSize), - int(cfg.MaxScale), - cfg.NoMinMax, + int(maxSize), + int(maxScale), + noMinMax, noSum, ), start: now(), diff --git a/sdk/metric/internal/aggregate/exponential_histogram_test.go b/sdk/metric/internal/aggregate/exponential_histogram_test.go index de949359ffd..f1db4139cdb 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram_test.go +++ b/sdk/metric/internal/aggregate/exponential_histogram_test.go @@ -25,7 +25,6 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/internal/global" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" ) @@ -645,31 +644,33 @@ func BenchmarkAppend(b *testing.B) { } } -var expoHistConf = aggregation.Base2ExponentialHistogram{ - MaxSize: 160, - MaxScale: 20, -} - func BenchmarkExponentialHistogram(b *testing.B) { + const ( + maxSize = 160 + maxScale = 20 + noMinMax = false + noSum = false + ) + b.Run("Int64/Cumulative", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ Temporality: metricdata.CumulativeTemporality, - }.ExponentialBucketHistogram(expoHistConf, false) + }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) })) b.Run("Int64/Delta", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ Temporality: metricdata.DeltaTemporality, - }.ExponentialBucketHistogram(expoHistConf, false) + }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) })) b.Run("Float64/Cumulative", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ Temporality: metricdata.CumulativeTemporality, - }.ExponentialBucketHistogram(expoHistConf, false) + }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) })) b.Run("Float64/Delta", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ Temporality: metricdata.DeltaTemporality, - }.ExponentialBucketHistogram(expoHistConf, false) + }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) })) } @@ -711,10 +712,12 @@ type exponentialHistogramAggregationTestCase[N int64 | float64] struct { } func testExponentialHistogramAggregation[N int64 | float64](t *testing.T) { - cfg := aggregation.Base2ExponentialHistogram{ - MaxSize: 4, - MaxScale: 20, - } + const ( + maxSize = 4 + maxScale = 20 + noMinMax = false + noSum = false + ) tests := []exponentialHistogramAggregationTestCase[N]{ { @@ -722,7 +725,7 @@ func testExponentialHistogramAggregation[N int64 | float64](t *testing.T) { build: func() (Measure[N], ComputeAggregation) { return Builder[N]{ Temporality: metricdata.DeltaTemporality, - }.ExponentialBucketHistogram(cfg, false) + }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) }, input: [][]N{ {4, 4, 4, 2, 16, 1}, @@ -750,7 +753,7 @@ func testExponentialHistogramAggregation[N int64 | float64](t *testing.T) { build: func() (Measure[N], ComputeAggregation) { return Builder[N]{ Temporality: metricdata.CumulativeTemporality, - }.ExponentialBucketHistogram(cfg, false) + }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) }, input: [][]N{ {4, 4, 4, 2, 16, 1}, @@ -778,7 +781,7 @@ func testExponentialHistogramAggregation[N int64 | float64](t *testing.T) { build: func() (Measure[N], ComputeAggregation) { return Builder[N]{ Temporality: metricdata.DeltaTemporality, - }.ExponentialBucketHistogram(cfg, false) + }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) }, input: [][]N{ {2, 3, 8}, @@ -807,7 +810,7 @@ func testExponentialHistogramAggregation[N int64 | float64](t *testing.T) { build: func() (Measure[N], ComputeAggregation) { return Builder[N]{ Temporality: metricdata.CumulativeTemporality, - }.ExponentialBucketHistogram(cfg, false) + }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) }, input: [][]N{ {2, 3, 8}, diff --git a/sdk/metric/internal/aggregate/histogram.go b/sdk/metric/internal/aggregate/histogram.go index cd030076fdc..62ec51e1f5e 100644 --- a/sdk/metric/internal/aggregate/histogram.go +++ b/sdk/metric/internal/aggregate/histogram.go @@ -21,7 +21,6 @@ import ( "time" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -109,10 +108,10 @@ func (s *histValues[N]) measure(_ context.Context, value N, attr attribute.Set) // newHistogram returns an Aggregator that summarizes a set of measurements as // an histogram. -func newHistogram[N int64 | float64](cfg aggregation.ExplicitBucketHistogram, noSum bool) *histogram[N] { +func newHistogram[N int64 | float64](boundaries []float64, noMinMax, noSum bool) *histogram[N] { return &histogram[N]{ - histValues: newHistValues[N](cfg.Boundaries, noSum), - noMinMax: cfg.NoMinMax, + histValues: newHistValues[N](boundaries, noSum), + noMinMax: noMinMax, start: now(), } } diff --git a/sdk/metric/internal/aggregate/histogram_test.go b/sdk/metric/internal/aggregate/histogram_test.go index 68a00f2a90f..ab44607e5f6 100644 --- a/sdk/metric/internal/aggregate/histogram_test.go +++ b/sdk/metric/internal/aggregate/histogram_test.go @@ -23,17 +23,13 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" ) var ( bounds = []float64{1, 5} - histConf = aggregation.ExplicitBucketHistogram{ - Boundaries: bounds, - NoMinMax: false, - } + noMinMax = false ) func TestHistogram(t *testing.T) { @@ -59,7 +55,7 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) { in, out := Builder[N]{ Temporality: metricdata.DeltaTemporality, Filter: attrFltr, - }.ExplicitBucketHistogram(histConf, c.noSum) + }.ExplicitBucketHistogram(bounds, noMinMax, c.noSum) ctx := context.Background() return test[N](in, out, []teststep[N]{ { @@ -125,7 +121,7 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) { in, out := Builder[N]{ Temporality: metricdata.CumulativeTemporality, Filter: attrFltr, - }.ExplicitBucketHistogram(histConf, c.noSum) + }.ExplicitBucketHistogram(bounds, noMinMax, c.noSum) ctx := context.Background() return test[N](in, out, []teststep[N]{ { @@ -277,7 +273,7 @@ func TestHistogramImmutableBounds(t *testing.T) { cpB := make([]float64, len(b)) copy(cpB, b) - h := newHistogram[int64](aggregation.ExplicitBucketHistogram{Boundaries: b}, false) + h := newHistogram[int64](b, false, false) require.Equal(t, cpB, h.bounds) b[0] = 10 @@ -293,7 +289,7 @@ func TestHistogramImmutableBounds(t *testing.T) { } func TestCumulativeHistogramImutableCounts(t *testing.T) { - h := newHistogram[int64](histConf, false) + h := newHistogram[int64](bounds, noMinMax, false) h.measure(context.Background(), 5, alice) var data metricdata.Aggregation = metricdata.Histogram[int64]{} @@ -311,7 +307,7 @@ func TestCumulativeHistogramImutableCounts(t *testing.T) { func TestDeltaHistogramReset(t *testing.T) { t.Cleanup(mockTime(now)) - h := newHistogram[int64](histConf, false) + h := newHistogram[int64](bounds, noMinMax, false) var data metricdata.Aggregation = metricdata.Histogram[int64]{} require.Equal(t, 0, h.delta(&data)) @@ -340,21 +336,21 @@ func BenchmarkHistogram(b *testing.B) { b.Run("Int64/Cumulative", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ Temporality: metricdata.CumulativeTemporality, - }.ExplicitBucketHistogram(histConf, false) + }.ExplicitBucketHistogram(bounds, noMinMax, false) })) b.Run("Int64/Delta", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ Temporality: metricdata.DeltaTemporality, - }.ExplicitBucketHistogram(histConf, false) + }.ExplicitBucketHistogram(bounds, noMinMax, false) })) b.Run("Float64/Cumulative", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ Temporality: metricdata.CumulativeTemporality, - }.ExplicitBucketHistogram(histConf, false) + }.ExplicitBucketHistogram(bounds, noMinMax, false) })) b.Run("Float64/Delta", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ Temporality: metricdata.DeltaTemporality, - }.ExplicitBucketHistogram(histConf, false) + }.ExplicitBucketHistogram(bounds, noMinMax, false) })) } diff --git a/sdk/metric/manual_reader.go b/sdk/metric/manual_reader.go index 3ab837c00d0..7d524de9ea1 100644 --- a/sdk/metric/manual_reader.go +++ b/sdk/metric/manual_reader.go @@ -22,7 +22,6 @@ import ( "sync/atomic" "go.opentelemetry.io/otel/internal/global" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -70,7 +69,7 @@ func (mr *ManualReader) temporality(kind InstrumentKind) metricdata.Temporality } // aggregation returns what Aggregation to use for kind. -func (mr *ManualReader) aggregation(kind InstrumentKind) aggregation.Aggregation { // nolint:revive // import-shadow for method scoped by type. +func (mr *ManualReader) aggregation(kind InstrumentKind) Aggregation { // nolint:revive // import-shadow for method scoped by type. return mr.aggregationSelector(kind) } @@ -201,25 +200,7 @@ func (t temporalitySelectorOption) applyManual(mrc manualReaderConfig) manualRea // this option is not used, the reader will use the DefaultAggregationSelector // or the aggregation explicitly passed for a view matching an instrument. func WithAggregationSelector(selector AggregationSelector) ManualReaderOption { - // Deep copy and validate before using. - wrapped := func(ik InstrumentKind) aggregation.Aggregation { - a := selector(ik) - if a == nil { - return nil - } - cpA := a.Copy() - if err := cpA.Err(); err != nil { - cpA = DefaultAggregationSelector(ik) - global.Error( - err, "using default aggregation instead", - "aggregation", a, - "replacement", cpA, - ) - } - return cpA - } - - return aggregationSelectorOption{selector: wrapped} + return aggregationSelectorOption{selector: selector} } type aggregationSelectorOption struct { diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index 338d0d7a0e2..edb1a400b2d 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -31,7 +31,6 @@ import ( "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/sdk/instrumentation" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" "go.opentelemetry.io/otel/sdk/resource" @@ -1137,8 +1136,8 @@ func TestUnregisterUnregisters(t *testing.T) { } func TestRegisterCallbackDropAggregations(t *testing.T) { - aggFn := func(InstrumentKind) aggregation.Aggregation { - return aggregation.Drop{} + aggFn := func(InstrumentKind) Aggregation { + return AggregationDrop{} } r := NewManualReader(WithAggregationSelector(aggFn)) mp := NewMeterProvider(WithReader(r)) @@ -1818,11 +1817,11 @@ func BenchmarkInstrumentCreation(b *testing.B) { } } -func testNilAggregationSelector(InstrumentKind) aggregation.Aggregation { +func testNilAggregationSelector(InstrumentKind) Aggregation { return nil } -func testDefaultAggregationSelector(InstrumentKind) aggregation.Aggregation { - return aggregation.Default{} +func testDefaultAggregationSelector(InstrumentKind) Aggregation { + return AggregationDefault{} } func testUndefinedTemporalitySelector(InstrumentKind) metricdata.Temporality { return metricdata.Temporality(0) diff --git a/sdk/metric/periodic_reader.go b/sdk/metric/periodic_reader.go index 1d18f961197..2a85456102a 100644 --- a/sdk/metric/periodic_reader.go +++ b/sdk/metric/periodic_reader.go @@ -24,7 +24,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/internal/global" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -204,7 +203,7 @@ func (r *PeriodicReader) temporality(kind InstrumentKind) metricdata.Temporality } // aggregation returns what Aggregation to use for kind. -func (r *PeriodicReader) aggregation(kind InstrumentKind) aggregation.Aggregation { // nolint:revive // import-shadow for method scoped by type. +func (r *PeriodicReader) aggregation(kind InstrumentKind) Aggregation { // nolint:revive // import-shadow for method scoped by type. return r.exporter.Aggregation(kind) } diff --git a/sdk/metric/periodic_reader_test.go b/sdk/metric/periodic_reader_test.go index 0a549042313..2f055796dd1 100644 --- a/sdk/metric/periodic_reader_test.go +++ b/sdk/metric/periodic_reader_test.go @@ -25,7 +25,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -168,7 +167,7 @@ func (e *fnExporter) Temporality(k InstrumentKind) metricdata.Temporality { return DefaultTemporalitySelector(k) } -func (e *fnExporter) Aggregation(k InstrumentKind) aggregation.Aggregation { +func (e *fnExporter) Aggregation(k InstrumentKind) Aggregation { if e.aggregationFunc != nil { return e.aggregationFunc(k) } diff --git a/sdk/metric/pipeline.go b/sdk/metric/pipeline.go index 6d1934a76fc..d76231cff7f 100644 --- a/sdk/metric/pipeline.go +++ b/sdk/metric/pipeline.go @@ -27,7 +27,6 @@ import ( "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/embedded" "go.opentelemetry.io/otel/sdk/instrumentation" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/internal" "go.opentelemetry.io/otel/sdk/metric/internal/aggregate" "go.opentelemetry.io/otel/sdk/metric/metricdata" @@ -308,14 +307,28 @@ type aggVal[N int64 | float64] struct { // is returned. func (i *inserter[N]) cachedAggregator(scope instrumentation.Scope, kind InstrumentKind, stream Stream) (meas aggregate.Measure[N], aggID uint64, err error) { switch stream.Aggregation.(type) { - case nil, aggregation.Default: + case nil: // Undefined, nil, means to use the default from the reader. stream.Aggregation = i.pipeline.reader.aggregation(kind) switch stream.Aggregation.(type) { - case nil, aggregation.Default: + case nil, AggregationDefault: // If the reader returns default or nil use the default selector. stream.Aggregation = DefaultAggregationSelector(kind) + default: + // Deep copy and validate before using. + stream.Aggregation = stream.Aggregation.copy() + if err := stream.Aggregation.err(); err != nil { + orig := stream.Aggregation + stream.Aggregation = DefaultAggregationSelector(kind) + global.Error( + err, "using default aggregation instead", + "aggregation", orig, + "replacement", stream.Aggregation, + ) + } } + case AggregationDefault: + stream.Aggregation = DefaultAggregationSelector(kind) } if err := isAggregatorCompatible(kind, stream.Aggregation); err != nil { @@ -423,15 +436,15 @@ func (i *inserter[N]) instID(kind InstrumentKind, stream Stream) instID { // aggregateFunc returns new aggregate functions matching agg, kind, and // monotonic. If the agg is unknown or temporality is invalid, an error is // returned. -func (i *inserter[N]) aggregateFunc(b aggregate.Builder[N], agg aggregation.Aggregation, kind InstrumentKind) (meas aggregate.Measure[N], comp aggregate.ComputeAggregation, err error) { +func (i *inserter[N]) aggregateFunc(b aggregate.Builder[N], agg Aggregation, kind InstrumentKind) (meas aggregate.Measure[N], comp aggregate.ComputeAggregation, err error) { switch a := agg.(type) { - case aggregation.Default: + case AggregationDefault: return i.aggregateFunc(b, DefaultAggregationSelector(kind), kind) - case aggregation.Drop: + case AggregationDrop: // Return nil in and out to signify the drop aggregator. - case aggregation.LastValue: + case AggregationLastValue: meas, comp = b.LastValue() - case aggregation.Sum: + case AggregationSum: switch kind { case InstrumentKindObservableCounter: meas, comp = b.PrecomputedSum(true) @@ -444,7 +457,7 @@ func (i *inserter[N]) aggregateFunc(b aggregate.Builder[N], agg aggregation.Aggr // instrumentKindUndefined or other invalid instrument kinds. meas, comp = b.Sum(false) } - case aggregation.ExplicitBucketHistogram: + case AggregationExplicitBucketHistogram: var noSum bool switch kind { case InstrumentKindUpDownCounter, InstrumentKindObservableUpDownCounter, InstrumentKindObservableGauge: @@ -453,8 +466,8 @@ func (i *inserter[N]) aggregateFunc(b aggregate.Builder[N], agg aggregation.Aggr // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/metrics/sdk.md#histogram-aggregations noSum = true } - meas, comp = b.ExplicitBucketHistogram(a, noSum) - case aggregation.Base2ExponentialHistogram: + meas, comp = b.ExplicitBucketHistogram(a.Boundaries, a.NoMinMax, noSum) + case AggregationBase2ExponentialHistogram: var noSum bool switch kind { case InstrumentKindUpDownCounter, InstrumentKindObservableUpDownCounter, InstrumentKindObservableGauge: @@ -463,7 +476,7 @@ func (i *inserter[N]) aggregateFunc(b aggregate.Builder[N], agg aggregation.Aggr // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/metrics/sdk.md#histogram-aggregations noSum = true } - meas, comp = b.ExponentialBucketHistogram(a, noSum) + meas, comp = b.ExponentialBucketHistogram(a.MaxSize, a.MaxScale, a.NoMinMax, noSum) default: err = errUnknownAggregation @@ -483,11 +496,11 @@ func (i *inserter[N]) aggregateFunc(b aggregate.Builder[N], agg aggregation.Aggr // | Observable Counter | ✓ | | ✓ | ✓ | ✓ | // | Observable UpDownCounter | ✓ | | ✓ | ✓ | ✓ | // | Observable Gauge | ✓ | ✓ | | ✓ | ✓ |. -func isAggregatorCompatible(kind InstrumentKind, agg aggregation.Aggregation) error { +func isAggregatorCompatible(kind InstrumentKind, agg Aggregation) error { switch agg.(type) { - case aggregation.Default: + case AggregationDefault: return nil - case aggregation.ExplicitBucketHistogram, aggregation.Base2ExponentialHistogram: + case AggregationExplicitBucketHistogram, AggregationBase2ExponentialHistogram: switch kind { case InstrumentKindCounter, InstrumentKindUpDownCounter, @@ -499,7 +512,7 @@ func isAggregatorCompatible(kind InstrumentKind, agg aggregation.Aggregation) er default: return errIncompatibleAggregation } - case aggregation.Sum: + case AggregationSum: switch kind { case InstrumentKindObservableCounter, InstrumentKindObservableUpDownCounter, InstrumentKindCounter, InstrumentKindHistogram, InstrumentKindUpDownCounter: return nil @@ -508,14 +521,14 @@ func isAggregatorCompatible(kind InstrumentKind, agg aggregation.Aggregation) er // https://github.com/open-telemetry/opentelemetry-specification/issues/2710 return errIncompatibleAggregation } - case aggregation.LastValue: + case AggregationLastValue: if kind == InstrumentKindObservableGauge { return nil } // TODO: review need for aggregation check after // https://github.com/open-telemetry/opentelemetry-specification/issues/2710 return errIncompatibleAggregation - case aggregation.Drop: + case AggregationDrop: return nil default: // This is used passed checking for default, it should be an error at this point. diff --git a/sdk/metric/pipeline_registry_test.go b/sdk/metric/pipeline_registry_test.go index 52fedd12471..89293e679e4 100644 --- a/sdk/metric/pipeline_registry_test.go +++ b/sdk/metric/pipeline_registry_test.go @@ -26,7 +26,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/internal/aggregate" "go.opentelemetry.io/otel/sdk/metric/metricdata" "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" @@ -35,14 +34,12 @@ import ( var defaultView = NewView(Instrument{Name: "*"}, Stream{}) -type invalidAggregation struct { - aggregation.Aggregation -} +type invalidAggregation struct{} -func (invalidAggregation) Copy() aggregation.Aggregation { +func (invalidAggregation) copy() Aggregation { return invalidAggregation{} } -func (invalidAggregation) Err() error { +func (invalidAggregation) err() error { return nil } @@ -146,7 +143,7 @@ func assertLastValue[N int64 | float64](t *testing.T, meas []aggregate.Measure[N func testCreateAggregators[N int64 | float64](t *testing.T) { changeAggView := NewView( Instrument{Name: "foo"}, - Stream{Aggregation: aggregation.ExplicitBucketHistogram{ + Stream{Aggregation: AggregationExplicitBucketHistogram{ Boundaries: []float64{0, 100}, NoMinMax: true, }}, @@ -176,7 +173,7 @@ func testCreateAggregators[N int64 | float64](t *testing.T) { }{ { name: "Default/Drop", - reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Drop{} })), + reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) Aggregation { return AggregationDrop{} })), inst: instruments[InstrumentKindCounter], validate: func(t *testing.T, meas []aggregate.Measure[N], comps []aggregate.ComputeAggregation, err error) { t.Helper() @@ -304,37 +301,37 @@ func testCreateAggregators[N int64 | float64](t *testing.T) { }, { name: "Reader/Default/Cumulative/Sum/Monotonic", - reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })), + reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) Aggregation { return AggregationDefault{} })), inst: instruments[InstrumentKindCounter], validate: assertSum[N](1, metricdata.CumulativeTemporality, true, [2]N{1, 4}), }, { name: "Reader/Default/Cumulative/Sum/NonMonotonic", - reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })), + reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) Aggregation { return AggregationDefault{} })), inst: instruments[InstrumentKindUpDownCounter], validate: assertSum[N](1, metricdata.CumulativeTemporality, false, [2]N{1, 4}), }, { name: "Reader/Default/Cumulative/ExplicitBucketHistogram", - reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })), + reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) Aggregation { return AggregationDefault{} })), inst: instruments[InstrumentKindHistogram], validate: assertHist[N](metricdata.CumulativeTemporality), }, { name: "Reader/Default/Cumulative/PrecomputedSum/Monotonic", - reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })), + reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) Aggregation { return AggregationDefault{} })), inst: instruments[InstrumentKindObservableCounter], validate: assertSum[N](1, metricdata.CumulativeTemporality, true, [2]N{1, 3}), }, { name: "Reader/Default/Cumulative/PrecomputedSum/NonMonotonic", - reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })), + reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) Aggregation { return AggregationDefault{} })), inst: instruments[InstrumentKindObservableUpDownCounter], validate: assertSum[N](1, metricdata.CumulativeTemporality, false, [2]N{1, 3}), }, { name: "Reader/Default/Gauge", - reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })), + reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) Aggregation { return AggregationDefault{} })), inst: instruments[InstrumentKindObservableGauge], validate: assertLastValue[N], }, @@ -409,7 +406,7 @@ func TestPipelinesAggregatorForEachReader(t *testing.T) { func TestPipelineRegistryCreateAggregators(t *testing.T) { renameView := NewView(Instrument{Name: "foo"}, Stream{Name: "bar"}) testRdr := NewManualReader() - testRdrHistogram := NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.ExplicitBucketHistogram{} })) + testRdrHistogram := NewManualReader(WithAggregationSelector(func(ik InstrumentKind) Aggregation { return AggregationExplicitBucketHistogram{} })) testCases := []struct { name string @@ -498,7 +495,7 @@ func TestPipelineRegistryResource(t *testing.T) { } func TestPipelineRegistryCreateAggregatorsIncompatibleInstrument(t *testing.T) { - testRdrHistogram := NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Sum{} })) + testRdrHistogram := NewManualReader(WithAggregationSelector(func(ik InstrumentKind) Aggregation { return AggregationSum{} })) readers := []Reader{testRdrHistogram} views := []View{defaultView} @@ -599,187 +596,187 @@ func TestIsAggregatorCompatible(t *testing.T) { testCases := []struct { name string kind InstrumentKind - agg aggregation.Aggregation + agg Aggregation want error }{ { name: "SyncCounter and Drop", kind: InstrumentKindCounter, - agg: aggregation.Drop{}, + agg: AggregationDrop{}, }, { name: "SyncCounter and LastValue", kind: InstrumentKindCounter, - agg: aggregation.LastValue{}, + agg: AggregationLastValue{}, want: errIncompatibleAggregation, }, { name: "SyncCounter and Sum", kind: InstrumentKindCounter, - agg: aggregation.Sum{}, + agg: AggregationSum{}, }, { name: "SyncCounter and ExplicitBucketHistogram", kind: InstrumentKindCounter, - agg: aggregation.ExplicitBucketHistogram{}, + agg: AggregationExplicitBucketHistogram{}, }, { name: "SyncCounter and ExponentialHistogram", kind: InstrumentKindCounter, - agg: aggregation.Base2ExponentialHistogram{}, + agg: AggregationBase2ExponentialHistogram{}, }, { name: "SyncUpDownCounter and Drop", kind: InstrumentKindUpDownCounter, - agg: aggregation.Drop{}, + agg: AggregationDrop{}, }, { name: "SyncUpDownCounter and LastValue", kind: InstrumentKindUpDownCounter, - agg: aggregation.LastValue{}, + agg: AggregationLastValue{}, want: errIncompatibleAggregation, }, { name: "SyncUpDownCounter and Sum", kind: InstrumentKindUpDownCounter, - agg: aggregation.Sum{}, + agg: AggregationSum{}, }, { name: "SyncUpDownCounter and ExplicitBucketHistogram", kind: InstrumentKindUpDownCounter, - agg: aggregation.ExplicitBucketHistogram{}, + agg: AggregationExplicitBucketHistogram{}, }, { name: "SyncUpDownCounter and ExponentialHistogram", kind: InstrumentKindUpDownCounter, - agg: aggregation.Base2ExponentialHistogram{}, + agg: AggregationBase2ExponentialHistogram{}, }, { name: "SyncHistogram and Drop", kind: InstrumentKindHistogram, - agg: aggregation.Drop{}, + agg: AggregationDrop{}, }, { name: "SyncHistogram and LastValue", kind: InstrumentKindHistogram, - agg: aggregation.LastValue{}, + agg: AggregationLastValue{}, want: errIncompatibleAggregation, }, { name: "SyncHistogram and Sum", kind: InstrumentKindHistogram, - agg: aggregation.Sum{}, + agg: AggregationSum{}, }, { name: "SyncHistogram and ExplicitBucketHistogram", kind: InstrumentKindHistogram, - agg: aggregation.ExplicitBucketHistogram{}, + agg: AggregationExplicitBucketHistogram{}, }, { name: "SyncHistogram and ExponentialHistogram", kind: InstrumentKindHistogram, - agg: aggregation.Base2ExponentialHistogram{}, + agg: AggregationBase2ExponentialHistogram{}, }, { name: "ObservableCounter and Drop", kind: InstrumentKindObservableCounter, - agg: aggregation.Drop{}, + agg: AggregationDrop{}, }, { name: "ObservableCounter and LastValue", kind: InstrumentKindObservableCounter, - agg: aggregation.LastValue{}, + agg: AggregationLastValue{}, want: errIncompatibleAggregation, }, { name: "ObservableCounter and Sum", kind: InstrumentKindObservableCounter, - agg: aggregation.Sum{}, + agg: AggregationSum{}, }, { name: "ObservableCounter and ExplicitBucketHistogram", kind: InstrumentKindObservableCounter, - agg: aggregation.ExplicitBucketHistogram{}, + agg: AggregationExplicitBucketHistogram{}, }, { name: "ObservableCounter and ExponentialHistogram", kind: InstrumentKindObservableCounter, - agg: aggregation.Base2ExponentialHistogram{}, + agg: AggregationBase2ExponentialHistogram{}, }, { name: "ObservableUpDownCounter and Drop", kind: InstrumentKindObservableUpDownCounter, - agg: aggregation.Drop{}, + agg: AggregationDrop{}, }, { name: "ObservableUpDownCounter and LastValue", kind: InstrumentKindObservableUpDownCounter, - agg: aggregation.LastValue{}, + agg: AggregationLastValue{}, want: errIncompatibleAggregation, }, { name: "ObservableUpDownCounter and Sum", kind: InstrumentKindObservableUpDownCounter, - agg: aggregation.Sum{}, + agg: AggregationSum{}, }, { name: "ObservableUpDownCounter and ExplicitBucketHistogram", kind: InstrumentKindObservableUpDownCounter, - agg: aggregation.ExplicitBucketHistogram{}, + agg: AggregationExplicitBucketHistogram{}, }, { name: "ObservableUpDownCounter and ExponentialHistogram", kind: InstrumentKindObservableUpDownCounter, - agg: aggregation.Base2ExponentialHistogram{}, + agg: AggregationBase2ExponentialHistogram{}, }, { name: "ObservableGauge and Drop", kind: InstrumentKindObservableGauge, - agg: aggregation.Drop{}, + agg: AggregationDrop{}, }, { - name: "ObservableGauge and aggregation.LastValue{}", + name: "ObservableGauge and LastValue{}", kind: InstrumentKindObservableGauge, - agg: aggregation.LastValue{}, + agg: AggregationLastValue{}, }, { name: "ObservableGauge and Sum", kind: InstrumentKindObservableGauge, - agg: aggregation.Sum{}, + agg: AggregationSum{}, want: errIncompatibleAggregation, }, { name: "ObservableGauge and ExplicitBucketHistogram", kind: InstrumentKindObservableGauge, - agg: aggregation.ExplicitBucketHistogram{}, + agg: AggregationExplicitBucketHistogram{}, }, { name: "ObservableGauge and ExponentialHistogram", kind: InstrumentKindObservableGauge, - agg: aggregation.Base2ExponentialHistogram{}, + agg: AggregationBase2ExponentialHistogram{}, }, { name: "unknown kind with Sum should error", kind: undefinedInstrument, - agg: aggregation.Sum{}, + agg: AggregationSum{}, want: errIncompatibleAggregation, }, { name: "unknown kind with LastValue should error", kind: undefinedInstrument, - agg: aggregation.LastValue{}, + agg: AggregationLastValue{}, want: errIncompatibleAggregation, }, { name: "unknown kind with Histogram should error", kind: undefinedInstrument, - agg: aggregation.ExplicitBucketHistogram{}, + agg: AggregationExplicitBucketHistogram{}, want: errIncompatibleAggregation, }, { name: "unknown kind with Histogram should error", kind: undefinedInstrument, - agg: aggregation.Base2ExponentialHistogram{}, + agg: AggregationBase2ExponentialHistogram{}, want: errIncompatibleAggregation, }, } diff --git a/sdk/metric/pipeline_test.go b/sdk/metric/pipeline_test.go index d30d0015c06..1026fd268ff 100644 --- a/sdk/metric/pipeline_test.go +++ b/sdk/metric/pipeline_test.go @@ -32,7 +32,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/sdk/instrumentation" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" "go.opentelemetry.io/otel/sdk/resource" @@ -366,7 +365,7 @@ func TestInserterCachedAggregatorNameConflict(t *testing.T) { kind := InstrumentKindCounter stream := Stream{ Name: name, - Aggregation: aggregation.Sum{}, + Aggregation: AggregationSum{}, } var vc cache[string, instID] diff --git a/sdk/metric/reader.go b/sdk/metric/reader.go index a4d05944d61..639dfc9653a 100644 --- a/sdk/metric/reader.go +++ b/sdk/metric/reader.go @@ -18,7 +18,6 @@ import ( "context" "fmt" - "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -67,7 +66,7 @@ type Reader interface { // // This method needs to be concurrent safe with itself and all the other // Reader methods. - aggregation(InstrumentKind) aggregation.Aggregation // nolint:revive // import-shadow for method scoped by type. + aggregation(InstrumentKind) Aggregation // nolint:revive // import-shadow for method scoped by type. // Collect gathers and returns all metric data related to the Reader from // the SDK and stores it in out. An error is returned if this is called @@ -137,7 +136,7 @@ func DefaultTemporalitySelector(InstrumentKind) metricdata.Temporality { // // If the Aggregation returned is nil or DefaultAggregation, the selection from // DefaultAggregationSelector will be used. -type AggregationSelector func(InstrumentKind) aggregation.Aggregation +type AggregationSelector func(InstrumentKind) Aggregation // DefaultAggregationSelector returns the default aggregation and parameters // that will be used to summarize measurement made from an instrument of @@ -145,14 +144,14 @@ type AggregationSelector func(InstrumentKind) aggregation.Aggregation // mapping: Counter ⇨ Sum, Observable Counter ⇨ Sum, UpDownCounter ⇨ Sum, // Observable UpDownCounter ⇨ Sum, Observable Gauge ⇨ LastValue, // Histogram ⇨ ExplicitBucketHistogram. -func DefaultAggregationSelector(ik InstrumentKind) aggregation.Aggregation { +func DefaultAggregationSelector(ik InstrumentKind) Aggregation { switch ik { case InstrumentKindCounter, InstrumentKindUpDownCounter, InstrumentKindObservableCounter, InstrumentKindObservableUpDownCounter: - return aggregation.Sum{} + return AggregationSum{} case InstrumentKindObservableGauge: - return aggregation.LastValue{} + return AggregationLastValue{} case InstrumentKindHistogram: - return aggregation.ExplicitBucketHistogram{ + return AggregationExplicitBucketHistogram{ Boundaries: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, NoMinMax: false, } diff --git a/sdk/metric/reader_test.go b/sdk/metric/reader_test.go index c2ab66ea08a..a1e9e507b9d 100644 --- a/sdk/metric/reader_test.go +++ b/sdk/metric/reader_test.go @@ -316,7 +316,7 @@ func TestDefaultAggregationSelector(t *testing.T) { } for _, ik := range iKinds { - assert.NoError(t, DefaultAggregationSelector(ik).Err(), ik) + assert.NoError(t, DefaultAggregationSelector(ik).err(), ik) } } diff --git a/sdk/metric/view.go b/sdk/metric/view.go index f1df24466bc..2d0fe18d7e9 100644 --- a/sdk/metric/view.go +++ b/sdk/metric/view.go @@ -20,7 +20,6 @@ import ( "strings" "go.opentelemetry.io/otel/internal/global" - "go.opentelemetry.io/otel/sdk/metric/aggregation" ) var ( @@ -92,10 +91,10 @@ func NewView(criteria Instrument, mask Stream) View { matchFunc = criteria.matches } - var agg aggregation.Aggregation + var agg Aggregation if mask.Aggregation != nil { - agg = mask.Aggregation.Copy() - if err := agg.Err(); err != nil { + agg = mask.Aggregation.copy() + if err := agg.err(); err != nil { global.Error( err, "not using aggregation with view", "criteria", criteria, diff --git a/sdk/metric/view_test.go b/sdk/metric/view_test.go index b8f6c921468..07f0c906cb8 100644 --- a/sdk/metric/view_test.go +++ b/sdk/metric/view_test.go @@ -28,7 +28,6 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/sdk/instrumentation" - "go.opentelemetry.io/otel/sdk/metric/aggregation" ) var ( @@ -395,13 +394,13 @@ func TestNewViewReplace(t *testing.T) { }, { name: "Aggregation", - mask: Stream{Aggregation: aggregation.LastValue{}}, + mask: Stream{Aggregation: AggregationLastValue{}}, want: func(i Instrument) Stream { return Stream{ Name: i.Name, Description: i.Description, Unit: i.Unit, - Aggregation: aggregation.LastValue{}, + Aggregation: AggregationLastValue{}, } }, }, @@ -423,14 +422,14 @@ func TestNewViewReplace(t *testing.T) { Name: alt, Description: alt, Unit: "1", - Aggregation: aggregation.LastValue{}, + Aggregation: AggregationLastValue{}, }, want: func(i Instrument) Stream { return Stream{ Name: alt, Description: alt, Unit: "1", - Aggregation: aggregation.LastValue{}, + Aggregation: AggregationLastValue{}, } }, }, @@ -446,20 +445,19 @@ func TestNewViewReplace(t *testing.T) { } type badAgg struct { - aggregation.Aggregation - err error + e error } -func (a badAgg) Copy() aggregation.Aggregation { return a } +func (a badAgg) copy() Aggregation { return a } -func (a badAgg) Err() error { return a.err } +func (a badAgg) err() error { return a.e } func TestNewViewAggregationErrorLogged(t *testing.T) { tLog := testr.NewWithOptions(t, testr.Options{Verbosity: 6}) l := &logCounter{LogSink: tLog.GetSink()} otel.SetLogger(logr.New(l)) - agg := badAgg{err: assert.AnError} + agg := badAgg{e: assert.AnError} mask := Stream{Aggregation: agg} got, match := NewView(completeIP, mask)(completeIP) require.True(t, match, "view did not match exact criteria") @@ -532,7 +530,7 @@ func ExampleNewView_drop() { // library. view := NewView( Instrument{Scope: instrumentation.Scope{Name: "db"}}, - Stream{Aggregation: aggregation.Drop{}}, + Stream{Aggregation: AggregationDrop{}}, ) // The created view can then be registered with the OpenTelemetry metric @@ -548,7 +546,7 @@ func ExampleNewView_drop() { fmt.Printf("aggregation: %#v", stream.Aggregation) // Output: // name: queries - // aggregation: aggregation.Drop{} + // aggregation: metric.AggregationDrop{} } func ExampleNewView_wildcard() {