Merge pull request #8 from opentracing/changes/split_tag_map_out

split the trace tags out from the core context ID in marshal/unmarshal

examples/dapperish/dapper.go

@@ -39,10 +39,9 @@ type DapperishTraceContext struct {
 const (
 	// Note that these strings are designed to be unchanged by the conversion
 	// into standard HTTP headers (which messes with capitalization).
-	fieldNameTraceID   = "Traceid"
-	fieldNameSpanID    = "Spanid"
-	fieldNameSampled   = "Sampled"
-	fieldNameTagPrefix = "Tag-"
+	fieldNameTraceID = "Traceid"
+	fieldNameSpanID  = "Spanid"
+	fieldNameSampled = "Sampled"
 )
 
 // NewChild complies with the opentracing.TraceContext interface.
@@ -63,20 +62,20 @@ func (d *DapperishTraceContext) NewChild() (opentracing.TraceContext, opentracin
 }
 
 // SetTraceTag complies with the opentracing.TraceContext interface.
-func (d *DapperishTraceContext) SetTraceTag(key, val string) opentracing.TraceContext {
+func (d *DapperishTraceContext) SetTraceTag(caseInsensitiveKey, val string) opentracing.TraceContext {
 	d.tagLock.Lock()
 	defer d.tagLock.Unlock()
 
-	d.traceTags[key] = val
+	d.traceTags[strings.ToLower(caseInsensitiveKey)] = val
 	return d
 }
 
 // TraceTag complies with the opentracing.TraceContext interface.
-func (d *DapperishTraceContext) TraceTag(key string) string {
+func (d *DapperishTraceContext) TraceTag(caseInsensitiveKey string) string {
 	d.tagLock.RLock()
 	defer d.tagLock.RUnlock()
 
-	return d.traceTags[key]
+	return d.traceTags[strings.ToLower(caseInsensitiveKey)]
 }
 
 // DapperishTraceContextSource is an implementation of
@@ -103,76 +102,77 @@ func (d *DapperishTraceContextSource) NewRootTraceContext() opentracing.TraceCon
 // opentracing.TraceContextSource interface.
 func (d *DapperishTraceContextSource) MarshalTraceContextStringMap(
 	ctx opentracing.TraceContext,
-) map[string]string {
+) (contextIDMap map[string]string, tagsMap map[string]string) {
 	dctx := ctx.(*DapperishTraceContext)
-	rval := map[string]string{
+	contextIDMap = map[string]string{
 		fieldNameTraceID: strconv.FormatInt(dctx.TraceID, 10),
 		fieldNameSpanID:  strconv.FormatInt(dctx.SpanID, 10),
 		fieldNameSampled: strconv.FormatBool(dctx.Sampled),
 	}
 	dctx.tagLock.RLock()
+	tagsMap = make(map[string]string, len(dctx.traceTags))
 	for k, v := range dctx.traceTags {
-		rval[fieldNameTagPrefix+k] = v
+		tagsMap[k] = v
 	}
 	dctx.tagLock.RUnlock()
-	return rval
+	return contextIDMap, tagsMap
 }
 
 // UnmarshalTraceContextStringMap complies with the
 // opentracing.TraceContextSource interface.
 func (d *DapperishTraceContextSource) UnmarshalTraceContextStringMap(
-	encoded map[string]string,
+	contextIDMap map[string]string,
+	tagsMap map[string]string,
 ) (opentracing.TraceContext, error) {
-	traceTags := make(map[string]string)
 	requiredFieldCount := 0
 	var traceID, spanID int64
 	var sampled bool
 	var err error
-	for k, v := range encoded {
+	for k, v := range contextIDMap {
 		switch k {
 		case fieldNameTraceID:
-			traceID, err = strconv.ParseInt(encoded[fieldNameTraceID], 10, 64)
+			traceID, err = strconv.ParseInt(v, 10, 64)
 			if err != nil {
 				return nil, err
 			}
 			requiredFieldCount++
 		case fieldNameSpanID:
-			spanID, err = strconv.ParseInt(encoded[fieldNameSpanID], 10, 64)
+			spanID, err = strconv.ParseInt(v, 10, 64)
 			if err != nil {
 				return nil, err
 			}
 			requiredFieldCount++
 		case fieldNameSampled:
-			sampled, err = strconv.ParseBool(encoded[fieldNameSampled])
+			sampled, err = strconv.ParseBool(v)
 			if err != nil {
 				return nil, err
 			}
 			requiredFieldCount++
 		default:
-			if strings.HasPrefix(k, fieldNameTagPrefix) {
-				traceTags[strings.TrimPrefix(k, fieldNameTagPrefix)] = v
-			} else {
-				return nil, fmt.Errorf("Unknown string map field: %v", k)
-			}
+			return nil, fmt.Errorf("Unknown contextIDMap field: %v", k)
 		}
 	}
 	if requiredFieldCount < 3 {
 		return nil, fmt.Errorf("Only found %v of 3 required fields", requiredFieldCount)
 	}
 
+	lowercaseTagsMap := make(map[string]string, len(tagsMap))
+	for k, v := range tagsMap {
+		lowercaseTagsMap[strings.ToLower(k)] = v
+	}
+
 	return &DapperishTraceContext{
 		TraceID:   traceID,
 		SpanID:    spanID,
 		Sampled:   sampled,
-		traceTags: traceTags,
+		traceTags: lowercaseTagsMap,
 	}, nil
 }
 
 // MarshalTraceContextBinary complies with the opentracing.TraceContextSource
 // interface.
-func (d *DapperishTraceContextSource) MarshalTraceContextBinary(ctx opentracing.TraceContext) []byte {
+func (d *DapperishTraceContextSource) MarshalTraceContextBinary(ctx opentracing.TraceContext) (contextID []byte, traceTags []byte) {
 	dtc := ctx.(*DapperishTraceContext)
-	// XXX: support tags
 	var err error
 	buf := new(bytes.Buffer)
 	err = binary.Write(buf, binary.BigEndian, dtc.TraceID)
@@ -191,17 +191,18 @@ func (d *DapperishTraceContextSource) MarshalTraceContextBinary(ctx opentracing.
 	if err != nil {
 		panic(err)
 	}
-	return buf.Bytes()
+	// XXX: support tags
+	return buf.Bytes(), []byte{}
 }
 
 // UnmarshalTraceContextBinary complies with the opentracing.TraceContextSource
 // interface.
 func (d *DapperishTraceContextSource) UnmarshalTraceContextBinary(
-	encoded []byte,
+	contextID []byte,
+	traceTags []byte,
 ) (opentracing.TraceContext, error) {
-	// XXX: support tags
 	var err error
-	reader := bytes.NewReader(encoded)
+	reader := bytes.NewReader(contextID)
 	var traceID, spanID int64
 	var sampledByte byte
 
@@ -217,6 +218,7 @@ func (d *DapperishTraceContextSource) UnmarshalTraceContextBinary(
 	if err != nil {
 		return nil, err
 	}
+	// XXX: support tags
 	return &DapperishTraceContext{
 		TraceID:   traceID,
 		SpanID:    spanID,

examples/dapperish/main.go

@@ -19,7 +19,7 @@ func client() {
 	reader := bufio.NewReader(os.Stdin)
 	for {
 		span := opentracing.StartTrace("getInput")
-		ctx := opentracing.BackgroundContextWithSpan(span)
+		ctx := opentracing.BackgroundGoContextWithSpan(span)
 		// Make sure that global trace tag propagation works.
 		span.TraceContext().SetTraceTag("User", os.Getenv("USER"))
 		span.Info("ctx: ", ctx)
@@ -66,10 +66,12 @@ func server() {
 			serverSpan.Error("body read error", err)
 		}
 		serverSpan.Info("got request with body: " + string(fullBody))
+		contextIDMap, tagsMap := opentracing.MarshalTraceContextStringMap(reqCtx)
 		fmt.Fprintf(
 			w,
-			"Hello: %v / %q",
-			opentracing.MarshalTraceContextStringMap(reqCtx),
+			"Hello: %v // %v //  %q",
+			contextIDMap,
+			tagsMap,
 			html.EscapeString(req.URL.Path))
 	})
 

opentracing/defaulttracer.go

@@ -35,30 +35,30 @@ func JoinTrace(operationName string, parent interface{}, keyValueTags ...interfa
 // `TraceContextMarshaler.MarshalTraceContextBinary`.
 //
 // See `DefaultTracer()`.
-func MarshalTraceContextBinary(ctx TraceContext) []byte {
+func MarshalTraceContextBinary(ctx TraceContext) ([]byte, []byte) {
 	return defaultOpenTracer.MarshalTraceContextBinary(ctx)
 }
 
 // MarshalTraceContextStringMap defers to
 // `TraceContextMarshaler.MarshalTraceContextStringMap`.
 //
 // See `DefaultTracer()`.
-func MarshalTraceContextStringMap(ctx TraceContext) map[string]string {
+func MarshalTraceContextStringMap(ctx TraceContext) (map[string]string, map[string]string) {
 	return defaultOpenTracer.MarshalTraceContextStringMap(ctx)
 }
 
 // UnmarshalTraceContextBinary defers to
 // `TraceContextUnmarshaler.UnmarshalTraceContextBinary`.
 //
 // See `DefaultTracer()`.
-func UnmarshalTraceContextBinary(encoded []byte) (TraceContext, error) {
-	return defaultOpenTracer.UnmarshalTraceContextBinary(encoded)
+func UnmarshalTraceContextBinary(traceContextID []byte, traceTags []byte) (TraceContext, error) {
+	return defaultOpenTracer.UnmarshalTraceContextBinary(traceContextID, traceTags)
 }
 
 // UnmarshalTraceContextStringMap defers to
 // `TraceContextUnmarshaler.UnmarshaTraceContextStringMap`.
 //
 // See `DefaultTracer()`.
-func UnmarshalTraceContextStringMap(encoded map[string]string) (TraceContext, error) {
-	return defaultOpenTracer.UnmarshalTraceContextStringMap(encoded)
+func UnmarshalTraceContextStringMap(traceContextID map[string]string, traceTags map[string]string) (TraceContext, error) {
+	return defaultOpenTracer.UnmarshalTraceContextStringMap(traceContextID, traceTags)
 }

opentracing/noop.go

@@ -48,16 +48,22 @@ func (n noopSpan) AddToGoContext(ctx context.Context) (Span, context.Context) {
 }
 
 // noopTraceContextSource:
-func (n noopTraceContextSource) MarshalTraceContextBinary(tcid TraceContext) []byte {
-	return emptyBytes
+func (n noopTraceContextSource) MarshalTraceContextBinary(tcid TraceContext) ([]byte, []byte) {
+	return emptyBytes, emptyBytes
 }
-func (n noopTraceContextSource) MarshalTraceContextStringMap(tcid TraceContext) map[string]string {
-	return emptyStringMap
+func (n noopTraceContextSource) MarshalTraceContextStringMap(tcid TraceContext) (map[string]string, map[string]string) {
+	return emptyStringMap, emptyStringMap
 }
-func (n noopTraceContextSource) UnmarshalTraceContextBinary(encoded []byte) (TraceContext, error) {
+func (n noopTraceContextSource) UnmarshalTraceContextBinary(
+	traceContextID []byte,
+	traceTags []byte,
+) (TraceContext, error) {
 	return defaultNoopTraceContext, nil
 }
-func (n noopTraceContextSource) UnmarshalTraceContextStringMap(encoded map[string]string) (TraceContext, error) {
+func (n noopTraceContextSource) UnmarshalTraceContextStringMap(
+	traceContextID map[string]string,
+	traceTags map[string]string,
+) (TraceContext, error) {
 	return defaultNoopTraceContext, nil
 }
 func (n noopTraceContextSource) NewRootTraceContext() TraceContext {

opentracing/tracecontext.go

@@ -39,34 +39,86 @@ type TraceContext interface {
 	// TraceContext, and that can add up to a lot of network and cpu
 	// overhead.
 	//
+	// IMPORTANT NOTE #3: Trace tags are case-insensitive: implementations may
+	// wish to use them as HTTP header keys (or key suffixes), and of course
+	// HTTP headers are case insensitive.
+	//
 	// Returns a reference to this TraceContext for chaining, etc.
-	SetTraceTag(key, value string) TraceContext
+	SetTraceTag(caseInsensitiveKey, value string) TraceContext
 
 	// Gets the value for a trace tag given its key. Returns the empty string
 	// if the value isn't found in this TraceContext.
-	TraceTag(key string) string
+	TraceTag(caseInsensitiveKey string) string
 }
 
 // TraceContextMarshaler is a simple interface to marshal a TraceContext to a
 // binary byte array or a string-to-string map.
 type TraceContextMarshaler interface {
 	// Converts the TraceContext into marshaled binary data (see
 	// TraceContextUnmarshaler.UnmarshalTraceContextBinary()).
-	MarshalTraceContextBinary(tcid TraceContext) []byte
+	//
+	// The first return value must represent the marshaler's serialization of
+	// the core identifying information in `tcid`.
+	//
+	// The second return value must represent the marshaler's serialization of
+	// the trace tags, per `SetTraceTag` and `TraceTag`.
+	MarshalTraceContextBinary(
+		tcid TraceContext,
+	) (
+		traceContextID []byte,
+		traceTags []byte,
+	)
+
 	// Converts the TraceContext into a marshaled string:string map (see
 	// TraceContextUnmarshaler.UnmarshalTraceContextStringMap()).
-	MarshalTraceContextStringMap(tcid TraceContext) map[string]string
+	//
+	// The first return value must represent the marshaler's serialization of
+	// the core identifying information in `tcid`.
+	//
+	// The second return value must represent the marshaler's serialization of
+	// the trace tags, per `SetTraceTag` and `TraceTag`.
+	MarshalTraceContextStringMap(
+		tcid TraceContext,
+	) (
+		traceContextID map[string]string,
+		traceTags map[string]string,
+	)
 }
 
 // TraceContextUnmarshaler is a simple interface to unmarshal a binary byte
 // array or a string-to-string map into a TraceContext.
 type TraceContextUnmarshaler interface {
 	// Converts the marshaled binary data (see
 	// TraceContextMarshaler.MarshalTraceContextBinary()) into a TraceContext.
-	UnmarshalTraceContextBinary(marshaled []byte) (TraceContext, error)
+	//
+	// The first parameter contains the marshaler's serialization of the core
+	// identifying information in a TraceContext instance.
+	//
+	// The second parameter contains the marshaler's serialization of the trace
+	// tags (per `SetTraceTag` and `TraceTag`) attached to a TraceContext
+	// instance.
+	UnmarshalTraceContextBinary(
+		traceContextID []byte,
+		traceTags []byte,
+	) (TraceContext, error)
+
 	// Converts the marshaled string:string map (see
 	// TraceContextMarshaler.MarshalTraceContextStringMap()) into a TraceContext.
-	UnmarshalTraceContextStringMap(marshaled map[string]string) (TraceContext, error)
+	//
+	// The first parameter contains the marshaler's serialization of the core
+	// identifying information in a TraceContext instance.
+	//
+	// The second parameter contains the marshaler's serialization of the trace
+	// tags (per `SetTraceTag` and `TraceTag`) attached to a TraceContext
+	// instance.
+	//
+	// It's permissable to pass the same map to both parameters (e.g., an HTTP
+	// request headers map): the implementation should only unmarshal the
+	// subset its interested in.
+	UnmarshalTraceContextStringMap(
+		traceContextID map[string]string,
+		traceTags map[string]string,
+	) (TraceContext, error)
 }
 
 // TraceContextSource is a long-lived interface that knows how to create a root