Skip to content

Commit

Permalink
Flatten sdk/metric/aggregation into sdk/metric (#4435)
Browse files Browse the repository at this point in the history
* Deprecate the aggregation pkg

* Decouple the internal/aggregate from aggregation pkg

* Add Aggregation to the metric pkg

* Do not use sdk/metric/aggregation in stdoutmetric exporter

* Update all generated templates

* Update prom exporter

* Fix view example

* Add changes to changelog

* Update CHANGELOG.md

Co-authored-by: Robert Pająk <[email protected]>

* Rename Sum to AggregationSum

* Fix comments

* Centralize validation of aggregation in pipeline

* Remove validation of agg in manual_reader selector opt

* Fix merge

---------

Co-authored-by: Robert Pająk <[email protected]>
  • Loading branch information
MrAlias and pellared authored Aug 14, 2023
1 parent 6f64e5b commit 3904523
Show file tree
Hide file tree
Showing 55 changed files with 513 additions and 347 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
3 changes: 1 addition & 2 deletions example/view/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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},
},
},
Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/otlpmetric/internal/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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.
//
Expand Down
5 changes: 2 additions & 3 deletions exporters/otlp/otlpmetric/internal/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/otlpmetric/internal/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
}

Expand Down
19 changes: 1 addition & 18 deletions exporters/otlp/otlpmetric/internal/oconf/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
})
}
7 changes: 3 additions & 4 deletions exporters/otlp/otlpmetric/internal/oconf/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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))
},
},
}
Expand All @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/otlpmetric/internal/otest/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}

Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/otlpmetric/otlpmetricgrpc/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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",
Expand All @@ -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),
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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))
},
},
}
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}

Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/otlpmetric/otlpmetrichttp/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 3904523

Please sign in to comment.