Remove support for expvar-backed metrics (jaegertracing#5437)

## Which problem is this PR solving?
- Part of jaegertracing#4722

## Description of the changes
- This PR removes deprecated expvar CLI flags for expvar-backed metrics
- The internal parameters/settings of some components are still exposed
via expvar. The implementation was changed to no longer depend on
go-kit, thus reducing the dependencies

## How was this change tested?
- Tested locally by running the application without the deprecated flags
to ensure that it functions correctly.

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ]  I have added unit tests for the new functionality
- [] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Joeyyy09 <[email protected]>
Signed-off-by: Harshith Mente <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>

cmd/agent/main.go

@@ -62,7 +62,7 @@ func main() {
 				Namespace(metrics.NSOptions{Name: "jaeger"}).
 				Namespace(metrics.NSOptions{Name: "agent"})
 			mFactory := fork.New("internal",
-				expvar.NewFactory(10), // backend for internal opts
+				expvar.NewFactory(), // expvar backend to report settings
 				baseFactory)
 			version.NewInfoMetrics(mFactory)
 

cmd/all-in-one/main.go

@@ -97,7 +97,7 @@ by default uses only in-memory database.`,
 			}
 			logger := svc.Logger // shortcut
 			baseFactory := fork.New("internal",
-				expvar.NewFactory(10), // backend for internal opts
+				expvar.NewFactory(), // expvar backend to report settings
 				svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"}))
 			version.NewInfoMetrics(baseFactory)
 			agentMetricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "agent"})

cmd/collector/main.go

@@ -74,7 +74,7 @@ func main() {
 			logger := svc.Logger // shortcut
 			baseFactory := svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})
 			metricsFactory := fork.New("internal",
-				expvar.NewFactory(10), // backend for internal opts
+				expvar.NewFactory(), // expvar backend to report settings
 				baseFactory.Namespace(metrics.NSOptions{Name: "collector"}))
 			version.NewInfoMetrics(metricsFactory)
 

examples/hotrod/cmd/flags.go

@@ -21,9 +21,8 @@ import (
 )
 
 var (
-	metricsBackend string
-	otelExporter   string // otlp, stdout
-	verbose        bool
+	otelExporter string // otlp, stdout
+	verbose      bool
 
 	fixDBConnDelay         time.Duration
 	fixDBConnDisableMutex  bool
@@ -38,11 +37,8 @@ var (
 	jaegerUI string
 )
 
-const expvarDepr = "(deprecated, will be removed after 2024-01-01 or in release v1.53.0, whichever is later) "
-
 // used by root command
 func addFlags(cmd *cobra.Command) {
-	cmd.PersistentFlags().StringVarP(&metricsBackend, "metrics", "m", "prometheus", expvarDepr+"Metrics backend (expvar|prometheus). ")
 	cmd.PersistentFlags().StringVarP(&otelExporter, "otel-exporter", "x", "otlp", "OpenTelemetry exporter (otlp|stdout)")
 
 	cmd.PersistentFlags().DurationVarP(&fixDBConnDelay, "fix-db-query-delay", "D", 300*time.Millisecond, "Average latency of MySQL DB query")

examples/hotrod/cmd/root.go

@@ -24,7 +24,6 @@ import (
 
 	"github.com/jaegertracing/jaeger/examples/hotrod/services/config"
 	"github.com/jaegertracing/jaeger/internal/jaegerclientenv2otel"
-	"github.com/jaegertracing/jaeger/internal/metrics/expvar"
 	"github.com/jaegertracing/jaeger/internal/metrics/prometheus"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
 )
@@ -57,6 +56,8 @@ func init() {
 
 // onInitialize is called before the command is executed.
 func onInitialize() {
+	jaegerclientenv2otel.MapJaegerToOtelEnvVars(logger)
+
 	zapOptions := []zap.Option{
 		zap.AddStacktrace(zapcore.FatalLevel),
 		zap.AddCallerSkip(1),
@@ -67,19 +68,8 @@ func onInitialize() {
 		)
 	}
 	logger, _ = zap.NewDevelopment(zapOptions...)
+	metricsFactory = prometheus.New().Namespace(metrics.NSOptions{Name: "hotrod", Tags: nil})
 
-	jaegerclientenv2otel.MapJaegerToOtelEnvVars(logger)
-
-	switch metricsBackend {
-	case "expvar":
-		metricsFactory = expvar.NewFactory(10) // 10 buckets for histograms
-		logger.Info("*** Using expvar as metrics backend " + expvarDepr)
-	case "prometheus":
-		metricsFactory = prometheus.New().Namespace(metrics.NSOptions{Name: "hotrod", Tags: nil})
-		logger.Info("Using Prometheus as metrics backend")
-	default:
-		logger.Fatal("unsupported metrics backend " + metricsBackend)
-	}
 	if config.MySQLGetDelay != fixDBConnDelay {
 		logger.Info("fix: overriding MySQL query delay", zap.Duration("old", config.MySQLGetDelay), zap.Duration("new", fixDBConnDelay))
 		config.MySQLGetDelay = fixDBConnDelay

go.mod

@@ -14,7 +14,6 @@ require (
 	github.com/dgraph-io/badger/v3 v3.2103.5
 	github.com/elastic/go-elasticsearch/v8 v8.13.1
 	github.com/fsnotify/fsnotify v1.7.0
-	github.com/go-kit/kit v0.13.0
 	github.com/go-logr/zapr v1.3.0
 	github.com/gocql/gocql v1.3.2
 	github.com/gogo/googleapis v1.4.1
@@ -84,7 +83,6 @@ require (
 
 require (
 	github.com/IBM/sarama v1.43.2 // indirect
-	github.com/VividCortex/gohistogram v1.0.0 // indirect
 	github.com/aws/aws-sdk-go v1.53.2 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cenkalti/backoff/v4 v4.3.0 // indirect

go.sum

@@ -12,8 +12,6 @@ github.com/Shopify/sarama v1.33.0 h1:2K4mB9M4fo46sAM7t6QTsmSO8dLX1OqznLM7vn3OjZ8
 github.com/Shopify/sarama v1.33.0/go.mod h1:lYO7LwEBkE0iAeTl94UfPSrDaavFzSFlmn+5isARATQ=
 github.com/Shopify/toxiproxy/v2 v2.3.0 h1:62YkpiP4bzdhKMH+6uC5E95y608k3zDwdzuBMsnn3uQ=
 github.com/Shopify/toxiproxy/v2 v2.3.0/go.mod h1:KvQTtB6RjCJY4zqNJn7C7JDFgsG5uoHYDirfUfpIm0c=
-github.com/VividCortex/gohistogram v1.0.0 h1:6+hBz+qvs0JOrrNhhmR7lFxo5sINxBCGXrdtl/UvroE=
-github.com/VividCortex/gohistogram v1.0.0/go.mod h1:Pf5mBqqDxYaXu3hDrrU+w6nw50o/4+TcAqDqk/vUH7g=
 github.com/ajstarks/svgo v0.0.0-20180226025133-644b8db467af/go.mod h1:K08gAheRH3/J6wwsYMMT4xOr94bZjxIelGM0+d/wbFw=
 github.com/apache/thrift v0.20.0 h1:631+KvYbsBZxmuJjYwhezVsrfc/TbqtZV4QcxOX1fOI=
 github.com/apache/thrift v0.20.0/go.mod h1:hOk1BQqcp2OLzGsyVXdfMk7YFlMxK3aoEVhjD06QhB8=
@@ -93,8 +91,6 @@ github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4
 github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
 github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
 github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
-github.com/go-kit/kit v0.13.0 h1:OoneCcHKHQ03LfBpoQCUfCluwd2Vt3ohz+kvbJneZAU=
-github.com/go-kit/kit v0.13.0/go.mod h1:phqEHMMUbyrCFCTgH48JueqrM3md2HcAZ8N3XE4FKDg=
 github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY=
 github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A=
 github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=

internal/metrics/expvar/factory.go

@@ -18,17 +18,16 @@ package expvar
 import (
 	"sort"
 
-	kexpvar "github.com/go-kit/kit/metrics/expvar"
-
 	"github.com/jaegertracing/jaeger/pkg/metrics"
 )
 
-// NewFactory creates a new metrics factory using go-kit expvar package.
-// buckets is the number of buckets to be used in histograms.
-// Custom buckets passed via options are not supported.
-func NewFactory(buckets int) metrics.Factory {
+// NewFactory creates a new metrics factory backed by expvar variables.
+// Histograms are not supported, only act as gauges.
+// This package is primarily used to expose config parameters / settings
+// via expvar variables for manual inspection, not for remove monitoring,
+// since they are usually emitted only once.
+func NewFactory() metrics.Factory {
 	return &factory{
-		buckets:  buckets,
 		scope:    "",
 		scopeSep: ".",
 		tagsSep:  ".",
@@ -38,8 +37,6 @@ func NewFactory(buckets int) metrics.Factory {
 }
 
 type factory struct {
-	buckets int
-
 	scope    string
 	tags     map[string]string
 	scopeSep string
@@ -96,34 +93,33 @@ func makeKey(name string, tags map[string]string, tagsSep string, tagKVSep strin
 func (f *factory) Counter(options metrics.Options) metrics.Counter {
 	key := f.getKey(options.Name, options.Tags)
 	return f.cache.getOrSetCounter(key, func() metrics.Counter {
-		return NewCounter(kexpvar.NewCounter(key))
+		return NewCounter(key)
 	})
 }
 
 func (f *factory) Gauge(options metrics.Options) metrics.Gauge {
 	key := f.getKey(options.Name, options.Tags)
 	return f.cache.getOrSetGauge(key, func() metrics.Gauge {
-		return NewGauge(kexpvar.NewGauge(key))
+		return NewGauge(key)
 	})
 }
 
 func (f *factory) Timer(options metrics.TimerOptions) metrics.Timer {
 	key := f.getKey(options.Name, options.Tags)
 	return f.cache.getOrSetTimer(key, func() metrics.Timer {
-		return NewTimer(kexpvar.NewHistogram(key, f.buckets))
+		return NewTimer(key)
 	})
 }
 
 func (f *factory) Histogram(options metrics.HistogramOptions) metrics.Histogram {
 	key := f.getKey(options.Name, options.Tags)
 	return f.cache.getOrSetHistogram(key, func() metrics.Histogram {
-		return NewHistogram(kexpvar.NewHistogram(key, f.buckets))
+		return NewHistogram(key)
 	})
 }
 
 func (f *factory) Namespace(options metrics.NSOptions) metrics.Factory {
 	return &factory{
-		buckets:  f.buckets,
 		scope:    f.subScope(options.Name),
 		tags:     f.mergeTags(options.Tags),
 		scopeSep: f.scopeSep,

internal/metrics/expvar/factory_test.go

@@ -40,29 +40,28 @@ var (
 )
 
 func TestFactory(t *testing.T) {
-	buckets := []float64{10, 20, 30, 40, 50, 60}
 	testCases := []struct {
 		name            string
 		tags            map[string]string
-		buckets         []float64
 		durationBuckets []time.Duration
 		namespace       string
 		nsTags          map[string]string
 		fullName        string
 		expectedCounter string
 	}{
-		{name: "x", fullName: "%sx", buckets: buckets},
-		{tags: tagsX, fullName: "%s.x_y", buckets: buckets},
-		{name: "x", tags: tagsA, fullName: "%sx.a_b", buckets: buckets},
-		{namespace: "y", fullName: "y.%s", buckets: buckets},
-		{nsTags: tagsA, fullName: "%s.a_b", buckets: buckets},
-		{namespace: "y", nsTags: tagsX, fullName: "y.%s.x_y", buckets: buckets},
-		{name: "x", namespace: "y", nsTags: tagsX, fullName: "y.%sx.x_y", buckets: buckets},
-		{name: "x", tags: tagsX, namespace: "y", nsTags: tagsX, fullName: "y.%sx.x_y", expectedCounter: "84", buckets: buckets},
-		{name: "x", tags: tagsA, namespace: "y", nsTags: tagsX, fullName: "y.%sx.a_b.x_y", buckets: buckets},
-		{name: "x", tags: tagsX, namespace: "y", nsTags: tagsA, fullName: "y.%sx.a_b.x_y", expectedCounter: "84", buckets: buckets},
+		{name: "x", fullName: "%sx"},
+		{name: "x", fullName: "%sx", expectedCounter: "84"}, // same as above to validate that the same vars are ok to create
+		{tags: tagsX, fullName: "%s.x_y"},
+		{name: "x", tags: tagsA, fullName: "%sx.a_b"},
+		{namespace: "y", fullName: "y.%s"},
+		{nsTags: tagsA, fullName: "%s.a_b"},
+		{namespace: "y", nsTags: tagsX, fullName: "y.%s.x_y"},
+		{name: "x", namespace: "y", nsTags: tagsX, fullName: "y.%sx.x_y"},
+		{name: "x", tags: tagsX, namespace: "y", nsTags: tagsX, fullName: "y.%sx.x_y", expectedCounter: "84"},
+		{name: "x", tags: tagsA, namespace: "y", nsTags: tagsX, fullName: "y.%sx.a_b.x_y"},
+		{name: "x", tags: tagsX, namespace: "y", nsTags: tagsA, fullName: "y.%sx.a_b.x_y", expectedCounter: "84"},
 	}
-	f := NewFactory(2)
+	f := NewFactory()
 	for _, testCase := range testCases {
 		t.Run("", func(t *testing.T) {
 			if testCase.expectedCounter == "" {
@@ -89,9 +88,8 @@ func TestFactory(t *testing.T) {
 				Buckets: testCase.durationBuckets,
 			})
 			histogram := ff.Histogram(metrics.HistogramOptions{
-				Name:    histogramPrefix + testCase.name,
-				Tags:    testCase.tags,
-				Buckets: testCase.buckets,
+				Name: histogramPrefix + testCase.name,
+				Tags: testCase.tags,
 			})
 
 			// register second time, should not panic
@@ -109,20 +107,19 @@ func TestFactory(t *testing.T) {
 				Buckets: testCase.durationBuckets,
 			})
 			ff.Histogram(metrics.HistogramOptions{
-				Name:    histogramPrefix + testCase.name,
-				Tags:    testCase.tags,
-				Buckets: testCase.buckets,
+				Name: histogramPrefix + testCase.name,
+				Tags: testCase.tags,
 			})
 
 			counter.Inc(42)
 			gauge.Update(42)
 			timer.Record(42 * time.Millisecond)
-			histogram.Record(42)
+			histogram.Record(42.42)
 
 			assertExpvar(t, fmt.Sprintf(testCase.fullName, counterPrefix), testCase.expectedCounter)
 			assertExpvar(t, fmt.Sprintf(testCase.fullName, gaugePrefix), "42")
-			assertExpvar(t, fmt.Sprintf(testCase.fullName, timerPrefix)+".p99", "0.042")
-			assertExpvar(t, fmt.Sprintf(testCase.fullName, histogramPrefix)+".p99", "42")
+			assertExpvar(t, fmt.Sprintf(testCase.fullName, timerPrefix)+"_ns", "42000000")
+			assertExpvar(t, fmt.Sprintf(testCase.fullName, histogramPrefix), "42.42")
 		})
 	}
 }

internal/metrics/expvar/metrics.go

@@ -16,67 +16,70 @@
 package expvar
 
 import (
+	"expvar"
+	"strings"
 	"time"
-
-	kit "github.com/go-kit/kit/metrics"
 )
 
 // Counter is an adapter from go-kit Counter to jaeger-lib Counter
 type Counter struct {
-	counter kit.Counter
+	intVar *expvar.Int
 }
 
 // NewCounter creates a new Counter
-func NewCounter(counter kit.Counter) *Counter {
-	return &Counter{counter: counter}
+func NewCounter(key string) *Counter {
+	return &Counter{intVar: expvar.NewInt(key)}
 }
 
 // Inc adds the given value to the counter.
 func (c *Counter) Inc(delta int64) {
-	c.counter.Add(float64(delta))
+	c.intVar.Add(delta)
 }
 
 // Gauge is an adapter from go-kit Gauge to jaeger-lib Gauge
 type Gauge struct {
-	gauge kit.Gauge
+	intVar *expvar.Int
 }
 
 // NewGauge creates a new Gauge
-func NewGauge(gauge kit.Gauge) *Gauge {
-	return &Gauge{gauge: gauge}
+func NewGauge(key string) *Gauge {
+	return &Gauge{intVar: expvar.NewInt(key)}
 }
 
 // Update the gauge to the value passed in.
 func (g *Gauge) Update(value int64) {
-	g.gauge.Set(float64(value))
+	g.intVar.Set(value)
 }
 
-// Timer is an adapter from go-kit Histogram to jaeger-lib Timer
+// Timer only records the latest value (like a Gauge).
 type Timer struct {
-	hist kit.Histogram
+	intVar *expvar.Int
 }
 
-// NewTimer creates a new Timer
-func NewTimer(hist kit.Histogram) *Timer {
-	return &Timer{hist: hist}
+// NewTimer creates a new Timer.
+func NewTimer(key string) *Timer {
+	if !strings.HasSuffix(key, "_ns") {
+		key += "_ns"
+	}
+	return &Timer{intVar: expvar.NewInt(key)}
 }
 
 // Record saves the time passed in.
 func (t *Timer) Record(delta time.Duration) {
-	t.hist.Observe(delta.Seconds())
+	t.intVar.Set(delta.Nanoseconds())
 }
 
-// Histogram is an adapter from go-kit Histogram to jaeger-lib Histogram
+// Histogram only records the latest value (like a Gauge).
 type Histogram struct {
-	hist kit.Histogram
+	floatVar *expvar.Float
 }
 
 // NewHistogram creates a new Histogram
-func NewHistogram(hist kit.Histogram) *Histogram {
-	return &Histogram{hist: hist}
+func NewHistogram(key string) *Histogram {
+	return &Histogram{floatVar: expvar.NewFloat(key)}
 }
 
 // Record saves the value passed in.
 func (t *Histogram) Record(value float64) {
-	t.hist.Observe(value)
+	t.floatVar.Set(value)
 }