Implement immutable SpanContext

Per opentracing/opentracing.io#106

context.go

@@ -25,9 +25,6 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
-	"sync"
-
-	"github.com/opentracing/opentracing-go"
 )
 
 const (
@@ -38,12 +35,12 @@ const (
 var (
 	errEmptyTracerStateString     = errors.New("Cannot convert empty string to tracer state")
 	errMalformedTracerStateString = errors.New("String does not match tracer state format")
+
+	emptyContext = SpanContext{}
 )
 
 // SpanContext represents propagated span identity and state
 type SpanContext struct {
-	sync.RWMutex
-
 	// traceID represents globally unique ID of the trace.
 	// Usually generated as a random number.
 	traceID uint64
@@ -59,34 +56,12 @@ type SpanContext struct {
 	// flags is a bitmap containing such bits as 'sampled' and 'debug'.
 	flags byte
 
-	// Distributed Context baggage
+	// Distributed Context baggage. The is a snapshot in time.
 	baggage map[string]string
 }
 
-// SetBaggageItem implements SetBaggageItem() of opentracing.SpanContext
-func (c *SpanContext) SetBaggageItem(key, value string) opentracing.SpanContext {
-	key = normalizeBaggageKey(key)
-	c.Lock()
-	defer c.Unlock()
-	if c.baggage == nil {
-		c.baggage = make(map[string]string)
-	}
-	c.baggage[key] = value
-	return c
-}
-
-// BaggageItem implements BaggageItem() of opentracing.SpanContext
-func (c *SpanContext) BaggageItem(key string) string {
-	key = normalizeBaggageKey(key)
-	c.RLock()
-	defer c.RUnlock()
-	return c.baggage[key]
-}
-
 // ForeachBaggageItem implements ForeachBaggageItem() of opentracing.SpanContext
-func (c *SpanContext) ForeachBaggageItem(handler func(k, v string) bool) {
-	c.RLock()
-	defer c.RUnlock()
+func (c SpanContext) ForeachBaggageItem(handler func(k, v string) bool) {
 	for k, v := range c.baggage {
 		if !handler(k, v) {
 			break
@@ -96,80 +71,74 @@ func (c *SpanContext) ForeachBaggageItem(handler func(k, v string) bool) {
 
 // IsSampled returns whether this trace was chosen for permanent storage
 // by the sampling mechanism of the tracer.
-func (c *SpanContext) IsSampled() bool {
+func (c SpanContext) IsSampled() bool {
 	return (c.flags & flagSampled) == flagSampled
 }
 
-func (c *SpanContext) String() string {
-	c.RLock()
-	defer c.RUnlock()
+func (c SpanContext) String() string {
 	return fmt.Sprintf("%x:%x:%x:%x", c.traceID, c.spanID, c.parentID, c.flags)
 }
 
 // ContextFromString reconstructs the Context encoded in a string
-func ContextFromString(value string) (*SpanContext, error) {
-	var context = new(SpanContext)
+func ContextFromString(value string) (SpanContext, error) {
+	var context SpanContext
 	if value == "" {
-		return nil, errEmptyTracerStateString
+		return emptyContext, errEmptyTracerStateString
 	}
 	parts := strings.Split(value, ":")
 	if len(parts) != 4 {
-		return nil, errMalformedTracerStateString
+		return emptyContext, errMalformedTracerStateString
 	}
 	var err error
 	if context.traceID, err = strconv.ParseUint(parts[0], 16, 64); err != nil {
-		return nil, err
+		return emptyContext, err
 	}
 	if context.spanID, err = strconv.ParseUint(parts[1], 16, 64); err != nil {
-		return nil, err
+		return emptyContext, err
 	}
 	if context.parentID, err = strconv.ParseUint(parts[2], 16, 64); err != nil {
-		return nil, err
+		return emptyContext, err
 	}
 	flags, err := strconv.ParseUint(parts[3], 10, 8)
 	if err != nil {
-		return nil, err
+		return emptyContext, err
 	}
 	context.flags = byte(flags)
 	return context, nil
 }
 
 // TraceID implements TraceID() of SpanID
-func (c *SpanContext) TraceID() uint64 {
+func (c SpanContext) TraceID() uint64 {
 	return c.traceID
 }
 
 // SpanID implements SpanID() of SpanID
-func (c *SpanContext) SpanID() uint64 {
+func (c SpanContext) SpanID() uint64 {
 	return c.spanID
 }
 
 // ParentID implements ParentID() of SpanID
-func (c *SpanContext) ParentID() uint64 {
+func (c SpanContext) ParentID() uint64 {
 	return c.parentID
 }
 
 // NewSpanContext creates a new instance of SpanContext
-func NewSpanContext(traceID, spanID, parentID uint64, sampled bool) *SpanContext {
+func NewSpanContext(traceID, spanID, parentID uint64, sampled bool, baggage map[string]string) SpanContext {
 	flags := byte(0)
 	if sampled {
 		flags = flagSampled
 	}
-	return &SpanContext{
+	return SpanContext{
 		traceID:  traceID,
 		spanID:   spanID,
 		parentID: parentID,
-		flags:    flags}
+		flags:    flags,
+		baggage:  baggage}
 }
 
 // CopyFrom copies data from ctx into this context, including span identity and baggage.
+// TODO This is only used by interop.go. Remove once TChannel Go supports OpenTracing.
 func (c *SpanContext) CopyFrom(ctx *SpanContext) {
-	c.Lock()
-	defer c.Unlock()
-
-	ctx.RLock()
-	defer ctx.RUnlock()
-
 	c.traceID = ctx.traceID
 	c.spanID = ctx.spanID
 	c.parentID = ctx.parentID
@@ -183,3 +152,19 @@ func (c *SpanContext) CopyFrom(ctx *SpanContext) {
 		c.baggage = nil
 	}
 }
+
+// WithBaggageItem creates a new context with an extra baggage item.
+func (c SpanContext) WithBaggageItem(key, value string) SpanContext {
+	var newBaggage map[string]string
+	if c.baggage == nil {
+		newBaggage = map[string]string{key: value}
+	} else {
+		newBaggage = make(map[string]string, len(c.baggage)+1)
+		for k, v := range c.baggage {
+			newBaggage[k] = v
+		}
+		newBaggage[key] = value
+	}
+	// Use positional parameters so the compiler will help catch new fields.
+	return SpanContext{c.traceID, c.spanID, c.parentID, c.flags, newBaggage}
+}

context_test.go

@@ -27,9 +27,17 @@ func TestContextFromString(t *testing.T) {
 	assert.EqualValues(t, 1, ctx.spanID)
 	assert.EqualValues(t, 1, ctx.parentID)
 	assert.EqualValues(t, 1, ctx.flags)
-	ctx = NewSpanContext(1, 1, 1, true)
+	ctx = NewSpanContext(1, 1, 1, true, nil)
 	assert.EqualValues(t, 1, ctx.traceID)
 	assert.EqualValues(t, 1, ctx.spanID)
 	assert.EqualValues(t, 1, ctx.parentID)
 	assert.EqualValues(t, 1, ctx.flags)
 }
+
+func TestSpanContext_WithBaggageItem(t *testing.T) {
+	var ctx SpanContext
+	ctx = ctx.WithBaggageItem("some-KEY", "Some-Value")
+	assert.Equal(t, map[string]string{"some-KEY": "Some-Value"}, ctx.baggage)
+	ctx = ctx.WithBaggageItem("some-KEY", "Some-Other-Value")
+	assert.Equal(t, map[string]string{"some-KEY": "Some-Other-Value"}, ctx.baggage)
+}

crossdock/server/tchannel.go

@@ -145,10 +145,7 @@ func setupOpenTracingContext(tracer opentracing.Tracer, ctx context.Context, met
 	if tSpan != nil {
 		// populate a fake carrier and try to create OpenTracing Span
 		sc := jaeger.NewSpanContext(
-			tSpan.TraceID(), tSpan.SpanID(), tSpan.ParentID(), tSpan.TracingEnabled())
-		for k, v := range headers {
-			sc.SetBaggageItem(k, v)
-		}
+			tSpan.TraceID(), tSpan.SpanID(), tSpan.ParentID(), tSpan.TracingEnabled(), headers)
 		if tracer == nil {
 			tracer = opentracing.GlobalTracer()
 		}

crossdock/server/trace.go

@@ -38,7 +38,7 @@ func (s *Server) doStartTrace(req *tracetest.StartTraceRequest) (*tracetest.Trac
 	if req.Sampled {
 		ext.SamplingPriority.Set(span, 1)
 	}
-	span.Context().SetBaggageItem(BaggageKey, req.Baggage)
+	span.SetBaggageItem(BaggageKey, req.Baggage)
 	defer span.Finish()
 
 	ctx := opentracing.ContextWithSpan(context.Background(), span)
@@ -102,6 +102,12 @@ func observeSpan(ctx context.Context, tracer opentracing.Tracer) (*tracetest.Obs
 	observedSpan := tracetest.NewObservedSpan()
 	observedSpan.TraceId = fmt.Sprintf("%x", sc.TraceID())
 	observedSpan.Sampled = sc.IsSampled()
-	observedSpan.Baggage = sc.BaggageItem(BaggageKey)
+	sc.ForeachBaggageItem(func(k, v string) bool {
+		if k == BaggageKey {
+			observedSpan.Baggage = v
+			return false
+		}
+		return true
+	})
 	return observedSpan, nil
 }

glide.yaml

@@ -5,7 +5,7 @@ import:
   subpackages:
   - lib/go/thrift
 - package: github.com/opentracing/opentracing-go
-  version: ce84834b482d236a829419f99eb78866b58751fd
+  version: 855519783f479520497c6b3445611b05fc42f009
   subpackages:
   - ext
 - package: golang.org/x/net

interop.go

@@ -38,24 +38,24 @@ type jaegerTraceContextPropagator struct {
 }
 
 func (p *jaegerTraceContextPropagator) Inject(
-	ctx *SpanContext,
+	ctx SpanContext,
 	abstractCarrier interface{},
 ) error {
 	carrier, ok := abstractCarrier.(*SpanContext)
 	if !ok {
 		return opentracing.ErrInvalidCarrier
 	}
 
-	carrier.CopyFrom(ctx)
+	carrier.CopyFrom(&ctx)
 	return nil
 }
 
-func (p *jaegerTraceContextPropagator) Extract(abstractCarrier interface{}) (*SpanContext, error) {
+func (p *jaegerTraceContextPropagator) Extract(abstractCarrier interface{}) (SpanContext, error) {
 	carrier, ok := abstractCarrier.(*SpanContext)
 	if !ok {
-		return nil, opentracing.ErrInvalidCarrier
+		return emptyContext, opentracing.ErrInvalidCarrier
 	}
 	ctx := new(SpanContext)
 	ctx.CopyFrom(carrier)
-	return ctx, nil
+	return *ctx, nil
 }