Implement immutable SpanContexts (#36)

Per opentracing/opentracing.io#106

context.go

@@ -1,11 +1,5 @@
 package basictracer
 
-import (
-	"sync"
-
-	"github.com/opentracing/opentracing-go"
-)
-
 // SpanContext holds the basic Span metadata.
 type SpanContext struct {
 	// A probabilistically unique identifier for a [multi-span] trace.
@@ -18,45 +12,31 @@ type SpanContext struct {
 	Sampled bool
 
 	// The span's associated baggage.
-	baggageLock sync.Mutex
-	Baggage     map[string]string // initialized on first use
-}
-
-// BaggageItem belongs to the opentracing.SpanContext interface
-func (c *SpanContext) BaggageItem(key string) string {
-	// TODO: if we want to support onBaggage, need a pointer to the bt.Span.
-	//   s.onBaggage(canonicalKey, val)
-	//   if s.trim() {
-	//   	return s
-	//   }
-
-	c.baggageLock.Lock()
-	defer c.baggageLock.Unlock()
-
-	if c.Baggage == nil {
-		return ""
-	}
-	return c.Baggage[key]
-}
-
-// SetBaggageItem belongs to the opentracing.SpanContext interface
-func (c *SpanContext) SetBaggageItem(key, val string) opentracing.SpanContext {
-	c.baggageLock.Lock()
-	defer c.baggageLock.Unlock()
-	if c.Baggage == nil {
-		c.Baggage = make(map[string]string)
-	}
-	c.Baggage[key] = val
-	return c
+	Baggage map[string]string // initialized on first use
 }
 
 // ForeachBaggageItem belongs to the opentracing.SpanContext interface
-func (c *SpanContext) ForeachBaggageItem(handler func(k, v string) bool) {
-	c.baggageLock.Lock()
-	defer c.baggageLock.Unlock()
+func (c SpanContext) ForeachBaggageItem(handler func(k, v string) bool) {
 	for k, v := range c.Baggage {
 		if !handler(k, v) {
 			break
 		}
 	}
 }
+
+// WithBaggageItem returns an entirely new basictracer SpanContext with the
+// given key:value baggage pair set.
+func (c SpanContext) WithBaggageItem(key, val string) SpanContext {
+	var newBaggage map[string]string
+	if c.Baggage == nil {
+		newBaggage = map[string]string{key: val}
+	} else {
+		newBaggage = make(map[string]string, len(c.Baggage)+1)
+		for k, v := range c.Baggage {
+			newBaggage[k] = v
+		}
+		newBaggage[key] = val
+	}
+	// Use positional parameters so the compiler will help catch new fields.
+	return SpanContext{c.TraceID, c.SpanID, c.Sampled, newBaggage}
+}

propagation.go

@@ -24,7 +24,7 @@ func (p *accessorPropagator) Inject(
 	if !ok || dc == nil {
 		return opentracing.ErrInvalidCarrier
 	}
-	sc, ok := spanContext.(*SpanContext)
+	sc, ok := spanContext.(SpanContext)
 	if !ok {
 		return opentracing.ErrInvalidSpanContext
 	}
@@ -44,7 +44,7 @@ func (p *accessorPropagator) Extract(
 	}
 
 	traceID, spanID, sampled := dc.State()
-	sc := &SpanContext{
+	sc := SpanContext{
 		TraceID: traceID,
 		SpanID:  spanID,
 		Sampled: sampled,

propagation_ot.go

@@ -32,7 +32,7 @@ func (p *textMapPropagator) Inject(
 	spanContext opentracing.SpanContext,
 	opaqueCarrier interface{},
 ) error {
-	sc, ok := spanContext.(*SpanContext)
+	sc, ok := spanContext.(SpanContext)
 	if !ok {
 		return opentracing.ErrInvalidSpanContext
 	}
@@ -44,11 +44,9 @@ func (p *textMapPropagator) Inject(
 	carrier.Set(fieldNameSpanID, strconv.FormatUint(sc.SpanID, 16))
 	carrier.Set(fieldNameSampled, strconv.FormatBool(sc.Sampled))
 
-	sc.baggageLock.Lock()
 	for k, v := range sc.Baggage {
 		carrier.Set(prefixBaggage+k, v)
 	}
-	sc.baggageLock.Unlock()
 	return nil
 }
 
@@ -102,7 +100,7 @@ func (p *textMapPropagator) Extract(
 		return nil, opentracing.ErrSpanContextCorrupted
 	}
 
-	return &SpanContext{
+	return SpanContext{
 		TraceID: traceID,
 		SpanID:  spanID,
 		Sampled: sampled,
@@ -114,7 +112,7 @@ func (p *binaryPropagator) Inject(
 	spanContext opentracing.SpanContext,
 	opaqueCarrier interface{},
 ) error {
-	sc, ok := spanContext.(*SpanContext)
+	sc, ok := spanContext.(SpanContext)
 	if !ok {
 		return opentracing.ErrInvalidSpanContext
 	}
@@ -173,7 +171,7 @@ func (p *binaryPropagator) Extract(
 		return nil, opentracing.ErrSpanContextCorrupted
 	}
 
-	return &SpanContext{
+	return SpanContext{
 		TraceID: ctx.TraceId,
 		SpanID:  ctx.SpanId,
 		Sampled: ctx.Sampled,

raw.go

@@ -8,9 +8,9 @@ import (
 
 // RawSpan encapsulates all state associated with a (finished) Span.
 type RawSpan struct {
-	// The RawSpan embeds its SpanContext. Those recording the RawSpan
-	// should also record the contents of its SpanContext.
-	*SpanContext
+	// Those recording the RawSpan should also record the contents of its
+	// SpanContext.
+	Context SpanContext
 
 	// The SpanID of this SpanContext's first intra-trace reference (i.e.,
 	// "parent"), or 0 if there is no parent.

recorder.go

@@ -45,7 +45,7 @@ func (r *InMemorySpanRecorder) GetSampledSpans() []RawSpan {
 	defer r.RUnlock()
 	spans := make([]RawSpan, 0, len(r.spans))
 	for _, span := range r.spans {
-		if span.Sampled {
+		if span.Context.Sampled {
 			spans = append(spans, span)
 		}
 	}

recorder_test.go

@@ -12,10 +12,10 @@ func TestInMemoryRecorderSpans(t *testing.T) {
 	recorder := NewInMemoryRecorder()
 	var apiRecorder SpanRecorder = recorder
 	span := RawSpan{
-		SpanContext: &SpanContext{},
-		Operation:   "test-span",
-		Start:       time.Now(),
-		Duration:    -1,
+		Context:   SpanContext{},
+		Operation: "test-span",
+		Start:     time.Now(),
+		Duration:  -1,
 	}
 	apiRecorder.RecordSpan(span)
 	assert.Equal(t, []RawSpan{span}, recorder.GetSpans())

span.go

@@ -52,7 +52,7 @@ func (s *spanImpl) reset() {
 	// some of the load. Hard to say how quickly that would be in practice
 	// though.
 	s.raw = RawSpan{
-		SpanContext: &SpanContext{},
+		Context: SpanContext{},
 	}
 }
 
@@ -64,7 +64,7 @@ func (s *spanImpl) SetOperationName(operationName string) opentracing.Span {
 }
 
 func (s *spanImpl) trim() bool {
-	return !s.raw.Sampled && s.tracer.options.TrimUnsampledSpans
+	return !s.raw.Context.Sampled && s.tracer.options.TrimUnsampledSpans
 }
 
 func (s *spanImpl) SetTag(key string, value interface{}) opentracing.Span {
@@ -73,7 +73,7 @@ func (s *spanImpl) SetTag(key string, value interface{}) opentracing.Span {
 	defer s.Unlock()
 	if key == string(ext.SamplingPriority) {
 		if v, ok := value.(uint16); ok {
-			s.raw.Sampled = v != 0
+			s.raw.Context.Sampled = v != 0
 			return s
 		}
 	}
@@ -155,7 +155,25 @@ func (s *spanImpl) Tracer() opentracing.Tracer {
 }
 
 func (s *spanImpl) Context() opentracing.SpanContext {
-	return s.raw.SpanContext
+	return s.raw.Context
+}
+
+func (s *spanImpl) SetBaggageItem(key, val string) opentracing.Span {
+	s.onBaggage(key, val)
+	if s.trim() {
+		return s
+	}
+
+	s.Lock()
+	defer s.Unlock()
+	s.raw.Context = s.raw.Context.WithBaggageItem(key, val)
+	return s
+}
+
+func (s *spanImpl) BaggageItem(key string) string {
+	s.Lock()
+	defer s.Unlock()
+	return s.raw.Context.Baggage[key]
 }
 
 func (s *spanImpl) Operation() string {

tracer.go

@@ -135,11 +135,7 @@ func (t *tracerImpl) getSpan() *spanImpl {
 		sp.reset()
 		return sp
 	}
-	return &spanImpl{
-		raw: RawSpan{
-			SpanContext: &SpanContext{},
-		},
-	}
+	return &spanImpl{}
 }
 
 func (t *tracerImpl) StartSpanWithOptions(
@@ -168,28 +164,26 @@ ReferencesLoop:
 		case opentracing.ChildOfRef,
 			opentracing.FollowsFromRef:
 
-			refMD := ref.ReferencedContext.(*SpanContext)
-			sp.raw.TraceID = refMD.TraceID
-			sp.raw.SpanID = randomID()
-			sp.raw.ParentSpanID = refMD.SpanID
-			sp.raw.Sampled = refMD.Sampled
+			refCtx := ref.ReferencedContext.(SpanContext)
+			sp.raw.Context.TraceID = refCtx.TraceID
+			sp.raw.Context.SpanID = randomID()
+			sp.raw.Context.Sampled = refCtx.Sampled
+			sp.raw.ParentSpanID = refCtx.SpanID
 
-			refMD.baggageLock.Lock()
-			if l := len(refMD.Baggage); l > 0 {
-				sp.raw.Baggage = make(map[string]string, len(refMD.Baggage))
-				for k, v := range refMD.Baggage {
-					sp.raw.Baggage[k] = v
+			if l := len(refCtx.Baggage); l > 0 {
+				sp.raw.Context.Baggage = make(map[string]string, l)
+				for k, v := range refCtx.Baggage {
+					sp.raw.Context.Baggage[k] = v
 				}
 			}
-			refMD.baggageLock.Unlock()
 			break ReferencesLoop
 		}
 	}
-	if sp.raw.TraceID == 0 {
+	if sp.raw.Context.TraceID == 0 {
 		// No parent Span found; allocate new trace and span ids and determine
 		// the Sampled status.
-		sp.raw.TraceID, sp.raw.SpanID = randomID2()
-		sp.raw.Sampled = t.options.ShouldSample(sp.raw.TraceID)
+		sp.raw.Context.TraceID, sp.raw.Context.SpanID = randomID2()
+		sp.raw.Context.Sampled = t.options.ShouldSample(sp.raw.Context.TraceID)
 	}
 
 	return t.startSpanInternal(