Make MockTracer thread-safe

.travis.yml

@@ -8,5 +8,5 @@ install:
   - go get -d -t github.com/opentracing/opentracing-go/...
   - go get -u github.com/golang/lint/...
 script:
-  - make test lint vet
+  - make test lint
   - go build ./...

Makefile

@@ -1,22 +1,32 @@
+PACKAGES := . ./mocktracer/... ./ext/...
 
+.DEFAULT_GOAL := test-and-lint
 
-.DEFAULT_GOAL := test
+.PHONE: test-and-lint
+
+test-and-lint: test lint
 
 .PHONY: test
 test:
 	go test -v -cover ./...
 
-.PHONY: fmt
-fmt:
-	go fmt ./...
+cover:
+	@rm -rf cover-all.out
+	$(foreach pkg, $(PACKAGES), $(MAKE) cover-pkg PKG=$(pkg) || true;)
+	@grep mode: cover.out > coverage.out
+	@cat cover-all.out >> coverage.out
+	go tool cover -html=coverage.out -o cover.html
+	@rm -rf cover.out cover-all.out coverage.out
+
+cover-pkg:
+	go test -coverprofile cover.out $(PKG)
+	@grep -v mode: cover.out >> cover-all.out
 
 .PHONY: lint
 lint:
+	go fmt ./...
 	golint ./...
 	@# Run again with magic to exit non-zero if golint outputs anything.
 	@! (golint ./... | read dummy)
-
-.PHONY: vet
-vet:
 	go vet ./...
 

ext/tags_test.go

