diff --git a/collector.go b/collector.go index c07e29c9f..0f6e7068f 100644 --- a/collector.go +++ b/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]. diff --git a/collector_test.go b/collector_test.go index e7d93928d..5df647a89 100644 --- a/collector_test.go +++ b/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"]) diff --git a/instrumentation_http_test.go b/instrumentation_http_test.go index 764854a7c..df479c06f 100644 --- a/instrumentation_http_test.go +++ b/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) { diff --git a/sensor.go b/sensor.go index 1010021a0..c67ab8333 100644 --- a/sensor.go +++ b/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,