Deprecated metrics deletion (#16086)

Signed-off-by: Manan Gupta <[email protected]>
vitessio · Jun 11, 2024 · 4f04b8b · 4f04b8b
1 parent e0325be
commit 4f04b8b
Show file tree

Hide file tree

Showing 19 changed files with 55 additions and 296 deletions.
diff --git a/changelog/21.0/21.0.0/summary.md b/changelog/21.0/21.0.0/summary.md
@@ -0,0 +1,36 @@
+
+## Summary
+
+### Table of Contents
+
+- **[Major Changes](#major-changes)**
+  - **[Deletions](#deletions)** 
+    - [Deletion of deprecated metrics](#metric-deletion)
+  - **[Breaking changes](#breaking-changes)**
+
+## <a id="major-changes"/>Major Changes
+
+### <a id="deletions"/>Deletion
+
+#### <a id="metric-deletion"/>Deletion of deprecated metrics
+
+The following metrics that were deprecated in the previous release, have now been deleted.
+
+
+|                 Metric Name                  |
+|:--------------------------------------------:|
+|           `analysis.change.write`            |        
+|                `audit.write`                 |     
+|            `discoveries.attempt`             |          
+|              `discoveries.fail`              |        
+| `discoveries.instance_poll_seconds_exceeded` | 
+|          `discoveries.queue_length`          |       
+|          `discoveries.recent_count`          |        
+|               `instance.read`                |            
+|           `instance.read_topology`           |       
+|         `emergency_reparent_counts`          |       
+|          `planned_reparent_counts`           |      
+|      `reparent_shard_operation_timings`      |  
+		
+
+
diff --git a/changelog/21.0/README.md b/changelog/21.0/README.md
@@ -0,0 +1,2 @@
+## v21.0
+* **[21.0.0](21.0.0)**
diff --git a/changelog/README.md b/changelog/README.md
@@ -1,4 +1,5 @@
 ## Releases
+* [21.0](21.0)
 * [20.0](20.0)
 * [19.0](19.0)
 * [18.0](18.0)

diff --git a/go/stats/counter.go b/go/stats/counter.go
@@ -17,8 +17,6 @@ limitations under the License.
 package stats
 
 import (
-	"expvar"
-	"fmt"
 	"math"
 	"strconv"
 	"sync/atomic"
@@ -47,22 +45,6 @@ func NewCounter(name string, help string) *Counter {
 	return v
 }
 
-// NewCounterWithDeprecatedName returns a new Counter 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 NewCounterWithDeprecatedName(name string, deprecatedName string, help string) *Counter {
-	// 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))
-	}
-
-	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.
-	expvar.Publish(name, v)
-	return v
-}
-
 // Add adds the provided value to the Counter.
 func (v *Counter) Add(delta int64) {
 	if delta < 0 {
@@ -154,21 +136,6 @@ func NewGauge(name string, help string) *Gauge {
 	return v
 }
 
-// NewGaugeWithDeprecatedName creates a new Gauge and publishes it if name is set 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 NewGaugeWithDeprecatedName(name string, deprecatedName string, help string) *Gauge {
-	// 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))
-	}
-
-	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.
-	expvar.Publish(name, v)
-	return v
-}
-
 // Set overwrites the current value.
 func (v *Gauge) Set(value int64) {
 	v.Counter.i.Store(value)

diff --git a/go/stats/counter_test.go b/go/stats/counter_test.go
@@ -18,12 +18,9 @@ package stats
 
 import (
 	"expvar"
-	"fmt"
-	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func TestCounter(t *testing.T) {
@@ -94,95 +91,3 @@ func TestGaugeFloat64(t *testing.T) {
 	v.Reset()
 	assert.Equal(t, float64(0), v.Get())
 }
-
-func TestNewCounterWithDeprecatedName(t *testing.T) {
-	clearStats()
-	Register(func(name string, v expvar.Var) {})
-
-	testcases := []struct {
-		name           string
-		deprecatedName string
-		shouldPanic    bool
-	}{
-		{
-			name:           "new_name",
-			deprecatedName: "deprecatedName",
-			shouldPanic:    true,
-		},
-		{
-			name:           "metricName_test",
-			deprecatedName: "metric.name-test",
-			shouldPanic:    false,
-		},
-		{
-			name:           "MetricNameTesting",
-			deprecatedName: "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()
-				}()
-				NewCounterWithDeprecatedName(testcase.name, testcase.deprecatedName, "help")
-			}()
-			wg.Wait()
-			require.EqualValues(t, testcase.shouldPanic, panicReceived)
-		})
-	}
-}
-
-func TestNewGaugeWithDeprecatedName(t *testing.T) {
-	clearStats()
-	Register(func(name string, v expvar.Var) {})
-
-	testcases := []struct {
-		name           string
-		deprecatedName string
-		shouldPanic    bool
-	}{
-		{
-			name:           "gauge_new_name",
-			deprecatedName: "gauge_deprecatedName",
-			shouldPanic:    true,
-		},
-		{
-			name:           "gauge-metricName_test",
-			deprecatedName: "gauge_metric.name-test",
-			shouldPanic:    false,
-		},
-		{
-			name:           "GaugeMetricNameTesting",
-			deprecatedName: "gauge.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()
-				}()
-				NewGaugeWithDeprecatedName(testcase.name, testcase.deprecatedName, "help")
-			}()
-			wg.Wait()
-			require.EqualValues(t, testcase.shouldPanic, panicReceived)
-		})
-	}
-}
diff --git a/go/stats/counters.go b/go/stats/counters.go
@@ -18,7 +18,6 @@ package stats
 
 import (
 	"bytes"
-	"expvar"
 	"fmt"
 	"strings"
 	"sync"
@@ -186,21 +185,6 @@ 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
@@ -18,17 +18,14 @@ 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) {
@@ -272,49 +269,3 @@ 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
@@ -18,7 +18,6 @@ package stats
 
 import (
 	"encoding/json"
-	"expvar"
 	"fmt"
 	"sync"
 	"sync/atomic"
@@ -62,21 +61,6 @@ 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()