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

Move metric.Descriptor out of metric API into sdkapi #2269

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion attribute/type_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions bridge/opencensus/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func convertResource(res *ocresource.Resource) *resource.Resource {
}

// convertDescriptor converts an OpenCensus Descriptor to an OpenTelemetry Descriptor
func convertDescriptor(ocDescriptor metricdata.Descriptor) (metric.Descriptor, error) {
func convertDescriptor(ocDescriptor metricdata.Descriptor) (sdkapi.Descriptor, error) {
var (
nkind number.Kind
ikind sdkapi.InstrumentKind
Expand All @@ -167,7 +167,7 @@ func convertDescriptor(ocDescriptor metricdata.Descriptor) (metric.Descriptor, e
ikind = sdkapi.CounterObserverInstrumentKind
default:
// Includes TypeGaugeDistribution, TypeCumulativeDistribution, TypeSummary
return metric.Descriptor{}, fmt.Errorf("%w; descriptor type: %v", errConversion, ocDescriptor.Type)
return sdkapi.Descriptor{}, fmt.Errorf("%w; descriptor type: %v", errConversion, ocDescriptor.Type)
}
opts := []metric.InstrumentOption{
metric.WithDescription(ocDescriptor.Description),
Expand All @@ -181,5 +181,5 @@ func convertDescriptor(ocDescriptor metricdata.Descriptor) (metric.Descriptor, e
opts = append(opts, metric.WithUnit(unit.Milliseconds))
}
cfg := metric.NewInstrumentConfig(opts...)
return metric.NewDescriptor(ocDescriptor.Name, ikind, nkind, cfg.Description(), cfg.Unit()), nil
return sdkapi.NewDescriptor(ocDescriptor.Name, ikind, nkind, cfg.Description(), cfg.Unit()), nil
}
2 changes: 1 addition & 1 deletion bridge/opencensus/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ func TestConvertDescriptor(t *testing.T) {
for _, tc := range []struct {
desc string
input metricdata.Descriptor
expected metric.Descriptor
expected sdkapi.Descriptor
expectedErr error
}{
{
Expand Down
4 changes: 2 additions & 2 deletions exporters/otlp/otlpmetric/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"sync"

"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/internal/metrictransform"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/sdkapi"
metricsdk "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
"go.opentelemetry.io/otel/sdk/resource"
Expand Down Expand Up @@ -96,7 +96,7 @@ func (e *Exporter) Shutdown(ctx context.Context) error {
return err
}

func (e *Exporter) ExportKindFor(descriptor *metric.Descriptor, aggregatorKind aggregation.Kind) metricsdk.ExportKind {
func (e *Exporter) ExportKindFor(descriptor *sdkapi.Descriptor, aggregatorKind aggregation.Kind) metricsdk.ExportKind {
return e.exportKindSelector.ExportKindFor(descriptor, aggregatorKind)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/metrictest"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
Expand Down Expand Up @@ -101,18 +100,18 @@ func TestStringKeyValues(t *testing.T) {
}

func TestMinMaxSumCountValue(t *testing.T) {
mmscs := minmaxsumcount.New(2, &metric.Descriptor{})
mmscs := minmaxsumcount.New(2, &sdkapi.Descriptor{})
mmsc, ckpt := &mmscs[0], &mmscs[1]

assert.NoError(t, mmsc.Update(context.Background(), 1, &metric.Descriptor{}))
assert.NoError(t, mmsc.Update(context.Background(), 10, &metric.Descriptor{}))
assert.NoError(t, mmsc.Update(context.Background(), 1, &sdkapi.Descriptor{}))
assert.NoError(t, mmsc.Update(context.Background(), 10, &sdkapi.Descriptor{}))

// Prior to checkpointing ErrNoData should be returned.
_, _, _, _, err := minMaxSumCountValues(ckpt)
assert.EqualError(t, err, aggregation.ErrNoData.Error())

// Checkpoint to set non-zero values
require.NoError(t, mmsc.SynchronizedMove(ckpt, &metric.Descriptor{}))
require.NoError(t, mmsc.SynchronizedMove(ckpt, &sdkapi.Descriptor{}))
min, max, sum, count, err := minMaxSumCountValues(ckpt)
if assert.NoError(t, err) {
assert.Equal(t, min, number.NewInt64Number(1))
Expand All @@ -125,7 +124,7 @@ func TestMinMaxSumCountValue(t *testing.T) {
func TestMinMaxSumCountDatapoints(t *testing.T) {
desc := metrictest.NewDescriptor("", sdkapi.HistogramInstrumentKind, number.Int64Kind)
labels := attribute.NewSet(attribute.String("one", "1"))
mmscs := minmaxsumcount.New(2, &metric.Descriptor{})
mmscs := minmaxsumcount.New(2, &sdkapi.Descriptor{})
mmsc, ckpt := &mmscs[0], &mmscs[1]

assert.NoError(t, mmsc.Update(context.Background(), 1, &desc))
Expand Down Expand Up @@ -172,7 +171,7 @@ func TestMinMaxSumCountPropagatesErrors(t *testing.T) {
// ErrNoData should be returned by both the Min and Max values of
// a MinMaxSumCount Aggregator. Use this fact to check the error is
// correctly returned.
mmsc := &minmaxsumcount.New(1, &metric.Descriptor{})[0]
mmsc := &minmaxsumcount.New(1, &sdkapi.Descriptor{})[0]
_, _, _, _, err := minMaxSumCountValues(mmsc)
assert.Error(t, err)
assert.Equal(t, aggregation.ErrNoData, err)
Expand Down Expand Up @@ -390,13 +389,13 @@ func (t *testAgg) Aggregation() aggregation.Aggregation {

// None of these three are used:

func (t *testAgg) Update(ctx context.Context, number number.Number, descriptor *metric.Descriptor) error {
func (t *testAgg) Update(ctx context.Context, number number.Number, descriptor *sdkapi.Descriptor) error {
return nil
}
func (t *testAgg) SynchronizedMove(destination export.Aggregator, descriptor *metric.Descriptor) error {
func (t *testAgg) SynchronizedMove(destination export.Aggregator, descriptor *sdkapi.Descriptor) error {
return nil
}
func (t *testAgg) Merge(aggregator export.Aggregator, descriptor *metric.Descriptor) error {
func (t *testAgg) Merge(aggregator export.Aggregator, descriptor *sdkapi.Descriptor) error {
return nil
}

Expand Down
3 changes: 2 additions & 1 deletion exporters/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
export "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
"go.opentelemetry.io/otel/sdk/instrumentation"
Expand Down Expand Up @@ -132,7 +133,7 @@ func (e *Exporter) Controller() *controller.Controller {
}

// ExportKindFor implements ExportKindSelector.
func (e *Exporter) ExportKindFor(desc *metric.Descriptor, kind aggregation.Kind) export.ExportKind {
func (e *Exporter) ExportKindFor(desc *sdkapi.Descriptor, kind aggregation.Kind) export.ExportKind {
return export.CumulativeExportKindSelector().ExportKindFor(desc, kind)
}

Expand Down
4 changes: 2 additions & 2 deletions exporters/stdout/stdoutmetric/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"time"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/sdkapi"
exportmetric "go.opentelemetry.io/otel/sdk/export/metric"
"go.opentelemetry.io/otel/sdk/export/metric/aggregation"
"go.opentelemetry.io/otel/sdk/instrumentation"
Expand All @@ -47,7 +47,7 @@ type line struct {
Timestamp *time.Time `json:"Timestamp,omitempty"`
}

func (e *metricExporter) ExportKindFor(desc *metric.Descriptor, kind aggregation.Kind) exportmetric.ExportKind {
func (e *metricExporter) ExportKindFor(desc *sdkapi.Descriptor, kind aggregation.Kind) exportmetric.ExportKind {
return exportmetric.StatelessExportKindSelector().ExportKindFor(desc, kind)
}

Expand Down
9 changes: 5 additions & 4 deletions internal/metric/global/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"go.opentelemetry.io/otel/internal/metric/registry"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
)

// This file contains the forwarding implementation of MeterProvider used as
Expand Down Expand Up @@ -78,7 +79,7 @@ type meterEntry struct {
}

type instrument struct {
descriptor metric.Descriptor
descriptor sdkapi.Descriptor
}

type syncImpl struct {
Expand Down Expand Up @@ -122,7 +123,7 @@ var _ metric.InstrumentImpl = &syncImpl{}
var _ metric.BoundSyncImpl = &syncHandle{}
var _ metric.AsyncImpl = &asyncImpl{}

func (inst *instrument) Descriptor() metric.Descriptor {
func (inst *instrument) Descriptor() sdkapi.Descriptor {
return inst.descriptor
}

Expand Down Expand Up @@ -197,7 +198,7 @@ func (m *meterImpl) setDelegate(key meterKey, provider metric.MeterProvider) {
m.asyncInsts = nil
}

func (m *meterImpl) NewSyncInstrument(desc metric.Descriptor) (metric.SyncImpl, error) {
func (m *meterImpl) NewSyncInstrument(desc sdkapi.Descriptor) (metric.SyncImpl, error) {
m.lock.Lock()
defer m.lock.Unlock()

Expand Down Expand Up @@ -265,7 +266,7 @@ func (bound *syncHandle) Unbind() {
// Async delegation

func (m *meterImpl) NewAsyncInstrument(
desc metric.Descriptor,
desc sdkapi.Descriptor,
runner metric.AsyncRunner,
) (metric.AsyncImpl, error) {

Expand Down
3 changes: 2 additions & 1 deletion internal/metric/global/meter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
metricglobal "go.opentelemetry.io/otel/metric/global"
"go.opentelemetry.io/otel/metric/metrictest"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
)

var Must = metric.Must
Expand Down Expand Up @@ -239,7 +240,7 @@ func (m *meterProviderWithConstructorError) Meter(iName string, opts ...metric.M
return metric.WrapMeterImpl(&meterWithConstructorError{m.MeterProvider.Meter(iName, opts...).MeterImpl()})
}

func (m *meterWithConstructorError) NewSyncInstrument(_ metric.Descriptor) (metric.SyncImpl, error) {
func (m *meterWithConstructorError) NewSyncInstrument(_ sdkapi.Descriptor) (metric.SyncImpl, error) {
return metric.NoopSync{}, errors.New("constructor error")
}

Expand Down
13 changes: 7 additions & 6 deletions internal/metric/registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/sdkapi"
)

// UniqueInstrumentMeterImpl implements the metric.MeterImpl interface, adding
Expand Down Expand Up @@ -60,17 +61,17 @@ func (u *UniqueInstrumentMeterImpl) RecordBatch(ctx context.Context, labels []at

// NewMetricKindMismatchError formats an error that describes a
// mismatched metric instrument definition.
func NewMetricKindMismatchError(desc metric.Descriptor) error {
func NewMetricKindMismatchError(desc sdkapi.Descriptor) error {
return fmt.Errorf("metric %s registered as %s %s: %w",
desc.Name(),
desc.NumberKind(),
desc.InstrumentKind(),
ErrMetricKindMismatch)
}

// Compatible determines whether two metric.Descriptors are considered
// Compatible determines whether two sdkapi.Descriptors are considered
// the same for the purpose of uniqueness checking.
func Compatible(candidate, existing metric.Descriptor) bool {
func Compatible(candidate, existing sdkapi.Descriptor) bool {
return candidate.InstrumentKind() == existing.InstrumentKind() &&
candidate.NumberKind() == existing.NumberKind()
}
Expand All @@ -80,7 +81,7 @@ func Compatible(candidate, existing metric.Descriptor) bool {
// `descriptor` argument. If there is an existing compatible
// registration, this returns the already-registered instrument. If
// there is no conflict and no prior registration, returns (nil, nil).
func (u *UniqueInstrumentMeterImpl) checkUniqueness(descriptor metric.Descriptor) (metric.InstrumentImpl, error) {
func (u *UniqueInstrumentMeterImpl) checkUniqueness(descriptor sdkapi.Descriptor) (metric.InstrumentImpl, error) {
impl, ok := u.state[descriptor.Name()]
if !ok {
return nil, nil
Expand All @@ -94,7 +95,7 @@ func (u *UniqueInstrumentMeterImpl) checkUniqueness(descriptor metric.Descriptor
}

// NewSyncInstrument implements metric.MeterImpl.
func (u *UniqueInstrumentMeterImpl) NewSyncInstrument(descriptor metric.Descriptor) (metric.SyncImpl, error) {
func (u *UniqueInstrumentMeterImpl) NewSyncInstrument(descriptor sdkapi.Descriptor) (metric.SyncImpl, error) {
u.lock.Lock()
defer u.lock.Unlock()

Expand All @@ -116,7 +117,7 @@ func (u *UniqueInstrumentMeterImpl) NewSyncInstrument(descriptor metric.Descript

// NewAsyncInstrument implements metric.MeterImpl.
func (u *UniqueInstrumentMeterImpl) NewAsyncInstrument(
descriptor metric.Descriptor,
descriptor sdkapi.Descriptor,
runner metric.AsyncRunner,
) (metric.AsyncImpl, error) {
u.lock.Lock()
Expand Down
55 changes: 2 additions & 53 deletions metric/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
"go.opentelemetry.io/otel/metric/unit"
)

// MeterProvider supports named Meter instances.
Expand Down Expand Up @@ -285,7 +284,7 @@ func (m Meter) newAsync(
return NoopAsync{}, nil
}
cfg := NewInstrumentConfig(opts...)
desc := NewDescriptor(name, mkind, nkind, cfg.description, cfg.unit)
desc := sdkapi.NewDescriptor(name, mkind, nkind, cfg.description, cfg.unit)
return m.impl.NewAsyncInstrument(desc, runner)
}

Expand All @@ -303,7 +302,7 @@ func (m Meter) newSync(
return NoopSync{}, nil
}
cfg := NewInstrumentConfig(opts...)
desc := NewDescriptor(name, metricKind, numberKind, cfg.description, cfg.unit)
desc := sdkapi.NewDescriptor(name, metricKind, numberKind, cfg.description, cfg.unit)
return m.impl.NewSyncInstrument(desc)
}

Expand Down Expand Up @@ -513,53 +512,3 @@ func (bm BatchObserverMust) NewFloat64UpDownCounterObserver(name string, oos ...
return inst
}
}

// Descriptor contains all the settings that describe an instrument,
// including its name, metric kind, number kind, and the configurable
// options.
type Descriptor struct {
name string
instrumentKind sdkapi.InstrumentKind
numberKind number.Kind
description string
unit unit.Unit
}

// NewDescriptor returns a Descriptor with the given contents.
func NewDescriptor(name string, ikind sdkapi.InstrumentKind, nkind number.Kind, description string, unit unit.Unit) Descriptor {
return Descriptor{
name: name,
instrumentKind: ikind,
numberKind: nkind,
description: description,
unit: unit,
}
}

// Name returns the metric instrument's name.
func (d Descriptor) Name() string {
return d.name
}

// InstrumentKind returns the specific kind of instrument.
func (d Descriptor) InstrumentKind() sdkapi.InstrumentKind {
return d.instrumentKind
}

// Description provides a human-readable description of the metric
// instrument.
func (d Descriptor) Description() string {
return d.description
}

// Unit describes the units of the metric instrument. Unitless
// metrics return the empty string.
func (d Descriptor) Unit() unit.Unit {
return d.unit
}

// NumberKind returns whether this instrument is declared over int64,
// float64, or uint64 values.
func (d Descriptor) NumberKind() number.Kind {
return d.numberKind
}
7 changes: 4 additions & 3 deletions metric/metric_sdkapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
)

// MeterImpl is the interface an SDK must implement to supply a Meter
Expand All @@ -30,13 +31,13 @@ type MeterImpl interface {
// NewSyncInstrument returns a newly constructed
// synchronous instrument implementation or an error, should
// one occur.
NewSyncInstrument(descriptor Descriptor) (SyncImpl, error)
NewSyncInstrument(descriptor sdkapi.Descriptor) (SyncImpl, error)

// NewAsyncInstrument returns a newly constructed
// asynchronous instrument implementation or an error, should
// one occur.
NewAsyncInstrument(
descriptor Descriptor,
descriptor sdkapi.Descriptor,
runner AsyncRunner,
) (AsyncImpl, error)
}
Expand All @@ -50,7 +51,7 @@ type InstrumentImpl interface {
Implementation() interface{}

// Descriptor returns a copy of the instrument's Descriptor.
Descriptor() Descriptor
Descriptor() sdkapi.Descriptor
}

// SyncImpl is the implementation-level interface to a generic
Expand Down
Loading