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

Metrics: Move the non-API types into sdkapi #2271

Merged
merged 14 commits into from Oct 14, 2021
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -11,6 +11,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
## Changed

- Skip links with invalid span context. (#2275)
- Metrics API cleanup. The `metric/sdkapi` package has been created to relocate the API-to-SDK interface:
- The following interface types simply moved from `metric` to `metric/sdkapi`: `Descriptor`, `MeterImpl`, `InstrumentImpl`, `SyncImpl`, `BoundSyncImpl`, `AsyncImpl`, `AsyncRunner`, `AsyncSingleRunner`, and `AsyncBatchRunner`
- The following struct types moved and are replaced with type aliases, since they are exposed to the user: `Observation`, `Measurement`.
- The No-op implementations of sync and async instruments are no longer exported, new functions `sdkapi.NewNoopAsyncInstrument()` and `sdkapi.NewNoopSyncInstrument()` are provided instead. (#2271)

### Added

Expand Down
6 changes: 3 additions & 3 deletions bridge/opencensus/exporter.go
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
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
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
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
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
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
20 changes: 10 additions & 10 deletions internal/metric/async.go
Expand Up @@ -22,7 +22,7 @@ import (

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

//nolint:revive // ignoring missing comments for exported error in an internal package
Expand All @@ -33,7 +33,7 @@ var ErrInvalidAsyncRunner = errors.New("unknown async runner type")
// the SDK to provide support for running observer callbacks.
type AsyncCollector interface {
// CollectAsync passes a batch of observations to the MeterImpl.
CollectAsync(labels []attribute.KeyValue, observation ...metric.Observation)
CollectAsync(labels []attribute.KeyValue, observation ...sdkapi.Observation)
}

// AsyncInstrumentState manages an ordered set of asynchronous
Expand Down Expand Up @@ -61,18 +61,18 @@ type AsyncInstrumentState struct {

// instruments maintains the set of instruments in the order
// they were registered.
instruments []metric.AsyncImpl
instruments []sdkapi.AsyncImpl
}

// asyncRunnerPair is a map entry for Observer callback runners.
type asyncRunnerPair struct {
// runner is used as a map key here. The API ensures
// that all callbacks are pointers for this reason.
runner metric.AsyncRunner
runner sdkapi.AsyncRunner

// inst refers to a non-nil instrument when `runner` is a
// AsyncSingleRunner.
inst metric.AsyncImpl
inst sdkapi.AsyncImpl
}

// NewAsyncInstrumentState returns a new *AsyncInstrumentState, for
Expand All @@ -87,7 +87,7 @@ func NewAsyncInstrumentState() *AsyncInstrumentState {
// Instruments returns the asynchronous instruments managed by this
// object, the set that should be checkpointed after observers are
// run.
func (a *AsyncInstrumentState) Instruments() []metric.AsyncImpl {
func (a *AsyncInstrumentState) Instruments() []sdkapi.AsyncImpl {
a.lock.Lock()
defer a.lock.Unlock()
return a.instruments
Expand All @@ -97,7 +97,7 @@ func (a *AsyncInstrumentState) Instruments() []metric.AsyncImpl {
// object. This should be called during NewAsyncInstrument() and
// assumes that errors (e.g., duplicate registration) have already
// been checked.
func (a *AsyncInstrumentState) Register(inst metric.AsyncImpl, runner metric.AsyncRunner) {
func (a *AsyncInstrumentState) Register(inst sdkapi.AsyncImpl, runner sdkapi.AsyncRunner) {
a.lock.Lock()
defer a.lock.Unlock()

Expand All @@ -111,7 +111,7 @@ func (a *AsyncInstrumentState) Register(inst metric.AsyncImpl, runner metric.Asy
rp := asyncRunnerPair{
runner: runner,
}
if _, ok := runner.(metric.AsyncSingleRunner); ok {
if _, ok := runner.(sdkapi.AsyncSingleRunner); ok {
rp.inst = inst
}

Expand All @@ -132,12 +132,12 @@ func (a *AsyncInstrumentState) Run(ctx context.Context, collector AsyncCollector
// other implementations are possible because the
// interface has un-exported methods.

if singleRunner, ok := rp.runner.(metric.AsyncSingleRunner); ok {
if singleRunner, ok := rp.runner.(sdkapi.AsyncSingleRunner); ok {
singleRunner.Run(ctx, rp.inst, collector.CollectAsync)
continue
}

if multiRunner, ok := rp.runner.(metric.AsyncBatchRunner); ok {
if multiRunner, ok := rp.runner.(sdkapi.AsyncBatchRunner); ok {
multiRunner.Run(ctx, collector.CollectAsync)
continue
}
Expand Down