Skip to content

Commit

Permalink
Remove metric options; rename "counter" aggregator to "sum" (#541)
Browse files Browse the repository at this point in the history 
* Remove options (mostly)

* Rename counter aggregator to 'sum'

* Fix prometheus test

* Rewordings from feedback
  • Loading branch information
@jmacd
jmacd authored Mar 12, 2020
1 parent d9210f5 commit 23e65ac
Show file tree
Hide file tree
Showing 36 changed files with 364 additions and 861 deletions.
4 changes: 2 additions & 2 deletions api/global/internal/benchmark_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (
"go.opentelemetry.io/otel/api/trace"
export "go.opentelemetry.io/otel/sdk/export/metric"
sdk "go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregator/counter"
"go.opentelemetry.io/otel/sdk/metric/aggregator/ddsketch"
"go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount"
"go.opentelemetry.io/otel/sdk/metric/aggregator/sum"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
)

Expand All @@ -41,7 +41,7 @@ func newFixture(b *testing.B) *benchFixture {
func (*benchFixture) AggregatorFor(descriptor *export.Descriptor) export.Aggregator {
switch descriptor.MetricKind() {
case export.CounterKind:
return counter.New()
return sum.New()
case export.MeasureKind:
if strings.HasSuffix(descriptor.Name(), "minmaxsumcount") {
return minmaxsumcount.New(descriptor)
Expand Down
36 changes: 18 additions & 18 deletions api/global/internal/meter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ type instImpl struct {
name string
mkind metricKind
nkind core.NumberKind
opts interface{}
opts []metric.Option
}

type obsImpl struct {
delegate unsafe.Pointer // (*metric.Int64Observer or *metric.Float64Observer)

name string
nkind core.NumberKind
opts []metric.ObserverOptionApplier
opts []metric.Option
meter *meter
callback interface{}
}
Expand Down Expand Up @@ -181,7 +181,7 @@ func (m *meter) setDelegate(provider metric.Provider) {
m.orderedObservers = nil
}

func (m *meter) newInst(name string, mkind metricKind, nkind core.NumberKind, opts interface{}) (metric.InstrumentImpl, error) {
func (m *meter) newInst(name string, mkind metricKind, nkind core.NumberKind, opts []metric.Option) (metric.InstrumentImpl, error) {
m.lock.Lock()
defer m.lock.Unlock()

Expand Down Expand Up @@ -209,18 +209,18 @@ func delegateCheck(has hasImpl, err error) (metric.InstrumentImpl, error) {
return nil, err
}

func newInstDelegate(m metric.Meter, name string, mkind metricKind, nkind core.NumberKind, opts interface{}) (metric.InstrumentImpl, error) {
func newInstDelegate(m metric.Meter, name string, mkind metricKind, nkind core.NumberKind, opts []metric.Option) (metric.InstrumentImpl, error) {
switch mkind {
case counterKind:
if nkind == core.Int64NumberKind {
return delegateCheck(m.NewInt64Counter(name, opts.([]metric.CounterOptionApplier)...))
return delegateCheck(m.NewInt64Counter(name, opts...))
}
return delegateCheck(m.NewFloat64Counter(name, opts.([]metric.CounterOptionApplier)...))
return delegateCheck(m.NewFloat64Counter(name, opts...))
case measureKind:
if nkind == core.Int64NumberKind {
return delegateCheck(m.NewInt64Measure(name, opts.([]metric.MeasureOptionApplier)...))
return delegateCheck(m.NewInt64Measure(name, opts...))
}
return delegateCheck(m.NewFloat64Measure(name, opts.([]metric.MeasureOptionApplier)...))
return delegateCheck(m.NewFloat64Measure(name, opts...))
}
return nil, errInvalidMetricKind
}
Expand Down Expand Up @@ -419,34 +419,34 @@ func (labels *labelSet) Delegate() metric.LabelSet {

// Constructors

func (m *meter) NewInt64Counter(name string, opts ...metric.CounterOptionApplier) (metric.Int64Counter, error) {
func (m *meter) NewInt64Counter(name string, opts ...metric.Option) (metric.Int64Counter, error) {
return metric.WrapInt64CounterInstrument(m.newInst(name, counterKind, core.Int64NumberKind, opts))
}

func (m *meter) NewFloat64Counter(name string, opts ...metric.CounterOptionApplier) (metric.Float64Counter, error) {
func (m *meter) NewFloat64Counter(name string, opts ...metric.Option) (metric.Float64Counter, error) {
return metric.WrapFloat64CounterInstrument(m.newInst(name, counterKind, core.Float64NumberKind, opts))
}

func (m *meter) NewInt64Measure(name string, opts ...metric.MeasureOptionApplier) (metric.Int64Measure, error) {
func (m *meter) NewInt64Measure(name string, opts ...metric.Option) (metric.Int64Measure, error) {
return metric.WrapInt64MeasureInstrument(m.newInst(name, measureKind, core.Int64NumberKind, opts))
}

func (m *meter) NewFloat64Measure(name string, opts ...metric.MeasureOptionApplier) (metric.Float64Measure, error) {
func (m *meter) NewFloat64Measure(name string, opts ...metric.Option) (metric.Float64Measure, error) {
return metric.WrapFloat64MeasureInstrument(m.newInst(name, measureKind, core.Float64NumberKind, opts))
}

func (m *meter) RegisterInt64Observer(name string, callback metric.Int64ObserverCallback, oos ...metric.ObserverOptionApplier) (metric.Int64Observer, error) {
func (m *meter) RegisterInt64Observer(name string, callback metric.Int64ObserverCallback, opts ...metric.Option) (metric.Int64Observer, error) {
m.lock.Lock()
defer m.lock.Unlock()

if meterPtr := (*metric.Meter)(atomic.LoadPointer(&m.delegate)); meterPtr != nil {
return (*meterPtr).RegisterInt64Observer(name, callback, oos...)
return (*meterPtr).RegisterInt64Observer(name, callback, opts...)
}

obs := &obsImpl{
name: name,
nkind: core.Int64NumberKind,
opts: oos,
opts: opts,
meter: m,
callback: callback,
}
Expand All @@ -456,18 +456,18 @@ func (m *meter) RegisterInt64Observer(name string, callback metric.Int64Observer
}, nil
}

func (m *meter) RegisterFloat64Observer(name string, callback metric.Float64ObserverCallback, oos ...metric.ObserverOptionApplier) (metric.Float64Observer, error) {
func (m *meter) RegisterFloat64Observer(name string, callback metric.Float64ObserverCallback, opts ...metric.Option) (metric.Float64Observer, error) {
m.lock.Lock()
defer m.lock.Unlock()

if meterPtr := (*metric.Meter)(atomic.LoadPointer(&m.delegate)); meterPtr != nil {
return (*meterPtr).RegisterFloat64Observer(name, callback, oos...)
return (*meterPtr).RegisterFloat64Observer(name, callback, opts...)
}

obs := &obsImpl{
name: name,
nkind: core.Float64NumberKind,
opts: oos,
opts: opts,
meter: m,
callback: callback,
}
Expand Down
2 changes: 1 addition & 1 deletion api/global/internal/meter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func (m *meterProviderWithConstructorError) Meter(name string) metric.Meter {
return &meterWithConstructorError{m.Provider.Meter(name)}
}

func (m *meterWithConstructorError) NewInt64Counter(name string, cos ...metric.CounterOptionApplier) (metric.Int64Counter, error) {
func (m *meterWithConstructorError) NewInt64Counter(name string, opts ...metric.Option) (metric.Int64Counter, error) {
return metric.Int64Counter{}, errors.New("constructor error")
}

Expand Down
190 changes: 29 additions & 161 deletions api/metric/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ type Provider interface {
type LabelSet interface {
}

// Options contains some options for metrics of any kind.
type Options struct {
// Config contains some options for metrics of any kind.
type Config struct {
// Description is an optional field describing the metric
// instrument.
Description string
Expand All @@ -43,42 +43,12 @@ type Options struct {
// Keys are recommended keys determined in the handles
// obtained for the metric.
Keys []core.Key
// Alternate defines the property of metric value dependent on
// a metric type.
//
// - for Counter, true implies that the metric is an up-down
// Counter
//
// - for Measure, true implies that the metric supports
// positive and negative values
//
// - for Observer, true implies that the metric is a
// non-descending Observer
Alternate bool
}

// CounterOptionApplier is an interface for applying metric options
// that are valid only for counter metrics.
type CounterOptionApplier interface {
// ApplyCounterOption is used to make some general or
// counter-specific changes in the Options.
ApplyCounterOption(*Options)
}

// MeasureOptionApplier is an interface for applying metric options
// that are valid only for measure metrics.
type MeasureOptionApplier interface {
// ApplyMeasureOption is used to make some general or
// measure-specific changes in the Options.
ApplyMeasureOption(*Options)
}

// ObserverOptionApplier is an interface for applying metric options
// that are valid only for observer metrics.
type ObserverOptionApplier interface {
// ApplyObserverOption is used to make some general or
// observer-specific changes in the Options.
ApplyObserverOption(*Options)
// Option is an interface for applying metric options.
type Option interface {
// Apply is used to set the Option value of a Config.
Apply(*Config)
}

// Measurement is used for reporting a batch of metric
Expand Down Expand Up @@ -119,25 +89,25 @@ type Meter interface {

// NewInt64Counter creates a new integral counter with a given
// name and customized with passed options.
NewInt64Counter(name string, cos ...CounterOptionApplier) (Int64Counter, error)
NewInt64Counter(name string, opts ...Option) (Int64Counter, error)
// NewFloat64Counter creates a new floating point counter with
// a given name and customized with passed options.
NewFloat64Counter(name string, cos ...CounterOptionApplier) (Float64Counter, error)
NewFloat64Counter(name string, opts ...Option) (Float64Counter, error)
// NewInt64Measure creates a new integral measure with a given
// name and customized with passed options.
NewInt64Measure(name string, mos ...MeasureOptionApplier) (Int64Measure, error)
NewInt64Measure(name string, opts ...Option) (Int64Measure, error)
// NewFloat64Measure creates a new floating point measure with
// a given name and customized with passed options.
NewFloat64Measure(name string, mos ...MeasureOptionApplier) (Float64Measure, error)
NewFloat64Measure(name string, opts ...Option) (Float64Measure, error)

// RegisterInt64Observer creates a new integral observer with a
// given name, running a given callback, and customized with passed
// options. Callback can be nil.
RegisterInt64Observer(name string, callback Int64ObserverCallback, oos ...ObserverOptionApplier) (Int64Observer, error)
RegisterInt64Observer(name string, callback Int64ObserverCallback, opts ...Option) (Int64Observer, error)
// RegisterFloat64Observer creates a new floating point observer
// with a given name, running a given callback, and customized with
// passed options. Callback can be nil.
RegisterFloat64Observer(name string, callback Float64ObserverCallback, oos ...ObserverOptionApplier) (Float64Observer, error)
RegisterFloat64Observer(name string, callback Float64ObserverCallback, opts ...Option) (Float64Observer, error)
}

// Int64ObserverResult is an interface for reporting integral
Expand Down Expand Up @@ -172,138 +142,36 @@ type Float64Observer interface {
Unregister()
}

// Option supports specifying the various metric options.
type Option func(*Options)

// OptionApplier is an interface for applying metric options that are
// valid for all the kinds of metrics.
type OptionApplier interface {
CounterOptionApplier
MeasureOptionApplier
ObserverOptionApplier
// ApplyOption is used to make some general changes in the
// Options.
ApplyOption(*Options)
}

// CounterObserverOptionApplier is an interface for applying metric
// options that are valid for counter or observer metrics.
type CounterObserverOptionApplier interface {
CounterOptionApplier
ObserverOptionApplier
}

type optionWrapper struct {
F Option
}

type counterOptionWrapper struct {
F Option
}

type measureOptionWrapper struct {
F Option
}

type observerOptionWrapper struct {
F Option
}

type counterObserverOptionWrapper struct {
FC Option
FO Option
}

var (
_ OptionApplier = optionWrapper{}
_ CounterOptionApplier = counterOptionWrapper{}
_ MeasureOptionApplier = measureOptionWrapper{}
_ ObserverOptionApplier = observerOptionWrapper{}
)

func (o optionWrapper) ApplyCounterOption(opts *Options) {
o.ApplyOption(opts)
}

func (o optionWrapper) ApplyMeasureOption(opts *Options) {
o.ApplyOption(opts)
}

func (o optionWrapper) ApplyObserverOption(opts *Options) {
o.ApplyOption(opts)
}

func (o optionWrapper) ApplyOption(opts *Options) {
o.F(opts)
}

func (o counterOptionWrapper) ApplyCounterOption(opts *Options) {
o.F(opts)
// WithDescription applies provided description.
func WithDescription(desc string) Option {
return descriptionOption(desc)
}

func (o measureOptionWrapper) ApplyMeasureOption(opts *Options) {
o.F(opts)
}
type descriptionOption string

func (o counterObserverOptionWrapper) ApplyCounterOption(opts *Options) {
o.FC(opts)
func (d descriptionOption) Apply(config *Config) {
config.Description = string(d)
}

func (o counterObserverOptionWrapper) ApplyObserverOption(opts *Options) {
o.FO(opts)
// WithUnit applies provided unit.
func WithUnit(unit unit.Unit) Option {
return unitOption(unit)
}

func (o observerOptionWrapper) ApplyObserverOption(opts *Options) {
o.F(opts)
}
type unitOption unit.Unit

// WithDescription applies provided description.
func WithDescription(desc string) OptionApplier {
return optionWrapper{
F: func(opts *Options) {
opts.Description = desc
},
}
}

// WithUnit applies provided unit.
func WithUnit(unit unit.Unit) OptionApplier {
return optionWrapper{
F: func(opts *Options) {
opts.Unit = unit
},
}
func (u unitOption) Apply(config *Config) {
config.Unit = unit.Unit(u)
}

// WithKeys applies recommended label keys. Multiple `WithKeys`
// options accumulate.
func WithKeys(keys ...core.Key) OptionApplier {
return optionWrapper{
F: func(opts *Options) {
opts.Keys = append(opts.Keys, keys...)
},
}
func WithKeys(keys ...core.Key) Option {
return keysOption(keys)
}

// WithMonotonic sets whether a counter or an observer is not
// permitted to go down.
func WithMonotonic(monotonic bool) CounterObserverOptionApplier {
return counterObserverOptionWrapper{
FC: func(opts *Options) {
opts.Alternate = !monotonic
},
FO: func(opts *Options) {
opts.Alternate = monotonic
},
}
}
type keysOption []core.Key

// WithAbsolute sets whether a measure is not permitted to be
// negative.
func WithAbsolute(absolute bool) MeasureOptionApplier {
return measureOptionWrapper{
F: func(opts *Options) {
opts.Alternate = !absolute
},
}
func (k keysOption) Apply(config *Config) {
config.Keys = append(config.Keys, k...)
}
Loading

0 comments on commit 23e65ac

Please sign in to comment.