From f49b9f75b89a586bbd73e74364634efe2b000b75 Mon Sep 17 00:00:00 2001 From: Mihai Parparita Date: Wed, 5 Apr 2023 14:19:40 -0700 Subject: [PATCH] util/clientmetric: allow client metric values to be provided by a function Adds NewGaugeFunc and NewCounterFunc (inspired by expvar.Func) which change the current value to be reported by a function. This allows some client metric values to be computed on-demand during uploading (at most every 15 seconds), instead of being continuously updated. clientmetric uploading had a bunch of micro-optimizations for memory access (#3331) which are not possible with this approach. However, any performance hit from function-based metrics is contained to those metrics only, and we expect to have very few. Also adds a DisableDeltas() option for client metrics, so that absolute values are always reported. This makes server-side processing of some metrics easier to reason about. Updates tailscale/corp#9230 Signed-off-by: Mihai Parparita --- util/clientmetric/clientmetric.go | 84 +++++++++++++++++++++----- util/clientmetric/clientmetric_test.go | 35 +++++++++++ 2 files changed, 105 insertions(+), 14 deletions(-) diff --git a/util/clientmetric/clientmetric.go b/util/clientmetric/clientmetric.go index e75b86858..b2d356b60 100644 --- a/util/clientmetric/clientmetric.go +++ b/util/clientmetric/clientmetric.go @@ -39,8 +39,9 @@ var ( // scanEntry contains the minimal data needed for quickly scanning // memory for changed values. It's small to reduce memory pressure. type scanEntry struct { - v *int64 // Metric.v - lastLogged int64 // last logged value + v *int64 // Metric.v + f func() int64 // Metric.f + lastLogged int64 // last logged value } // Type is a metric type: counter or gauge. @@ -55,10 +56,12 @@ const ( // // It's safe for concurrent use. type Metric struct { - v *int64 // atomic; the metric value - regIdx int // index into lastLogVal and unsorted - name string - typ Type + v *int64 // atomic; the metric value + f func() int64 // value function (v is ignored if f is non-nil) + regIdx int // index into lastLogVal and unsorted + name string + typ Type + deltasDisabled bool // The following fields are owned by the package-level 'mu': @@ -74,13 +77,29 @@ type Metric struct { } func (m *Metric) Name() string { return m.name } -func (m *Metric) Value() int64 { return atomic.LoadInt64(m.v) } -func (m *Metric) Type() Type { return m.typ } + +func (m *Metric) Value() int64 { + if m.f != nil { + return m.f() + } + return atomic.LoadInt64(m.v) +} + +func (m *Metric) Type() Type { return m.typ } + +// DisableDeltas disables uploading of deltas for this metric (absolute values +// are always uploaded). +func (m *Metric) DisableDeltas() { + m.deltasDisabled = true +} // Add increments m's value by n. // // If m is of type counter, n should not be negative. func (m *Metric) Add(n int64) { + if m.f != nil { + panic("Add() called on metric with value function") + } atomic.AddInt64(m.v, n) } @@ -88,6 +107,9 @@ func (m *Metric) Add(n int64) { // // If m is of type counter, Set should not be used. func (m *Metric) Set(v int64) { + if m.f != nil { + panic("Set() called on metric with value function") + } atomic.StoreInt64(m.v, v) } @@ -105,15 +127,19 @@ func (m *Metric) Publish() { metrics[m.name] = m sortedDirty = true - if len(valFreeList) == 0 { - valFreeList = make([]int64, 256) + if m.f != nil { + lastLogVal = append(lastLogVal, scanEntry{f: m.f}) + } else { + if len(valFreeList) == 0 { + valFreeList = make([]int64, 256) + } + m.v = &valFreeList[0] + valFreeList = valFreeList[1:] + lastLogVal = append(lastLogVal, scanEntry{v: m.v}) } - m.v = &valFreeList[0] - valFreeList = valFreeList[1:] m.regIdx = len(unsorted) unsorted = append(unsorted, m) - lastLogVal = append(lastLogVal, scanEntry{v: m.v}) } // Metrics returns the sorted list of metrics. @@ -177,6 +203,26 @@ func NewGauge(name string) *Metric { return m } +// NewCounterFunc returns a counter metric that has its value determined by +// calling the provided function (calling Add() and Set() will panic). No +// locking guarantees are made for the invocation. +func NewCounterFunc(name string, f func() int64) *Metric { + m := NewUnpublished(name, TypeCounter) + m.f = f + m.Publish() + return m +} + +// NewGaugeFunc returns a gauge metric that has its value determined by +// calling the provided function (calling Add() and Set() will panic). No +// locking guarantees are made for the invocation. +func NewGaugeFunc(name string, f func() int64) *Metric { + m := NewUnpublished(name, TypeGauge) + m.f = f + m.Publish() + return m +} + // WritePrometheusExpositionFormat writes all client metrics to w in // the Prometheus text-based exposition format. // @@ -233,7 +279,12 @@ func EncodeLogTailMetricsDelta() string { var enc *deltaEncBuf // lazy for i, ent := range lastLogVal { - val := atomic.LoadInt64(ent.v) + var val int64 + if ent.f != nil { + val = ent.f() + } else { + val = atomic.LoadInt64(ent.v) + } delta := val - ent.lastLogged if delta == 0 { continue @@ -248,9 +299,14 @@ func EncodeLogTailMetricsDelta() string { numWireID++ m.wireID = numWireID } + + writeValue := m.deltasDisabled if m.lastNamed.IsZero() || now.Sub(m.lastNamed) > metricLogNameFrequency { enc.writeName(m.Name(), m.Type()) m.lastNamed = now + writeValue = true + } + if writeValue { enc.writeValue(m.wireID, val) } else { enc.writeDelta(m.wireID, delta) diff --git a/util/clientmetric/clientmetric_test.go b/util/clientmetric/clientmetric_test.go index 0bdb4e409..ab6c4335a 100644 --- a/util/clientmetric/clientmetric_test.go +++ b/util/clientmetric/clientmetric_test.go @@ -72,3 +72,38 @@ func TestEncodeLogTailMetricsDelta(t *testing.T) { t.Errorf("with increments = %q; want %q", got, want) } } + +func TestDisableDeltas(t *testing.T) { + clearMetrics() + + c := NewCounter("foo") + c.DisableDeltas() + c.Set(123) + + if got, want := EncodeLogTailMetricsDelta(), "N06fooS02f601"; got != want { + t.Errorf("first = %q; want %q", got, want) + } + + c.Set(456) + advanceTime() + if got, want := EncodeLogTailMetricsDelta(), "S029007"; got != want { + t.Errorf("second = %q; want %q", got, want) + } +} + +func TestWithFunc(t *testing.T) { + clearMetrics() + + v := int64(123) + NewCounterFunc("foo", func() int64 { return v }) + + if got, want := EncodeLogTailMetricsDelta(), "N06fooS02f601"; got != want { + t.Errorf("first = %q; want %q", got, want) + } + + v = 456 + advanceTime() + if got, want := EncodeLogTailMetricsDelta(), "I029a05"; got != want { + t.Errorf("second = %q; want %q", got, want) + } +}