Skip to content

Commit 852cd21

Browse files
committed
code review changes and cleanup
1 parent 5762b53 commit 852cd21

File tree

3 files changed

+39
-26
lines changed

3 files changed

+39
-26
lines changed

collector.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package instana
44

55
import (
66
"context"
7+
"fmt"
78
"sync"
89

910
ot "github.com/opentracing/opentracing-go"
@@ -27,8 +28,8 @@ type Collector struct {
2728
var _ TracerLogger = (*Collector)(nil)
2829

2930
var (
30-
once sync.Once
31-
muCollector sync.Mutex
31+
once sync.Once
32+
muc sync.Mutex
3233
)
3334

3435
// InitCollector creates a new [Collector]
@@ -51,8 +52,8 @@ func InitCollector(opts *Options) TracerLogger {
5152
recorder: opts.Recorder,
5253
}
5354

54-
muCollector.Lock()
55-
defer muCollector.Unlock()
55+
muc.Lock()
56+
defer muc.Unlock()
5657

5758
c = &Collector{
5859
t: tracer,
@@ -65,11 +66,16 @@ func InitCollector(opts *Options) TracerLogger {
6566
return c
6667
}
6768

68-
// GetC return the instance of instana Collector
69-
func GetC() TracerLogger {
70-
muCollector.Lock()
71-
defer muCollector.Unlock()
72-
return c
69+
// GetCollector return the instance of instana Collector
70+
func GetCollector() (TracerLogger, error) {
71+
muc.Lock()
72+
defer muc.Unlock()
73+
74+
if _, ok := c.(*Collector); !ok {
75+
return c, fmt.Errorf("collector is not initialized")
76+
}
77+
78+
return c, nil
7379
}
7480

7581
// Extract() returns a SpanContext instance given `format` and `carrier`. It matches [opentracing.Tracer.Extract].

collector_test.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,19 @@ import (
1515
"github.com/stretchr/testify/assert"
1616
)
1717

18+
func getInstanaCollector() instana.TracerLogger {
19+
c, _ := instana.GetCollector()
20+
return c
21+
}
22+
1823
func Test_Collector_Noop(t *testing.T) {
19-
assert.NotNil(t, instana.GetC(), "instana collector should never be nil and be initialized as noop")
24+
assert.NotNil(t, getInstanaCollector(), "instana collector should never be nil and be initialized as noop")
2025

21-
sc, err := instana.GetC().Extract(nil, nil)
26+
sc, err := getInstanaCollector().Extract(nil, nil)
2227
assert.Nil(t, sc)
2328
assert.Error(t, err)
24-
assert.Nil(t, instana.GetC().StartSpan(""))
25-
assert.Nil(t, instana.GetC().LegacySensor())
29+
assert.Nil(t, getInstanaCollector().StartSpan(""))
30+
assert.Nil(t, getInstanaCollector().LegacySensor())
2631
}
2732

2833
func Test_Collector_LegacySensor(t *testing.T) {
@@ -31,7 +36,7 @@ func Test_Collector_LegacySensor(t *testing.T) {
3136
s := c.LegacySensor()
3237
defer instana.ShutdownCollector()
3338

34-
assert.NotNil(t, instana.GetC().LegacySensor())
39+
assert.NotNil(t, getInstanaCollector().LegacySensor())
3540

3641
h := instana.TracingHandlerFunc(s, "/{action}", func(w http.ResponseWriter, req *http.Request) {
3742
fmt.Fprintln(w, "Ok")
@@ -41,7 +46,7 @@ func Test_Collector_LegacySensor(t *testing.T) {
4146

4247
h.ServeHTTP(httptest.NewRecorder(), req)
4348

44-
assert.Len(t, recorder.GetQueuedSpans(), 1, "Instrumentations should still work fine with instana.GetC().LegacySensor()")
49+
assert.Len(t, recorder.GetQueuedSpans(), 1, "Instrumentations should still work fine with getInstanaCollector().LegacySensor()")
4550
}
4651

4752
func Test_Collector_Singleton(t *testing.T) {
@@ -50,17 +55,17 @@ func Test_Collector_Singleton(t *testing.T) {
5055

5156
defer instana.ShutdownCollector()
5257

53-
_, ok = instana.GetC().(*instana.Collector)
58+
_, ok = getInstanaCollector().(*instana.Collector)
5459
assert.False(t, ok, "instana collector is noop before InitCollector is called")
5560

5661
instana.InitCollector(instana.DefaultOptions())
5762

58-
instance, ok = instana.GetC().(*instana.Collector)
63+
instance, ok = getInstanaCollector().(*instana.Collector)
5964
assert.True(t, ok, "instana collector is of type instana.Collector after InitCollector is called")
6065

6166
instana.InitCollector(instana.DefaultOptions())
6267

63-
assert.Equal(t, instana.GetC(), instance, "instana collector is singleton and should not be reassigned if InitCollector is called again")
68+
assert.Equal(t, getInstanaCollector(), instance, "instana collector is singleton and should not be reassigned if InitCollector is called again")
6469
}
6570

6671
func Test_Collector_EmbeddedTracer(t *testing.T) {
@@ -98,13 +103,15 @@ func Test_Collector_Logger(t *testing.T) {
98103

99104
l := &mylogger{}
100105

101-
instana.GetC().SetLogger(l)
106+
c := getInstanaCollector()
107+
108+
c.SetLogger(l)
102109

103-
instana.GetC().Debug()
104-
instana.GetC().Info()
105-
instana.GetC().Warn()
106-
instana.GetC().Error()
107-
instana.GetC().Error()
110+
c.Debug()
111+
c.Info()
112+
c.Warn()
113+
c.Error()
114+
c.Error()
108115

109116
assert.Equal(t, 1, l.counter["debug"])
110117
assert.Equal(t, 1, l.counter["info"])

sensor.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,8 @@ func ShutdownSensor() {
290290
// It will also reset the singleton as the next time that instana.InitCollector API is called,
291291
// collector and sensor will be reinitialized.
292292
func ShutdownCollector() {
293-
muCollector.Lock()
294-
defer muCollector.Unlock()
293+
muc.Lock()
294+
defer muc.Unlock()
295295
if sensor != nil {
296296
sensor = nil
297297
}

0 commit comments

Comments
 (0)