diff --git a/changelog/20.0/20.0.0/summary.md b/changelog/20.0/20.0.0/summary.md index 20e17744f8d..1bf7b16d296 100644 --- a/changelog/20.0/20.0.0/summary.md +++ b/changelog/20.0/20.0.0/summary.md @@ -140,6 +140,9 @@ The following metric names have been changed in VTOrc. The old metrics are still | `discoveries.recent_count` | `DiscoveriesRecentCount` | `vtorc_discoveries_recent_count` | | `instance.read` | `InstanceRead` | `vtorc_instance_read` | | `instance.read_topology` | `InstanceReadTopology` | `vtorc_instance_read_topology` | +| `emergency_reparent_counts` | `EmergencyReparentCounts` | `vtorc_emergency_reparent_counts` | +| `planned_reparent_counts` | `PlannedReparentCounts` | `vtorc_planned_reparent_counts` | +| `reparent_shard_operation_timings` | `ReparentShardOperationTimings` | `vtorc_reparent_shard_operation_timings_bucket` | diff --git a/go/stats/counter.go b/go/stats/counter.go index d38929d64b6..e74969039f6 100644 --- a/go/stats/counter.go +++ b/go/stats/counter.go @@ -55,10 +55,10 @@ func NewCounterWithDeprecatedName(name string, deprecatedName string, help strin if deprecatedName == "" || GetSnakeName(name) != GetSnakeName(deprecatedName) { panic(fmt.Sprintf("New name for deprecated metric doesn't have the same snake case - %v", deprecatedName)) } - v := &Counter{help: help} - // We want to publish the deprecated name for backward compatibility. + + v := NewCounter(deprecatedName, help) + // We have already published the deprecated name for backward compatibility. // At the same time we want the new metric to be visible on the `/debug/vars` page, so we publish the new name in expvar. - publish(deprecatedName, v) expvar.Publish(name, v) return v } @@ -161,11 +161,10 @@ func NewGaugeWithDeprecatedName(name string, deprecatedName string, help string) if deprecatedName == "" || GetSnakeName(name) != GetSnakeName(deprecatedName) { panic(fmt.Sprintf("New name for deprecated metric doesn't have the same snake case - %v", deprecatedName)) } - v := &Gauge{Counter: Counter{help: help}} - // We want to publish the deprecated name for backward compatibility. + v := NewGauge(deprecatedName, help) + // We have already published the deprecated name for backward compatibility. // At the same time we want the new metric to be visible on the `/debug/vars` page, so we publish the new name in expvar. - publish(deprecatedName, v) expvar.Publish(name, v) return v } diff --git a/go/stats/counters.go b/go/stats/counters.go index bcf7fc3a8b6..b3099993b09 100644 --- a/go/stats/counters.go +++ b/go/stats/counters.go @@ -18,6 +18,7 @@ package stats import ( "bytes" + "expvar" "fmt" "strings" "sync" @@ -185,6 +186,21 @@ func NewCountersWithMultiLabels(name, help string, labels []string) *CountersWit return t } +// NewCountersWithMultiLabelsWithDeprecatedName returns a new CountersWithMultiLabels that also has a deprecated name that can be removed in a future release. +// It is important to ensure that we only call this function with values for name and deprecatedName such that they match to the same +// metric name in snake case. +func NewCountersWithMultiLabelsWithDeprecatedName(name string, deprecatedName string, help string, labels []string) *CountersWithMultiLabels { + // Ensure that the snake case for the deprecated name and the new name are the same. + if deprecatedName == "" || GetSnakeName(name) != GetSnakeName(deprecatedName) { + panic(fmt.Sprintf("New name for deprecated metric doesn't have the same snake case - %v", deprecatedName)) + } + t := NewCountersWithMultiLabels(deprecatedName, help, labels) + // We have already published the deprecated name for backward compatibility. + // At the same time we want the new metric to be visible on the `/debug/vars` page, so we publish the new name in expvar. + expvar.Publish(name, t) + return t +} + // Labels returns the list of labels. func (mc *CountersWithMultiLabels) Labels() []string { return mc.labels diff --git a/go/stats/counters_test.go b/go/stats/counters_test.go index 72eb11e1a10..e1b171a2765 100644 --- a/go/stats/counters_test.go +++ b/go/stats/counters_test.go @@ -18,14 +18,17 @@ package stats import ( "expvar" + "fmt" "math/rand/v2" "reflect" "sort" "strings" + "sync" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCounters(t *testing.T) { @@ -269,3 +272,49 @@ func TestCountersCombineDimension(t *testing.T) { c4.Add([]string{"c4", "c2", "c5"}, 1) assert.Equal(t, `{"all.c2.all": 2}`, c4.String()) } + +func TestNewCountersWithMultiLabelsWithDeprecatedName(t *testing.T) { + clearStats() + Register(func(name string, v expvar.Var) {}) + + testcases := []struct { + name string + deprecatedName string + shouldPanic bool + }{ + { + name: "counterWithMultiLabels_new_name", + deprecatedName: "counterWithMultiLabels_deprecatedName", + shouldPanic: true, + }, + { + name: "counterWithMultiLabels-metricName_test", + deprecatedName: "counterWithMultiLabels_metric.name-test", + shouldPanic: false, + }, + { + name: "CounterWithMultiLabelsMetricNameTesting", + deprecatedName: "counterWithMultiLabels.metric.name.testing", + shouldPanic: false, + }, + } + + for _, testcase := range testcases { + t.Run(fmt.Sprintf("%v-%v", testcase.name, testcase.deprecatedName), func(t *testing.T) { + wg := sync.WaitGroup{} + wg.Add(1) + panicReceived := false + go func() { + defer func() { + if x := recover(); x != nil { + panicReceived = true + } + wg.Done() + }() + NewCountersWithMultiLabelsWithDeprecatedName(testcase.name, testcase.deprecatedName, "help", []string{"1", "2", "3"}) + }() + wg.Wait() + require.EqualValues(t, testcase.shouldPanic, panicReceived) + }) + } +} diff --git a/go/stats/timings.go b/go/stats/timings.go index d0fb82ebedf..9742ad4d67c 100644 --- a/go/stats/timings.go +++ b/go/stats/timings.go @@ -18,6 +18,7 @@ package stats import ( "encoding/json" + "expvar" "fmt" "sync" "sync/atomic" @@ -61,6 +62,21 @@ func NewTimings(name, help, label string, categories ...string) *Timings { return t } +// NewTimingsWithDeprecatedName returns a new Timings that also has a deprecated name that can be removed in a future release. +// It is important to ensure that we only call this function with values for name and deprecatedName such that they match to the same +// metric name in snake case. +func NewTimingsWithDeprecatedName(name string, deprecatedName string, help, label string, categories ...string) *Timings { + // Ensure that the snake case for the deprecated name and the new name are the same. + if deprecatedName == "" || GetSnakeName(name) != GetSnakeName(deprecatedName) { + panic(fmt.Sprintf("New name for deprecated metric doesn't have the same snake case - %v", deprecatedName)) + } + t := NewTimings(deprecatedName, help, label, categories...) + // We have already published the deprecated name for backward compatibility. + // At the same time we want the new metric to be visible on the `/debug/vars` page, so we publish the new name in expvar. + expvar.Publish(name, t) + return t +} + // Reset will clear histograms and counters: used during testing func (t *Timings) Reset() { t.mu.RLock() diff --git a/go/stats/timings_test.go b/go/stats/timings_test.go index a632f3fba6a..518d38947e5 100644 --- a/go/stats/timings_test.go +++ b/go/stats/timings_test.go @@ -18,11 +18,14 @@ package stats import ( "expvar" + "fmt" "strings" + "sync" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestTimings(t *testing.T) { @@ -99,3 +102,49 @@ func TestTimingsCombineDimension(t *testing.T) { want = `{"TotalCount":1,"TotalTime":1,"Histograms":{"all.c2.all":{"500000":1,"1000000":0,"5000000":0,"10000000":0,"50000000":0,"100000000":0,"500000000":0,"1000000000":0,"5000000000":0,"10000000000":0,"inf":0,"Count":1,"Time":1}}}` assert.Equal(t, want, t3.String()) } + +func TestNewTimingsWithDeprecatedName(t *testing.T) { + clearStats() + Register(func(name string, v expvar.Var) {}) + + testcases := []struct { + name string + deprecatedName string + shouldPanic bool + }{ + { + name: "timings_new_name", + deprecatedName: "timings_deprecatedName", + shouldPanic: true, + }, + { + name: "timings-metricName_test", + deprecatedName: "timings_metric.name-test", + shouldPanic: false, + }, + { + name: "TimingsMetricNameTesting", + deprecatedName: "timings.metric.name.testing", + shouldPanic: false, + }, + } + + for _, testcase := range testcases { + t.Run(fmt.Sprintf("%v-%v", testcase.name, testcase.deprecatedName), func(t *testing.T) { + wg := sync.WaitGroup{} + wg.Add(1) + panicReceived := false + go func() { + defer func() { + if x := recover(); x != nil { + panicReceived = true + } + wg.Done() + }() + NewTimingsWithDeprecatedName(testcase.name, testcase.deprecatedName, "help", "label", []string{"1", "2", "3"}...) + }() + wg.Wait() + require.EqualValues(t, testcase.shouldPanic, panicReceived) + }) + } +} diff --git a/go/test/endtoend/vtorc/primaryfailure/primary_failure_test.go b/go/test/endtoend/vtorc/primaryfailure/primary_failure_test.go index d91dadddcb4..645b413799c 100644 --- a/go/test/endtoend/vtorc/primaryfailure/primary_failure_test.go +++ b/go/test/endtoend/vtorc/primaryfailure/primary_failure_test.go @@ -101,6 +101,22 @@ func TestDownPrimary(t *testing.T) { utils.VerifyWritesSucceed(t, clusterInfo, replica, []*cluster.Vttablet{crossCellReplica}, 10*time.Second) utils.WaitForSuccessfulRecoveryCount(t, vtOrcProcess, logic.RecoverDeadPrimaryRecoveryName, 1) utils.WaitForSuccessfulERSCount(t, vtOrcProcess, keyspace.Name, shard0.Name, 1) + t.Run("Check ERS and PRS Vars and Metrics", func(t *testing.T) { + // These are vars that will be deprecated in v21. + utils.CheckVarExists(t, vtOrcProcess, "emergency_reparent_counts") + utils.CheckVarExists(t, vtOrcProcess, "planned_reparent_counts") + utils.CheckVarExists(t, vtOrcProcess, "reparent_shard_operation_timings") + + // Newly added vars + utils.CheckVarExists(t, vtOrcProcess, "EmergencyReparentCounts") + utils.CheckVarExists(t, vtOrcProcess, "PlannedReparentCounts") + utils.CheckVarExists(t, vtOrcProcess, "ReparentShardOperationTimings") + + // Metrics registered in prometheus + utils.CheckMetricExists(t, vtOrcProcess, "vtorc_emergency_reparent_counts") + utils.CheckMetricExists(t, vtOrcProcess, "vtorc_planned_reparent_counts") + utils.CheckMetricExists(t, vtOrcProcess, "vtorc_reparent_shard_operation_timings_bucket") + }) } // bring down primary before VTOrc has started, let vtorc repair. diff --git a/go/test/endtoend/vtorc/utils/utils.go b/go/test/endtoend/vtorc/utils/utils.go index 7df3898d9f3..5982589af85 100644 --- a/go/test/endtoend/vtorc/utils/utils.go +++ b/go/test/endtoend/vtorc/utils/utils.go @@ -1057,7 +1057,7 @@ func CheckVarExists(t *testing.T, vtorcInstance *cluster.VTOrcProcess, metricNam t.Helper() vars := vtorcInstance.GetVars() _, exists := vars[metricName] - assert.True(t, exists) + assert.True(t, exists, vars) } // CheckMetricExists checks whether the given metric exists or not in /metrics. diff --git a/go/vt/vtctl/reparentutil/emergency_reparenter.go b/go/vt/vtctl/reparentutil/emergency_reparenter.go index c3542850bee..d7e5b8a8445 100644 --- a/go/vt/vtctl/reparentutil/emergency_reparenter.go +++ b/go/vt/vtctl/reparentutil/emergency_reparenter.go @@ -68,7 +68,7 @@ type EmergencyReparentOptions struct { } // counters for Emergency Reparent Shard -var ersCounter = stats.NewCountersWithMultiLabels("emergency_reparent_counts", "Number of times Emergency Reparent Shard has been run", +var ersCounter = stats.NewCountersWithMultiLabelsWithDeprecatedName("EmergencyReparentCounts", "emergency_reparent_counts", "Number of times Emergency Reparent Shard has been run", []string{"Keyspace", "Shard", "Result"}, ) diff --git a/go/vt/vtctl/reparentutil/planned_reparenter.go b/go/vt/vtctl/reparentutil/planned_reparenter.go index 7221508b702..3ef327987e3 100644 --- a/go/vt/vtctl/reparentutil/planned_reparenter.go +++ b/go/vt/vtctl/reparentutil/planned_reparenter.go @@ -41,7 +41,7 @@ import ( // counters for Planned Reparent Shard var ( - prsCounter = stats.NewCountersWithMultiLabels("planned_reparent_counts", "Number of times Planned Reparent Shard has been run", + prsCounter = stats.NewCountersWithMultiLabelsWithDeprecatedName("PlannedReparentCounts", "planned_reparent_counts", "Number of times Planned Reparent Shard has been run", []string{"Keyspace", "Shard", "Result"}, ) ) diff --git a/go/vt/vtctl/reparentutil/util.go b/go/vt/vtctl/reparentutil/util.go index b3c4061ab70..4ea4d25ed83 100644 --- a/go/vt/vtctl/reparentutil/util.go +++ b/go/vt/vtctl/reparentutil/util.go @@ -44,7 +44,7 @@ import ( ) var ( - reparentShardOpTimings = stats.NewTimings("reparent_shard_operation_timings", "Timings of reparent shard operations", "Operation") + reparentShardOpTimings = stats.NewTimingsWithDeprecatedName("ReparentShardOperationTimings", "reparent_shard_operation_timings", "Timings of reparent shard operations", "Operation") failureResult = "failure" successResult = "success" )