@@ -31,12 +31,16 @@ func TestPeerTags(t *testing.T) {
 	ext.SpanKind.Set(span, ext.SpanKindRPCClient)
 	span.Finish()
 
-	rawSpan := span.(*mocktracer.MockSpan)
-	assertEqual(t, "my-service", rawSpan.Tags["peer.service"])
-	assertEqual(t, "my-hostname", rawSpan.Tags["peer.hostname"])
-	assertEqual(t, uint32(127<<24|1), rawSpan.Tags["peer.ipv4"])
-	assertEqual(t, "::", rawSpan.Tags["peer.ipv6"])
-	assertEqual(t, uint16(8080), rawSpan.Tags["peer.port"])
+	rawSpan := tracer.GetFinishedSpans()[0]
+	assertEqual(t, map[string]interface{}{
+		"peer.service":      "my-service",
+		"peer.hostname":     "my-hostname",
+		"peer.ipv4":         uint32(127<<24 | 1),
+		"peer.ipv6":         "::",
+		"peer.port":         uint16(8080),
+		"span.kind":         ext.SpanKindRPCClient,
+		"sampling.priority": uint16(1),
+	}, rawSpan.GetTags())
 }
 
 func TestHTTPTags(t *testing.T) {
@@ -47,10 +51,12 @@ func TestHTTPTags(t *testing.T) {
 	ext.HTTPStatusCode.Set(span, 301)
 	span.Finish()
 
-	rawSpan := span.(*mocktracer.MockSpan)
-	assertEqual(t, "test.biz/uri?protocol=false", rawSpan.Tags["http.url"])
-	assertEqual(t, "GET", rawSpan.Tags["http.method"])
-	assertEqual(t, uint16(301), rawSpan.Tags["http.status_code"])
+	rawSpan := tracer.GetFinishedSpans()[0]
+	assertEqual(t, map[string]interface{}{
+		"http.url":         "test.biz/uri?protocol=false",
+		"http.method":      "GET",
+		"http.status_code": uint16(301),
+	}, rawSpan.GetTags())
 }
 
 func TestMiscTags(t *testing.T) {
@@ -62,8 +68,10 @@ func TestMiscTags(t *testing.T) {
 
 	span.Finish()
 
-	rawSpan := span.(*mocktracer.MockSpan)
-	assertEqual(t, "my-awesome-library", rawSpan.Tags["component"])
-	assertEqual(t, uint16(1), rawSpan.Tags["sampling.priority"])
-	assertEqual(t, true, rawSpan.Tags["error"])
+	rawSpan := tracer.GetFinishedSpans()[0]
+	assertEqual(t, map[string]interface{}{
+		"component":         "my-awesome-library",
+		"sampling.priority": uint16(1),
+		"error":             true,
+	}, rawSpan.GetTags())
 }

mocktracer/mocktracer.go

@@ -3,6 +3,8 @@ package mocktracer
 import (
 	"strconv"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/opentracing/opentracing-go"
@@ -12,38 +14,92 @@ import (
 // to facilitate tests of OpenTracing instrumentation.
 func New() *MockTracer {
 	return &MockTracer{
-		FinishedSpans: []*MockSpan{},
+		finishedSpans: []*MockSpan{},
 	}
 }
 
-// MockTracer is a for-testing-only opentracing.Tracer implementation. It is
-// entirely unsuitable for production use but appropriate for tests that want
-// to verify tracing behavior.
+// MockTracer is only intended for testing OpenTracing instrumentation.
+// It is entirely unsuitable for production use, but appropriate for tests
+// that want to verify tracing behavior in other frameworks/applications.
 type MockTracer struct {
-	FinishedSpans []*MockSpan
+	sync.RWMutex
+	finishedSpans []*MockSpan
 }
 
 // MockSpan is an opentracing.Span implementation that exports its internal
 // state for testing purposes.
 type MockSpan struct {
+	sync.RWMutex
+
 	SpanID   int
 	ParentID int
 
 	OperationName string
 	StartTime     time.Time
 	FinishTime    time.Time
-	Tags          map[string]interface{}
-	Baggage       map[string]string
-	Logs          []opentracing.LogData
+
+	tags    map[string]interface{}
+	baggage map[string]string
+	logs    []opentracing.LogData
 
 	tracer *MockTracer
 }
 
-// Reset clears the exported MockTracer.FinishedSpans field. Note that any
-// extant MockSpans will still append to FinishedSpans when they Finish(), even
-// after a call to Reset().
+// GetFinishedSpans returns all spans that have been Finish()'ed since the
+// MockTracer was constructed or since the last call to its Reset() method.
+func (t *MockTracer) GetFinishedSpans() []*MockSpan {
+	t.RLock()
+	defer t.RUnlock()
+	spans := make([]*MockSpan, len(t.finishedSpans))
+	copy(spans, t.finishedSpans)
+	return spans
+}
+
+// Reset clears the internally accumulated finished spans. Note that any
+// extant MockSpans will still append to finishedSpans when they Finish(),
+// even after a call to Reset().
 func (t *MockTracer) Reset() {
-	t.FinishedSpans = []*MockSpan{}
+	t.Lock()
+	defer t.Unlock()
+	t.finishedSpans = []*MockSpan{}
+}
+
+// GetTags returns a copy of tags accumulated by the span so far
+func (s *MockSpan) GetTags() map[string]interface{} {
+	s.RLock()
+	defer s.RUnlock()
+	tags := make(map[string]interface{})
+	for k, v := range s.tags {
+		tags[k] = v
+	}
+	return tags
+}
+
+// GetTag returns a single tag
+func (s *MockSpan) GetTag(k string) interface{} {
+	s.RLock()
+	defer s.RUnlock()
+	return s.tags[k]
+}
+
+// GetBaggage returns a copy of baggage items in the span
+func (s *MockSpan) GetBaggage() map[string]string {
+	s.RLock()
+	defer s.RUnlock()
+	baggage := make(map[string]string)
+	for k, v := range s.baggage {
+		baggage[k] = v
+	}
+	return baggage
+}
+
+// GetLogs returns a copy of logs accumulated in the span so far
+func (s *MockSpan) GetLogs() []opentracing.LogData {
+	s.RLock()
+	defer s.RUnlock()
+	logs := make([]opentracing.LogData, len(s.logs))
+	copy(logs, s.logs)
+	return logs
 }
 
 // StartSpan belongs to the Tracer interface.
@@ -64,13 +120,19 @@ const mockTextMapBaggagePrefix = "mockpfx-baggage-"
 // Inject belongs to the Tracer interface.
 func (t *MockTracer) Inject(sp opentracing.Span, format interface{}, carrier interface{}) error {
 	span := sp.(*MockSpan)
+	span.RLock()
+	defer span.RUnlock()
+
 	switch format {
 	case opentracing.TextMap:
-		writer := carrier.(opentracing.TextMapWriter)
+		writer, ok := carrier.(opentracing.TextMapWriter)
+		if !ok {
+			return opentracing.ErrInvalidCarrier
+		}
 		// Ids:
 		writer.Set(mockTextMapIdsPrefix+"spanid", strconv.Itoa(span.SpanID))
 		// Baggage:
-		for baggageKey, baggageVal := range span.Baggage {
+		for baggageKey, baggageVal := range span.baggage {
 			writer.Set(mockTextMapBaggagePrefix+baggageKey, baggageVal)
 		}
 		return nil
@@ -85,7 +147,12 @@ func (t *MockTracer) Join(operationName string, format interface{}, carrier inte
 		rval := newMockSpan(t, opentracing.StartSpanOptions{
 			OperationName: operationName,
 		})
-		err := carrier.(opentracing.TextMapReader).ForeachKey(func(key, val string) error {
+		reader, ok := carrier.(opentracing.TextMapReader)
+		if !ok {
+			return nil, opentracing.ErrInvalidCarrier
+		}
+
+		err := reader.ForeachKey(func(key, val string) error {
 			lowerKey := strings.ToLower(key)
 			switch {
 			case lowerKey == mockTextMapIdsPrefix+"spanid":
@@ -97,20 +164,20 @@ func (t *MockTracer) Join(operationName string, format interface{}, carrier inte
 				rval.ParentID = i
 			case strings.HasPrefix(lowerKey, mockTextMapBaggagePrefix):
 				// Baggage:
-				rval.Baggage[lowerKey[len(mockTextMapBaggagePrefix):]] = val
+				rval.baggage[lowerKey[len(mockTextMapBaggagePrefix):]] = val
 			}
 			return nil
 		})
 		return rval, err
 	}
-	return nil, opentracing.ErrTraceNotFound
+	return nil, opentracing.ErrUnsupportedFormat
 }
 
-var mockIDSource = 1
+var mockIDSource = uint32(42)
 
 func nextMockID() int {
-	mockIDSource++
-	return mockIDSource
+	atomic.AddUint32(&mockIDSource, 1)
+	return int(atomic.LoadUint32(&mockIDSource))
 }
 
 func newMockSpan(t *MockTracer, opts opentracing.StartSpanOptions) *MockSpan {
@@ -132,47 +199,65 @@ func newMockSpan(t *MockTracer, opts opentracing.StartSpanOptions) *MockSpan {
 
 		OperationName: opts.OperationName,
 		StartTime:     startTime,
-		Tags:          tags,
-		Baggage:       map[string]string{},
-		Logs:          []opentracing.LogData{},
+		tags:          tags,
+		baggage:       map[string]string{},
+		logs:          []opentracing.LogData{},
 
 		tracer: t,
 	}
 }
 
 // SetTag belongs to the Span interface
 func (s *MockSpan) SetTag(key string, value interface{}) opentracing.Span {
-	s.Tags[key] = value
+	s.Lock()
+	defer s.Unlock()
+	s.tags[key] = value
 	return s
 }
 
 // Finish belongs to the Span interface
 func (s *MockSpan) Finish() {
+	s.Lock()
 	s.FinishTime = time.Now()
-	s.tracer.FinishedSpans = append(s.tracer.FinishedSpans, s)
+	s.Unlock()
+	s.tracer.recordSpan(s)
 }
 
 // FinishWithOptions belongs to the Span interface
 func (s *MockSpan) FinishWithOptions(opts opentracing.FinishOptions) {
+	s.Lock()
 	s.FinishTime = opts.FinishTime
-	s.Logs = append(s.Logs, opts.BulkLogData...)
-	s.tracer.FinishedSpans = append(s.tracer.FinishedSpans, s)
+	s.logs = append(s.logs, opts.BulkLogData...)
+	s.Unlock()
+	s.tracer.recordSpan(s)
+}
+
+func (t *MockTracer) recordSpan(span *MockSpan) {
+	t.Lock()
+	defer t.Unlock()
+	t.finishedSpans = append(t.finishedSpans, span)
 }
 
 // SetBaggageItem belongs to the Span interface
 func (s *MockSpan) SetBaggageItem(key, val string) opentracing.Span {
-	s.Baggage[key] = val
+	s.Lock()
+	defer s.Unlock()
+	s.baggage[key] = val
 	return s
 }
 
 // BaggageItem belongs to the Span interface
 func (s *MockSpan) BaggageItem(key string) string {
-	return s.Baggage[key]
+	s.RLock()
+	defer s.RUnlock()
+	return s.baggage[key]
 }
 
 // ForeachBaggageItem belongs to the Span interface
 func (s *MockSpan) ForeachBaggageItem(handler func(k, v string) bool) {
-	for k, v := range s.Baggage {
+	s.RLock()
+	defer s.RUnlock()
+	for k, v := range s.baggage {
 		if !handler(k, v) {
 			break
 		}
@@ -196,11 +281,15 @@ func (s *MockSpan) LogEventWithPayload(event string, payload interface{}) {
 
 // Log belongs to the Span interface
 func (s *MockSpan) Log(data opentracing.LogData) {
-	s.Logs = append(s.Logs, data)
+	s.Lock()
+	defer s.Unlock()
+	s.logs = append(s.logs, data)
 }
 
 // SetOperationName belongs to the Span interface
 func (s *MockSpan) SetOperationName(operationName string) opentracing.Span {
+	s.Lock()
+	defer s.Unlock()
 	s.OperationName = operationName
 	return s
 }