Skip to content

Commit

Permalink
Remove Meter.RecordBatch functionality, not specified in the specs
Browse files Browse the repository at this point in the history
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
  • Loading branch information
bogdandrutu committed Dec 14, 2021
1 parent 43daab9 commit d5f3b92
Show file tree
Hide file tree
Showing 14 changed files with 19 additions and 300 deletions.
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

0 comments on commit d5f3b92

Please sign in to comment.