Implement immutable SpanContexts (#36)

Per opentracing/opentracing.io#106

bench_test.go

@@ -26,7 +26,7 @@ func executeOps(sp opentracing.Span, numEvent, numTag, numItems int) {
 		sp.SetTag(tags[j], nil)
 	}
 	for j := 0; j < numItems; j++ {
-		sp.Context().SetBaggageItem(tags[j], tags[j])
+		sp.SetBaggageItem(tags[j], tags[j])
 	}
 }
 

concurrency_test.go

@@ -83,7 +83,7 @@ func TestConcurrentUsage(t *testing.T) {
 				sp := tracer.StartSpan(op)
 				sp.LogEvent("test event")
 				sp.SetTag("foo", "bar")
-				sp.Context().SetBaggageItem("boo", "far")
+				sp.SetBaggageItem("boo", "far")
 				sp.SetOperationName("x")
 				csp := tracer.StartSpan(
 					"csp",

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}
+}

examples/dapperish.go

@@ -24,7 +24,7 @@ func client() {
 		span := opentracing.StartSpan("getInput")
 		ctx := opentracing.ContextWithSpan(context.Background(), span)
 		// Make sure that global baggage propagation works.
-		span.Context().SetBaggageItem("User", os.Getenv("USER"))
+		span.SetBaggageItem("User", os.Getenv("USER"))
 		span.LogEventWithPayload("ctx", ctx)
 		fmt.Print("\n\nEnter text (empty string to exit): ")
 		text, _ := reader.ReadString('\n')

examples/dapperish/trivialrecorder.go

@@ -33,9 +33,9 @@ func (t *TrivialRecorder) SetTag(key string, val interface{}) *TrivialRecorder {
 // RecordSpan complies with the basictracer.Recorder interface.
 func (t *TrivialRecorder) RecordSpan(span basictracer.RawSpan) {
 	fmt.Printf(
-		"RecordSpan: %v[%v, %v us] --> %v logs. std context: %v; baggage: %v\n",
+		"RecordSpan: %v[%v, %v us] --> %v logs. context: %v; baggage: %v\n",
 		span.Operation, span.Start, span.Duration, len(span.Logs),
-		span.SpanContext, span.Baggage)
+		span.Context, span.Context.Baggage)
 	for i, l := range span.Logs {
 		fmt.Printf(
 			"    log %v @ %v: %v --> %v\n", i, l.Timestamp, l.Event, reflect.TypeOf(l.Payload))

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,

propagation_test.go

@@ -43,7 +43,7 @@ func TestSpanPropagator(t *testing.T) {
 	tracer := basictracer.New(recorder)
 
 	sp := tracer.StartSpan(op)
-	sp.Context().SetBaggageItem("foo", "bar")
+	sp.SetBaggageItem("foo", "bar")
 
 	tmc := opentracing.HTTPHeadersCarrier(http.Header{})
 	tests := []struct {
@@ -81,14 +81,14 @@ func TestSpanPropagator(t *testing.T) {
 	exp.Start = time.Time{}.Add(1)
 
 	for i, sp := range spans {
-		if a, e := sp.ParentSpanID, exp.SpanID; a != e {
+		if a, e := sp.ParentSpanID, exp.Context.SpanID; a != e {
 			t.Fatalf("%d: ParentSpanID %d does not match expectation %d", i, a, e)
 		} else {
 			// Prepare for comparison.
-			sp.SpanID, sp.ParentSpanID = exp.SpanID, 0
+			sp.Context.SpanID, sp.ParentSpanID = exp.Context.SpanID, 0
 			sp.Duration, sp.Start = exp.Duration, exp.Start
 		}
-		if a, e := sp.TraceID, exp.TraceID; a != e {
+		if a, e := sp.Context.TraceID, exp.Context.TraceID; a != e {
 			t.Fatalf("%d: TraceID changed from %d to %d", i, e, a)
 		}
 		if !reflect.DeepEqual(exp, sp) {

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 {

span_test.go

@@ -15,24 +15,24 @@ func TestSpan_Baggage(t *testing.T) {
 		ShouldSample: func(traceID uint64) bool { return true }, // always sample
 	})
 	span := tracer.StartSpan("x")
-	span.Context().SetBaggageItem("x", "y")
-	assert.Equal(t, "y", span.Context().BaggageItem("x"))
+	span.SetBaggageItem("x", "y")
+	assert.Equal(t, "y", span.BaggageItem("x"))
 	span.Finish()
 	spans := recorder.GetSpans()
 	assert.Equal(t, 1, len(spans))
-	assert.Equal(t, map[string]string{"x": "y"}, spans[0].Baggage)
+	assert.Equal(t, map[string]string{"x": "y"}, spans[0].Context.Baggage)
 
 	recorder.Reset()
 	span = tracer.StartSpan("x")
-	span.Context().SetBaggageItem("x", "y")
+	span.SetBaggageItem("x", "y")
 	baggage := make(map[string]string)
 	span.Context().ForeachBaggageItem(func(k, v string) bool {
 		baggage[k] = v
 		return true
 	})
 	assert.Equal(t, map[string]string{"x": "y"}, baggage)
 
-	span.Context().SetBaggageItem("a", "b")
+	span.SetBaggageItem("a", "b")
 	baggage = make(map[string]string)
 	span.Context().ForeachBaggageItem(func(k, v string) bool {
 		baggage[k] = v
@@ -42,7 +42,7 @@ func TestSpan_Baggage(t *testing.T) {
 	span.Finish()
 	spans = recorder.GetSpans()
 	assert.Equal(t, 1, len(spans))
-	assert.Equal(t, 2, len(spans[0].Baggage))
+	assert.Equal(t, 2, len(spans[0].Context.Baggage))
 }
 
 func TestSpan_Sampling(t *testing.T) {

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(