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

[Proposal] Refactor view implementation #3397

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e8b62c0
Add view implementation to sdk/metric pkg
MrAlias Oct 23, 2022
ba9993a
Deprecated view pkg in favor of sdk/metric views
MrAlias Oct 23, 2022
d3ee653
Drop MatchFuncs
MrAlias Oct 24, 2022
02742b6
Use new view implementation
MrAlias Oct 24, 2022
bdee2ed
Rename InstrumentStream to DataStream
MrAlias Oct 25, 2022
9829b1e
Fix InstrumentProperties.mask for Unit
MrAlias Oct 25, 2022
f291038
Add view tests
MrAlias Oct 25, 2022
e7a25b1
Add NewView example tests
MrAlias Oct 25, 2022
88b5315
Rename InstrumentProperties to Instrument
MrAlias Oct 26, 2022
1dcd612
Add temporality to data stream
MrAlias Oct 26, 2022
488f6c3
Rename DataStream to Stream
MrAlias Oct 26, 2022
0ced1d0
Add deprecation notices to exported types of view
MrAlias Oct 26, 2022
fd3cd30
Fix spelling errors
MrAlias Oct 26, 2022
8e22a21
Add docs for private methods of Instrument
MrAlias Oct 26, 2022
267ae59
Fix view example
MrAlias Oct 26, 2022
5714ca6
Lint sdk/metric
MrAlias Oct 26, 2022
8a243f2
Fix prom exp use of views
MrAlias Oct 26, 2022
f061ec4
Ignore lint false-positive of used nonComparable
MrAlias Oct 26, 2022
45b1008
Add View example
MrAlias Oct 26, 2022
2a8d1f6
Merge branch 'main' into view-deprecate
MrAlias Oct 27, 2022
49ba1cd
Merge branch 'main' into view-deprecate
MrAlias Oct 27, 2022
3b9543c
Merge branch 'main' into view-deprecate
MrAlias Nov 1, 2022
da15cac
Update sdk/metric docs
MrAlias Nov 1, 2022
657af01
Merge branch 'main' into view-deprecate
MrAlias Nov 1, 2022
e2a92f3
Merge branch 'main' into view-deprecate
MrAlias Nov 2, 2022
4a45cf0
Fix benchmark_test
MrAlias Nov 2, 2022
384e7f0
Merge branch 'main' into view-deprecate
MrAlias Nov 4, 2022
f5a9033
Merge branch 'main' into view-deprecate
MrAlias Nov 7, 2022
a6b8e10
Merge branch 'main' into view-deprecate
MrAlias Nov 8, 2022
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
33 changes: 12 additions & 21 deletions example/view/main.go
Expand Up @@ -30,7 +30,6 @@ import (
"go.opentelemetry.io/otel/sdk/instrumentation"
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/view"
)

const meterName = "github.com/open-telemetry/opentelemetry-go/example/view"
Expand All @@ -45,30 +44,22 @@ func main() {
}

