fix: unit test failures while using instana.InitCollector() API (#1012)

* fix: unit test failures while using instana.InitCollector() API 
Co-authored-by: Nithin Puthenveettil <[email protected]>
Co-authored-by: Sanoj Subran <[email protected]>

Author: Angith

collector.go

@@ -4,6 +4,8 @@ package instana
 
 import (
 	"context"
+	"fmt"
+	"sync"
 
 	ot "github.com/opentracing/opentracing-go"
 )
@@ -25,38 +27,52 @@ type Collector struct {
 
 var _ TracerLogger = (*Collector)(nil)
 
+var (
+	once sync.Once
+	muc  sync.Mutex
+)
+
 // InitCollector creates a new [Collector]
 func InitCollector(opts *Options) TracerLogger {
 
-	// if instana.C is already an instance of Collector, we just return
-	if _, ok := C.(*Collector); ok {
-		C.Warn("InitCollector was previously called. instana.C is reused")
-		return C
-	}
+	once.Do(func() {
+		if opts == nil {
+			opts = &Options{
+				Recorder: NewRecorder(),
+			}
+		}
 
-	if opts == nil {
-		opts = &Options{
-			Recorder: NewRecorder(),
+		if opts.Recorder == nil {
+			opts.Recorder = NewRecorder()
 		}
-	}
 
-	if opts.Recorder == nil {
-		opts.Recorder = NewRecorder()
-	}
+		StartMetrics(opts)
 
-	StartMetrics(opts)
+		tracer := &tracerS{
+			recorder: opts.Recorder,
+		}
 
-	tracer := &tracerS{
-		recorder: opts.Recorder,
-	}
+		c = &Collector{
+			t:             tracer,
+			LeveledLogger: defaultLogger,
+			Sensor:        NewSensorWithTracer(tracer),
+		}
+
+	})
+
+	return c
+}
+
+// GetCollector return the instance of instana Collector
+func GetCollector() (TracerLogger, error) {
+	muc.Lock()
+	defer muc.Unlock()
 
-	C = &Collector{
-		t:             tracer,
-		LeveledLogger: defaultLogger,
-		Sensor:        NewSensorWithTracer(tracer),
+	if _, ok := c.(*Collector); !ok {
+		return c, fmt.Errorf("collector has not been initialised yet. Please use InitCollector first")
 	}
 
-	return C
+	return c, nil
 }
 
 // Extract() returns a SpanContext instance given `format` and `carrier`. It matches [opentracing.Tracer.Extract].

collector_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"sync"
 	"testing"
 
 	instana "github.com/instana/go-sensor"
@@ -16,22 +17,25 @@ import (
 )
 
 func Test_Collector_Noop(t *testing.T) {
-	assert.NotNil(t, instana.C, "instana.C should never be nil and be initialized as noop")
+	c, err := instana.GetCollector()
+	assert.Error(t, err, "should return error as collector has not been initialized yet.")
 
-	sc, err := instana.C.Extract(nil, nil)
+	assert.NotNil(t, c, "instana collector should never be nil and be initialized as noop")
+
+	sc, err := c.Extract(nil, nil)
 	assert.Nil(t, sc)
 	assert.Error(t, err)
-	assert.Nil(t, instana.C.StartSpan(""))
-	assert.Nil(t, instana.C.LegacySensor())
+	assert.Nil(t, c.StartSpan(""))
+	assert.Nil(t, c.LegacySensor())
 }
 
 func Test_Collector_LegacySensor(t *testing.T) {
 	recorder := instana.NewTestRecorder()
 	c := instana.InitCollector(&instana.Options{AgentClient: alwaysReadyClient{}, Recorder: recorder})
 	s := c.LegacySensor()
-	defer instana.ShutdownSensor()
+	defer instana.ShutdownCollector()
 
-	assert.NotNil(t, instana.C.LegacySensor())
+	assert.NotNil(t, c.LegacySensor())
 
 	h := instana.TracingHandlerFunc(s, "/{action}", func(w http.ResponseWriter, req *http.Request) {
 		fmt.Fprintln(w, "Ok")
@@ -41,30 +45,55 @@ func Test_Collector_LegacySensor(t *testing.T) {
 
 	h.ServeHTTP(httptest.NewRecorder(), req)
 
-	assert.Len(t, recorder.GetQueuedSpans(), 1, "Instrumentations should still work fine with instana.C.LegacySensor()")
+	assert.Len(t, recorder.GetQueuedSpans(), 1, "Instrumentations should still work fine with LegacySensor() method")
 }
 
 func Test_Collector_Singleton(t *testing.T) {
-	instana.C = nil
 	var ok bool
 	var instance instana.TracerLogger
 
-	_, ok = instana.C.(*instana.Collector)
-	assert.False(t, ok, "instana.C is noop before InitCollector is called")
+	c, err := instana.GetCollector()
+	assert.Error(t, err, "should return error as collector has not been initialized yet.")
+
+	defer instana.ShutdownCollector()
+
+	_, ok = c.(*instana.Collector)
+	assert.False(t, ok, "instana collector is noop before InitCollector is called")
+
+	c = instana.InitCollector(instana.DefaultOptions())
+
+	instance, ok = c.(*instana.Collector)
+	assert.True(t, ok, "instana collector is of type instana.Collector after InitCollector is called")
+
+	c = instana.InitCollector(instana.DefaultOptions())
 
-	instana.InitCollector(instana.DefaultOptions())
+	assert.Equal(t, c, instance, "instana collector is singleton and should not be reassigned if InitCollector is called again")
+}
+
+func Test_InitCollector_With_Goroutines(t *testing.T) {
+
+	defer instana.ShutdownCollector()
 
-	instance, ok = instana.C.(*instana.Collector)
-	assert.True(t, ok, "instana.C is of type instana.Collector after InitCollector is called")
+	var wg sync.WaitGroup
+	wg.Add(3)
 
-	instana.InitCollector(instana.DefaultOptions())
+	for i := 0; i < 3; i++ {
+		go func(id int) {
+			defer wg.Done()
+			c := instana.InitCollector(instana.DefaultOptions())
 
-	assert.Equal(t, instana.C, instance, "instana.C is singleton and should not be reassigned if InitCollector is called again")
+			_, ok := c.(*instana.Collector)
+			assert.True(t, ok, "instana collector is of type instana.Collector after InitCollector is called")
+
+			assert.NotNil(t, c)
+		}(i)
+	}
+	wg.Wait()
 }
 
 func Test_Collector_EmbeddedTracer(t *testing.T) {
-	instana.C = nil
 	c := instana.InitCollector(nil)
+	defer instana.ShutdownCollector()
 
 	sp := c.StartSpan("my-span")
 
@@ -92,18 +121,18 @@ func Test_Collector_EmbeddedTracer(t *testing.T) {
 }
 
 func Test_Collector_Logger(t *testing.T) {
-	instana.C = nil
-	instana.InitCollector(nil)
+	c := instana.InitCollector(nil)
+	defer instana.ShutdownCollector()
 
 	l := &mylogger{}
 
-	instana.C.SetLogger(l)
+	c.SetLogger(l)
 
-	instana.C.Debug()
-	instana.C.Info()
-	instana.C.Warn()
-	instana.C.Error()
-	instana.C.Error()
+	c.Debug()
+	c.Info()
+	c.Warn()
+	c.Error()
+	c.Error()
 
 	assert.Equal(t, 1, l.counter["debug"])
 	assert.Equal(t, 1, l.counter["info"])

instrumentation_http_test.go

@@ -46,16 +46,17 @@ func BenchmarkTracingNamedHandlerFunc(b *testing.B) {
 }
 
 func TestTracingNamedHandlerFunc_Write(t *testing.T) {
+	recorder := instana.NewTestRecorder()
 	opts := &instana.Options{
 		Service: "go-sensor-test",
 		Tracer: instana.TracerOptions{
 			CollectableHTTPHeaders: []string{"x-custom-header-1", "x-custom-header-2"},
 		},
 		AgentClient: alwaysReadyClient{},
+		Recorder:    recorder,
 	}
 
-	recorder := instana.NewTestRecorder()
-	s := instana.NewSensorWithTracer(instana.NewTracerWithEverything(opts, recorder))
+	s := instana.InitCollector(opts)
 	defer instana.ShutdownSensor()
 
 	h := instana.TracingNamedHandlerFunc(s, "action", "/{action}", func(w http.ResponseWriter, req *http.Request) {

sensor.go

@@ -86,11 +86,11 @@ var (
 	muSensor         sync.Mutex
 	binaryName       = filepath.Base(os.Args[0])
 	processStartedAt = time.Now()
-	C                TracerLogger
+	c                TracerLogger
 )
 
 func init() {
-	C = newNoopCollector()
+	c = newNoopCollector()
 }
 
 func newSensor(options *Options) *sensorS {
@@ -276,12 +276,26 @@ func Flush(ctx context.Context) error {
 
 // ShutdownSensor cleans up the internal global sensor reference. The next time that instana.InitSensor is called,
 // directly or indirectly, the internal sensor will be reinitialized.
+//
+// Deprecated: Use [ShutdownCollector] instead.
 func ShutdownSensor() {
 	muSensor.Lock()
+	defer muSensor.Unlock()
 	if sensor != nil {
 		sensor = nil
 	}
-	muSensor.Unlock()
+}
+
+// ShutdownCollector cleans up the collector and sensor reference.
+// It will also reset the singleton as the next time that instana.InitCollector API is called,
+// collector and sensor will be reinitialized.
+func ShutdownCollector() {
+
+	ShutdownSensor()
+	muc.Lock()
+	defer muc.Unlock()
+	c = newNoopCollector()
+	once = sync.Once{}
 }
 
 func newServerlessAgent(serviceName, agentEndpoint, agentKey string,