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

Remove Meter.RecordBatch functionality, not specified in the specs #2449

Closed
wants to merge 1 commit 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### Removed

- Remove Meter.RecordBatch functionality, not specified in the OpenTelemetry Specification (#2449)

### Deprecated

- Deprecate module `"go.opentelemetry.io/otel/sdk/export/metric"`, new functionality available in "go.opentelemetry.io/otel/sdk/metric" module:
Expand Down
24 changes: 6 additions & 18 deletions example/prometheus/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,38 +92,26 @@ func main() {
*observerValueToReport = 1.0
*observerLabelsToReport = commonLabels
(*observerLock).Unlock()
meter.RecordBatch(
ctx,
commonLabels,
histogram.Measurement(2.0),
counter.Measurement(12.0),
)
histogram.Record(ctx, 2.0, commonLabels...)
counter.Add(ctx, 12.0, commonLabels...)

time.Sleep(5 * time.Second)

(*observerLock).Lock()
*observerValueToReport = 1.0
*observerLabelsToReport = notSoCommonLabels
(*observerLock).Unlock()
meter.RecordBatch(
ctx,
notSoCommonLabels,
histogram.Measurement(2.0),
counter.Measurement(22.0),
)
histogram.Record(ctx, 2.0, notSoCommonLabels...)
counter.Add(ctx, 22.0, notSoCommonLabels...)

time.Sleep(5 * time.Second)

(*observerLock).Lock()
*observerValueToReport = 13.0
*observerLabelsToReport = commonLabels
(*observerLock).Unlock()
meter.RecordBatch(
ctx,
commonLabels,
histogram.Measurement(12.0),
counter.Measurement(13.0),
)
histogram.Record(ctx, 12.0, commonLabels...)
counter.Add(ctx, 13.0, commonLabels...)

fmt.Println("Example finished updating, please visit :2222")

Expand Down
6 changes: 0 additions & 6 deletions internal/metric/global/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,6 @@ func (obs *asyncImpl) setDelegate(d sdkapi.MeterImpl) {

// Metric updates

func (m *meterImpl) RecordBatch(ctx context.Context, labels []attribute.KeyValue, measurements ...sdkapi.Measurement) {
if delegatePtr := (*sdkapi.MeterImpl)(atomic.LoadPointer(&m.delegate)); delegatePtr != nil {
(*delegatePtr).RecordBatch(ctx, labels, measurements...)
}
}

func (inst *syncImpl) RecordOne(ctx context.Context, number number.Number, labels []attribute.KeyValue) {
if instPtr := (*sdkapi.SyncImpl)(atomic.LoadPointer(&inst.delegate)); instPtr != nil {
(*instPtr).RecordOne(ctx, number, labels)
Expand Down
32 changes: 2 additions & 30 deletions internal/metric/global/meter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func TestImplementationIndirection(t *testing.T) {
// Sync: no SDK yet
counter := Must(meter1).NewInt64Counter("interface.counter")

ival := counter.Measurement(1).SyncImpl().Implementation()
ival := counter.SyncImpl().Implementation()
require.NotNil(t, ival)

_, ok := ival.(*metrictest.Sync)
Expand All @@ -210,7 +210,7 @@ func TestImplementationIndirection(t *testing.T) {
// Repeat the above tests

// Sync
ival = counter.Measurement(1).SyncImpl().Implementation()
ival = counter.SyncImpl().Implementation()
require.NotNil(t, ival)

_, ok = ival.(*metrictest.Sync)
Expand All @@ -223,31 +223,3 @@ func TestImplementationIndirection(t *testing.T) {
_, ok = ival.(*metrictest.Async)
require.True(t, ok)
}

func TestRecordBatchMock(t *testing.T) {
global.ResetForTest()

meter := metricglobal.GetMeterProvider().Meter("builtin")

counter := Must(meter).NewInt64Counter("test.counter")

meter.RecordBatch(context.Background(), nil, counter.Measurement(1))

provider := metrictest.NewMeterProvider()
metricglobal.SetMeterProvider(provider)

meter.RecordBatch(context.Background(), nil, counter.Measurement(1))

require.EqualValues(t,
[]metrictest.Measured{
{
Name: "test.counter",
Library: metrictest.Library{
InstrumentationName: "builtin",
},
Labels: metrictest.LabelsToMap(),
Number: asInt(1),
},
},
metrictest.AsStructs(provider.MeasurementBatches))
}
7 changes: 0 additions & 7 deletions internal/metric/registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@
package registry // import "go.opentelemetry.io/otel/internal/metric/registry"

import (
"context"
"fmt"
"sync"

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

Expand Down Expand Up @@ -53,11 +51,6 @@ func (u *UniqueInstrumentMeterImpl) MeterImpl() sdkapi.MeterImpl {
return u.impl
}

// RecordBatch implements sdkapi.MeterImpl.
func (u *UniqueInstrumentMeterImpl) RecordBatch(ctx context.Context, labels []attribute.KeyValue, ms ...sdkapi.Measurement) {
u.impl.RecordBatch(ctx, labels, ms...)
}

// NewMetricKindMismatchError formats an error that describes a
// mismatched metric instrument definition.
func NewMetricKindMismatchError(desc sdkapi.Descriptor) error {
Expand Down
19 changes: 0 additions & 19 deletions metric/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
package metric // import "go.opentelemetry.io/otel/metric"

import (
"context"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric/number"
"go.opentelemetry.io/otel/metric/sdkapi"
)
Expand Down Expand Up @@ -48,14 +45,6 @@ func WrapMeterImpl(impl sdkapi.MeterImpl) Meter {
}
}

// Measurement is used for reporting a synchronous batch of metric
// values. Instances of this type should be created by synchronous
// instruments (e.g., Int64Counter.Measurement()).
//
// Note: This is an alias because it is a first-class member of the
// API but is also part of the lower-level sdkapi interface.
type Measurement = sdkapi.Measurement

// Observation is used for reporting an asynchronous batch of metric
// values. Instances of this type should be created by asynchronous
// instruments (e.g., Int64GaugeObserver.Observation()).
Expand All @@ -64,14 +53,6 @@ type Measurement = sdkapi.Measurement
// API but is also part of the lower-level sdkapi interface.
type Observation = sdkapi.Observation

// RecordBatch atomically records a batch of measurements.
func (m Meter) RecordBatch(ctx context.Context, ls []attribute.KeyValue, ms ...Measurement) {
if m.impl == nil {
return
}
m.impl.RecordBatch(ctx, ls, ms...)
}

// NewBatchObserver creates a new BatchObserver that supports
// making batches of observations for multiple instruments.
func (m Meter) NewBatchObserver(callback BatchObserverFunc) BatchObserver {
Expand Down
44 changes: 0 additions & 44 deletions metric/metric_instrument.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,14 +275,6 @@ func (s syncInstrument) SyncImpl() sdkapi.SyncImpl {
return s.instrument
}

func (s syncInstrument) float64Measurement(value float64) Measurement {
return sdkapi.NewMeasurement(s.instrument, number.NewFloat64Number(value))
}

func (s syncInstrument) int64Measurement(value int64) Measurement {
return sdkapi.NewMeasurement(s.instrument, number.NewInt64Number(value))
}

func (s syncInstrument) directRecord(ctx context.Context, number number.Number, labels []attribute.KeyValue) {
s.instrument.RecordOne(ctx, number, labels)
}
Expand Down Expand Up @@ -368,18 +360,6 @@ type Int64Counter struct {
syncInstrument
}

// Measurement creates a Measurement object to use with batch
// recording.
func (c Float64Counter) Measurement(value float64) Measurement {
return c.float64Measurement(value)
}

// Measurement creates a Measurement object to use with batch
// recording.
func (c Int64Counter) Measurement(value int64) Measurement {
return c.int64Measurement(value)
}

// Add adds the value to the counter's sum. The labels should contain
// the keys and values to be associated with this value.
func (c Float64Counter) Add(ctx context.Context, value float64, labels ...attribute.KeyValue) {
Expand All @@ -403,18 +383,6 @@ type Int64UpDownCounter struct {
syncInstrument
}

// Measurement creates a Measurement object to use with batch
// recording.
func (c Float64UpDownCounter) Measurement(value float64) Measurement {
return c.float64Measurement(value)
}

// Measurement creates a Measurement object to use with batch
// recording.
func (c Int64UpDownCounter) Measurement(value int64) Measurement {
return c.int64Measurement(value)
}

// Add adds the value to the counter's sum. The labels should contain
// the keys and values to be associated with this value.
func (c Float64UpDownCounter) Add(ctx context.Context, value float64, labels ...attribute.KeyValue) {
Expand All @@ -437,18 +405,6 @@ type Int64Histogram struct {
syncInstrument
}

// Measurement creates a Measurement object to use with batch
// recording.
func (c Float64Histogram) Measurement(value float64) Measurement {
return c.float64Measurement(value)
}

// Measurement creates a Measurement object to use with batch
// recording.
func (c Int64Histogram) Measurement(value int64) Measurement {
return c.int64Measurement(value)
}

// Record adds a new value to the list of Histogram's records. The
// labels should contain the keys and values to be associated with
// this value.
Expand Down
15 changes: 6 additions & 9 deletions metric/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func TestCounter(t *testing.T) {
ctx := context.Background()
labels := []attribute.KeyValue{attribute.String("A", "B")}
c.Add(ctx, 1994.1, labels...)
meter.RecordBatch(ctx, labels, c.Measurement(42))
c.Add(ctx, 42, labels...)
checkSyncBatches(ctx, t, labels, provider, number.Float64Kind, sdkapi.CounterInstrumentKind, c.SyncImpl(),
1994.1, 42,
)
Expand All @@ -261,7 +261,7 @@ func TestCounter(t *testing.T) {
ctx := context.Background()
labels := []attribute.KeyValue{attribute.String("A", "B"), attribute.String("C", "D")}
c.Add(ctx, 42, labels...)
meter.RecordBatch(ctx, labels, c.Measurement(420000))
c.Add(ctx, 420000, labels...)
checkSyncBatches(ctx, t, labels, provider, number.Int64Kind, sdkapi.CounterInstrumentKind, c.SyncImpl(),
42, 420000,
)
Expand All @@ -273,7 +273,7 @@ func TestCounter(t *testing.T) {
ctx := context.Background()
labels := []attribute.KeyValue{attribute.String("A", "B"), attribute.String("C", "D")}
c.Add(ctx, 100, labels...)
meter.RecordBatch(ctx, labels, c.Measurement(42))
c.Add(ctx, 42, labels...)
checkSyncBatches(ctx, t, labels, provider, number.Int64Kind, sdkapi.UpDownCounterInstrumentKind, c.SyncImpl(),
100, 42,
)
Expand All @@ -284,7 +284,7 @@ func TestCounter(t *testing.T) {
ctx := context.Background()
labels := []attribute.KeyValue{attribute.String("A", "B"), attribute.String("C", "D")}
c.Add(ctx, 100.1, labels...)
meter.RecordBatch(ctx, labels, c.Measurement(-100.1))
c.Add(ctx, -100.1, labels...)
checkSyncBatches(ctx, t, labels, provider, number.Float64Kind, sdkapi.UpDownCounterInstrumentKind, c.SyncImpl(),
100.1, -100.1,
)
Expand All @@ -298,7 +298,7 @@ func TestHistogram(t *testing.T) {
ctx := context.Background()
labels := []attribute.KeyValue{}
m.Record(ctx, 42, labels...)
meter.RecordBatch(ctx, labels, m.Measurement(-100.5))
m.Record(ctx, -100.5, labels...)
checkSyncBatches(ctx, t, labels, provider, number.Float64Kind, sdkapi.HistogramInstrumentKind, m.SyncImpl(),
42, -100.5,
)
Expand All @@ -309,7 +309,7 @@ func TestHistogram(t *testing.T) {
ctx := context.Background()
labels := []attribute.KeyValue{attribute.Int("I", 1)}
m.Record(ctx, 173, labels...)
meter.RecordBatch(ctx, labels, m.Measurement(0))
m.Record(ctx, 0, labels...)
checkSyncBatches(ctx, t, labels, provider, number.Int64Kind, sdkapi.HistogramInstrumentKind, m.SyncImpl(),
173, 0,
)
Expand Down Expand Up @@ -458,9 +458,6 @@ type testWrappedMeter struct {

var _ sdkapi.MeterImpl = testWrappedMeter{}

func (testWrappedMeter) RecordBatch(context.Context, []attribute.KeyValue, ...sdkapi.Measurement) {
}

func (testWrappedMeter) NewSyncInstrument(_ sdkapi.Descriptor) (sdkapi.SyncImpl, error) {
return nil, nil
}
Expand Down
13 changes: 0 additions & 13 deletions metric/metrictest/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,19 +175,6 @@ func (m *MeterImpl) NewAsyncInstrument(descriptor sdkapi.Descriptor, runner sdka
return a, nil
}

// RecordBatch implements sdkapi.MeterImpl.
func (m *MeterImpl) RecordBatch(ctx context.Context, labels []attribute.KeyValue, measurements ...sdkapi.Measurement) {
mm := make([]Measurement, len(measurements))
for i := 0; i < len(measurements); i++ {
m := measurements[i]
mm[i] = Measurement{
Instrument: m.SyncImpl().Implementation().(*Sync),
Number: m.Number(),
}
}
m.collect(ctx, labels, mm)
}

// CollectAsync is called from asyncInstruments.Run() with the lock held.
func (m *MeterImpl) CollectAsync(labels []attribute.KeyValue, obs ...sdkapi.Observation) {
mm := make([]Measurement, len(obs))
Expand Down
32 changes: 0 additions & 32 deletions metric/sdkapi/sdkapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ import (
// MeterImpl is the interface an SDK must implement to supply a Meter
// implementation.
type MeterImpl interface {
// RecordBatch atomically records a batch of measurements.
RecordBatch(ctx context.Context, labels []attribute.KeyValue, measurement ...Measurement)

// NewSyncInstrument returns a newly constructed
// synchronous instrument implementation or an error, should
// one occur.
Expand Down Expand Up @@ -100,35 +97,6 @@ type AsyncBatchRunner interface {
AsyncRunner
}

// NewMeasurement constructs a single observation, a binding between
// an asynchronous instrument and a number.
func NewMeasurement(instrument SyncImpl, number number.Number) Measurement {
return Measurement{
instrument: instrument,
number: number,
}
}

// Measurement is a low-level type used with synchronous instruments
// as a direct interface to the SDK via `RecordBatch`.
type Measurement struct {
// number needs to be aligned for 64-bit atomic operations.
number number.Number
instrument SyncImpl
}

// SyncImpl returns the instrument that created this measurement.
// This returns an implementation-level object for use by the SDK,
// users should not refer to this.
func (m Measurement) SyncImpl() SyncImpl {
return m.instrument
}

// Number returns a number recorded in this measurement.
func (m Measurement) Number() number.Number {
return m.number
}

// NewObservation constructs a single observation, a binding between
// an asynchronous instrument and a number.
func NewObservation(instrument AsyncImpl, number number.Number) Observation {
Expand Down