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 Labelset #595

Merged
merged 24 commits into from
Mar 27, 2020
Merged
Show file tree
Hide file tree
Changes from 19 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
8 changes: 4 additions & 4 deletions api/global/internal/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@ func BenchmarkGlobalInt64CounterAddNoSDK(b *testing.B) {
internal.ResetForTest()
ctx := context.Background()
sdk := global.Meter("test")
labs := sdk.Labels(key.String("A", "B"))
labs := metric.Labels(key.String("A", "B"))
cnt := Must(sdk).NewInt64Counter("int64.counter")

b.ResetTimer()

for i := 0; i < b.N; i++ {
cnt.Add(ctx, 1, labs)
cnt.Add(ctx, 1, labs...)
}
}

Expand All @@ -109,13 +109,13 @@ func BenchmarkGlobalInt64CounterAddWithSDK(b *testing.B) {

global.SetMeterProvider(fix)

labs := sdk.Labels(key.String("A", "B"))
labs := metric.Labels(key.String("A", "B"))
cnt := Must(sdk).NewInt64Counter("int64.counter")

b.ResetTimer()

for i := 0; i < b.N; i++ {
cnt.Add(ctx, 1, labs)
cnt.Add(ctx, 1, labs...)
}
}

Expand Down
52 changes: 4 additions & 48 deletions api/global/internal/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ import (
// provider. Mutexes in the Provider and Meters ensure that each
// instrument has a delegate before the global provider is set.
//
// LabelSets are implemented by delegating to the Meter instance using
// the metric.LabelSetDelegator interface.
//
// Bound instrument operations are implemented by delegating to the
// instrument after it is registered, with a sync.Once initializer to
// protect against races with Release().
Expand Down Expand Up @@ -100,28 +97,17 @@ type AsyncImpler interface {
AsyncImpl() metric.AsyncImpl
}

type labelSet struct {
delegate unsafe.Pointer // (* metric.LabelSet)

meter *meter
value []core.KeyValue

initialize sync.Once
}

type syncHandle struct {
delegate unsafe.Pointer // (*metric.HandleImpl)

inst *syncImpl
labels metric.LabelSet
labels []core.KeyValue

initialize sync.Once
}

var _ metric.Provider = &meterProvider{}
var _ metric.Meter = &meter{}
var _ metric.LabelSet = &labelSet{}
var _ metric.LabelSetDelegate = &labelSet{}
var _ metric.InstrumentImpl = &syncImpl{}
var _ metric.BoundSyncImpl = &syncHandle{}
var _ metric.AsyncImpl = &asyncImpl{}
Expand Down Expand Up @@ -254,7 +240,7 @@ func (inst *syncImpl) Implementation() interface{} {
return inst
}

func (inst *syncImpl) Bind(labels metric.LabelSet) metric.BoundSyncImpl {
func (inst *syncImpl) Bind(labels []core.KeyValue) metric.BoundSyncImpl {
if implPtr := (*metric.SyncImpl)(atomic.LoadPointer(&inst.delegate)); implPtr != nil {
return (*implPtr).Bind(labels)
}
Expand Down Expand Up @@ -340,13 +326,13 @@ func (obs *asyncImpl) setDelegate(d metric.Meter) {

// Metric updates

func (m *meter) RecordBatch(ctx context.Context, labels metric.LabelSet, measurements ...metric.Measurement) {
func (m *meter) RecordBatch(ctx context.Context, labels []core.KeyValue, measurements ...metric.Measurement) {
if delegatePtr := (*metric.Meter)(atomic.LoadPointer(&m.delegate)); delegatePtr != nil {
(*delegatePtr).RecordBatch(ctx, labels, measurements...)
}
}

func (inst *syncImpl) RecordOne(ctx context.Context, number core.Number, labels metric.LabelSet) {
func (inst *syncImpl) RecordOne(ctx context.Context, number core.Number, labels []core.KeyValue) {
if instPtr := (*metric.SyncImpl)(atomic.LoadPointer(&inst.delegate)); instPtr != nil {
(*instPtr).RecordOne(ctx, number, labels)
}
Expand Down Expand Up @@ -377,35 +363,6 @@ func (bound *syncHandle) RecordOne(ctx context.Context, number core.Number) {
(*implPtr).RecordOne(ctx, number)
}

// LabelSet initialization

func (m *meter) Labels(labels ...core.KeyValue) metric.LabelSet {
return &labelSet{
meter: m,
value: labels,
}
}

func (labels *labelSet) Delegate() metric.LabelSet {
meterPtr := (*metric.Meter)(atomic.LoadPointer(&labels.meter.delegate))
if meterPtr == nil {
// This is technically impossible, provided the global
// Meter is updated after the meters and instruments
// have been delegated.
return labels
}
var implPtr *metric.LabelSet
labels.initialize.Do(func() {
implPtr = new(metric.LabelSet)
*implPtr = (*meterPtr).Labels(labels.value...)
atomic.StorePointer(&labels.delegate, unsafe.Pointer(implPtr))
})
if implPtr == nil {
implPtr = (*metric.LabelSet)(atomic.LoadPointer(&labels.delegate))
}
return (*implPtr)
}

// Constructors

func (m *meter) withName(opts []metric.Option) []metric.Option {
Expand Down Expand Up @@ -466,7 +423,6 @@ func AtomicFieldOffsets() map[string]uintptr {
"meter.delegate": unsafe.Offsetof(meter{}.delegate),
"syncImpl.delegate": unsafe.Offsetof(syncImpl{}.delegate),
"asyncImpl.delegate": unsafe.Offsetof(asyncImpl{}.delegate),
"labelSet.delegate": unsafe.Offsetof(labelSet{}.delegate),
"syncHandle.delegate": unsafe.Offsetof(syncHandle{}.delegate),
}
}
84 changes: 39 additions & 45 deletions api/global/internal/meter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func asStructs(batches []metrictest.Batch) []measured {
r = append(r, measured{
Name: m.Instrument.Descriptor().Name(),
LibraryName: m.Instrument.Descriptor().LibraryName(),
Labels: batch.LabelSet.Labels,
Labels: asMap(batch.Labels...),
Number: m.Number,
})
}
Expand All @@ -72,41 +72,38 @@ func TestDirect(t *testing.T) {
ctx := context.Background()
meter1 := global.Meter("test1")
meter2 := global.Meter("test2")
lvals1 := key.String("A", "B")
labels1 := meter1.Labels(lvals1)
lvals2 := key.String("C", "D")
labels2 := meter1.Labels(lvals2)
lvals3 := key.String("E", "F")
labels3 := meter2.Labels(lvals3)
labels1 := metric.Labels(key.String("A", "B"))
labels2 := metric.Labels(key.String("C", "D"))
labels3 := metric.Labels(key.String("E", "F"))

counter := Must(meter1).NewInt64Counter("test.counter")
counter.Add(ctx, 1, labels1)
counter.Add(ctx, 1, labels1)
counter.Add(ctx, 1, labels1...)
counter.Add(ctx, 1, labels1...)

measure := Must(meter1).NewFloat64Measure("test.measure")
measure.Record(ctx, 1, labels1)
measure.Record(ctx, 2, labels1)
measure.Record(ctx, 1, labels1...)
measure.Record(ctx, 2, labels1...)

_ = Must(meter1).RegisterFloat64Observer("test.observer.float", func(result metric.Float64ObserverResult) {
result.Observe(1., labels1)
result.Observe(2., labels2)
result.Observe(1., labels1...)
result.Observe(2., labels2...)
})

_ = Must(meter1).RegisterInt64Observer("test.observer.int", func(result metric.Int64ObserverResult) {
result.Observe(1, labels1)
result.Observe(2, labels2)
result.Observe(1, labels1...)
result.Observe(2, labels2...)
})

second := Must(meter2).NewFloat64Measure("test.second")
second.Record(ctx, 1, labels3)
second.Record(ctx, 2, labels3)
second.Record(ctx, 1, labels3...)
second.Record(ctx, 2, labels3...)

mock, provider := metrictest.NewProvider()
global.SetMeterProvider(provider)

counter.Add(ctx, 1, labels1)
measure.Record(ctx, 3, labels1)
second.Record(ctx, 3, labels3)
counter.Add(ctx, 1, labels1...)
measure.Record(ctx, 3, labels1...)
second.Record(ctx, 3, labels3...)

mock.RunAsyncInstruments()

Expand All @@ -117,43 +114,43 @@ func TestDirect(t *testing.T) {
{
Name: "test.counter",
LibraryName: "test1",
Labels: asMap(lvals1),
Labels: asMap(labels1...),
Number: asInt(1),
},
{
Name: "test.measure",
LibraryName: "test1",
Labels: asMap(lvals1),
Labels: asMap(labels1...),
Number: asFloat(3),
},
{
Name: "test.second",
LibraryName: "test2",
Labels: asMap(lvals3),
Labels: asMap(labels3...),
Number: asFloat(3),
},
{
Name: "test.observer.float",
LibraryName: "test1",
Labels: asMap(lvals1),
Labels: asMap(labels1...),
Number: asFloat(1),
},
{
Name: "test.observer.float",
LibraryName: "test1",
Labels: asMap(lvals2),
Labels: asMap(labels2...),
Number: asFloat(2),
},
{
Name: "test.observer.int",
LibraryName: "test1",
Labels: asMap(lvals1),
Labels: asMap(labels1...),
Number: asInt(1),
},
{
Name: "test.observer.int",
LibraryName: "test1",
Labels: asMap(lvals2),
Labels: asMap(labels2...),
Number: asInt(2),
},
},
Expand All @@ -168,16 +165,15 @@ func TestBound(t *testing.T) {
// vs. the above, to cover all the instruments.
ctx := context.Background()
glob := global.Meter("test")
lvals1 := key.String("A", "B")
labels1 := glob.Labels(lvals1)
labels1 := metric.Labels(key.String("A", "B"))

counter := Must(glob).NewFloat64Counter("test.counter")
boundC := counter.Bind(labels1)
boundC := counter.Bind(labels1...)
boundC.Add(ctx, 1)
boundC.Add(ctx, 1)

measure := Must(glob).NewInt64Measure("test.measure")
boundM := measure.Bind(labels1)
boundM := measure.Bind(labels1...)
boundM.Record(ctx, 1)
boundM.Record(ctx, 2)

Expand All @@ -192,13 +188,13 @@ func TestBound(t *testing.T) {
{
Name: "test.counter",
LibraryName: "test",
Labels: asMap(lvals1),
Labels: asMap(labels1...),
Number: asFloat(1),
},
{
Name: "test.measure",
LibraryName: "test",
Labels: asMap(lvals1),
Labels: asMap(labels1...),
Number: asInt(3),
},
},
Expand All @@ -213,14 +209,13 @@ func TestUnbind(t *testing.T) {
internal.ResetForTest()

glob := global.Meter("test")
lvals1 := key.New("A").String("B")
labels1 := glob.Labels(lvals1)
labels1 := metric.Labels(key.String("A", "B"))

counter := Must(glob).NewFloat64Counter("test.counter")
boundC := counter.Bind(labels1)
boundC := counter.Bind(labels1...)

measure := Must(glob).NewInt64Measure("test.measure")
boundM := measure.Bind(labels1)
boundM := measure.Bind(labels1...)

boundC.Unbind()
boundM.Unbind()
Expand All @@ -231,12 +226,11 @@ func TestDefaultSDK(t *testing.T) {

ctx := context.Background()
meter1 := global.Meter("builtin")
lvals1 := key.String("A", "B")
labels1 := meter1.Labels(lvals1)
labels1 := metric.Labels(key.String("A", "B"))

counter := Must(meter1).NewInt64Counter("test.builtin")
counter.Add(ctx, 1, labels1)
counter.Add(ctx, 1, labels1)
counter.Add(ctx, 1, labels1...)
counter.Add(ctx, 1, labels1...)

in, out := io.Pipe()
pusher, err := stdout.InstallNewPipeline(stdout.Config{
Expand All @@ -247,7 +241,7 @@ func TestDefaultSDK(t *testing.T) {
panic(err)
}

counter.Add(ctx, 1, labels1)
counter.Add(ctx, 1, labels1...)

ch := make(chan string)
go func() {
Expand All @@ -270,7 +264,7 @@ func TestUnbindThenRecordOne(t *testing.T) {

meter := global.Meter("test")
counter := Must(meter).NewInt64Counter("test.counter")
boundC := counter.Bind(meter.Labels())
boundC := counter.Bind()
global.SetMeterProvider(provider)
boundC.Unbind()

Expand Down Expand Up @@ -312,8 +306,8 @@ func TestErrorInDeferredConstructor(t *testing.T) {
global.SetMeterProvider(sdk)
})

c1.Add(ctx, 1, meter.Labels())
c2.Add(ctx, 2, meter.Labels())
c1.Add(ctx, 1)
c2.Add(ctx, 2)
}

func TestImplementationIndirection(t *testing.T) {
Expand Down
19 changes: 11 additions & 8 deletions api/metric/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,16 @@ type Provider interface {
Meter(name string) Meter
}

// LabelSet is an implementation-level interface that represents a
// []core.KeyValue for use as pre-defined labels in the metrics API.
type LabelSet interface {
// Labels is a convenience method to build a []core.KeyValue{}. For example,
jmacd marked this conversation as resolved.
Show resolved Hide resolved
//
// meter.RecordBatch(ctx, metric.Labels(...), ...)
//
// as opposed to:
//
// meter.RecordBatch(ctx, []core.KeyValue{...}, ...)
//
func Labels(labels ...core.KeyValue) []core.KeyValue {
return labels
}

// Config contains some options for metrics of any kind.
Expand Down Expand Up @@ -161,12 +168,8 @@ func (d Descriptor) LibraryName() string {

// Meter is an interface to the metrics portion of the OpenTelemetry SDK.
type Meter interface {
// Labels returns a reference to a set of labels that cannot
// be read by the application.
Labels(...core.KeyValue) LabelSet

// RecordBatch atomically records a batch of measurements.
RecordBatch(context.Context, LabelSet, ...Measurement)
RecordBatch(context.Context, []core.KeyValue, ...Measurement)

// All instrument constructors may return an error for
// conditions such as:
Expand Down
Loading