Replace internal metrics.Factory usage with direct calls to expvar (#…

…5496)

## Which problem is this PR solving?
- fixes: #5495

## Description of the changes
 * removed fork.New factory usage
* identify which internal components use "internal" namespace to report
settings and replace with expvar directly
 * removed internal/metrics/fork/*
 * removed internal/metrics/expvar/*
 * fixed tests

## How was this change tested?
- 

## Checklist
- [ ] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [ ] 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: Griffin <prakritimandal611@gmail.com>

cmd/agent/app/agent_test.go

@@ -28,7 +28,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 
-	"github.com/jaegertracing/jaeger/internal/metrics/fork"
 	"github.com/jaegertracing/jaeger/internal/metrics/metricsbuilder"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
 	"github.com/jaegertracing/jaeger/pkg/testutils"
@@ -103,11 +102,10 @@ func withRunningAgent(t *testing.T, testcase func(string, chan error)) {
 	logger, logBuf := testutils.NewLogger()
 	mBldr := &metricsbuilder.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
 	metricsFactory, err := mBldr.CreateMetricsFactory("jaeger")
-	mFactory := fork.New("internal", metrics.NullFactory, metricsFactory)
 	require.NoError(t, err)
-	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, mFactory)
+	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, metricsFactory)
 	require.NoError(t, err)
-	if h := mBldr.Handler(); mFactory != nil && h != nil {
+	if h := mBldr.Handler(); metricsFactory != nil && h != nil {
 		logger.Info("Registering metrics handler with HTTP server", zap.String("route", mBldr.HTTPRoute))
 		agent.GetHTTPRouter().Handle(mBldr.HTTPRoute, h).Methods(http.MethodGet)
 	}
@@ -158,9 +156,8 @@ func TestStartStopRace(t *testing.T) {
 	logger, logBuf := testutils.NewEchoLogger(t)
 	mBldr := &metricsbuilder.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
 	metricsFactory, err := mBldr.CreateMetricsFactory("jaeger")
-	mFactory := fork.New("internal", metrics.NullFactory, metricsFactory)
 	require.NoError(t, err)
-	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, mFactory)
+	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, metricsFactory)
 	require.NoError(t, err)
 
 	// This test attempts to hit the data race bug when Stop() is called
@@ -202,9 +199,8 @@ func TestStartStopWithSocketBufferSet(t *testing.T) {
 	}
 	mBldr := &metricsbuilder.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
 	metricsFactory, err := mBldr.CreateMetricsFactory("jaeger")
-	mFactory := fork.New("internal", metrics.NullFactory, metricsFactory)
 	require.NoError(t, err)
-	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), mFactory)
+	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metricsFactory)
 	require.NoError(t, err)
 
 	if err := agent.Run(); err != nil {

cmd/agent/app/builder.go

@@ -31,6 +31,7 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/agent/app/reporter"
 	"github.com/jaegertracing/jaeger/cmd/agent/app/servers"
 	"github.com/jaegertracing/jaeger/cmd/agent/app/servers/thriftudp"
+	"github.com/jaegertracing/jaeger/internal/safeexpvar"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
 	"github.com/jaegertracing/jaeger/ports"
 	agentThrift "github.com/jaegertracing/jaeger/thrift-gen/agent"
@@ -111,7 +112,7 @@ func (b *Builder) CreateAgent(primaryProxy CollectorProxy, logger *zap.Logger, m
 		return nil, fmt.Errorf("cannot create processors: %w", err)
 	}
 	server := b.HTTPServer.getHTTPServer(primaryProxy.GetManager(), mFactory, logger)
-	b.publishOpts(mFactory)
+	b.publishOpts()
 
 	return NewAgent(processors, server, logger), nil
 }
@@ -128,16 +129,12 @@ func (b *Builder) getReporter(primaryProxy CollectorProxy) reporter.Reporter {
 	return reporter.NewMultiReporter(rep...)
 }
 
-func (b *Builder) publishOpts(mFactory metrics.Factory) {
-	internalFactory := mFactory.Namespace(metrics.NSOptions{Name: "internal"})
+func (b *Builder) publishOpts() {
 	for _, p := range b.Processors {
 		prefix := fmt.Sprintf(processorPrefixFmt, p.Model, p.Protocol)
-		internalFactory.Gauge(metrics.Options{Name: prefix + suffixServerMaxPacketSize}).
-			Update(int64(p.Server.MaxPacketSize))
-		internalFactory.Gauge(metrics.Options{Name: prefix + suffixServerQueueSize}).
-			Update(int64(p.Server.QueueSize))
-		internalFactory.Gauge(metrics.Options{Name: prefix + suffixWorkers}).
-			Update(int64(p.Workers))
+		safeexpvar.SetInt(prefix+suffixServerMaxPacketSize, int64(p.Server.MaxPacketSize))
+		safeexpvar.SetInt(prefix+suffixServerQueueSize, int64(p.Server.QueueSize))
+		safeexpvar.SetInt(prefix+suffixWorkers, int64(p.Workers))
 	}
 }
 

cmd/agent/app/builder_test.go

@@ -18,7 +18,9 @@ package app
 import (
 	"context"
 	"errors"
+	"expvar"
 	"flag"
+	"fmt"
 	"strings"
 	"testing"
 	"time"
@@ -33,7 +35,6 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/agent/app/configmanager"
 	"github.com/jaegertracing/jaeger/cmd/agent/app/reporter"
 	"github.com/jaegertracing/jaeger/cmd/agent/app/reporter/grpc"
-	"github.com/jaegertracing/jaeger/internal/metrics/fork"
 	"github.com/jaegertracing/jaeger/internal/metricstest"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
 	"github.com/jaegertracing/jaeger/proto-gen/api_v2"
@@ -308,25 +309,15 @@ func TestPublishOpts(t *testing.T) {
 
 	baseMetrics := metricstest.NewFactory(time.Second)
 	defer baseMetrics.Stop()
-	forkFactory := metricstest.NewFactory(time.Second)
-	defer forkFactory.Stop()
-	metricsFactory := fork.New("internal", forkFactory, baseMetrics)
-	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metricsFactory)
+	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), baseMetrics)
 	require.NoError(t, err)
 	assert.NotNil(t, agent)
 	require.NoError(t, agent.Run())
 	defer agent.Stop()
 
-	forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
-		Name:  "internal.processor.jaeger-binary.server-max-packet-size",
-		Value: 4242,
-	})
-	forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
-		Name:  "internal.processor.jaeger-binary.server-queue-size",
-		Value: 24,
-	})
-	forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
-		Name:  "internal.processor.jaeger-binary.workers",
-		Value: 42,
-	})
+	p := cfg.Processors[2]
+	prefix := fmt.Sprintf(processorPrefixFmt, p.Model, p.Protocol)
+	assert.EqualValues(t, 4242, expvar.Get(prefix+suffixServerMaxPacketSize).(*expvar.Int).Value())
+	assert.EqualValues(t, 24, expvar.Get(prefix+suffixServerQueueSize).(*expvar.Int).Value())
+	assert.EqualValues(t, 42, expvar.Get(prefix+suffixWorkers).(*expvar.Int).Value())
 }

cmd/agent/main.go

@@ -31,8 +31,6 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/internal/docs"
 	"github.com/jaegertracing/jaeger/cmd/internal/flags"
 	"github.com/jaegertracing/jaeger/cmd/internal/status"
-	"github.com/jaegertracing/jaeger/internal/metrics/expvar"
-	"github.com/jaegertracing/jaeger/internal/metrics/fork"
 	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
 	"github.com/jaegertracing/jaeger/pkg/version"
@@ -58,12 +56,9 @@ func main() {
 			}
 			logger := svc.Logger // shortcut
 
-			baseFactory := svc.MetricsFactory.
+			mFactory := svc.MetricsFactory.
 				Namespace(metrics.NSOptions{Name: "jaeger"}).
 				Namespace(metrics.NSOptions{Name: "agent"})
-			mFactory := fork.New("internal",
-				expvar.NewFactory(), // expvar backend to report settings
-				baseFactory)
 			version.NewInfoMetrics(mFactory)
 
 			rOpts := new(reporter.Options).InitFromViper(v, logger)

cmd/all-in-one/main.go

@@ -40,8 +40,6 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/internal/status"
 	queryApp "github.com/jaegertracing/jaeger/cmd/query/app"
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
-	"github.com/jaegertracing/jaeger/internal/metrics/expvar"
-	"github.com/jaegertracing/jaeger/internal/metrics/fork"
 	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/jaegertracing/jaeger/pkg/jtracer"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
@@ -96,9 +94,7 @@ by default uses only in-memory database.`,
 				return err
 			}
 			logger := svc.Logger // shortcut
-			baseFactory := fork.New("internal",
-				expvar.NewFactory(), // expvar backend to report settings
-				svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"}))
+			baseFactory := svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})
 			version.NewInfoMetrics(baseFactory)
 			agentMetricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "agent"})
 			collectorMetricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "collector"})

cmd/collector/app/collector.go

@@ -30,6 +30,7 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/collector/app/processor"
 	"github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore"
 	"github.com/jaegertracing/jaeger/cmd/collector/app/server"
+	"github.com/jaegertracing/jaeger/internal/safeexpvar"
 	"github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
@@ -170,9 +171,8 @@ func (c *Collector) Start(options *flags.CollectorOptions) error {
 }
 
 func (c *Collector) publishOpts(cOpts *flags.CollectorOptions) {
-	internalFactory := c.metricsFactory.Namespace(metrics.NSOptions{Name: "internal"})
-	internalFactory.Gauge(metrics.Options{Name: metricNumWorkers}).Update(int64(cOpts.NumWorkers))
-	internalFactory.Gauge(metrics.Options{Name: metricQueueSize}).Update(int64(cOpts.QueueSize))
+	safeexpvar.SetInt(metricNumWorkers, int64(cOpts.NumWorkers))
+	safeexpvar.SetInt(metricQueueSize, int64(cOpts.QueueSize))
 }
 
 // Close the component and all its underlying dependencies

cmd/collector/app/collector_test.go

@@ -16,6 +16,7 @@ package app
 
 import (
 	"context"
+	"expvar"
 	"io"
 	"sync/atomic"
 	"testing"
@@ -27,7 +28,6 @@ import (
 
 	"github.com/jaegertracing/jaeger/cmd/collector/app/flags"
 	"github.com/jaegertracing/jaeger/cmd/collector/app/processor"
-	"github.com/jaegertracing/jaeger/internal/metrics/fork"
 	"github.com/jaegertracing/jaeger/internal/metricstest"
 	"github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
@@ -158,11 +158,8 @@ func TestCollector_PublishOpts(t *testing.T) {
 	// prepare
 	hc := healthcheck.New()
 	logger := zap.NewNop()
-	baseMetrics := metricstest.NewFactory(time.Second)
-	defer baseMetrics.Backend.Stop()
-	forkFactory := metricstest.NewFactory(time.Second)
-	defer forkFactory.Backend.Stop()
-	metricsFactory := fork.New("internal", forkFactory, baseMetrics)
+	metricsFactory := metricstest.NewFactory(time.Second)
+	defer metricsFactory.Backend.Stop()
 	spanWriter := &fakeSpanWriter{}
 	strategyStore := &mockStrategyStore{}
 	tm := &tenancy.Manager{}
@@ -182,15 +179,9 @@ func TestCollector_PublishOpts(t *testing.T) {
 
 	require.NoError(t, c.Start(collectorOpts))
 	defer c.Close()
-
-	forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
-		Name:  "internal.collector.num-workers",
-		Value: 24,
-	})
-	forkFactory.AssertGaugeMetrics(t, metricstest.ExpectedMetric{
-		Name:  "internal.collector.queue-size",
-		Value: 42,
-	})
+	c.publishOpts(collectorOpts)
+	assert.EqualValues(t, 24, expvar.Get(metricNumWorkers).(*expvar.Int).Value())
+	assert.EqualValues(t, 42, expvar.Get(metricQueueSize).(*expvar.Int).Value())
 }
 
 func TestAggregator(t *testing.T) {

cmd/collector/main.go

@@ -33,8 +33,6 @@ import (
 	cmdFlags "github.com/jaegertracing/jaeger/cmd/internal/flags"
 	"github.com/jaegertracing/jaeger/cmd/internal/printconfig"
 	"github.com/jaegertracing/jaeger/cmd/internal/status"
-	"github.com/jaegertracing/jaeger/internal/metrics/expvar"
-	"github.com/jaegertracing/jaeger/internal/metrics/fork"
 	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/jaegertracing/jaeger/pkg/metrics"
 	"github.com/jaegertracing/jaeger/pkg/tenancy"
@@ -73,9 +71,7 @@ func main() {
 			}
 			logger := svc.Logger // shortcut
 			baseFactory := svc.MetricsFactory.Namespace(metrics.NSOptions{Name: "jaeger"})
-			metricsFactory := fork.New("internal",
-				expvar.NewFactory(), // expvar backend to report settings
-				baseFactory.Namespace(metrics.NSOptions{Name: "collector"}))
+			metricsFactory := baseFactory.Namespace(metrics.NSOptions{Name: "collector"})
 			version.NewInfoMetrics(metricsFactory)
 
 			storageFactory.InitFromViper(v, logger)