// View to customize histogram buckets and rename a single histogram instrument.
customBucketsView, err := view.New(
// Match* to match instruments
view.MatchInstrumentName("custom_histogram"),
view.MatchInstrumentationScope(instrumentation.Scope{Name: meterName}),

// With* to modify instruments
view.WithSetAggregation(aggregation.ExplicitBucketHistogram{
Boundaries: []float64{64, 128, 256, 512, 1024, 2048, 4096},
}),
view.WithRename("bar"),
customBucketsView := metric.NewView(
metric.Instrument{
Name: "custom_histogram",
Scope: instrumentation.Scope{Name: meterName},
},
metric.Stream{
Instrument: metric.Instrument{Name: "bar"},
Aggregation: aggregation.ExplicitBucketHistogram{
Boundaries: []float64{64, 128, 256, 512, 1024, 2048, 4096},
},
},
)
if err != nil {
log.Fatal(err)
}

// Default view to keep all instruments
defaultView, err := view.New(view.MatchInstrumentName("*"))
if err != nil {
log.Fatal(err)
}

provider := metric.NewMeterProvider(
metric.WithReader(exporter),
metric.WithView(customBucketsView, defaultView),
metric.WithView(customBucketsView),
)
meter := provider.Meter(meterName)

Expand Down
6 changes: 3 additions & 3 deletions exporters/otlp/otlpmetric/client.go
Expand Up @@ -17,19 +17,19 @@ package otlpmetric // import "go.opentelemetry.io/otel/exporters/otlp/otlpmetric
import (
"context"

"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
mpb "go.opentelemetry.io/proto/otlp/metrics/v1"
)

// Client handles the transmission of OTLP data to an OTLP receiving endpoint.
type Client interface {
// Temporality returns the Temporality to use for an instrument kind.
Temporality(view.InstrumentKind) metricdata.Temporality
Temporality(metric.InstrumentKind) metricdata.Temporality

// Aggregation returns the Aggregation to use for an instrument kind.
Aggregation(view.InstrumentKind) aggregation.Aggregation
Aggregation(metric.InstrumentKind) aggregation.Aggregation

// UploadMetrics transmits metric data to an OTLP receiver.
//
Expand Down
9 changes: 4 additions & 5 deletions exporters/otlp/otlpmetric/exporter.go
Expand Up @@ -23,7 +23,6 @@ import (
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
mpb "go.opentelemetry.io/proto/otlp/metrics/v1"
)

Expand All @@ -37,14 +36,14 @@ type exporter struct {
}

// Temporality returns the Temporality to use for an instrument kind.
func (e *exporter) Temporality(k view.InstrumentKind) metricdata.Temporality {
func (e *exporter) Temporality(k metric.InstrumentKind) metricdata.Temporality {
e.clientMu.Lock()
defer e.clientMu.Unlock()
return e.client.Temporality(k)
}

// Aggregation returns the Aggregation to use for an instrument kind.
func (e *exporter) Aggregation(k view.InstrumentKind) aggregation.Aggregation {
func (e *exporter) Aggregation(k metric.InstrumentKind) aggregation.Aggregation {
e.clientMu.Lock()
defer e.clientMu.Unlock()
return e.client.Aggregation(k)
Expand Down Expand Up @@ -113,11 +112,11 @@ func (c shutdownClient) err(ctx context.Context) error {
return errShutdown
}

func (c shutdownClient) Temporality(k view.InstrumentKind) metricdata.Temporality {
func (c shutdownClient) Temporality(k metric.InstrumentKind) metricdata.Temporality {
return c.temporalitySelector(k)
}

func (c shutdownClient) Aggregation(k view.InstrumentKind) aggregation.Aggregation {
func (c shutdownClient) Aggregation(k metric.InstrumentKind) aggregation.Aggregation {
return c.aggregationSelector(k)
}

Expand Down
5 changes: 2 additions & 3 deletions exporters/otlp/otlpmetric/exporter_test.go
Expand Up @@ -24,7 +24,6 @@ import (
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
mpb "go.opentelemetry.io/proto/otlp/metrics/v1"
)

Expand All @@ -34,11 +33,11 @@ type client struct {
n int
}

func (c *client) Temporality(k view.InstrumentKind) metricdata.Temporality {
func (c *client) Temporality(k metric.InstrumentKind) metricdata.Temporality {
return metric.DefaultTemporalitySelector(k)
}

func (c *client) Aggregation(k view.InstrumentKind) aggregation.Aggregation {
func (c *client) Aggregation(k metric.InstrumentKind) aggregation.Aggregation {
return metric.DefaultAggregationSelector(k)
}

Expand Down
3 changes: 1 addition & 2 deletions exporters/otlp/otlpmetric/internal/oconf/options.go
Expand Up @@ -30,7 +30,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/view"
)

const (
Expand Down Expand Up @@ -336,7 +335,7 @@ func WithTemporalitySelector(selector metric.TemporalitySelector) GenericOption

func WithAggregationSelector(selector metric.AggregationSelector) GenericOption {
// Deep copy and validate before using.
wrapped := func(ik view.InstrumentKind) aggregation.Aggregation {
wrapped := func(ik metric.InstrumentKind) aggregation.Aggregation {
a := selector(ik)
cpA := a.Copy()
if err := cpA.Err(); err != nil {
Expand Down
10 changes: 5 additions & 5 deletions exporters/otlp/otlpmetric/internal/oconf/options_test.go
Expand Up @@ -23,9 +23,9 @@ import (

"go.opentelemetry.io/otel/exporters/otlp/internal/envconfig"
"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"
"go.opentelemetry.io/otel/sdk/metric/view"
)

const (
Expand Down Expand Up @@ -397,7 +397,7 @@ func TestConfigs(t *testing.T) {
// Function value comparisons are disallowed, test non-default
// behavior of a TemporalitySelector here to ensure our "catch
// all" was set.
var undefinedKind view.InstrumentKind
var undefinedKind metric.InstrumentKind
got := c.Metrics.TemporalitySelector
assert.Equal(t, metricdata.DeltaTemporality, got(undefinedKind))
},
Expand All @@ -413,7 +413,7 @@ func TestConfigs(t *testing.T) {
// Function value comparisons are disallowed, test non-default
// behavior of a AggregationSelector here to ensure our "catch
// all" was set.
var undefinedKind view.InstrumentKind
var undefinedKind metric.InstrumentKind
got := c.Metrics.AggregationSelector
assert.Equal(t, aggregation.Drop{}, got(undefinedKind))
},
Expand Down Expand Up @@ -441,11 +441,11 @@ func TestConfigs(t *testing.T) {
}
}

func dropSelector(view.InstrumentKind) aggregation.Aggregation {
func dropSelector(metric.InstrumentKind) aggregation.Aggregation {
return aggregation.Drop{}
}

func deltaSelector(view.InstrumentKind) metricdata.Temporality {
func deltaSelector(metric.InstrumentKind) metricdata.Temporality {
return metricdata.DeltaTemporality
}

Expand Down
5 changes: 2 additions & 3 deletions exporters/otlp/otlpmetric/internal/otest/client_test.go
Expand Up @@ -22,7 +22,6 @@ import (
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
cpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
mpb "go.opentelemetry.io/proto/otlp/metrics/v1"
)
Expand All @@ -31,11 +30,11 @@ type client struct {
storage *Storage
}

func (c *client) Temporality(k view.InstrumentKind) metricdata.Temporality {
func (c *client) Temporality(k metric.InstrumentKind) metricdata.Temporality {
return metric.DefaultTemporalitySelector(k)
}

func (c *client) Aggregation(k view.InstrumentKind) aggregation.Aggregation {
func (c *client) Aggregation(k metric.InstrumentKind) aggregation.Aggregation {
return metric.DefaultAggregationSelector(k)
}

Expand Down
5 changes: 2 additions & 3 deletions exporters/otlp/otlpmetric/otlpmetricgrpc/client.go
Expand Up @@ -30,7 +30,6 @@ import (
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
colmetricpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
)
Expand Down Expand Up @@ -104,12 +103,12 @@ func newClient(ctx context.Context, options ...Option) (otlpmetric.Client, error
}

// Temporality returns the Temporality to use for an instrument kind.
func (c *client) Temporality(k view.InstrumentKind) metricdata.Temporality {
func (c *client) Temporality(k metric.InstrumentKind) metricdata.Temporality {
return c.temporalitySelector(k)
}

// Aggregation returns the Aggregation to use for an instrument kind.
func (c *client) Aggregation(k view.InstrumentKind) aggregation.Aggregation {
func (c *client) Aggregation(k metric.InstrumentKind) aggregation.Aggregation {
return c.aggregationSelector(k)
}

Expand Down
5 changes: 2 additions & 3 deletions exporters/otlp/otlpmetric/otlpmetrichttp/client.go
Expand Up @@ -36,7 +36,6 @@ import (
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
colmetricpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
)
Expand Down Expand Up @@ -129,12 +128,12 @@ func newClient(opts ...Option) (otlpmetric.Client, error) {
}

// Temporality returns the Temporality to use for an instrument kind.
func (c *client) Temporality(k view.InstrumentKind) metricdata.Temporality {
func (c *client) Temporality(k metric.InstrumentKind) metricdata.Temporality {
return c.temporalitySelector(k)
}

// Aggregation returns the Aggregation to use for an instrument kind.
func (c *client) Aggregation(k view.InstrumentKind) aggregation.Aggregation {
func (c *client) Aggregation(k metric.InstrumentKind) aggregation.Aggregation {
return c.aggregationSelector(k)
}

Expand Down
6 changes: 3 additions & 3 deletions exporters/prometheus/config_test.go
Expand Up @@ -20,14 +20,14 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/view"
)

func TestNewConfig(t *testing.T) {
registry := prometheus.NewRegistry()

aggregationSelector := func(view.InstrumentKind) aggregation.Aggregation { return nil }
aggregationSelector := func(metric.InstrumentKind) aggregation.Aggregation { return nil }

testCases := []struct {
name string
Expand Down Expand Up @@ -112,7 +112,7 @@ func TestNewConfig(t *testing.T) {
}

func TestConfigManualReaderOptions(t *testing.T) {
aggregationSelector := func(view.InstrumentKind) aggregation.Aggregation { return nil }
aggregationSelector := func(metric.InstrumentKind) aggregation.Aggregation { return nil }

testCases := []struct {
name string
Expand Down
20 changes: 8 additions & 12 deletions exporters/prometheus/exporter_test.go
Expand Up @@ -30,7 +30,6 @@ import (
"go.opentelemetry.io/otel/metric/unit"
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/view"
"go.opentelemetry.io/otel/sdk/resource"
semconv "go.opentelemetry.io/otel/semconv/v1.12.0"
)
Expand Down Expand Up @@ -269,16 +268,6 @@ func TestPrometheusExporter(t *testing.T) {
exporter, err := New(append(tc.options, WithRegisterer(registry))...)
require.NoError(t, err)

customBucketsView, err := view.New(
view.MatchInstrumentName("histogram_*"),
view.WithSetAggregation(aggregation.ExplicitBucketHistogram{
Boundaries: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000},
}),
)
require.NoError(t, err)
defaultView, err := view.New(view.MatchInstrumentName("*"))
require.NoError(t, err)

var res *resource.Resource

if tc.emptyResource {
Expand All @@ -300,7 +289,14 @@ func TestPrometheusExporter(t *testing.T) {
provider := metric.NewMeterProvider(
metric.WithResource(res),
metric.WithReader(exporter),
metric.WithView(customBucketsView, defaultView),
metric.WithView(metric.NewView(
metric.Instrument{Name: "histogram_*"},
metric.Stream{
Aggregation: aggregation.ExplicitBucketHistogram{
Boundaries: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000},
},
},
)),
)
meter := provider.Meter(
"testmeter",
Expand Down
3 changes: 1 addition & 2 deletions exporters/stdout/stdoutmetric/config.go
Expand Up @@ -20,7 +20,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/view"
)

// config contains options for the exporter.
Expand Down Expand Up @@ -101,7 +100,7 @@ func (t temporalitySelectorOption) apply(c config) config {
// instrument.
func WithAggregationSelector(selector metric.AggregationSelector) Option {
// Deep copy and validate before using.
wrapped := func(ik view.InstrumentKind) aggregation.Aggregation {
wrapped := func(ik metric.InstrumentKind) aggregation.Aggregation {
a := selector(ik)
cpA := a.Copy()
if err := cpA.Err(); err != nil {
Expand Down
5 changes: 2 additions & 3 deletions exporters/stdout/stdoutmetric/exporter.go
Expand Up @@ -22,7 +22,6 @@ import (
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
)

// exporter is an OpenTelemetry metric exporter.
Expand All @@ -49,11 +48,11 @@ func New(options ...Option) (metric.Exporter, error) {
return exp, nil
}

func (e *exporter) Temporality(k view.InstrumentKind) metricdata.Temporality {
func (e *exporter) Temporality(k metric.InstrumentKind) metricdata.Temporality {
return e.temporalitySelector(k)
}

func (e *exporter) Aggregation(k view.InstrumentKind) aggregation.Aggregation {
func (e *exporter) Aggregation(k metric.InstrumentKind) aggregation.Aggregation {
return e.aggregationSelector(k)
}

Expand Down