From 039df12e9325d7d12cc8a25644b457b65ae9732f Mon Sep 17 00:00:00 2001 From: costin Date: Sun, 7 May 2023 13:11:56 -0700 Subject: [PATCH 1/4] Initial refactoring to separate OC --- monitoring/monitoring.go | 350 +++++----------------------- monitoring/monitoring_expvar.go | 98 ++++++++ monitoring/monitoring_opencensus.go | 320 +++++++++++++++++++++++++ 3 files changed, 474 insertions(+), 294 deletions(-) create mode 100644 monitoring/monitoring_expvar.go create mode 100644 monitoring/monitoring_opencensus.go diff --git a/monitoring/monitoring.go b/monitoring/monitoring.go index 4e7deb5a..96c20b27 100644 --- a/monitoring/monitoring.go +++ b/monitoring/monitoring.go @@ -15,21 +15,13 @@ package monitoring import ( - "context" - "fmt" - "math" "sync" - - "go.opencensus.io/metric" - "go.opencensus.io/metric/metricdata" - "go.opencensus.io/metric/metricproducer" - "go.opencensus.io/stats" - "go.opencensus.io/stats/view" - "go.opencensus.io/tag" - - "istio.io/pkg/log" ) +// OpenCensus independent metrics +// Removed unused code: +// - Metric.Decrement + type ( // A Metric collects numerical observations. Metric interface { @@ -43,25 +35,42 @@ type ( // this is equivalent to subtracting -1 to the current value. For Gauges, // this is equivalent to setting the value to -1. For Distributions, // this is equivalent to making an observation of value -1. - Decrement() + // + // Not used in istio, removed pending new interface. + //Decrement() // Name returns the name value of a Metric. + // TODO: internal use only Name() string // Record makes an observation of the provided value for the given measure. + // Majority of Istio is setting this to an int Record(value float64) // RecordInt makes an observation of the provided value for the measure. - RecordInt(value int64) + // Not actually used in Istio. + //RecordInt(value int64) + + // Prometheus uses ConterVec and With(map[string]string) returning Counter + // + // Expvar doesn't support labels - but can be used in the name, creating a new expvar in a map + // The labels are really made part of the name when exporting and is the name of the TS + // + // Otel uses attribute.Int("name", val) and similar - from the stable package - + // but doesn't create a new Metric, it is an option to Add(). + // + // Istio only uses string/string - so using the pattern from slog works. + // Once we adopt slog, we can start using slog.Attr pattern // With creates a new Metric, with the LabelValues provided. This allows creating // a set of pre-dimensioned data for recording purposes. This is primarily used // for documentation and convenience. Metrics created with this method do not need // to be registered (they share the registration of their parent Metric). - With(labelValues ...LabelValue) Metric + With(labelValues ...string) Metric // Register configures the Metric for export. It MUST be called before collection // of values for the Metric. An error will be returned if registration fails. + // TODO: internal use only Register() error } @@ -96,15 +105,11 @@ type ( DerivedOptions func(*derivedOptions) // A Label provides a named dimension for a Metric. - Label tag.Key - - // A LabelValue represents a Label with a specific value. It is used to record - // values for a Metric. - LabelValue tag.Mutator + //Label Key options struct { unit Unit - labels []Label + labels []string // Label useInt64 bool } @@ -112,14 +117,11 @@ type ( labelKeys []string valueFn func() float64 } - - // RecordHook has a callback function which a measure is recorded. - RecordHook interface { - OnRecordFloat64Measure(f *stats.Float64Measure, tags []tag.Mutator, value float64) - OnRecordInt64Measure(i *stats.Int64Measure, tags []tag.Mutator, value int64) - } ) +func (dm *disabledMetric) ValueFrom(valueFn func() float64, labelValues ...string) { +} + // Decrement implements Metric func (dm *disabledMetric) Decrement() {} @@ -143,35 +145,19 @@ func (dm *disabledMetric) Register() error { } // With implements Metric -func (dm *disabledMetric) With(labelValues ...LabelValue) Metric { +func (dm *disabledMetric) With(labelValues ...string) Metric { return dm } var _ Metric = &disabledMetric{} var ( - recordHooks map[string]RecordHook recordHookMutex sync.RWMutex - - derivedRegistry = metric.NewRegistry() ) -func init() { - recordHooks = make(map[string]RecordHook) - // ensures exporters can see any derived metrics - metricproducer.GlobalManager().AddProducer(derivedRegistry) -} - -// RegisterRecordHook adds a RecordHook for a given measure. -func RegisterRecordHook(name string, h RecordHook) { - recordHookMutex.Lock() - defer recordHookMutex.Unlock() - recordHooks[name] = h -} - // WithLabels provides configuration options for a new Metric, providing the expected // dimensions for data collection for that Metric. -func WithLabels(labels ...Label) Options { +func WithLabels(labels ...string) Options { return func(opts *options) { opts.labels = labels } @@ -212,20 +198,20 @@ func WithValueFrom(valueFn func() float64) DerivedOptions { } } -// Value creates a new LabelValue for the Label. -func (l Label) Value(value string) LabelValue { - return tag.Upsert(tag.Key(l), value) -} - -// MustCreateLabel will attempt to create a new Label. If -// creation fails, then this method will panic. -func MustCreateLabel(key string) Label { - k, err := tag.NewKey(key) - if err != nil { - panic(fmt.Errorf("could not create label %q: %v", key, err)) - } - return Label(k) -} +//// Value creates a new LabelValue for the Label. +//func (l Label) Value(value string) LabelValue { +// return tag.Upsert(tag.Key(l), value) +//} +// +//// MustCreateLabel will attempt to create a new Label. If +//// creation fails, then this method will panic. +//func MustCreateLabel(key string) Label { +// k, err := NewKey(key) +// if err != nil { +// panic(fmt.Errorf("could not create label %q: %v", key, err)) +// } +// return Label(k) +//} // MustRegister is a helper function that will ensure that the provided Metrics are // registered. If a metric fails to register, this method will panic. @@ -255,97 +241,37 @@ func RegisterIf(metric Metric, enabled func() bool) Metric { // NewSum creates a new Metric with an aggregation type of Sum (the values will be cumulative). // That means that data collected by the new Metric will be summed before export. func NewSum(name, description string, opts ...Options) Metric { - return newMetric(name, description, view.Sum(), opts...) + return newSum(name, description, opts...) } // NewGauge creates a new Metric with an aggregation type of LastValue. That means that data collected // by the new Metric will export only the last recorded value. func NewGauge(name, description string, opts ...Options) Metric { - return newMetric(name, description, view.LastValue(), opts...) + return newGauge(name, description, opts...) } // NewDerivedGauge creates a new Metric with an aggregation type of LastValue that generates the value // dynamically according to the provided function. This can be used for values based on querying some // state within a system (when event-driven recording is not appropriate). +// +// Only 2 usages (uptime and cache expiry in node agent) in istio func NewDerivedGauge(name, description string, opts ...DerivedOptions) DerivedMetric { - options := createDerivedOptions(opts...) - m, err := derivedRegistry.AddFloat64DerivedGauge(name, - metric.WithDescription(description), - metric.WithLabelKeys(options.labelKeys...), - metric.WithUnit(metricdata.UnitDimensionless)) // TODO: allow unit in options - if err != nil { - log.Warnf("failed to add metric %q: %v", name, err) - } - derived := &derivedFloat64Metric{ - base: m, - name: name, - } - if options.valueFn != nil { - derived.ValueFrom(options.valueFn) - } - return derived + return newDerivedGauge(name, description, opts...) } // NewDistribution creates a new Metric with an aggregation type of Distribution. This means that the // data collected by the Metric will be collected and exported as a histogram, with the specified bounds. func NewDistribution(name, description string, bounds []float64, opts ...Options) Metric { - return newMetric(name, description, view.Distribution(bounds...), opts...) -} - -func newMetric(name, description string, aggregation *view.Aggregation, opts ...Options) Metric { - o := createOptions(opts...) - if o.useInt64 { - return newInt64Metric(name, description, aggregation, o) - } - return newFloat64Metric(name, description, aggregation, o) -} - -type derivedFloat64Metric struct { - base *metric.Float64DerivedGauge - - name string -} - -func (d *derivedFloat64Metric) Name() string { - return d.name -} - -// no-op -func (d *derivedFloat64Metric) Register() error { - return nil -} - -func (d *derivedFloat64Metric) ValueFrom(valueFn func() float64, labelValues ...string) { - if len(labelValues) == 0 { - if err := d.base.UpsertEntry(valueFn); err != nil { - log.Errorf("failed to add value for derived metric %q: %v", d.name, err) - } - return - } - lv := make([]metricdata.LabelValue, 0, len(labelValues)) - for _, l := range labelValues { - lv = append(lv, metricdata.NewLabelValue(l)) - } - if err := d.base.UpsertEntry(valueFn, lv...); err != nil { - log.Errorf("failed to add value for derived metric %q: %v", d.name, err) - } + return newDistribution(name, description, bounds, opts...) } -type float64Metric struct { - *stats.Float64Measure - - // tags stores all tags for the metrics - tags []tag.Mutator - // ctx is a precomputed context holding tags, as an optimization - ctx context.Context - view *view.View - - incrementMeasure []stats.Measurement - decrementMeasure []stats.Measurement -} +var newSum func(name, description string, opts ...Options) Metric +var newGauge func(name, description string, opts ...Options) Metric +var newDistribution func(name, description string, bounds []float64, opts ...Options) Metric +var newDerivedGauge func(name, description string, opts ...DerivedOptions) DerivedMetric func createOptions(opts ...Options) *options { - o := &options{unit: None, labels: make([]Label, 0)} + o := &options{unit: None, labels: make([]string, 0)} for _, opt := range opts { opt(o) } @@ -364,167 +290,3 @@ func createDerivedOptions(opts ...DerivedOptions) *derivedOptions { } return o } - -func newFloat64Metric(name, description string, aggregation *view.Aggregation, opts *options) *float64Metric { - measure := stats.Float64(name, description, string(opts.unit)) - tagKeys := make([]tag.Key, 0, len(opts.labels)) - for _, l := range opts.labels { - tagKeys = append(tagKeys, tag.Key(l)) - } - ctx, _ := tag.New(context.Background()) //nolint:errcheck - return &float64Metric{ - Float64Measure: measure, - tags: make([]tag.Mutator, 0), - ctx: ctx, - view: &view.View{Measure: measure, TagKeys: tagKeys, Aggregation: aggregation}, - incrementMeasure: []stats.Measurement{measure.M(1)}, - decrementMeasure: []stats.Measurement{measure.M(-1)}, - } -} - -func (f *float64Metric) Increment() { - f.recordMeasurements(f.incrementMeasure) -} - -func (f *float64Metric) Decrement() { - f.recordMeasurements(f.decrementMeasure) -} - -func (f *float64Metric) Name() string { - return f.Float64Measure.Name() -} - -func (f *float64Metric) Record(value float64) { - recordHookMutex.RLock() - if rh, ok := recordHooks[f.Name()]; ok { - rh.OnRecordFloat64Measure(f.Float64Measure, f.tags, value) - } - recordHookMutex.RUnlock() - m := f.M(value) - stats.Record(f.ctx, m) //nolint:errcheck -} - -func (f *float64Metric) recordMeasurements(m []stats.Measurement) { - recordHookMutex.RLock() - if rh, ok := recordHooks[f.Name()]; ok { - for _, mv := range m { - rh.OnRecordFloat64Measure(f.Float64Measure, f.tags, mv.Value()) - } - } - recordHookMutex.RUnlock() - stats.Record(f.ctx, m...) -} - -func (f *float64Metric) RecordInt(value int64) { - f.Record(float64(value)) -} - -func (f *float64Metric) With(labelValues ...LabelValue) Metric { - t := make([]tag.Mutator, len(f.tags), len(f.tags)+len(labelValues)) - copy(t, f.tags) - for _, tagValue := range labelValues { - t = append(t, tag.Mutator(tagValue)) - } - ctx, _ := tag.New(context.Background(), t...) //nolint:errcheck - return &float64Metric{ - Float64Measure: f.Float64Measure, - tags: t, - ctx: ctx, - view: f.view, - incrementMeasure: f.incrementMeasure, - decrementMeasure: f.decrementMeasure, - } -} - -func (f *float64Metric) Register() error { - return view.Register(f.view) -} - -type int64Metric struct { - *stats.Int64Measure - - // tags stores all tags for the metrics - tags []tag.Mutator - // ctx is a precomputed context holding tags, as an optimization - ctx context.Context - view *view.View - - // incrementMeasure is a precomputed +1 measurement to avoid extra allocations in Increment() - incrementMeasure []stats.Measurement - // decrementMeasure is a precomputed -1 measurement to avoid extra allocations in Decrement() - decrementMeasure []stats.Measurement -} - -func newInt64Metric(name, description string, aggregation *view.Aggregation, opts *options) *int64Metric { - measure := stats.Int64(name, description, string(opts.unit)) - tagKeys := make([]tag.Key, 0, len(opts.labels)) - for _, l := range opts.labels { - tagKeys = append(tagKeys, tag.Key(l)) - } - ctx, _ := tag.New(context.Background()) //nolint:errcheck - return &int64Metric{ - Int64Measure: measure, - tags: make([]tag.Mutator, 0), - ctx: ctx, - view: &view.View{Measure: measure, TagKeys: tagKeys, Aggregation: aggregation}, - incrementMeasure: []stats.Measurement{measure.M(1)}, - decrementMeasure: []stats.Measurement{measure.M(-1)}, - } -} - -func (i *int64Metric) Increment() { - i.recordMeasurements(i.incrementMeasure) -} - -func (i *int64Metric) Decrement() { - i.recordMeasurements(i.decrementMeasure) -} - -func (i *int64Metric) Name() string { - return i.Int64Measure.Name() -} - -func (i *int64Metric) Record(value float64) { - i.RecordInt(int64(math.Floor(value))) -} - -func (i *int64Metric) recordMeasurements(m []stats.Measurement) { - recordHookMutex.RLock() - if rh, ok := recordHooks[i.Name()]; ok { - for _, mv := range m { - rh.OnRecordInt64Measure(i.Int64Measure, i.tags, int64(math.Floor(mv.Value()))) - } - } - recordHookMutex.RUnlock() - stats.Record(i.ctx, m...) //nolint:errcheck -} - -func (i *int64Metric) RecordInt(value int64) { - recordHookMutex.RLock() - if rh, ok := recordHooks[i.Name()]; ok { - rh.OnRecordInt64Measure(i.Int64Measure, i.tags, value) - } - recordHookMutex.RUnlock() - stats.Record(i.ctx, i.M(value)) //nolint:errcheck -} - -func (i *int64Metric) With(labelValues ...LabelValue) Metric { - t := make([]tag.Mutator, len(i.tags), len(i.tags)+len(labelValues)) - copy(t, i.tags) - for _, tagValue := range labelValues { - t = append(t, tag.Mutator(tagValue)) - } - ctx, _ := tag.New(context.Background(), t...) //nolint:errcheck - return &int64Metric{ - Int64Measure: i.Int64Measure, - tags: t, - ctx: ctx, - view: i.view, - incrementMeasure: i.incrementMeasure, - decrementMeasure: i.decrementMeasure, - } -} - -func (i *int64Metric) Register() error { - return view.Register(i.view) -} diff --git a/monitoring/monitoring_expvar.go b/monitoring/monitoring_expvar.go new file mode 100644 index 00000000..dc03e283 --- /dev/null +++ b/monitoring/monitoring_expvar.go @@ -0,0 +1,98 @@ +//go:build !opencensus +// +build !opencensus + +package monitoring + +import "expvar" + +func init() { + newSum = newSumEV + newGauge = newGaugeEV + newDistribution = newDistributionEV + newDerivedGauge = newDerivedGaugeEV +} + +func newDerivedGaugeEV(name string, description string, opts ...DerivedOptions) DerivedMetric { + return &disabledMetric{} +} + +func newDistributionEV(name string, description string, bounds []float64, opts ...Options) Metric { + return &disabledMetric{} +} + +func newGaugeEV(name string, description string, opts ...Options) Metric { + return &expvarInt{name: name} +} + +func newSumEV(name string, description string, opts ...Options) Metric { + return &disabledMetric{} +} + +type expvarInt struct { + name string + expvar.Int +} + +func (dm *expvarInt) ValueFrom(valueFn func() float64, labelValues ...string) { +} + +// Decrement implements Metric +func (dm *expvarInt) Decrement() {} + +// Increment implements Metric +func (dm *expvarInt) Increment() {} + +// Name implements Metric +func (dm *expvarInt) Name() string { + return dm.name +} + +// Record implements Metric +func (dm *expvarInt) Record(value float64) {} + +// RecordInt implements Metric +func (dm *expvarInt) RecordInt(value int64) {} + +// Register implements Metric +func (dm *expvarInt) Register() error { + return nil +} + +// With implements Metric +func (dm *expvarInt) With(labelValues ...string) Metric { + return dm +} + +type expvarFloat struct { + name string +} + +func (dm *expvarFloat) ValueFrom(valueFn func() float64, labelValues ...string) { +} + +// Decrement implements Metric +func (dm *expvarFloat) Decrement() {} + +// Increment implements Metric +func (dm *expvarFloat) Increment() {} + +// Name implements Metric +func (dm *expvarFloat) Name() string { + return dm.name +} + +// Record implements Metric +func (dm *expvarFloat) Record(value float64) {} + +// RecordInt implements Metric +func (dm *expvarFloat) RecordInt(value int64) {} + +// Register implements Metric +func (dm *expvarFloat) Register() error { + return nil +} + +// With implements Metric +func (dm *expvarFloat) With(labelValues ...string) Metric { + return dm +} diff --git a/monitoring/monitoring_opencensus.go b/monitoring/monitoring_opencensus.go new file mode 100644 index 00000000..678d6897 --- /dev/null +++ b/monitoring/monitoring_opencensus.go @@ -0,0 +1,320 @@ +//go:build !opencensus +// +build !opencensus + +// Copyright 2019 Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package monitoring + +import ( + "context" + "go.opencensus.io/metric" + "go.opencensus.io/metric/metricdata" + "go.opencensus.io/metric/metricproducer" + "go.opencensus.io/stats" + "go.opencensus.io/stats/view" + "go.opencensus.io/tag" + "math" + + "istio.io/pkg/log" +) + +var ( + recordHooks map[string]RecordHook + derivedRegistry = metric.NewRegistry() +) + +// RecordHook has a callback function which a measure is recorded. +type RecordHook interface { + OnRecordFloat64Measure(f *stats.Float64Measure, tags []tag.Mutator, value float64) + OnRecordInt64Measure(i *stats.Int64Measure, tags []tag.Mutator, value int64) +} + +func init() { + recordHooks = make(map[string]RecordHook) + // ensures exporters can see any derived metrics + metricproducer.GlobalManager().AddProducer(derivedRegistry) + newSum = newSumOC + newGauge = newGaugeOC + newDistribution = newDistributionOC + newDerivedGauge = newDerivedGaugeOpenCensus +} + +// RegisterRecordHook adds a RecordHook for a given measure. +func RegisterRecordHook(name string, h RecordHook) { + recordHookMutex.Lock() + defer recordHookMutex.Unlock() + recordHooks[name] = h +} + +// NewDistribution creates a new Metric with an aggregation type of Distribution. This means that the +// data collected by the Metric will be collected and exported as a histogram, with the specified bounds. +func newDistributionOC(name, description string, bounds []float64, opts ...Options) Metric { + return newMetricOC(name, description, view.Distribution(bounds...), opts...) +} + +func newSumOC(name, description string, opts ...Options) Metric { + return newMetricOC(name, description, view.Sum(), opts...) +} + +// NewGauge creates a new Metric with an aggregation type of LastValue. That means that data collected +// by the new Metric will export only the last recorded value. +func newGaugeOC(name, description string, opts ...Options) Metric { + return newMetricOC(name, description, view.LastValue(), opts...) +} + +func newMetricOC(name, description string, aggregation *view.Aggregation, opts ...Options) Metric { + o := createOptions(opts...) + if o.useInt64 { + return newInt64Metric(name, description, aggregation, o) + } + return newFloat64Metric(name, description, aggregation, o) +} + +func newDerivedGaugeOpenCensus(name, description string, opts ...DerivedOptions) DerivedMetric { + options := createDerivedOptions(opts...) + m, err := derivedRegistry.AddFloat64DerivedGauge(name, + metric.WithDescription(description), + metric.WithLabelKeys(options.labelKeys...), + metric.WithUnit(metricdata.UnitDimensionless)) // TODO: allow unit in options + if err != nil { + log.Warnf("failed to add metric %q: %v", name, err) + } + derived := &derivedFloat64Metric{ + base: m, + name: name, + } + if options.valueFn != nil { + derived.ValueFrom(options.valueFn) + } + return derived +} + +type derivedFloat64Metric struct { + base *metric.Float64DerivedGauge + + name string +} + +func (d *derivedFloat64Metric) Name() string { + return d.name +} + +// no-op +func (d *derivedFloat64Metric) Register() error { + return nil +} + +func (d *derivedFloat64Metric) ValueFrom(valueFn func() float64, labelValues ...string) { + if len(labelValues) == 0 { + if err := d.base.UpsertEntry(valueFn); err != nil { + log.Errorf("failed to add value for derived metric %q: %v", d.name, err) + } + return + } + lv := make([]metricdata.LabelValue, 0, len(labelValues)) + for _, l := range labelValues { + lv = append(lv, metricdata.NewLabelValue(l)) + } + if err := d.base.UpsertEntry(valueFn, lv...); err != nil { + log.Errorf("failed to add value for derived metric %q: %v", d.name, err) + } +} + +type float64Metric struct { + *stats.Float64Measure + + // tags stores all tags for the metrics + tags []tag.Mutator + // ctx is a precomputed context holding tags, as an optimization + ctx context.Context + view *view.View + + incrementMeasure []stats.Measurement + decrementMeasure []stats.Measurement +} + +func newFloat64Metric(name, description string, aggregation *view.Aggregation, opts *options) *float64Metric { + measure := stats.Float64(name, description, string(opts.unit)) + tagKeys := make([]tag.Key, 0, len(opts.labels)) + for _, l := range opts.labels { + tagKeys = append(tagKeys, tag.MustNewKey(l)) + } + ctx, _ := tag.New(context.Background()) //nolint:errcheck + return &float64Metric{ + Float64Measure: measure, + tags: make([]tag.Mutator, 0), + ctx: ctx, + view: &view.View{Measure: measure, TagKeys: tagKeys, Aggregation: aggregation}, + incrementMeasure: []stats.Measurement{measure.M(1)}, + decrementMeasure: []stats.Measurement{measure.M(-1)}, + } +} + +func (f *float64Metric) Increment() { + f.recordMeasurements(f.incrementMeasure) +} + +func (f *float64Metric) Decrement() { + f.recordMeasurements(f.decrementMeasure) +} + +func (f *float64Metric) Name() string { + return f.Float64Measure.Name() +} + +func (f *float64Metric) Record(value float64) { + recordHookMutex.RLock() + if rh, ok := recordHooks[f.Name()]; ok { + rh.OnRecordFloat64Measure(f.Float64Measure, f.tags, value) + } + recordHookMutex.RUnlock() + m := f.M(value) + stats.Record(f.ctx, m) //nolint:errcheck +} + +func (f *float64Metric) recordMeasurements(m []stats.Measurement) { + recordHookMutex.RLock() + if rh, ok := recordHooks[f.Name()]; ok { + for _, mv := range m { + rh.OnRecordFloat64Measure(f.Float64Measure, f.tags, mv.Value()) + } + } + recordHookMutex.RUnlock() + stats.Record(f.ctx, m...) +} + +func (f *float64Metric) RecordInt(value int64) { + f.Record(float64(value)) +} + +// A LabelValue represents a Label with a specific value. It is used to record +// values for a Metric. +type LabelValue tag.Mutator + +func toLabelValues(args ...string) []LabelValue { + return nil +} + +func (f *float64Metric) With(labelValues ...string) Metric { + t := make([]tag.Mutator, len(f.tags), len(f.tags)+len(labelValues)) + copy(t, f.tags) + lv := toLabelValues(labelValues...) + for _, tagValue := range lv { + t = append(t, tag.Mutator(tagValue)) + } + ctx, _ := tag.New(context.Background(), t...) //nolint:errcheck + return &float64Metric{ + Float64Measure: f.Float64Measure, + tags: t, + ctx: ctx, + view: f.view, + incrementMeasure: f.incrementMeasure, + decrementMeasure: f.decrementMeasure, + } +} + +func (f *float64Metric) Register() error { + return view.Register(f.view) +} + +type int64Metric struct { + *stats.Int64Measure + + // tags stores all tags for the metrics + tags []tag.Mutator + // ctx is a precomputed context holding tags, as an optimization + ctx context.Context + view *view.View + + // incrementMeasure is a precomputed +1 measurement to avoid extra allocations in Increment() + incrementMeasure []stats.Measurement + // decrementMeasure is a precomputed -1 measurement to avoid extra allocations in Decrement() + decrementMeasure []stats.Measurement +} + +func newInt64Metric(name, description string, aggregation *view.Aggregation, opts *options) *int64Metric { + measure := stats.Int64(name, description, string(opts.unit)) + tagKeys := make([]tag.Key, 0, len(opts.labels)) + for _, l := range opts.labels { + tagKeys = append(tagKeys, tag.MustNewKey(l)) + } + ctx, _ := tag.New(context.Background()) //nolint:errcheck + return &int64Metric{ + Int64Measure: measure, + tags: make([]tag.Mutator, 0), + ctx: ctx, + view: &view.View{Measure: measure, TagKeys: tagKeys, Aggregation: aggregation}, + incrementMeasure: []stats.Measurement{measure.M(1)}, + decrementMeasure: []stats.Measurement{measure.M(-1)}, + } +} + +func (i *int64Metric) Increment() { + i.recordMeasurements(i.incrementMeasure) +} + +func (i *int64Metric) Decrement() { + i.recordMeasurements(i.decrementMeasure) +} + +func (i *int64Metric) Name() string { + return i.Int64Measure.Name() +} + +func (i *int64Metric) Record(value float64) { + i.RecordInt(int64(math.Floor(value))) +} + +func (i *int64Metric) recordMeasurements(m []stats.Measurement) { + recordHookMutex.RLock() + if rh, ok := recordHooks[i.Name()]; ok { + for _, mv := range m { + rh.OnRecordInt64Measure(i.Int64Measure, i.tags, int64(math.Floor(mv.Value()))) + } + } + recordHookMutex.RUnlock() + stats.Record(i.ctx, m...) //nolint:errcheck +} + +func (i *int64Metric) RecordInt(value int64) { + recordHookMutex.RLock() + if rh, ok := recordHooks[i.Name()]; ok { + rh.OnRecordInt64Measure(i.Int64Measure, i.tags, value) + } + recordHookMutex.RUnlock() + stats.Record(i.ctx, i.M(value)) //nolint:errcheck +} + +func (i *int64Metric) With(labelValues ...string) Metric { + t := make([]tag.Mutator, len(i.tags), len(i.tags)+len(labelValues)) + copy(t, i.tags) + lv := toLabelValues(labelValues...) + for _, tagValue := range lv { + t = append(t, tag.Mutator(tagValue)) + } + ctx, _ := tag.New(context.Background(), t...) //nolint:errcheck + return &int64Metric{ + Int64Measure: i.Int64Measure, + tags: t, + ctx: ctx, + view: i.view, + incrementMeasure: i.incrementMeasure, + decrementMeasure: i.decrementMeasure, + } +} + +func (i *int64Metric) Register() error { + return view.Register(i.view) +} From 4319753f83b4589049da29b374f29dd82c83714c Mon Sep 17 00:00:00 2001 From: Costin Manolache Date: Mon, 8 May 2023 10:41:07 -0700 Subject: [PATCH 2/4] Separate opencensus from the core API and remove unused methods. --- monitoring/monitoring.go | 120 ++++++++++++++++++++------- monitoring/monitoring_opencensus.go | 121 ++++------------------------ 2 files changed, 104 insertions(+), 137 deletions(-) diff --git a/monitoring/monitoring.go b/monitoring/monitoring.go index 96c20b27..6bcf9049 100644 --- a/monitoring/monitoring.go +++ b/monitoring/monitoring.go @@ -15,6 +15,8 @@ package monitoring import ( + "fmt" + "strings" "sync" ) @@ -66,7 +68,7 @@ type ( // a set of pre-dimensioned data for recording purposes. This is primarily used // for documentation and convenience. Metrics created with this method do not need // to be registered (they share the registration of their parent Metric). - With(labelValues ...string) Metric + With(labelValues ...Attr) Metric // Register configures the Metric for export. It MUST be called before collection // of values for the Metric. An error will be returned if registration fails. @@ -91,7 +93,7 @@ type ( // ValueFrom may be called without any labelValues. Otherwise, the labelValues // supplied MUST match the label keys supplied at creation time both in number // and in order. - ValueFrom(valueFn func() float64, labelValues ...string) + ValueFrom(valueFn func() float64, labelValues ...Attr) } disabledMetric struct { @@ -108,18 +110,18 @@ type ( //Label Key options struct { - unit Unit - labels []string // Label - useInt64 bool + unit Unit + labels []Label // Label + //useInt64 bool } derivedOptions struct { labelKeys []string - valueFn func() float64 + //valueFn func() float64 } ) -func (dm *disabledMetric) ValueFrom(valueFn func() float64, labelValues ...string) { +func (dm *disabledMetric) ValueFrom(valueFn func() float64, labelValues ...Attr) { } // Decrement implements Metric @@ -145,7 +147,7 @@ func (dm *disabledMetric) Register() error { } // With implements Metric -func (dm *disabledMetric) With(labelValues ...string) Metric { +func (dm *disabledMetric) With(labelValues ...Attr) Metric { return dm } @@ -157,7 +159,7 @@ var ( // WithLabels provides configuration options for a new Metric, providing the expected // dimensions for data collection for that Metric. -func WithLabels(labels ...string) Options { +func WithLabels(labels ...Label) Options { return func(opts *options) { opts.labels = labels } @@ -165,20 +167,23 @@ func WithLabels(labels ...string) Options { // WithUnit provides configuration options for a new Metric, providing unit of measure // information for a new Metric. +// Used only 2x - once the type is part of the name ( as recommended), once is not. +// TODO: get rid of it. func WithUnit(unit Unit) Options { return func(opts *options) { opts.unit = unit } } -// WithInt64Values provides configuration options for a new Metric, indicating that -// recorded values will be saved as int64 values. Any float64 values recorded will -// converted to int64s via math.Floor-based conversion. -func WithInt64Values() Options { - return func(opts *options) { - opts.useInt64 = true - } -} +// Not used +//// WithInt64Values provides configuration options for a new Metric, indicating that +//// recorded values will be saved as int64 values. Any float64 values recorded will +//// converted to int64s via math.Floor-based conversion. +//func WithInt64Values() Options { +// return func(opts *options) { +// opts.useInt64 = true +// } +//} // WithLabelKeys is used to configure the label keys used by a DerivedMetric. This // option is mutually exclusive with the derived option `WithValueFrom` and will be ignored @@ -189,14 +194,14 @@ func WithLabelKeys(keys ...string) DerivedOptions { } } -// WithValueFrom is used to configure the derivation of a DerivedMetric. This option -// is mutually exclusive with the derived option `WithLabelKeys`. It acts as syntactic sugar -// that elides the need to create a DerivedMetric (with no labels) and then call `ValueFrom`. -func WithValueFrom(valueFn func() float64) DerivedOptions { - return func(opts *derivedOptions) { - opts.valueFn = valueFn - } -} +//// WithValueFrom is used to configure the derivation of a DerivedMetric. This option +//// is mutually exclusive with the derived option `WithLabelKeys`. It acts as syntactic sugar +//// that elides the need to create a DerivedMetric (with no labels) and then call `ValueFrom`. +//func WithValueFrom(valueFn func() float64) DerivedOptions { +// return func(opts *derivedOptions) { +// opts.valueFn = valueFn +// } +//} //// Value creates a new LabelValue for the Label. //func (l Label) Value(value string) LabelValue { @@ -213,6 +218,25 @@ func WithValueFrom(valueFn func() float64) DerivedOptions { // return Label(k) //} +type Label string + +type Attr struct { + Key string + Value string +} + +// MustCreateLabel is a temporary method to ease migration. +func MustCreateLabel(key string) Label { + return Label(key) +} + +func (l Label) Value(v string) Attr { + return Attr{ + Key: string(l), + Value: v, + } +} + // MustRegister is a helper function that will ensure that the provided Metrics are // registered. If a metric fails to register, this method will panic. func MustRegister(metrics ...Metric) { @@ -240,6 +264,37 @@ func RegisterIf(metric Metric, enabled func() bool) Metric { // NewSum creates a new Metric with an aggregation type of Sum (the values will be cumulative). // That means that data collected by the new Metric will be summed before export. +// Per prom conventions, must have a name ending in _total or _unit_total +// +// _count _sum and _bucket should be used with histograms +// +// Istio doesn't do this for: +// +// num_outgoing_retries +// pilot_total_rejected_configs +// provider_lookup_cluster_failures +// xds_cache_reads +// xds_cache_evictions +// pilot_k8s_cfg_events +// pilot_k8s_reg_events +// pilot_k8s_endpoints_with_no_pods +// pilot_total_xds_rejects +// pilot_xds_expired_nonce +// pilot_xds_write_timeout +// pilot_xds_pushes +// pilot_push_triggers +// pilot_xds_push_context_errors +// pilot_total_xds_internal_errors +// pilot_inbound_updates +// wasm_cache_lookup_count +// wasm_remote_fetch_count +// wasm_config_conversion_count +// citadel_server_csr_count +// citadel_server_authentication_failure_count +// citadel_server_csr_parsing_err_count +// citadel_server_id_extraction_err_count +// citadel_server_csr_sign_err_count +// citadel_server_success_cert_issuance_count func NewSum(name, description string, opts ...Options) Metric { return newSum(name, description, opts...) } @@ -247,6 +302,9 @@ func NewSum(name, description string, opts ...Options) Metric { // NewGauge creates a new Metric with an aggregation type of LastValue. That means that data collected // by the new Metric will export only the last recorded value. func NewGauge(name, description string, opts ...Options) Metric { + if strings.HasSuffix(name, "_total") { + fmt.Println(name) + } return newGauge(name, description, opts...) } @@ -271,7 +329,7 @@ var newDistribution func(name, description string, bounds []float64, opts ...Opt var newDerivedGauge func(name, description string, opts ...DerivedOptions) DerivedMetric func createOptions(opts ...Options) *options { - o := &options{unit: None, labels: make([]string, 0)} + o := &options{unit: None, labels: make([]Label, 0)} for _, opt := range opts { opt(o) } @@ -283,10 +341,10 @@ func createDerivedOptions(opts ...DerivedOptions) *derivedOptions { for _, opt := range opts { opt(o) } - // if a valueFn is supplied, then no label values can be supplied. - // to prevent issues, drop the label keys - if o.valueFn != nil { - o.labelKeys = []string{} - } + //// if a valueFn is supplied, then no label values can be supplied. + //// to prevent issues, drop the label keys + //if o.valueFn != nil { + // o.labelKeys = []string{} + //} return o } diff --git a/monitoring/monitoring_opencensus.go b/monitoring/monitoring_opencensus.go index 678d6897..143e7a43 100644 --- a/monitoring/monitoring_opencensus.go +++ b/monitoring/monitoring_opencensus.go @@ -1,5 +1,4 @@ -//go:build !opencensus -// +build !opencensus +//go:build !skip_opencensus // Copyright 2019 Istio Authors // @@ -19,14 +18,13 @@ package monitoring import ( "context" + "go.opencensus.io/metric" "go.opencensus.io/metric/metricdata" "go.opencensus.io/metric/metricproducer" "go.opencensus.io/stats" "go.opencensus.io/stats/view" "go.opencensus.io/tag" - "math" - "istio.io/pkg/log" ) @@ -76,9 +74,6 @@ func newGaugeOC(name, description string, opts ...Options) Metric { func newMetricOC(name, description string, aggregation *view.Aggregation, opts ...Options) Metric { o := createOptions(opts...) - if o.useInt64 { - return newInt64Metric(name, description, aggregation, o) - } return newFloat64Metric(name, description, aggregation, o) } @@ -95,9 +90,9 @@ func newDerivedGaugeOpenCensus(name, description string, opts ...DerivedOptions) base: m, name: name, } - if options.valueFn != nil { - derived.ValueFrom(options.valueFn) - } + //if options.valueFn != nil { + // derived.ValueFrom(options.valueFn) + //} return derived } @@ -116,7 +111,7 @@ func (d *derivedFloat64Metric) Register() error { return nil } -func (d *derivedFloat64Metric) ValueFrom(valueFn func() float64, labelValues ...string) { +func (d *derivedFloat64Metric) ValueFrom(valueFn func() float64, labelValues ...Attr) { if len(labelValues) == 0 { if err := d.base.UpsertEntry(valueFn); err != nil { log.Errorf("failed to add value for derived metric %q: %v", d.name, err) @@ -125,7 +120,7 @@ func (d *derivedFloat64Metric) ValueFrom(valueFn func() float64, labelValues ... } lv := make([]metricdata.LabelValue, 0, len(labelValues)) for _, l := range labelValues { - lv = append(lv, metricdata.NewLabelValue(l)) + lv = append(lv, metricdata.NewLabelValue(l.Value)) } if err := d.base.UpsertEntry(valueFn, lv...); err != nil { log.Errorf("failed to add value for derived metric %q: %v", d.name, err) @@ -149,7 +144,7 @@ func newFloat64Metric(name, description string, aggregation *view.Aggregation, o measure := stats.Float64(name, description, string(opts.unit)) tagKeys := make([]tag.Key, 0, len(opts.labels)) for _, l := range opts.labels { - tagKeys = append(tagKeys, tag.MustNewKey(l)) + tagKeys = append(tagKeys, tag.MustNewKey(string(l))) } ctx, _ := tag.New(context.Background()) //nolint:errcheck return &float64Metric{ @@ -203,16 +198,20 @@ func (f *float64Metric) RecordInt(value int64) { // values for a Metric. type LabelValue tag.Mutator -func toLabelValues(args ...string) []LabelValue { +func toLabelValues(args ...Attr) []tag.Mutator { + t := make([]tag.Mutator, len(args)) + for _, a := range args { + t = append(t, tag.Insert(tag.MustNewKey(a.Key), a.Value)) + } return nil } -func (f *float64Metric) With(labelValues ...string) Metric { +func (f *float64Metric) With(labelValues ...Attr) Metric { t := make([]tag.Mutator, len(f.tags), len(f.tags)+len(labelValues)) copy(t, f.tags) lv := toLabelValues(labelValues...) for _, tagValue := range lv { - t = append(t, tag.Mutator(tagValue)) + t = append(t, tagValue) } ctx, _ := tag.New(context.Background(), t...) //nolint:errcheck return &float64Metric{ @@ -228,93 +227,3 @@ func (f *float64Metric) With(labelValues ...string) Metric { func (f *float64Metric) Register() error { return view.Register(f.view) } - -type int64Metric struct { - *stats.Int64Measure - - // tags stores all tags for the metrics - tags []tag.Mutator - // ctx is a precomputed context holding tags, as an optimization - ctx context.Context - view *view.View - - // incrementMeasure is a precomputed +1 measurement to avoid extra allocations in Increment() - incrementMeasure []stats.Measurement - // decrementMeasure is a precomputed -1 measurement to avoid extra allocations in Decrement() - decrementMeasure []stats.Measurement -} - -func newInt64Metric(name, description string, aggregation *view.Aggregation, opts *options) *int64Metric { - measure := stats.Int64(name, description, string(opts.unit)) - tagKeys := make([]tag.Key, 0, len(opts.labels)) - for _, l := range opts.labels { - tagKeys = append(tagKeys, tag.MustNewKey(l)) - } - ctx, _ := tag.New(context.Background()) //nolint:errcheck - return &int64Metric{ - Int64Measure: measure, - tags: make([]tag.Mutator, 0), - ctx: ctx, - view: &view.View{Measure: measure, TagKeys: tagKeys, Aggregation: aggregation}, - incrementMeasure: []stats.Measurement{measure.M(1)}, - decrementMeasure: []stats.Measurement{measure.M(-1)}, - } -} - -func (i *int64Metric) Increment() { - i.recordMeasurements(i.incrementMeasure) -} - -func (i *int64Metric) Decrement() { - i.recordMeasurements(i.decrementMeasure) -} - -func (i *int64Metric) Name() string { - return i.Int64Measure.Name() -} - -func (i *int64Metric) Record(value float64) { - i.RecordInt(int64(math.Floor(value))) -} - -func (i *int64Metric) recordMeasurements(m []stats.Measurement) { - recordHookMutex.RLock() - if rh, ok := recordHooks[i.Name()]; ok { - for _, mv := range m { - rh.OnRecordInt64Measure(i.Int64Measure, i.tags, int64(math.Floor(mv.Value()))) - } - } - recordHookMutex.RUnlock() - stats.Record(i.ctx, m...) //nolint:errcheck -} - -func (i *int64Metric) RecordInt(value int64) { - recordHookMutex.RLock() - if rh, ok := recordHooks[i.Name()]; ok { - rh.OnRecordInt64Measure(i.Int64Measure, i.tags, value) - } - recordHookMutex.RUnlock() - stats.Record(i.ctx, i.M(value)) //nolint:errcheck -} - -func (i *int64Metric) With(labelValues ...string) Metric { - t := make([]tag.Mutator, len(i.tags), len(i.tags)+len(labelValues)) - copy(t, i.tags) - lv := toLabelValues(labelValues...) - for _, tagValue := range lv { - t = append(t, tag.Mutator(tagValue)) - } - ctx, _ := tag.New(context.Background(), t...) //nolint:errcheck - return &int64Metric{ - Int64Measure: i.Int64Measure, - tags: t, - ctx: ctx, - view: i.view, - incrementMeasure: i.incrementMeasure, - decrementMeasure: i.decrementMeasure, - } -} - -func (i *int64Metric) Register() error { - return view.Register(i.view) -} From b8bc4deb241ebaea54bc6bdbcedd0de36033c3cd Mon Sep 17 00:00:00 2001 From: Costin Manolache Date: Mon, 8 May 2023 10:45:30 -0700 Subject: [PATCH 3/4] Remove experimental expvar file --- monitoring/monitoring_expvar.go | 98 --------------------------------- 1 file changed, 98 deletions(-) delete mode 100644 monitoring/monitoring_expvar.go diff --git a/monitoring/monitoring_expvar.go b/monitoring/monitoring_expvar.go deleted file mode 100644 index dc03e283..00000000 --- a/monitoring/monitoring_expvar.go +++ /dev/null @@ -1,98 +0,0 @@ -//go:build !opencensus -// +build !opencensus - -package monitoring - -import "expvar" - -func init() { - newSum = newSumEV - newGauge = newGaugeEV - newDistribution = newDistributionEV - newDerivedGauge = newDerivedGaugeEV -} - -func newDerivedGaugeEV(name string, description string, opts ...DerivedOptions) DerivedMetric { - return &disabledMetric{} -} - -func newDistributionEV(name string, description string, bounds []float64, opts ...Options) Metric { - return &disabledMetric{} -} - -func newGaugeEV(name string, description string, opts ...Options) Metric { - return &expvarInt{name: name} -} - -func newSumEV(name string, description string, opts ...Options) Metric { - return &disabledMetric{} -} - -type expvarInt struct { - name string - expvar.Int -} - -func (dm *expvarInt) ValueFrom(valueFn func() float64, labelValues ...string) { -} - -// Decrement implements Metric -func (dm *expvarInt) Decrement() {} - -// Increment implements Metric -func (dm *expvarInt) Increment() {} - -// Name implements Metric -func (dm *expvarInt) Name() string { - return dm.name -} - -// Record implements Metric -func (dm *expvarInt) Record(value float64) {} - -// RecordInt implements Metric -func (dm *expvarInt) RecordInt(value int64) {} - -// Register implements Metric -func (dm *expvarInt) Register() error { - return nil -} - -// With implements Metric -func (dm *expvarInt) With(labelValues ...string) Metric { - return dm -} - -type expvarFloat struct { - name string -} - -func (dm *expvarFloat) ValueFrom(valueFn func() float64, labelValues ...string) { -} - -// Decrement implements Metric -func (dm *expvarFloat) Decrement() {} - -// Increment implements Metric -func (dm *expvarFloat) Increment() {} - -// Name implements Metric -func (dm *expvarFloat) Name() string { - return dm.name -} - -// Record implements Metric -func (dm *expvarFloat) Record(value float64) {} - -// RecordInt implements Metric -func (dm *expvarFloat) RecordInt(value int64) {} - -// Register implements Metric -func (dm *expvarFloat) Register() error { - return nil -} - -// With implements Metric -func (dm *expvarFloat) With(labelValues ...string) Metric { - return dm -} From 2bb7ef98ea9f78156f9c9a0a8372d6bedb3c8d7e Mon Sep 17 00:00:00 2001 From: Costin Manolache Date: Mon, 8 May 2023 10:53:41 -0700 Subject: [PATCH 4/4] Cleanup --- monitoring/monitoring.go | 93 +++-------------------------- monitoring/monitoring_opencensus.go | 2 + 2 files changed, 10 insertions(+), 85 deletions(-) diff --git a/monitoring/monitoring.go b/monitoring/monitoring.go index 6bcf9049..d9770348 100644 --- a/monitoring/monitoring.go +++ b/monitoring/monitoring.go @@ -14,15 +14,7 @@ package monitoring -import ( - "fmt" - "strings" - "sync" -) - // OpenCensus independent metrics -// Removed unused code: -// - Metric.Decrement type ( // A Metric collects numerical observations. @@ -42,7 +34,7 @@ type ( //Decrement() // Name returns the name value of a Metric. - // TODO: internal use only + // TODO: internal use only, make private Name() string // Record makes an observation of the provided value for the given measure. @@ -53,17 +45,6 @@ type ( // Not actually used in Istio. //RecordInt(value int64) - // Prometheus uses ConterVec and With(map[string]string) returning Counter - // - // Expvar doesn't support labels - but can be used in the name, creating a new expvar in a map - // The labels are really made part of the name when exporting and is the name of the TS - // - // Otel uses attribute.Int("name", val) and similar - from the stable package - - // but doesn't create a new Metric, it is an option to Add(). - // - // Istio only uses string/string - so using the pattern from slog works. - // Once we adopt slog, we can start using slog.Attr pattern - // With creates a new Metric, with the LabelValues provided. This allows creating // a set of pre-dimensioned data for recording purposes. This is primarily used // for documentation and convenience. Metrics created with this method do not need @@ -106,57 +87,39 @@ type ( // DerivedOptions encode changes to the options passed to a DerivedMetric at creation time. DerivedOptions func(*derivedOptions) - // A Label provides a named dimension for a Metric. - //Label Key - options struct { unit Unit labels []Label // Label - //useInt64 bool } derivedOptions struct { labelKeys []string - //valueFn func() float64 } ) -func (dm *disabledMetric) ValueFrom(valueFn func() float64, labelValues ...Attr) { +func (dm *disabledMetric) ValueFrom(func() float64, ...Attr) { } -// Decrement implements Metric -func (dm *disabledMetric) Decrement() {} - -// Increment implements Metric func (dm *disabledMetric) Increment() {} -// Name implements Metric func (dm *disabledMetric) Name() string { return dm.name } -// Record implements Metric func (dm *disabledMetric) Record(value float64) {} -// RecordInt implements Metric func (dm *disabledMetric) RecordInt(value int64) {} -// Register implements Metric func (dm *disabledMetric) Register() error { return nil } -// With implements Metric func (dm *disabledMetric) With(labelValues ...Attr) Metric { return dm } var _ Metric = &disabledMetric{} -var ( - recordHookMutex sync.RWMutex -) - // WithLabels provides configuration options for a new Metric, providing the expected // dimensions for data collection for that Metric. func WithLabels(labels ...Label) Options { @@ -168,23 +131,13 @@ func WithLabels(labels ...Label) Options { // WithUnit provides configuration options for a new Metric, providing unit of measure // information for a new Metric. // Used only 2x - once the type is part of the name ( as recommended), once is not. -// TODO: get rid of it. +// TODO: get rid of it or use consistently in ALL metrics. func WithUnit(unit Unit) Options { return func(opts *options) { opts.unit = unit } } -// Not used -//// WithInt64Values provides configuration options for a new Metric, indicating that -//// recorded values will be saved as int64 values. Any float64 values recorded will -//// converted to int64s via math.Floor-based conversion. -//func WithInt64Values() Options { -// return func(opts *options) { -// opts.useInt64 = true -// } -//} - // WithLabelKeys is used to configure the label keys used by a DerivedMetric. This // option is mutually exclusive with the derived option `WithValueFrom` and will be ignored // if that option is provided. @@ -194,32 +147,11 @@ func WithLabelKeys(keys ...string) DerivedOptions { } } -//// WithValueFrom is used to configure the derivation of a DerivedMetric. This option -//// is mutually exclusive with the derived option `WithLabelKeys`. It acts as syntactic sugar -//// that elides the need to create a DerivedMetric (with no labels) and then call `ValueFrom`. -//func WithValueFrom(valueFn func() float64) DerivedOptions { -// return func(opts *derivedOptions) { -// opts.valueFn = valueFn -// } -//} - -//// Value creates a new LabelValue for the Label. -//func (l Label) Value(value string) LabelValue { -// return tag.Upsert(tag.Key(l), value) -//} -// -//// MustCreateLabel will attempt to create a new Label. If -//// creation fails, then this method will panic. -//func MustCreateLabel(key string) Label { -// k, err := NewKey(key) -// if err != nil { -// panic(fmt.Errorf("could not create label %q: %v", key, err)) -// } -// return Label(k) -//} - +// Label is used to ease migration from opencensus. Will be eventually replaced +// with the otel attributes, but in a second stage. type Label string +// Attr is used to ease migration and minimize changes. Will be replaced by otel attributes. type Attr struct { Key string Value string @@ -264,9 +196,7 @@ func RegisterIf(metric Metric, enabled func() bool) Metric { // NewSum creates a new Metric with an aggregation type of Sum (the values will be cumulative). // That means that data collected by the new Metric will be summed before export. -// Per prom conventions, must have a name ending in _total or _unit_total -// -// _count _sum and _bucket should be used with histograms +// Per prom conventions, must have a name ending in _total. // // Istio doesn't do this for: // @@ -302,9 +232,6 @@ func NewSum(name, description string, opts ...Options) Metric { // NewGauge creates a new Metric with an aggregation type of LastValue. That means that data collected // by the new Metric will export only the last recorded value. func NewGauge(name, description string, opts ...Options) Metric { - if strings.HasSuffix(name, "_total") { - fmt.Println(name) - } return newGauge(name, description, opts...) } @@ -323,6 +250,7 @@ func NewDistribution(name, description string, bounds []float64, opts ...Options return newDistribution(name, description, bounds, opts...) } +// Internal methods used to hook one of the conditionally compiled implementations. var newSum func(name, description string, opts ...Options) Metric var newGauge func(name, description string, opts ...Options) Metric var newDistribution func(name, description string, bounds []float64, opts ...Options) Metric @@ -341,10 +269,5 @@ func createDerivedOptions(opts ...DerivedOptions) *derivedOptions { for _, opt := range opts { opt(o) } - //// if a valueFn is supplied, then no label values can be supplied. - //// to prevent issues, drop the label keys - //if o.valueFn != nil { - // o.labelKeys = []string{} - //} return o } diff --git a/monitoring/monitoring_opencensus.go b/monitoring/monitoring_opencensus.go index 143e7a43..4e2e0701 100644 --- a/monitoring/monitoring_opencensus.go +++ b/monitoring/monitoring_opencensus.go @@ -18,6 +18,7 @@ package monitoring import ( "context" + "sync" "go.opencensus.io/metric" "go.opencensus.io/metric/metricdata" @@ -29,6 +30,7 @@ import ( ) var ( + recordHookMutex sync.RWMutex recordHooks map[string]RecordHook derivedRegistry = metric.NewRegistry() )