diff --git a/context.go b/context.go index 3e7c8590..889bdd9c 100644 --- a/context.go +++ b/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} +} diff --git a/context_test.go b/context_test.go index 6bf0be36..ba306f1e 100644 --- a/context_test.go +++ b/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) +} diff --git a/crossdock/server/tchannel.go b/crossdock/server/tchannel.go index b99050a3..e17bbbf2 100644 --- a/crossdock/server/tchannel.go +++ b/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() } diff --git a/crossdock/server/trace.go b/crossdock/server/trace.go index f855f537..179d107a 100644 --- a/crossdock/server/trace.go +++ b/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 } diff --git a/glide.lock b/glide.lock index 2182b783..d540bab2 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ -hash: 64cfc62ba3a3bba279ca0a16690bfad46232914612f3fc91ca00c7e0a57a26a2 -updated: 2016-07-27T00:31:43.551854959-04:00 +hash: 90167d4d6839383076d496e454b1a56cae23267141889705214504fbd081acfe +updated: 2016-08-05T13:13:39.333339713-04:00 imports: - name: github.com/apache/thrift version: 23d6746079d7b5fdb38214387c63f987e68a6d8f @@ -7,12 +7,15 @@ imports: - lib/go/thrift - name: github.com/crossdock/crossdock-go version: 228792ab861c86f8900b2db0439577feef9ec9d8 + subpackages: + - assert + - require - name: github.com/davecgh/go-spew version: 5215b55f46b2b919f50a1df0eaa5886afe4e3b3d subpackages: - spew - name: github.com/opentracing/opentracing-go - version: ce84834b482d236a829419f99eb78866b58751fd + version: 855519783f479520497c6b3445611b05fc42f009 subpackages: - ext - name: github.com/pmezard/go-difflib diff --git a/glide.yaml b/glide.yaml index 4d61bd57..a7923475 100644 --- a/glide.yaml +++ b/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 diff --git a/interop.go b/interop.go index 1f7aefc9..556cf7cd 100644 --- a/interop.go +++ b/interop.go @@ -38,7 +38,7 @@ type jaegerTraceContextPropagator struct { } func (p *jaegerTraceContextPropagator) Inject( - ctx *SpanContext, + ctx SpanContext, abstractCarrier interface{}, ) error { carrier, ok := abstractCarrier.(*SpanContext) @@ -46,16 +46,16 @@ func (p *jaegerTraceContextPropagator) Inject( 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 } diff --git a/propagation.go b/propagation.go index a8744240..f9217087 100644 --- a/propagation.go +++ b/propagation.go @@ -41,7 +41,7 @@ type Injector interface { // // Implementations may return opentracing.ErrInvalidCarrier or any other // implementation-specific error if injection fails. - Inject(ctx *SpanContext, carrier interface{}) error + Inject(ctx SpanContext, carrier interface{}) error } // Extractor is responsible for extracting SpanContext instances from a @@ -52,7 +52,7 @@ type Extractor interface { // Extract decodes a SpanContext instance from the given `carrier`, // or (nil, opentracing.ErrSpanContextNotFound) if no context could // be found in the `carrier`. - Extract(carrier interface{}) (*SpanContext, error) + Extract(carrier interface{}) (SpanContext, error) } type textMapPropagator struct { @@ -84,7 +84,7 @@ func newBinaryPropagator(tracer *tracer) *binaryPropagator { } func (p *textMapPropagator) Inject( - sc *SpanContext, + sc SpanContext, abstractCarrier interface{}, ) error { textMapWriter, ok := abstractCarrier.(opentracing.TextMapWriter) @@ -92,9 +92,6 @@ func (p *textMapPropagator) Inject( return opentracing.ErrInvalidCarrier } - sc.RLock() - defer sc.RUnlock() - textMapWriter.Set(TracerStateHeaderName, sc.String()) for k, v := range sc.baggage { textMapWriter.Set(encodeBaggageKeyAsHeader(k), v) @@ -102,12 +99,12 @@ func (p *textMapPropagator) Inject( return nil } -func (p *textMapPropagator) Extract(abstractCarrier interface{}) (*SpanContext, error) { +func (p *textMapPropagator) Extract(abstractCarrier interface{}) (SpanContext, error) { textMapReader, ok := abstractCarrier.(opentracing.TextMapReader) if !ok { - return nil, opentracing.ErrInvalidCarrier + return emptyContext, opentracing.ErrInvalidCarrier } - var ctx *SpanContext + var ctx SpanContext var baggage map[string]string err := textMapReader.ForeachKey(func(key, value string) error { if key == TracerStateHeaderName { @@ -126,17 +123,17 @@ func (p *textMapPropagator) Extract(abstractCarrier interface{}) (*SpanContext, }) if err != nil { p.tracer.metrics.DecodingErrors.Inc(1) - return nil, err + return emptyContext, err } - if ctx == nil || ctx.traceID == 0 { - return nil, opentracing.ErrSpanContextNotFound + if ctx.traceID == 0 { + return emptyContext, opentracing.ErrSpanContextNotFound } ctx.baggage = baggage return ctx, nil } func (p *httpHeaderPropagator) Inject( - sc *SpanContext, + sc SpanContext, abstractCarrier interface{}, ) error { textMapWriter, ok := abstractCarrier.(opentracing.TextMapWriter) @@ -144,9 +141,6 @@ func (p *httpHeaderPropagator) Inject( return opentracing.ErrInvalidCarrier } - sc.RLock() - defer sc.RUnlock() - textMapWriter.Set(TracerStateHeaderName, sc.String()) for k, v := range sc.baggage { safeKey := encodeBaggageKeyAsHeader(k) @@ -155,12 +149,12 @@ func (p *httpHeaderPropagator) Inject( return nil } -func (p *httpHeaderPropagator) Extract(abstractCarrier interface{}) (*SpanContext, error) { +func (p *httpHeaderPropagator) Extract(abstractCarrier interface{}) (SpanContext, error) { textMapReader, ok := abstractCarrier.(opentracing.TextMapReader) if !ok { - return nil, opentracing.ErrInvalidCarrier + return emptyContext, opentracing.ErrInvalidCarrier } - var ctx *SpanContext + var ctx SpanContext var baggage map[string]string err := textMapReader.ForeachKey(func(key, value string) error { lowerCaseKey := strings.ToLower(key) @@ -180,27 +174,23 @@ func (p *httpHeaderPropagator) Extract(abstractCarrier interface{}) (*SpanContex }) if err != nil { p.tracer.metrics.DecodingErrors.Inc(1) - return nil, err + return emptyContext, err } - if ctx == nil || ctx.traceID == 0 { - return nil, opentracing.ErrSpanContextNotFound + if ctx.traceID == 0 { + return emptyContext, opentracing.ErrSpanContextNotFound } ctx.baggage = baggage return ctx, nil } func (p *binaryPropagator) Inject( - sc *SpanContext, + sc SpanContext, abstractCarrier interface{}, ) error { carrier, ok := abstractCarrier.(io.Writer) if !ok { return opentracing.ErrInvalidCarrier } - var err error - - sc.RLock() - defer sc.RUnlock() // Handle the tracer context if err := binary.Write(carrier, binary.BigEndian, sc.traceID); err != nil { @@ -221,11 +211,11 @@ func (p *binaryPropagator) Inject( return err } for k, v := range sc.baggage { - if err = binary.Write(carrier, binary.BigEndian, int32(len(k))); err != nil { + if err := binary.Write(carrier, binary.BigEndian, int32(len(k))); err != nil { return err } io.WriteString(carrier, k) - if err = binary.Write(carrier, binary.BigEndian, int32(len(v))); err != nil { + if err := binary.Write(carrier, binary.BigEndian, int32(len(v))); err != nil { return err } io.WriteString(carrier, v) @@ -234,62 +224,60 @@ func (p *binaryPropagator) Inject( return nil } -func (p *binaryPropagator) Extract(abstractCarrier interface{}) (*SpanContext, error) { +func (p *binaryPropagator) Extract(abstractCarrier interface{}) (SpanContext, error) { carrier, ok := abstractCarrier.(io.Reader) if !ok { - return nil, opentracing.ErrInvalidCarrier + return emptyContext, opentracing.ErrInvalidCarrier } - ctx := new(SpanContext) + var ctx SpanContext if err := binary.Read(carrier, binary.BigEndian, &ctx.traceID); err != nil { - return nil, opentracing.ErrSpanContextCorrupted + return emptyContext, opentracing.ErrSpanContextCorrupted } if err := binary.Read(carrier, binary.BigEndian, &ctx.spanID); err != nil { - return nil, opentracing.ErrSpanContextCorrupted + return emptyContext, opentracing.ErrSpanContextCorrupted } if err := binary.Read(carrier, binary.BigEndian, &ctx.parentID); err != nil { - return nil, opentracing.ErrSpanContextCorrupted + return emptyContext, opentracing.ErrSpanContextCorrupted } if err := binary.Read(carrier, binary.BigEndian, &ctx.flags); err != nil { - return nil, opentracing.ErrSpanContextCorrupted + return emptyContext, opentracing.ErrSpanContextCorrupted } // Handle the baggage items var numBaggage int32 if err := binary.Read(carrier, binary.BigEndian, &numBaggage); err != nil { - return nil, opentracing.ErrSpanContextCorrupted + return emptyContext, opentracing.ErrSpanContextCorrupted } - var baggage map[string]string if iNumBaggage := int(numBaggage); iNumBaggage > 0 { - baggage = make(map[string]string, iNumBaggage) + ctx.baggage = make(map[string]string, iNumBaggage) buf := p.buffers.Get().(*bytes.Buffer) defer p.buffers.Put(buf) var keyLen, valLen int32 for i := 0; i < iNumBaggage; i++ { if err := binary.Read(carrier, binary.BigEndian, &keyLen); err != nil { - return nil, opentracing.ErrSpanContextCorrupted + return emptyContext, opentracing.ErrSpanContextCorrupted } buf.Reset() buf.Grow(int(keyLen)) if n, err := io.CopyN(buf, carrier, int64(keyLen)); err != nil || int32(n) != keyLen { - return nil, opentracing.ErrSpanContextCorrupted + return emptyContext, opentracing.ErrSpanContextCorrupted } key := buf.String() if err := binary.Read(carrier, binary.BigEndian, &valLen); err != nil { - return nil, opentracing.ErrSpanContextCorrupted + return emptyContext, opentracing.ErrSpanContextCorrupted } buf.Reset() buf.Grow(int(valLen)) if n, err := io.CopyN(buf, carrier, int64(valLen)); err != nil || int32(n) != valLen { - return nil, opentracing.ErrSpanContextCorrupted + return emptyContext, opentracing.ErrSpanContextCorrupted } - baggage[key] = buf.String() + ctx.baggage[key] = buf.String() } } - ctx.baggage = baggage return ctx, nil } diff --git a/propagation_test.go b/propagation_test.go index b0e02543..4fef2f31 100644 --- a/propagation_test.go +++ b/propagation_test.go @@ -24,7 +24,6 @@ func TestSpanPropagator(t *testing.T) { tests := []struct { format, carrier, formatName interface{} }{ - {SpanContextFormat, new(SpanContext), "TraceContextFormat"}, {opentracing.Binary, new(bytes.Buffer), "Binary"}, {opentracing.TextMap, mapc, "TextMap"}, {opentracing.HTTPHeaders, httpc, "HTTPHeaders"}, @@ -32,7 +31,7 @@ func TestSpanPropagator(t *testing.T) { sp := tracer.StartSpan(op) sp.SetTag("x", "y") // to avoid later comparing nil vs. [] - sp.Context().SetBaggageItem("foo", "bar") + sp.SetBaggageItem("foo", "bar") for _, test := range tests { // starting normal child to extract its serialized context child := tracer.StartSpan(op, opentracing.ChildOf(sp.Context())) @@ -129,17 +128,18 @@ func TestBaggagePropagationHTTP(t *testing.T) { tracer, closer := NewTracer("DOOP", NewConstSampler(true), NewNullReporter()) defer closer.Close() - sp1 := tracer.StartSpan("s1").(*span) - sp1.Context().SetBaggageItem("Some_Key", "12345") - assert.Equal(t, "12345", sp1.Context().BaggageItem("some-KEY")) - sp1.Context().SetBaggageItem("Some_Key", "98765") - assert.Equal(t, "98765", sp1.Context().BaggageItem("some-KEY")) + sp1 := tracer.StartSpan("s1") + sp1.SetBaggageItem("Some_Key", "12345") + assert.Equal(t, "12345", sp1.BaggageItem("some-KEY"), "baggage: %+v", sp1.(*span).context.baggage) + sp1.SetBaggageItem("Some_Key", "98765") + assert.Equal(t, "98765", sp1.BaggageItem("some-KEY"), "baggage: %+v", sp1.(*span).context.baggage) h := http.Header{} - err := tracer.Inject(sp1.context, opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(h)) + err := tracer.Inject(sp1.Context(), opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(h)) require.NoError(t, err) sp2, err := tracer.Extract(opentracing.HTTPHeaders, opentracing.HTTPHeadersCarrier(h)) require.NoError(t, err) - assert.Equal(t, "98765", sp2.BaggageItem("some-KEY")) + assert.Empty(t, sp2.(SpanContext).baggage["some-KEY"]) + assert.Equal(t, "98765", sp2.(SpanContext).baggage["some-key"]) } diff --git a/span.go b/span.go index 625c34fc..e9dfe295 100644 --- a/span.go +++ b/span.go @@ -22,6 +22,7 @@ package jaeger import ( "strings" + "sync" "time" "github.com/opentracing/opentracing-go" @@ -31,9 +32,11 @@ import ( ) type span struct { + sync.RWMutex + tracer *tracer - context *SpanContext + context SpanContext // The name of the "operation" this span is an instance of. // Known as a "span name" in some implementations. @@ -77,8 +80,8 @@ type tag struct { // Sets or changes the operation name. func (s *span) SetOperationName(operationName string) opentracing.Span { - s.context.Lock() - defer s.context.Unlock() + s.Lock() + defer s.Unlock() if s.context.IsSampled() { s.operationName = operationName } @@ -90,8 +93,8 @@ func (s *span) SetTag(key string, value interface{}) opentracing.Span { if key == string(ext.SamplingPriority) && setSamplingPriority(s, key, value) { return s } - s.context.Lock() - defer s.context.Unlock() + s.Lock() + defer s.Unlock() if s.context.IsSampled() { s.setTagNoLocking(key, value) } @@ -117,8 +120,8 @@ func (s *span) LogEventWithPayload(event string, payload interface{}) { } func (s *span) Log(ld opentracing.LogData) { - s.context.Lock() - defer s.context.Unlock() + s.Lock() + defer s.Unlock() if s.context.IsSampled() { if ld.Timestamp.IsZero() { ld.Timestamp = s.tracer.timeNow() @@ -127,12 +130,29 @@ func (s *span) Log(ld opentracing.LogData) { } } +// SetBaggageItem implements SetBaggageItem() of opentracing.SpanContext +func (s *span) SetBaggageItem(key, value string) opentracing.Span { + key = normalizeBaggageKey(key) + s.Lock() + defer s.Unlock() + s.context = s.context.WithBaggageItem(key, value) + return s +} + +// BaggageItem implements BaggageItem() of opentracing.SpanContext +func (s *span) BaggageItem(key string) string { + key = normalizeBaggageKey(key) + s.RLock() + defer s.RUnlock() + return s.context.baggage[key] +} + func (s *span) Finish() { s.FinishWithOptions(opentracing.FinishOptions{}) } func (s *span) FinishWithOptions(options opentracing.FinishOptions) { - s.context.Lock() + s.Lock() if s.context.IsSampled() { finishTime := options.FinishTime if finishTime.IsZero() { @@ -143,7 +163,7 @@ func (s *span) FinishWithOptions(options opentracing.FinishOptions) { s.logs = append(s.logs, options.BulkLogData...) } } - s.context.Unlock() + s.Unlock() // call reportSpan even for non-sampled traces, to return span to the pool s.tracer.reportSpan(s) } @@ -165,14 +185,14 @@ func (s *span) peerDefined() bool { } func (s *span) isRPC() bool { - s.context.RLock() - defer s.context.RUnlock() + s.RLock() + defer s.RUnlock() return s.spanKind == string(ext.SpanKindRPCClientEnum) || s.spanKind == string(ext.SpanKindRPCServerEnum) } func (s *span) isRPCClient() bool { - s.context.RLock() - defer s.context.RUnlock() + s.RLock() + defer s.RUnlock() return s.spanKind == string(ext.SpanKindRPCClientEnum) } @@ -240,8 +260,8 @@ func setPeerService(s *span, key string, value interface{}) bool { } func setSamplingPriority(s *span, key string, value interface{}) bool { - s.context.Lock() - defer s.context.Unlock() + s.Lock() + defer s.Unlock() if val, ok := value.(uint16); ok { if val > 0 { s.context.flags = s.context.flags | flagDebug | flagSampled diff --git a/span_test.go b/span_test.go index afc6aa34..9d79c035 100644 --- a/span_test.go +++ b/span_test.go @@ -10,8 +10,8 @@ func TestBaggageIterator(t *testing.T) { defer closer.Close() sp1 := tracer.StartSpan("s1").(*span) - sp1.Context().SetBaggageItem("Some_Key", "12345") - sp1.Context().SetBaggageItem("Some-other-key", "42") + sp1.SetBaggageItem("Some_Key", "12345") + sp1.SetBaggageItem("Some-other-key", "42") b := make(map[string]string) sp1.Context().ForeachBaggageItem(func(k, v string) bool { diff --git a/tracer.go b/tracer.go index 344bf303..ea05b844 100644 --- a/tracer.go +++ b/tracer.go @@ -87,10 +87,6 @@ func NewTracer( t.injectors[opentracing.Binary] = binaryPropagator t.extractors[opentracing.Binary] = binaryPropagator - interopPropagator := &jaegerTraceContextPropagator{tracer: t} - t.injectors[SpanContextFormat] = interopPropagator - t.extractors[SpanContextFormat] = interopPropagator - zipkinPropagator := &zipkinPropagator{tracer: t} t.injectors[ZipkinSpanFormat] = zipkinPropagator t.extractors[ZipkinSpanFormat] = zipkinPropagator @@ -140,15 +136,14 @@ func (t *tracer) startSpanWithOptions( startTime = t.timeNow() } - sp := t.newSpan() - ctx := new(SpanContext) - sp.context = ctx - - var parent *SpanContext + var parent SpanContext + var hasParent bool for _, ref := range options.References { if ref.Type == opentracing.ChildOfRef { - if p, ok := ref.ReferencedContext.(*SpanContext); ok { + if p, ok := ref.ReferencedContext.(SpanContext); ok { parent = p + hasParent = true + break } else { t.logger.Error(fmt.Sprintf( "ChildOf reference contains invalid type of SpanReference: %s", @@ -164,7 +159,8 @@ func (t *tracer) startSpanWithOptions( rpcServer = (v == ext.SpanKindRPCServerEnum || v == string(ext.SpanKindRPCServerEnum)) } - if parent == nil { + var ctx SpanContext + if !hasParent { ctx.traceID = t.randomID() ctx.spanID = ctx.traceID ctx.parentID = 0 @@ -173,7 +169,6 @@ func (t *tracer) startSpanWithOptions( ctx.flags |= flagSampled } } else { - parent.RLock() ctx.traceID = parent.traceID if rpcServer { // Support Zipkin's one-span-per-RPC model @@ -191,9 +186,10 @@ func (t *tracer) startSpanWithOptions( ctx.baggage[k] = v } } - parent.RUnlock() } + sp := t.newSpan() + sp.context = ctx return t.startSpanInternal( sp, operationName, @@ -205,7 +201,7 @@ func (t *tracer) startSpanWithOptions( // Inject implements Inject() method of opentracing.Tracer func (t *tracer) Inject(ctx opentracing.SpanContext, format interface{}, carrier interface{}) error { - c, ok := ctx.(*SpanContext) + c, ok := ctx.(SpanContext) if !ok { return opentracing.ErrInvalidSpanContext } @@ -240,7 +236,7 @@ func (t *tracer) newSpan() *span { return &span{} } sp := t.spanPool.Get().(*span) - sp.context = nil + sp.context = emptyContext sp.tracer = nil sp.tags = nil sp.logs = nil diff --git a/tracer_test.go b/tracer_test.go index 4375cda5..a37cf0f0 100644 --- a/tracer_test.go +++ b/tracer_test.go @@ -200,7 +200,7 @@ type dummyCarrier struct { ok bool } -func (p *dummyPropagator) Inject(ctx *SpanContext, carrier interface{}) error { +func (p *dummyPropagator) Inject(ctx SpanContext, carrier interface{}) error { c, ok := carrier.(*dummyCarrier) if !ok { return opentracing.ErrInvalidCarrier @@ -209,13 +209,13 @@ func (p *dummyPropagator) Inject(ctx *SpanContext, carrier interface{}) error { return nil } -func (p *dummyPropagator) Extract(carrier interface{}) (*SpanContext, error) { +func (p *dummyPropagator) Extract(carrier interface{}) (SpanContext, error) { c, ok := carrier.(*dummyCarrier) if !ok { - return nil, opentracing.ErrInvalidCarrier + return emptyContext, opentracing.ErrInvalidCarrier } if c.ok { - return nil, nil + return emptyContext, nil } - return nil, opentracing.ErrSpanContextNotFound + return emptyContext, opentracing.ErrSpanContextNotFound } diff --git a/zipkin.go b/zipkin.go index d022bd0f..6afef28c 100644 --- a/zipkin.go +++ b/zipkin.go @@ -30,7 +30,7 @@ type zipkinPropagator struct { } func (p *zipkinPropagator) Inject( - ctx *SpanContext, + ctx SpanContext, abstractCarrier interface{}, ) error { carrier, ok := abstractCarrier.(InjectableZipkinSpan) @@ -38,9 +38,6 @@ func (p *zipkinPropagator) Inject( return opentracing.ErrInvalidCarrier } - ctx.RLock() - defer ctx.RUnlock() - carrier.SetTraceID(ctx.TraceID()) carrier.SetSpanID(ctx.SpanID()) carrier.SetParentID(ctx.ParentID()) @@ -48,15 +45,15 @@ func (p *zipkinPropagator) Inject( return nil } -func (p *zipkinPropagator) Extract(abstractCarrier interface{}) (*SpanContext, error) { +func (p *zipkinPropagator) Extract(abstractCarrier interface{}) (SpanContext, error) { carrier, ok := abstractCarrier.(ExtractableZipkinSpan) if !ok { - return nil, opentracing.ErrInvalidCarrier + return emptyContext, opentracing.ErrInvalidCarrier } if carrier.TraceID() == 0 { - return nil, opentracing.ErrSpanContextNotFound + return emptyContext, opentracing.ErrSpanContextNotFound } - ctx := new(SpanContext) + var ctx SpanContext ctx.traceID = carrier.TraceID() ctx.spanID = carrier.SpanID() ctx.parentID = carrier.ParentID()