Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flatten sdk/metric/aggregation into sdk/metric #4435

Merged
merged 20 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
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
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
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
Expand Up @@ -25,7 +25,6 @@

"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 @@
}

// 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 {

Check warning on line 49 in exporters/otlp/otlpmetric/internal/exporter.go

View check run for this annotation

Codecov / codecov/patch

exporters/otlp/otlpmetric/internal/exporter.go#L49

Added line #L49 was not covered by tests
e.clientMu.Lock()
defer e.clientMu.Unlock()
return e.client.Aggregation(k)
Expand Down Expand Up @@ -120,7 +119,7 @@
return c.temporalitySelector(k)
}

func (c shutdownClient) Aggregation(k metric.InstrumentKind) aggregation.Aggregation {
func (c shutdownClient) Aggregation(k metric.InstrumentKind) metric.Aggregation {

Check warning on line 122 in exporters/otlp/otlpmetric/internal/exporter.go

View check run for this annotation

Codecov / codecov/patch

exporters/otlp/otlpmetric/internal/exporter.go#L122

Added line #L122 was not covered by tests
return c.aggregationSelector(k)
}

Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/otlpmetric/internal/exporter_test.go
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
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 {
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
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
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
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
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
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
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
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
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
})
}
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
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
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
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
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