Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 19 commits into from
Jan 28, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 37 additions & 21 deletions collector.go
Original file line number Diff line number Diff line change
@@ -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
Angith marked this conversation as resolved.
Show resolved Hide resolved
}

// 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].
77 changes: 53 additions & 24 deletions collector_test.go
Original file line number Diff line number Diff line change
@@ -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"])
5 changes: 3 additions & 2 deletions instrumentation_http_test.go
Original file line number Diff line number Diff line change
@@ -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) {
20 changes: 17 additions & 3 deletions sensor.go
Original file line number Diff line number Diff line change
@@ -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()
Angith marked this conversation as resolved.
Show resolved Hide resolved
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,