From 50403f55bd4a3aca7175cbe186e5399e8030915f Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Thu, 6 Feb 2020 08:07:35 -0800 Subject: [PATCH 1/7] stats: stop using CountersFuncWithMultiLabels Signed-off-by: Sugu Sougoumarane --- .../tabletmanager/vreplication/stats.go | 4 +- go/vt/vttablet/tabletserver/query_engine.go | 105 +++--------------- 2 files changed, 16 insertions(+), 93 deletions(-) diff --git a/go/vt/vttablet/tabletmanager/vreplication/stats.go b/go/vt/vttablet/tabletmanager/vreplication/stats.go index be66fae9997..ea688fa4774 100644 --- a/go/vt/vttablet/tabletmanager/vreplication/stats.go +++ b/go/vt/vttablet/tabletmanager/vreplication/stats.go @@ -58,11 +58,9 @@ type vrStats struct { func (st *vrStats) register() { stats.NewGaugeFunc("VReplicationStreamCount", "Number of vreplication streams", st.numControllers) stats.NewGaugeFunc("VReplicationSecondsBehindMasterMax", "Max vreplication seconds behind master", st.maxSecondsBehindMaster) - stats.NewCountersFuncWithMultiLabels( + stats.NewGaugesFuncWithMultiLabels( "VReplicationSecondsBehindMaster", "vreplication seconds behind master per stream", - // CAUTION: Always keep this label as "counts" because the Google - // internal monitoring depends on this specific value. []string{"counts"}, func() map[string]int64 { st.mu.Lock() diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go index b182b7ba12a..be7e121ec0c 100644 --- a/go/vt/vttablet/tabletserver/query_engine.go +++ b/go/vt/vttablet/tabletserver/query_engine.go @@ -107,6 +107,12 @@ func (ep *TabletPlan) buildAuthorized() { } } +var ( + // Global stats vars. + // TODO(sougou): unglobalize after componentizing TabletServer. + queryCounts, queryTimes, queryRowCounts, queryErrorCounts *stats.CountersWithMultiLabels +) + //_______________________________________________ // QueryEngine implements the core functionality of tabletserver. @@ -126,9 +132,6 @@ type QueryEngine struct { plans *cache.LRUCache queryRuleSources *rules.Map - queryStatsMu sync.RWMutex - queryStats map[string]*QueryStats - // Pools conns *connpool.Pool streamConns *connpool.Pool @@ -186,7 +189,6 @@ func NewQueryEngine(checker connpool.MySQLChecker, se *schema.Engine, config tab plans: cache.NewLRUCache(int64(config.QueryPlanCacheSize)), queryRuleSources: rules.NewMap(), queryPoolWaiterCap: sync2.NewAtomicInt64(int64(config.QueryPoolWaiterCap)), - queryStats: make(map[string]*QueryStats), } qe.conns = connpool.New( @@ -259,10 +261,10 @@ func NewQueryEngine(checker connpool.MySQLChecker, se *schema.Engine, config tab stats.Publish("QueryCacheOldest", stats.StringFunc(func() string { return fmt.Sprintf("%v", qe.plans.Oldest()) })) - _ = stats.NewCountersFuncWithMultiLabels("QueryCounts", "query counts", []string{"Table", "Plan"}, qe.getQueryCount) - _ = stats.NewCountersFuncWithMultiLabels("QueryTimesNs", "query times in ns", []string{"Table", "Plan"}, qe.getQueryTime) - _ = stats.NewCountersFuncWithMultiLabels("QueryRowCounts", "query row counts", []string{"Table", "Plan"}, qe.getQueryRowCount) - _ = stats.NewCountersFuncWithMultiLabels("QueryErrorCounts", "query error counts", []string{"Table", "Plan"}, qe.getQueryErrorCount) + queryCounts = stats.NewCountersWithMultiLabels("QueryCounts", "query counts", []string{"Table", "Plan"}) + queryTimes = stats.NewCountersWithMultiLabels("QueryTimesNs", "query times in ns", []string{"Table", "Plan"}) + queryRowCounts = stats.NewCountersWithMultiLabels("QueryRowCounts", "query row counts", []string{"Table", "Plan"}) + queryErrorCounts = stats.NewCountersWithMultiLabels("QueryErrorCounts", "query error counts", []string{"Table", "Plan"}) http.Handle("/debug/hotrows", qe.txSerializer) @@ -483,91 +485,14 @@ func (qe *QueryEngine) QueryPlanCacheCap() int { return int(qe.plans.Capacity()) } -// QueryStats tracks query stats for export per planName/tableName -type QueryStats struct { - mu sync.Mutex - queryCount int64 - time time.Duration - mysqlTime time.Duration - rowCount int64 - errorCount int64 -} - // AddStats adds the given stats for the planName.tableName func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) { // table names can contain "." characters, replace them! - key := strings.Replace(tableName, ".", "_", -1) + "." + planName - - qe.queryStatsMu.RLock() - stats, ok := qe.queryStats[key] - qe.queryStatsMu.RUnlock() - - if !ok { - // Check again with the write lock held and - // create a new record only if none exists - qe.queryStatsMu.Lock() - if stats, ok = qe.queryStats[key]; !ok { - stats = &QueryStats{} - qe.queryStats[key] = stats - } - qe.queryStatsMu.Unlock() - } - - stats.mu.Lock() - stats.queryCount += queryCount - stats.time += duration - stats.mysqlTime += mysqlTime - stats.rowCount += rowCount - stats.errorCount += errorCount - stats.mu.Unlock() -} - -func (qe *QueryEngine) getQueryCount() map[string]int64 { - qstats := make(map[string]int64) - qe.queryStatsMu.RLock() - defer qe.queryStatsMu.RUnlock() - for k, qs := range qe.queryStats { - qs.mu.Lock() - qstats[k] = qs.queryCount - qs.mu.Unlock() - } - return qstats -} - -func (qe *QueryEngine) getQueryTime() map[string]int64 { - qstats := make(map[string]int64) - qe.queryStatsMu.RLock() - defer qe.queryStatsMu.RUnlock() - for k, qs := range qe.queryStats { - qs.mu.Lock() - qstats[k] = int64(qs.time) - qs.mu.Unlock() - } - return qstats -} - -func (qe *QueryEngine) getQueryRowCount() map[string]int64 { - qstats := make(map[string]int64) - qe.queryStatsMu.RLock() - defer qe.queryStatsMu.RUnlock() - for k, qs := range qe.queryStats { - qs.mu.Lock() - qstats[k] = qs.rowCount - qs.mu.Unlock() - } - return qstats -} - -func (qe *QueryEngine) getQueryErrorCount() map[string]int64 { - qstats := make(map[string]int64) - qe.queryStatsMu.RLock() - defer qe.queryStatsMu.RUnlock() - for k, qs := range qe.queryStats { - qs.mu.Lock() - qstats[k] = qs.errorCount - qs.mu.Unlock() - } - return qstats + keys := []string{strings.Replace(tableName, ".", "_", -1), planName} + queryCounts.Add(keys, queryCount) + queryTimes.Add(keys, int64(duration)) + queryRowCounts.Add(keys, rowCount) + queryErrorCounts.Add(keys, errorCount) } type perQueryStats struct { From 4a3a98bfeab4fa4388fbc9d7f9b955f13519d248 Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Sun, 9 Feb 2020 01:07:15 -0800 Subject: [PATCH 2/7] stats: refactor counters for drop dimensions Also, the old code was badly written. Signed-off-by: Sugu Sougoumarane --- go/stats/counters.go | 144 +++++++++++++++---------------------------- 1 file changed, 50 insertions(+), 94 deletions(-) diff --git a/go/stats/counters.go b/go/stats/counters.go index 6307cf10f2b..1f7cc416b07 100644 --- a/go/stats/counters.go +++ b/go/stats/counters.go @@ -19,104 +19,70 @@ package stats import ( "bytes" "fmt" + "strings" "sync" - "sync/atomic" ) // counters is similar to expvar.Map, except that it doesn't allow floats. // It is used to build CountersWithSingleLabel and GaugesWithSingleLabel. type counters struct { - // mu only protects adding and retrieving the value (*int64) from the - // map. - // The modification to the actual number (int64) must be done with - // atomic funcs. - // If a value for a given name already exists in the map, we only have - // to use a read-lock to retrieve it. This is an important performance - // optimizations because it allows to concurrently increment a counter. - mu sync.RWMutex - counts map[string]*int64 - help string + mu sync.Mutex + counts map[string]int64 + + help string } -// String implements the expvar.Var interface. func (c *counters) String() string { - b := bytes.NewBuffer(make([]byte, 0, 4096)) - - c.mu.RLock() - defer c.mu.RUnlock() + c.mu.Lock() + defer c.mu.Unlock() + b := &strings.Builder{} fmt.Fprintf(b, "{") - firstValue := true - for k, a := range c.counts { - if firstValue { - firstValue = false - } else { - fmt.Fprintf(b, ", ") - } - fmt.Fprintf(b, "%q: %v", k, atomic.LoadInt64(a)) + prefix := "" + for k, v := range c.counts { + fmt.Fprintf(b, "%s%q: %v", prefix, k, v) + prefix = ", " } fmt.Fprintf(b, "}") return b.String() } -func (c *counters) getValueAddr(name string) *int64 { - c.mu.RLock() - a, ok := c.counts[name] - c.mu.RUnlock() - - if ok { - return a - } - +func (c *counters) add(name string, value int64) { c.mu.Lock() defer c.mu.Unlock() - // we need to check the existence again - // as it may be created by other goroutine. - a, ok = c.counts[name] - if ok { - return a - } - a = new(int64) - c.counts[name] = a - return a + c.counts[name] = c.counts[name] + value } -// Add adds a value to a named counter. -func (c *counters) Add(name string, value int64) { - a := c.getValueAddr(name) - atomic.AddInt64(a, value) +func (c *counters) set(name string, value int64) { + c.mu.Lock() + defer c.mu.Unlock() + c.counts[name] = value } -// ResetAll resets all counter values and clears all keys. -func (c *counters) ResetAll() { +func (c *counters) reset() { c.mu.Lock() defer c.mu.Unlock() - c.counts = make(map[string]*int64) + c.counts = make(map[string]int64) } -// ZeroAll resets all counter values to zero +// ZeroAll zeroes out all values func (c *counters) ZeroAll() { c.mu.Lock() defer c.mu.Unlock() - for _, a := range c.counts { - atomic.StoreInt64(a, int64(0)) - } -} -// Reset resets a specific counter value to 0. -func (c *counters) Reset(name string) { - a := c.getValueAddr(name) - atomic.StoreInt64(a, int64(0)) + for k := range c.counts { + c.counts[k] = 0 + } } // Counts returns a copy of the Counters' map. func (c *counters) Counts() map[string]int64 { - c.mu.RLock() - defer c.mu.RUnlock() + c.mu.Lock() + defer c.mu.Unlock() counts := make(map[string]int64, len(c.counts)) - for k, a := range c.counts { - counts[k] = atomic.LoadInt64(a) + for k, v := range c.counts { + counts[k] = v } return counts } @@ -143,14 +109,14 @@ type CountersWithSingleLabel struct { func NewCountersWithSingleLabel(name, help, label string, tags ...string) *CountersWithSingleLabel { c := &CountersWithSingleLabel{ counters: counters{ - counts: make(map[string]*int64), + counts: make(map[string]int64), help: help, }, label: label, } for _, tag := range tags { - c.counts[tag] = new(int64) + c.counts[tag] = 0 } if name != "" { publish(name, c) @@ -168,13 +134,17 @@ func (c *CountersWithSingleLabel) Add(name string, value int64) { if value < 0 { logCounterNegative.Warningf("Adding a negative value to a counter, %v should be a gauge instead", c) } - a := c.getValueAddr(name) - atomic.AddInt64(a, value) + c.counters.add(name, value) +} + +// Reset resets the value for the name. +func (c *CountersWithSingleLabel) Reset(name string) { + c.counters.set(name, 0) } // ResetAll clears the counters func (c *CountersWithSingleLabel) ResetAll() { - c.counters.ResetAll() + c.counters.reset() } // CountersWithMultiLabels is a multidimensional counters implementation. @@ -190,7 +160,7 @@ type CountersWithMultiLabels struct { func NewCountersWithMultiLabels(name, help string, labels []string) *CountersWithMultiLabels { t := &CountersWithMultiLabels{ counters: counters{ - counts: make(map[string]*int64), + counts: make(map[string]int64), help: help}, labels: labels, } @@ -215,8 +185,7 @@ func (mc *CountersWithMultiLabels) Add(names []string, value int64) { if value < 0 { logCounterNegative.Warningf("Adding a negative value to a counter, %v should be a gauge instead", mc) } - - mc.counters.Add(safeJoinLabels(names), value) + mc.counters.add(safeJoinLabels(names), value) } // Reset resets the value of a named counter back to 0. @@ -226,7 +195,12 @@ func (mc *CountersWithMultiLabels) Reset(names []string) { panic("CountersWithMultiLabels: wrong number of values in Reset") } - mc.counters.Reset(safeJoinLabels(names)) + mc.counters.set(safeJoinLabels(names), 0) +} + +// ResetAll clears the counters +func (mc *CountersWithMultiLabels) ResetAll() { + mc.counters.reset() } // Counts returns a copy of the Counters' map. @@ -317,7 +291,7 @@ func NewGaugesWithSingleLabel(name, help, label string, tags ...string) *GaugesW g := &GaugesWithSingleLabel{ CountersWithSingleLabel: CountersWithSingleLabel{ counters: counters{ - counts: make(map[string]*int64), + counts: make(map[string]int64), help: help, }, label: label, @@ -325,7 +299,7 @@ func NewGaugesWithSingleLabel(name, help, label string, tags ...string) *GaugesW } for _, tag := range tags { - g.counts[tag] = new(int64) + g.counts[tag] = 0 } if name != "" { publish(name, g) @@ -335,14 +309,7 @@ func NewGaugesWithSingleLabel(name, help, label string, tags ...string) *GaugesW // Set sets the value of a named gauge. func (g *GaugesWithSingleLabel) Set(name string, value int64) { - a := g.getValueAddr(name) - atomic.StoreInt64(a, value) -} - -// Add adds a value to a named gauge. -func (g *GaugesWithSingleLabel) Add(name string, value int64) { - a := g.getValueAddr(name) - atomic.AddInt64(a, value) + g.counters.set(name, value) } // GaugesWithMultiLabels is a CountersWithMultiLabels implementation where @@ -357,7 +324,7 @@ func NewGaugesWithMultiLabels(name, help string, labels []string) *GaugesWithMul t := &GaugesWithMultiLabels{ CountersWithMultiLabels: CountersWithMultiLabels{ counters: counters{ - counts: make(map[string]*int64), + counts: make(map[string]int64), help: help, }, labels: labels, @@ -375,18 +342,7 @@ func (mg *GaugesWithMultiLabels) Set(names []string, value int64) { if len(names) != len(mg.CountersWithMultiLabels.labels) { panic("GaugesWithMultiLabels: wrong number of values in Set") } - a := mg.getValueAddr(safeJoinLabels(names)) - atomic.StoreInt64(a, value) -} - -// Add adds a value to a named gauge. -// len(names) must be equal to len(Labels). -func (mg *GaugesWithMultiLabels) Add(names []string, value int64) { - if len(names) != len(mg.labels) { - panic("CountersWithMultiLabels: wrong number of values in Add") - } - - mg.counters.Add(safeJoinLabels(names), value) + mg.counters.set(safeJoinLabels(names), value) } // GaugesFuncWithMultiLabels is a wrapper around CountersFuncWithMultiLabels From b5066e3f847fcace049ed6eab0bb7a37db9a157a Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Sun, 9 Feb 2020 02:18:41 -0800 Subject: [PATCH 3/7] stats: drop dimension This feature implements the drop dimension part of #5791. The dropping of dimensions is not applicable to all stats vars. We drop dimensions for counters and timings, but not gauges. This also doesn't work for func based vars, but those are now unused for counters and timings. The feature doesn't actually drop the dimension. Instead, it consolidates all the different values into a single "all" category. This approach is more backward compatible and allows for going back and forth, because the label itself continues to exist in both scenarios. Signed-off-by: Sugu Sougoumarane --- go/stats/counters.go | 27 +++++++++++++++------ go/stats/counters_test.go | 19 +++++++++++++++ go/stats/export.go | 44 +++++++++++++++++++++++++++++++++ go/stats/export_test.go | 2 ++ go/stats/safe_label.go | 27 --------------------- go/stats/timings.go | 51 ++++++++++++++++++--------------------- go/stats/timings_test.go | 22 +++++++++++++++++ 7 files changed, 130 insertions(+), 62 deletions(-) delete mode 100644 go/stats/safe_label.go diff --git a/go/stats/counters.go b/go/stats/counters.go index 1f7cc416b07..d545b75c10c 100644 --- a/go/stats/counters.go +++ b/go/stats/counters.go @@ -97,7 +97,8 @@ func (c *counters) Help() string { // It provides a Counts method which can be used for tracking rates. type CountersWithSingleLabel struct { counters - label string + label string + labelDropped bool } // NewCountersWithSingleLabel create a new Counters instance. @@ -112,7 +113,8 @@ func NewCountersWithSingleLabel(name, help, label string, tags ...string) *Count counts: make(map[string]int64), help: help, }, - label: label, + label: label, + labelDropped: IsDimensionDropped(label), } for _, tag := range tags { @@ -134,11 +136,17 @@ func (c *CountersWithSingleLabel) Add(name string, value int64) { if value < 0 { logCounterNegative.Warningf("Adding a negative value to a counter, %v should be a gauge instead", c) } + if c.labelDropped { + name = StatsAllStr + } c.counters.add(name, value) } // Reset resets the value for the name. func (c *CountersWithSingleLabel) Reset(name string) { + if c.labelDropped { + name = StatsAllStr + } c.counters.set(name, 0) } @@ -152,7 +160,8 @@ func (c *CountersWithSingleLabel) ResetAll() { // label value where all label values are joined with ".". type CountersWithMultiLabels struct { counters - labels []string + labels []string + droppedLabels []bool } // NewCountersWithMultiLabels creates a new CountersWithMultiLabels @@ -162,7 +171,11 @@ func NewCountersWithMultiLabels(name, help string, labels []string) *CountersWit counters: counters{ counts: make(map[string]int64), help: help}, - labels: labels, + labels: labels, + droppedLabels: make([]bool, len(labels)), + } + for i, label := range labels { + t.droppedLabels[i] = IsDimensionDropped(label) } if name != "" { publish(name, t) @@ -185,7 +198,7 @@ func (mc *CountersWithMultiLabels) Add(names []string, value int64) { if value < 0 { logCounterNegative.Warningf("Adding a negative value to a counter, %v should be a gauge instead", mc) } - mc.counters.add(safeJoinLabels(names), value) + mc.counters.add(safeJoinLabels(names, mc.droppedLabels), value) } // Reset resets the value of a named counter back to 0. @@ -195,7 +208,7 @@ func (mc *CountersWithMultiLabels) Reset(names []string) { panic("CountersWithMultiLabels: wrong number of values in Reset") } - mc.counters.set(safeJoinLabels(names), 0) + mc.counters.set(safeJoinLabels(names, mc.droppedLabels), 0) } // ResetAll clears the counters @@ -342,7 +355,7 @@ func (mg *GaugesWithMultiLabels) Set(names []string, value int64) { if len(names) != len(mg.CountersWithMultiLabels.labels) { panic("GaugesWithMultiLabels: wrong number of values in Set") } - mg.counters.set(safeJoinLabels(names), value) + mg.counters.set(safeJoinLabels(names, nil), value) } // GaugesFuncWithMultiLabels is a wrapper around CountersFuncWithMultiLabels diff --git a/go/stats/counters_test.go b/go/stats/counters_test.go index a9ea7485604..801447d9057 100644 --- a/go/stats/counters_test.go +++ b/go/stats/counters_test.go @@ -24,6 +24,8 @@ import ( "strings" "testing" "time" + + "github.com/stretchr/testify/assert" ) func TestCounters(t *testing.T) { @@ -240,3 +242,20 @@ func TestCountersFuncWithMultiLabels_Hook(t *testing.T) { t.Errorf("want %#v, got %#v", v, gotv) } } + +func TestCountersDropDimension(t *testing.T) { + clear() + *dropDimensions = "a,c" + + c1 := NewCountersWithSingleLabel("counter_dropdim1", "help", "label") + c1.Add("c1", 1) + assert.Equal(t, `{"c1": 1}`, c1.String()) + + c2 := NewCountersWithSingleLabel("counter_dropdim2", "help", "a") + c2.Add("c1", 1) + assert.Equal(t, `{"all": 1}`, c2.String()) + + c3 := NewCountersWithMultiLabels("counter_dropdim3", "help", []string{"a", "b", "c"}) + c3.Add([]string{"c1", "c2", "c3"}, 1) + assert.Equal(t, `{"all.c2.all": 1}`, c3.String()) +} diff --git a/go/stats/export.go b/go/stats/export.go index 54aa38d94d7..f26cc7a5f92 100644 --- a/go/stats/export.go +++ b/go/stats/export.go @@ -33,6 +33,7 @@ import ( "flag" "fmt" "strconv" + "strings" "sync" "time" @@ -42,6 +43,10 @@ import ( var emitStats = flag.Bool("emit_stats", false, "true iff we should emit stats to push-based monitoring/stats backends") var statsEmitPeriod = flag.Duration("stats_emit_period", time.Duration(60*time.Second), "Interval between emitting stats to all registered backends") var statsBackend = flag.String("stats_backend", "", "The name of the registered push-based monitoring/stats backend to use") +var dropDimensions = flag.String("drop_dimension", "", "List of dimensions to be dropped from stats vars") + +// StatsAllStr is the consolidated name if a dimension gets dropped. +const StatsAllStr = "all" // NewVarHook is the type of a hook to export variables in a different way type NewVarHook func(name string, v expvar.Var) @@ -247,3 +252,42 @@ func stringMapToString(m map[string]string) string { fmt.Fprintf(b, "}") return b.String() } + +var ( + dimMu sync.Mutex + droppedDimensions map[string]bool +) + +// IsDimensionDropped returns true if the specified dimension is dropped. +func IsDimensionDropped(name string) bool { + dimMu.Lock() + defer dimMu.Unlock() + + if droppedDimensions == nil { + dims := strings.Split(*dropDimensions, ",") + droppedDimensions = make(map[string]bool, len(dims)) + for _, dim := range dims { + droppedDimensions[dim] = true + } + } + return droppedDimensions[name] +} + +// safeJoinLabels joins the label values with ".", but first replaces any existing +// "." characters in the labels with the proper replacement, to avoid issues parsing +// them apart later. +func safeJoinLabels(labels []string, droppedLabels []bool) string { + sanitizedLabels := make([]string, len(labels)) + for idx, label := range labels { + if droppedLabels != nil && droppedLabels[idx] { + sanitizedLabels[idx] = StatsAllStr + } else { + sanitizedLabels[idx] = safeLabel(label) + } + } + return strings.Join(sanitizedLabels, ".") +} + +func safeLabel(label string) string { + return strings.Replace(label, ".", "_", -1) +} diff --git a/go/stats/export_test.go b/go/stats/export_test.go index 86c07064a7d..8aa5f07f4c6 100644 --- a/go/stats/export_test.go +++ b/go/stats/export_test.go @@ -24,6 +24,8 @@ import ( func clear() { defaultVarGroup.vars = make(map[string]expvar.Var) defaultVarGroup.newVarHook = nil + *dropDimensions = "" + droppedDimensions = nil } func TestNoHook(t *testing.T) { diff --git a/go/stats/safe_label.go b/go/stats/safe_label.go deleted file mode 100644 index a9ca0720f26..00000000000 --- a/go/stats/safe_label.go +++ /dev/null @@ -1,27 +0,0 @@ -/* -Copyright 2019 The Vitess 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 stats - -import ( - "strings" -) - -// safeLabel turns a label into a safe label for stats export. -// It is in its own file so it can be customized. -func safeLabel(label string) string { - return strings.Replace(label, ".", "_", -1) -} diff --git a/go/stats/timings.go b/go/stats/timings.go index 8d32ad94bea..d9195176d31 100644 --- a/go/stats/timings.go +++ b/go/stats/timings.go @@ -19,7 +19,6 @@ package stats import ( "encoding/json" "fmt" - "strings" "sync" "time" @@ -32,13 +31,12 @@ type Timings struct { totalCount sync2.AtomicInt64 totalTime sync2.AtomicInt64 - // mu protects get and set of hook and the map. - // Modification to the value in the map is not protected. mu sync.RWMutex histograms map[string]*Histogram - hook func(string, time.Duration) - help string - label string + + help string + label string + labelDropped bool } // NewTimings creates a new Timings object, and publishes it if name is set. @@ -47,9 +45,10 @@ type Timings struct { // first time they are updated. func NewTimings(name, help, label string, categories ...string) *Timings { t := &Timings{ - histograms: make(map[string]*Histogram), - help: help, - label: label, + histograms: make(map[string]*Histogram), + help: help, + label: label, + labelDropped: IsDimensionDropped(label), } for _, cat := range categories { t.histograms[cat] = NewGenericHistogram("", "", bucketCutoffs, bucketLabels, "Count", "Time") @@ -63,10 +62,12 @@ func NewTimings(name, help, label string, categories ...string) *Timings { // Add will add a new value to the named histogram. func (t *Timings) Add(name string, elapsed time.Duration) { + if t.labelDropped { + name = StatsAllStr + } // Get existing Histogram. t.mu.RLock() hist, ok := t.histograms[name] - hook := t.hook t.mu.RUnlock() // Create Histogram if it does not exist. @@ -84,14 +85,14 @@ func (t *Timings) Add(name string, elapsed time.Duration) { hist.Add(elapsedNs) t.totalCount.Add(1) t.totalTime.Add(elapsedNs) - if hook != nil { - hook(name, elapsed) - } } // Record is a convenience function that records completion // timing data based on the provided start time of an event. func (t *Timings) Record(name string, startTime time.Time) { + if t.labelDropped { + name = StatsAllStr + } t.Add(name, time.Since(startTime)) } @@ -184,7 +185,8 @@ func init() { // with joining multiple strings with '.'. type MultiTimings struct { Timings - labels []string + labels []string + droppedLabels []bool } // NewMultiTimings creates a new MultiTimings object. @@ -194,7 +196,11 @@ func NewMultiTimings(name string, help string, labels []string) *MultiTimings { histograms: make(map[string]*Histogram), help: help, }, - labels: labels, + labels: labels, + droppedLabels: make([]bool, len(labels)), + } + for i, label := range labels { + t.droppedLabels[i] = IsDimensionDropped(label) } if name != "" { publish(name, t) @@ -208,23 +214,12 @@ func (mt *MultiTimings) Labels() []string { return mt.labels } -// safeJoinLabels joins the label values with ".", but first replaces any existing -// "." characters in the labels with the proper replacement, to avoid issues parsing -// them apart later. -func safeJoinLabels(labels []string) string { - sanitizedLabels := make([]string, len(labels)) - for idx, label := range labels { - sanitizedLabels[idx] = safeLabel(label) - } - return strings.Join(sanitizedLabels, ".") -} - // Add will add a new value to the named histogram. func (mt *MultiTimings) Add(names []string, elapsed time.Duration) { if len(names) != len(mt.labels) { panic("MultiTimings: wrong number of values in Add") } - mt.Timings.Add(safeJoinLabels(names), elapsed) + mt.Timings.Add(safeJoinLabels(names, mt.droppedLabels), elapsed) } // Record is a convenience function that records completion @@ -233,7 +228,7 @@ func (mt *MultiTimings) Record(names []string, startTime time.Time) { if len(names) != len(mt.labels) { panic("MultiTimings: wrong number of values in Record") } - mt.Timings.Record(safeJoinLabels(names), startTime) + mt.Timings.Record(safeJoinLabels(names, mt.droppedLabels), startTime) } // Cutoffs returns the cutoffs used in the component histograms. diff --git a/go/stats/timings_test.go b/go/stats/timings_test.go index 808d1aa15b5..6ee89a312b5 100644 --- a/go/stats/timings_test.go +++ b/go/stats/timings_test.go @@ -21,6 +21,8 @@ import ( "strings" "testing" "time" + + "github.com/stretchr/testify/assert" ) func TestTimings(t *testing.T) { @@ -77,3 +79,23 @@ func TestTimingsHook(t *testing.T) { t.Errorf("got %#v, want %#v", gotv, v) } } + +func TestTimingsDropDimension(t *testing.T) { + clear() + *dropDimensions = "a,c" + + t1 := NewTimings("timing_dropdim1", "help", "label") + t1.Add("t1", 1*time.Nanosecond) + want := `{"TotalCount":1,"TotalTime":1,"Histograms":{"t1":{"500000":1,"1000000":1,"5000000":1,"10000000":1,"50000000":1,"100000000":1,"500000000":1,"1000000000":1,"5000000000":1,"10000000000":1,"inf":1,"Count":1,"Time":1}}}` + assert.Equal(t, want, t1.String()) + + t2 := NewTimings("timing_dropdim2", "help", "a") + t2.Add("t1", 1) + want = `{"TotalCount":1,"TotalTime":1,"Histograms":{"all":{"500000":1,"1000000":1,"5000000":1,"10000000":1,"50000000":1,"100000000":1,"500000000":1,"1000000000":1,"5000000000":1,"10000000000":1,"inf":1,"Count":1,"Time":1}}}` + assert.Equal(t, want, t2.String()) + + t3 := NewMultiTimings("timing_dropdim3", "help", []string{"a", "b", "c"}) + t3.Add([]string{"c1", "c2", "c3"}, 1) + want = `{"TotalCount":1,"TotalTime":1,"Histograms":{"all.c2.all":{"500000":1,"1000000":1,"5000000":1,"10000000":1,"50000000":1,"100000000":1,"500000000":1,"1000000000":1,"5000000000":1,"10000000000":1,"inf":1,"Count":1,"Time":1}}}` + assert.Equal(t, want, t3.String()) +} From 468ead013f00cde4404f34f0d8483cc66e3f24e2 Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Sun, 9 Feb 2020 23:02:42 -0800 Subject: [PATCH 4/7] stats: dropped dimension: fix empty label bug Signed-off-by: Sugu Sougoumarane --- go/stats/counters_test.go | 6 ++++++ go/stats/export.go | 3 +++ 2 files changed, 9 insertions(+) diff --git a/go/stats/counters_test.go b/go/stats/counters_test.go index 801447d9057..21267ec026e 100644 --- a/go/stats/counters_test.go +++ b/go/stats/counters_test.go @@ -244,6 +244,12 @@ func TestCountersFuncWithMultiLabels_Hook(t *testing.T) { } func TestCountersDropDimension(t *testing.T) { + clear() + // Empty labels shouldn't be dropped. + c4 := NewCountersWithSingleLabel("counter_dropdim4", "help", "") + c4.Add("c1", 1) + assert.Equal(t, `{"c1": 1}`, c4.String()) + clear() *dropDimensions = "a,c" diff --git a/go/stats/export.go b/go/stats/export.go index f26cc7a5f92..0a9296b7511 100644 --- a/go/stats/export.go +++ b/go/stats/export.go @@ -267,6 +267,9 @@ func IsDimensionDropped(name string) bool { dims := strings.Split(*dropDimensions, ",") droppedDimensions = make(map[string]bool, len(dims)) for _, dim := range dims { + if dim == "" { + continue + } droppedDimensions[dim] = true } } From cb2f6aaf997d3cc111141d894c3d7145e0eef221 Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Sun, 9 Feb 2020 23:27:57 -0800 Subject: [PATCH 5/7] stats: drop dimension: address review comments Signed-off-by: Sugu Sougoumarane --- go/stats/counters.go | 34 ++++++++++++--------- go/stats/counters_test.go | 25 ++++++++------- go/stats/export.go | 29 +++++++++--------- go/stats/export_test.go | 4 +-- go/stats/timings.go | 32 +++++++++---------- go/stats/timings_test.go | 10 +++--- go/vt/vttablet/tabletserver/query_engine.go | 3 +- 7 files changed, 72 insertions(+), 65 deletions(-) diff --git a/go/stats/counters.go b/go/stats/counters.go index d545b75c10c..46e9b8f904c 100644 --- a/go/stats/counters.go +++ b/go/stats/counters.go @@ -97,8 +97,8 @@ func (c *counters) Help() string { // It provides a Counts method which can be used for tracking rates. type CountersWithSingleLabel struct { counters - label string - labelDropped bool + label string + labelCombined bool } // NewCountersWithSingleLabel create a new Counters instance. @@ -113,12 +113,16 @@ func NewCountersWithSingleLabel(name, help, label string, tags ...string) *Count counts: make(map[string]int64), help: help, }, - label: label, - labelDropped: IsDimensionDropped(label), + label: label, + labelCombined: IsDimensionCombined(label), } - for _, tag := range tags { - c.counts[tag] = 0 + if c.labelCombined { + c.counts[StatsAllStr] = 0 + } else { + for _, tag := range tags { + c.counts[tag] = 0 + } } if name != "" { publish(name, c) @@ -136,7 +140,7 @@ func (c *CountersWithSingleLabel) Add(name string, value int64) { if value < 0 { logCounterNegative.Warningf("Adding a negative value to a counter, %v should be a gauge instead", c) } - if c.labelDropped { + if c.labelCombined { name = StatsAllStr } c.counters.add(name, value) @@ -144,7 +148,7 @@ func (c *CountersWithSingleLabel) Add(name string, value int64) { // Reset resets the value for the name. func (c *CountersWithSingleLabel) Reset(name string) { - if c.labelDropped { + if c.labelCombined { name = StatsAllStr } c.counters.set(name, 0) @@ -160,8 +164,8 @@ func (c *CountersWithSingleLabel) ResetAll() { // label value where all label values are joined with ".". type CountersWithMultiLabels struct { counters - labels []string - droppedLabels []bool + labels []string + combinedLabels []bool } // NewCountersWithMultiLabels creates a new CountersWithMultiLabels @@ -171,11 +175,11 @@ func NewCountersWithMultiLabels(name, help string, labels []string) *CountersWit counters: counters{ counts: make(map[string]int64), help: help}, - labels: labels, - droppedLabels: make([]bool, len(labels)), + labels: labels, + combinedLabels: make([]bool, len(labels)), } for i, label := range labels { - t.droppedLabels[i] = IsDimensionDropped(label) + t.combinedLabels[i] = IsDimensionCombined(label) } if name != "" { publish(name, t) @@ -198,7 +202,7 @@ func (mc *CountersWithMultiLabels) Add(names []string, value int64) { if value < 0 { logCounterNegative.Warningf("Adding a negative value to a counter, %v should be a gauge instead", mc) } - mc.counters.add(safeJoinLabels(names, mc.droppedLabels), value) + mc.counters.add(safeJoinLabels(names, mc.combinedLabels), value) } // Reset resets the value of a named counter back to 0. @@ -208,7 +212,7 @@ func (mc *CountersWithMultiLabels) Reset(names []string) { panic("CountersWithMultiLabels: wrong number of values in Reset") } - mc.counters.set(safeJoinLabels(names, mc.droppedLabels), 0) + mc.counters.set(safeJoinLabels(names, mc.combinedLabels), 0) } // ResetAll clears the counters diff --git a/go/stats/counters_test.go b/go/stats/counters_test.go index 21267ec026e..2fd9bc0fea1 100644 --- a/go/stats/counters_test.go +++ b/go/stats/counters_test.go @@ -243,25 +243,28 @@ func TestCountersFuncWithMultiLabels_Hook(t *testing.T) { } } -func TestCountersDropDimension(t *testing.T) { +func TestCountersCombineDimension(t *testing.T) { clear() - // Empty labels shouldn't be dropped. - c4 := NewCountersWithSingleLabel("counter_dropdim4", "help", "") - c4.Add("c1", 1) - assert.Equal(t, `{"c1": 1}`, c4.String()) + // Empty labels shouldn't be combined. + c0 := NewCountersWithSingleLabel("counter_combine_dim0", "help", "") + c0.Add("c1", 1) + assert.Equal(t, `{"c1": 1}`, c0.String()) clear() - *dropDimensions = "a,c" + *combineDimensions = "a,c" - c1 := NewCountersWithSingleLabel("counter_dropdim1", "help", "label") + c1 := NewCountersWithSingleLabel("counter_combine_dim1", "help", "label") c1.Add("c1", 1) assert.Equal(t, `{"c1": 1}`, c1.String()) - c2 := NewCountersWithSingleLabel("counter_dropdim2", "help", "a") + c2 := NewCountersWithSingleLabel("counter_combine_dim2", "help", "a") c2.Add("c1", 1) assert.Equal(t, `{"all": 1}`, c2.String()) - c3 := NewCountersWithMultiLabels("counter_dropdim3", "help", []string{"a", "b", "c"}) - c3.Add([]string{"c1", "c2", "c3"}, 1) - assert.Equal(t, `{"all.c2.all": 1}`, c3.String()) + c3 := NewCountersWithSingleLabel("counter_combine_dim3", "help", "a") + assert.Equal(t, `{"all": 0}`, c3.String()) + + c4 := NewCountersWithMultiLabels("counter_combine_dim4", "help", []string{"a", "b", "c"}) + c4.Add([]string{"c1", "c2", "c3"}, 1) + assert.Equal(t, `{"all.c2.all": 1}`, c4.String()) } diff --git a/go/stats/export.go b/go/stats/export.go index 0a9296b7511..ac4416adf58 100644 --- a/go/stats/export.go +++ b/go/stats/export.go @@ -43,9 +43,9 @@ import ( var emitStats = flag.Bool("emit_stats", false, "true iff we should emit stats to push-based monitoring/stats backends") var statsEmitPeriod = flag.Duration("stats_emit_period", time.Duration(60*time.Second), "Interval between emitting stats to all registered backends") var statsBackend = flag.String("stats_backend", "", "The name of the registered push-based monitoring/stats backend to use") -var dropDimensions = flag.String("drop_dimension", "", "List of dimensions to be dropped from stats vars") +var combineDimensions = flag.String("stats_combine_dimensions", "", `List of dimensions to be combined into a single "all" value in exported stats vars`) -// StatsAllStr is the consolidated name if a dimension gets dropped. +// StatsAllStr is the consolidated name if a dimension gets combined. const StatsAllStr = "all" // NewVarHook is the type of a hook to export variables in a different way @@ -254,35 +254,36 @@ func stringMapToString(m map[string]string) string { } var ( - dimMu sync.Mutex - droppedDimensions map[string]bool + dimMu sync.Mutex + combinedDimensions map[string]bool ) -// IsDimensionDropped returns true if the specified dimension is dropped. -func IsDimensionDropped(name string) bool { +// IsDimensionCombined returns true if the specified dimension should be combined. +func IsDimensionCombined(name string) bool { dimMu.Lock() defer dimMu.Unlock() - if droppedDimensions == nil { - dims := strings.Split(*dropDimensions, ",") - droppedDimensions = make(map[string]bool, len(dims)) + if combinedDimensions == nil { + dims := strings.Split(*combineDimensions, ",") + combinedDimensions = make(map[string]bool, len(dims)) for _, dim := range dims { if dim == "" { continue } - droppedDimensions[dim] = true + combinedDimensions[dim] = true } } - return droppedDimensions[name] + return combinedDimensions[name] } // safeJoinLabels joins the label values with ".", but first replaces any existing // "." characters in the labels with the proper replacement, to avoid issues parsing -// them apart later. -func safeJoinLabels(labels []string, droppedLabels []bool) string { +// them apart later. The function also replaces specific label values with "all" +// if a dimenstion is marked as true in combinedLabels. +func safeJoinLabels(labels []string, combinedLabels []bool) string { sanitizedLabels := make([]string, len(labels)) for idx, label := range labels { - if droppedLabels != nil && droppedLabels[idx] { + if combinedLabels != nil && combinedLabels[idx] { sanitizedLabels[idx] = StatsAllStr } else { sanitizedLabels[idx] = safeLabel(label) diff --git a/go/stats/export_test.go b/go/stats/export_test.go index 8aa5f07f4c6..b12939cd5c7 100644 --- a/go/stats/export_test.go +++ b/go/stats/export_test.go @@ -24,8 +24,8 @@ import ( func clear() { defaultVarGroup.vars = make(map[string]expvar.Var) defaultVarGroup.newVarHook = nil - *dropDimensions = "" - droppedDimensions = nil + *combineDimensions = "" + combinedDimensions = nil } func TestNoHook(t *testing.T) { diff --git a/go/stats/timings.go b/go/stats/timings.go index d9195176d31..697963c4773 100644 --- a/go/stats/timings.go +++ b/go/stats/timings.go @@ -34,9 +34,9 @@ type Timings struct { mu sync.RWMutex histograms map[string]*Histogram - help string - label string - labelDropped bool + help string + label string + labelCombined bool } // NewTimings creates a new Timings object, and publishes it if name is set. @@ -45,10 +45,10 @@ type Timings struct { // first time they are updated. func NewTimings(name, help, label string, categories ...string) *Timings { t := &Timings{ - histograms: make(map[string]*Histogram), - help: help, - label: label, - labelDropped: IsDimensionDropped(label), + histograms: make(map[string]*Histogram), + help: help, + label: label, + labelCombined: IsDimensionCombined(label), } for _, cat := range categories { t.histograms[cat] = NewGenericHistogram("", "", bucketCutoffs, bucketLabels, "Count", "Time") @@ -62,7 +62,7 @@ func NewTimings(name, help, label string, categories ...string) *Timings { // Add will add a new value to the named histogram. func (t *Timings) Add(name string, elapsed time.Duration) { - if t.labelDropped { + if t.labelCombined { name = StatsAllStr } // Get existing Histogram. @@ -90,7 +90,7 @@ func (t *Timings) Add(name string, elapsed time.Duration) { // Record is a convenience function that records completion // timing data based on the provided start time of an event. func (t *Timings) Record(name string, startTime time.Time) { - if t.labelDropped { + if t.labelCombined { name = StatsAllStr } t.Add(name, time.Since(startTime)) @@ -185,8 +185,8 @@ func init() { // with joining multiple strings with '.'. type MultiTimings struct { Timings - labels []string - droppedLabels []bool + labels []string + combinedLabels []bool } // NewMultiTimings creates a new MultiTimings object. @@ -196,11 +196,11 @@ func NewMultiTimings(name string, help string, labels []string) *MultiTimings { histograms: make(map[string]*Histogram), help: help, }, - labels: labels, - droppedLabels: make([]bool, len(labels)), + labels: labels, + combinedLabels: make([]bool, len(labels)), } for i, label := range labels { - t.droppedLabels[i] = IsDimensionDropped(label) + t.combinedLabels[i] = IsDimensionCombined(label) } if name != "" { publish(name, t) @@ -219,7 +219,7 @@ func (mt *MultiTimings) Add(names []string, elapsed time.Duration) { if len(names) != len(mt.labels) { panic("MultiTimings: wrong number of values in Add") } - mt.Timings.Add(safeJoinLabels(names, mt.droppedLabels), elapsed) + mt.Timings.Add(safeJoinLabels(names, mt.combinedLabels), elapsed) } // Record is a convenience function that records completion @@ -228,7 +228,7 @@ func (mt *MultiTimings) Record(names []string, startTime time.Time) { if len(names) != len(mt.labels) { panic("MultiTimings: wrong number of values in Record") } - mt.Timings.Record(safeJoinLabels(names, mt.droppedLabels), startTime) + mt.Timings.Record(safeJoinLabels(names, mt.combinedLabels), startTime) } // Cutoffs returns the cutoffs used in the component histograms. diff --git a/go/stats/timings_test.go b/go/stats/timings_test.go index 6ee89a312b5..435f5106ba2 100644 --- a/go/stats/timings_test.go +++ b/go/stats/timings_test.go @@ -80,21 +80,21 @@ func TestTimingsHook(t *testing.T) { } } -func TestTimingsDropDimension(t *testing.T) { +func TestTimingsCombineDimension(t *testing.T) { clear() - *dropDimensions = "a,c" + *combineDimensions = "a,c" - t1 := NewTimings("timing_dropdim1", "help", "label") + t1 := NewTimings("timing_combine_dim1", "help", "label") t1.Add("t1", 1*time.Nanosecond) want := `{"TotalCount":1,"TotalTime":1,"Histograms":{"t1":{"500000":1,"1000000":1,"5000000":1,"10000000":1,"50000000":1,"100000000":1,"500000000":1,"1000000000":1,"5000000000":1,"10000000000":1,"inf":1,"Count":1,"Time":1}}}` assert.Equal(t, want, t1.String()) - t2 := NewTimings("timing_dropdim2", "help", "a") + t2 := NewTimings("timing_combine_dim2", "help", "a") t2.Add("t1", 1) want = `{"TotalCount":1,"TotalTime":1,"Histograms":{"all":{"500000":1,"1000000":1,"5000000":1,"10000000":1,"50000000":1,"100000000":1,"500000000":1,"1000000000":1,"5000000000":1,"10000000000":1,"inf":1,"Count":1,"Time":1}}}` assert.Equal(t, want, t2.String()) - t3 := NewMultiTimings("timing_dropdim3", "help", []string{"a", "b", "c"}) + t3 := NewMultiTimings("timing_combine_dim3", "help", []string{"a", "b", "c"}) t3.Add([]string{"c1", "c2", "c3"}, 1) want = `{"TotalCount":1,"TotalTime":1,"Histograms":{"all.c2.all":{"500000":1,"1000000":1,"5000000":1,"10000000":1,"50000000":1,"100000000":1,"500000000":1,"1000000000":1,"5000000000":1,"10000000000":1,"inf":1,"Count":1,"Time":1}}}` assert.Equal(t, want, t3.String()) diff --git a/go/vt/vttablet/tabletserver/query_engine.go b/go/vt/vttablet/tabletserver/query_engine.go index be7e121ec0c..862f9950bd5 100644 --- a/go/vt/vttablet/tabletserver/query_engine.go +++ b/go/vt/vttablet/tabletserver/query_engine.go @@ -21,7 +21,6 @@ import ( "encoding/json" "fmt" "net/http" - "strings" "sync" "time" @@ -488,7 +487,7 @@ func (qe *QueryEngine) QueryPlanCacheCap() int { // AddStats adds the given stats for the planName.tableName func (qe *QueryEngine) AddStats(planName, tableName string, queryCount int64, duration, mysqlTime time.Duration, rowCount, errorCount int64) { // table names can contain "." characters, replace them! - keys := []string{strings.Replace(tableName, ".", "_", -1), planName} + keys := []string{tableName, planName} queryCounts.Add(keys, queryCount) queryTimes.Add(keys, int64(duration)) queryRowCounts.Add(keys, rowCount) From 308b4eca7c5f0fc944cec01a09a08ca4c60f60ff Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Fri, 14 Feb 2020 05:46:06 -0800 Subject: [PATCH 6/7] stats: drop variables: ability to drop variables It turns out that combining dimensions doesn't address all use cases. Gauges are also susceptible to cause huge /debug/vars. This additional feature allows you to drop specific variables from being exported. Ideally, they should also not be tracked. But we can iterate on this improvement later. Signed-off-by: Sugu Sougoumarane --- go/stats/export.go | 28 +++++++++++++++++++++++++--- go/stats/export_test.go | 11 +++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/go/stats/export.go b/go/stats/export.go index ac4416adf58..b61c7278118 100644 --- a/go/stats/export.go +++ b/go/stats/export.go @@ -44,6 +44,7 @@ var emitStats = flag.Bool("emit_stats", false, "true iff we should emit stats to var statsEmitPeriod = flag.Duration("stats_emit_period", time.Duration(60*time.Second), "Interval between emitting stats to all registered backends") var statsBackend = flag.String("stats_backend", "", "The name of the registered push-based monitoring/stats backend to use") var combineDimensions = flag.String("stats_combine_dimensions", "", `List of dimensions to be combined into a single "all" value in exported stats vars`) +var dropVariables = flag.String("stats_drop_variables", "", `Variables to be dropped from the list of exported variables.`) // StatsAllStr is the consolidated name if a dimension gets combined. const StatsAllStr = "all" @@ -76,6 +77,9 @@ func (vg *varGroup) register(nvh NewVarHook) { } func (vg *varGroup) publish(name string, v expvar.Var) { + if isVarDropped(name) { + return + } vg.Lock() defer vg.Unlock() @@ -254,14 +258,15 @@ func stringMapToString(m map[string]string) string { } var ( - dimMu sync.Mutex + varsMu sync.Mutex combinedDimensions map[string]bool + droppedVars map[string]bool ) // IsDimensionCombined returns true if the specified dimension should be combined. func IsDimensionCombined(name string) bool { - dimMu.Lock() - defer dimMu.Unlock() + varsMu.Lock() + defer varsMu.Unlock() if combinedDimensions == nil { dims := strings.Split(*combineDimensions, ",") @@ -295,3 +300,20 @@ func safeJoinLabels(labels []string, combinedLabels []bool) string { func safeLabel(label string) string { return strings.Replace(label, ".", "_", -1) } + +func isVarDropped(name string) bool { + varsMu.Lock() + defer varsMu.Unlock() + + if droppedVars == nil { + dims := strings.Split(*dropVariables, ",") + droppedVars = make(map[string]bool, len(dims)) + for _, dim := range dims { + if dim == "" { + continue + } + droppedVars[dim] = true + } + } + return droppedVars[name] +} diff --git a/go/stats/export_test.go b/go/stats/export_test.go index b12939cd5c7..a765af1ee50 100644 --- a/go/stats/export_test.go +++ b/go/stats/export_test.go @@ -25,7 +25,9 @@ func clear() { defaultVarGroup.vars = make(map[string]expvar.Var) defaultVarGroup.newVarHook = nil *combineDimensions = "" + *dropVariables = "" combinedDimensions = nil + droppedVars = nil } func TestNoHook(t *testing.T) { @@ -118,3 +120,12 @@ func TestPublishFunc(t *testing.T) { t.Errorf("want %v, got %#v", f(), gotv()) } } + +func TestDropVariable(t *testing.T) { + clear() + *dropVariables = "dropTest" + + // This should not panic. + _ = NewGaugesWithSingleLabel("dropTest", "help", "label") + _ = NewGaugesWithSingleLabel("dropTest", "help", "label") +} From dd1b5698c718acb7b7f9f6f2dec47f75c0100130 Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Sun, 1 Mar 2020 21:50:48 -0800 Subject: [PATCH 7/7] stats: address review comments Signed-off-by: Sugu Sougoumarane --- go/stats/counters_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/go/stats/counters_test.go b/go/stats/counters_test.go index 2fd9bc0fea1..03c3b244345 100644 --- a/go/stats/counters_test.go +++ b/go/stats/counters_test.go @@ -264,7 +264,10 @@ func TestCountersCombineDimension(t *testing.T) { c3 := NewCountersWithSingleLabel("counter_combine_dim3", "help", "a") assert.Equal(t, `{"all": 0}`, c3.String()) + // Anything under "a" and "c" should get reported under a consolidated "all" value + // instead of the specific supplied values. c4 := NewCountersWithMultiLabels("counter_combine_dim4", "help", []string{"a", "b", "c"}) c4.Add([]string{"c1", "c2", "c3"}, 1) - assert.Equal(t, `{"all.c2.all": 1}`, c4.String()) + c4.Add([]string{"c4", "c2", "c5"}, 1) + assert.Equal(t, `{"all.c2.all": 2}`, c4.String()) }