From 6e695b37065f09cc6a7eed2fb21a7aead92fa6e1 Mon Sep 17 00:00:00 2001 From: Saumya Shah <115284013+Saumya40-codes@users.noreply.github.com> Date: Tue, 12 Nov 2024 22:42:24 +0530 Subject: [PATCH] fix: TestCreateCollectorProxy unit test failing on go-tip (#6204) ## Which problem is this PR solving? - Resolves #6198 ## Description of the changes - After the addition of `t.Parallel` in `cmd/agent/app/builder_test.go` in `TestCreateCollectorProxy`, its test started to fail during random ittterations. This was because in `/cmd/agent/app/reporter/connect_metrics.go` under `NewConnectMetrics` function we are doing `expvar.NewString("gRPCTarget")` which isnt thread-safe and it should not be changed on initialized - Now we use sync.Once to tackle the above bug/problem ## How was this change tested? - running `go test -v` ## 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 - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: Saumya Shah Co-authored-by: Yuri Shkuro --- cmd/agent/app/reporter/connect_metrics.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/cmd/agent/app/reporter/connect_metrics.go b/cmd/agent/app/reporter/connect_metrics.go index 833f0c67f07..d0f6f5770c1 100644 --- a/cmd/agent/app/reporter/connect_metrics.go +++ b/cmd/agent/app/reporter/connect_metrics.go @@ -5,6 +5,7 @@ package reporter import ( "expvar" + "sync" "github.com/jaegertracing/jaeger/pkg/metrics" ) @@ -23,17 +24,21 @@ type ConnectMetrics struct { target *expvar.String } +var ( + gRPCTarget *expvar.String + gRPCTargetOnce sync.Once +) + // NewConnectMetrics will be initialize ConnectMetrics func NewConnectMetrics(mf metrics.Factory) *ConnectMetrics { cm := &ConnectMetrics{} metrics.MustInit(&cm.metrics, mf.Namespace(metrics.NSOptions{Name: "connection_status"}), nil) - if r := expvar.Get("gRPCTarget"); r == nil { - cm.target = expvar.NewString("gRPCTarget") - } else { - cm.target = r.(*expvar.String) - } - + // expvar.String is not thread-safe, so we need to initialize it only once + gRPCTargetOnce.Do(func() { + gRPCTarget = expvar.NewString("gRPCTarget") + }) + cm.target = gRPCTarget return cm }