diff --git a/ext/tags_test.go b/ext/tags_test.go index 1150f25..b09efa1 100644 --- a/ext/tags_test.go +++ b/ext/tags_test.go @@ -35,9 +35,9 @@ func TestPeerTags(t *testing.T) { "peer.port": uint16(8080), "span.kind": ext.SpanKindRPCClientEnum, }, rawSpan.Tags()) - assert.True(t, span.Context().(*mocktracer.MockSpanContext).Sampled) + assert.True(t, span.Context().(mocktracer.MockSpanContext).Sampled) ext.SamplingPriority.Set(span, uint16(0)) - assert.False(t, span.Context().(*mocktracer.MockSpanContext).Sampled) + assert.False(t, span.Context().(mocktracer.MockSpanContext).Sampled) } func TestHTTPTags(t *testing.T) { @@ -76,7 +76,7 @@ func TestMiscTags(t *testing.T) { func TestRPCServerOption(t *testing.T) { tracer := mocktracer.New() parent := tracer.StartSpan("my-trace") - parent.Context().SetBaggageItem("bag", "gage") + parent.SetBaggageItem("bag", "gage") carrier := opentracing.HTTPHeadersCarrier{} err := tracer.Inject(parent.Context(), opentracing.HTTPHeaders, carrier) @@ -97,5 +97,5 @@ func TestRPCServerOption(t *testing.T) { }, rawSpan.Tags()) assert.Equal(t, map[string]string{ "bag": "gage", - }, rawSpan.Context().(*mocktracer.MockSpanContext).Baggage()) + }, rawSpan.Context().(mocktracer.MockSpanContext).Baggage) } diff --git a/mocktracer/mockspan.go b/mocktracer/mockspan.go index 9830f98..b22dff8 100644 --- a/mocktracer/mockspan.go +++ b/mocktracer/mockspan.go @@ -18,12 +18,10 @@ import ( // By default all spans have Sampled=true flag, unless {"sampling.priority": 0} // tag is set. type MockSpanContext struct { - sync.RWMutex - TraceID int SpanID int Sampled bool - baggage map[string]string + Baggage map[string]string } var mockIDSource = uint32(42) @@ -33,71 +31,44 @@ func nextMockID() int { return int(atomic.LoadUint32(&mockIDSource)) } -func newMockSpanContext(traceID int, spanID int, sampled bool, baggage map[string]string) *MockSpanContext { - baggageCopy := make(map[string]string) - for k, v := range baggage { - baggageCopy[k] = v - } - return &MockSpanContext{ - TraceID: traceID, - SpanID: spanID, - Sampled: sampled, - baggage: baggageCopy, - } -} - -// SetBaggageItem belongs to the SpanContext interface -func (s *MockSpanContext) SetBaggageItem(key, val string) opentracing.SpanContext { - s.Lock() - defer s.Unlock() - if s.baggage == nil { - s.baggage = make(map[string]string) - } - s.baggage[key] = val - return s -} - -// BaggageItem belongs to the SpanContext interface -func (s *MockSpanContext) BaggageItem(key string) string { - s.RLock() - defer s.RUnlock() - return s.baggage[key] -} - // ForeachBaggageItem belongs to the SpanContext interface -func (s *MockSpanContext) ForeachBaggageItem(handler func(k, v string) bool) { - s.RLock() - defer s.RUnlock() - for k, v := range s.baggage { +func (c MockSpanContext) ForeachBaggageItem(handler func(k, v string) bool) { + for k, v := range c.Baggage { if !handler(k, v) { break } } } -// Baggage returns a copy of baggage items in the span -func (s *MockSpanContext) Baggage() map[string]string { - s.RLock() - defer s.RUnlock() - baggage := make(map[string]string) - for k, v := range s.baggage { - baggage[k] = v +// WithBaggageItem creates a new context with an extra baggage item. +func (c MockSpanContext) WithBaggageItem(key, value string) MockSpanContext { + 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 } - return baggage + // Use positional parameters so the compiler will help catch new fields. + return MockSpanContext{c.TraceID, c.SpanID, c.Sampled, newBaggage} } // MockSpan is an opentracing.Span implementation that exports its internal // state for testing purposes. type MockSpan struct { + sync.RWMutex + ParentID int OperationName string StartTime time.Time FinishTime time.Time - // All of the below (including SpanContext) are protected by SpanContext's - // embedded RWMutex. - SpanContext *MockSpanContext + // All of the below are protected by the embedded RWMutex. + SpanContext MockSpanContext tags map[string]interface{} logs []opentracing.LogData tracer *MockTracer @@ -113,12 +84,12 @@ func newMockSpan(t *MockTracer, name string, opts opentracing.StartSpanOptions) var baggage map[string]string sampled := true if len(opts.References) > 0 { - traceID = opts.References[0].ReferencedContext.(*MockSpanContext).TraceID - parentID = opts.References[0].ReferencedContext.(*MockSpanContext).SpanID - sampled = opts.References[0].ReferencedContext.(*MockSpanContext).Sampled - baggage = opts.References[0].ReferencedContext.(*MockSpanContext).Baggage() + traceID = opts.References[0].ReferencedContext.(MockSpanContext).TraceID + parentID = opts.References[0].ReferencedContext.(MockSpanContext).SpanID + sampled = opts.References[0].ReferencedContext.(MockSpanContext).Sampled + baggage = opts.References[0].ReferencedContext.(MockSpanContext).Baggage } - spanContext := newMockSpanContext(traceID, nextMockID(), sampled, baggage) + spanContext := MockSpanContext{traceID, nextMockID(), sampled, baggage} startTime := opts.StartTime if startTime.IsZero() { startTime = time.Now() @@ -137,8 +108,8 @@ func newMockSpan(t *MockTracer, name string, opts opentracing.StartSpanOptions) // Tags returns a copy of tags accumulated by the span so far func (s *MockSpan) Tags() map[string]interface{} { - s.SpanContext.RLock() - defer s.SpanContext.RUnlock() + s.RLock() + defer s.RUnlock() tags := make(map[string]interface{}) for k, v := range s.tags { tags[k] = v @@ -148,15 +119,15 @@ func (s *MockSpan) Tags() map[string]interface{} { // Tag returns a single tag func (s *MockSpan) Tag(k string) interface{} { - s.SpanContext.RLock() - defer s.SpanContext.RUnlock() + s.RLock() + defer s.RUnlock() return s.tags[k] } // Logs returns a copy of logs accumulated in the span so far func (s *MockSpan) Logs() []opentracing.LogData { - s.SpanContext.RLock() - defer s.SpanContext.RUnlock() + s.RLock() + defer s.RUnlock() logs := make([]opentracing.LogData, len(s.logs)) copy(logs, s.logs) return logs @@ -169,8 +140,8 @@ func (s *MockSpan) Context() opentracing.SpanContext { // SetTag belongs to the Span interface func (s *MockSpan) SetTag(key string, value interface{}) opentracing.Span { - s.SpanContext.Lock() - defer s.SpanContext.Unlock() + s.Lock() + defer s.Unlock() if key == string(ext.SamplingPriority) { if v, ok := value.(uint16); ok { s.SpanContext.Sampled = v > 0 @@ -185,20 +156,35 @@ func (s *MockSpan) SetTag(key string, value interface{}) opentracing.Span { return s } +// SetBaggageItem belongs to the Span interface +func (s *MockSpan) SetBaggageItem(key, val string) opentracing.Span { + s.Lock() + defer s.Unlock() + s.SpanContext = s.SpanContext.WithBaggageItem(key, val) + return s +} + +// BaggageItem belongs to the Span interface +func (s *MockSpan) BaggageItem(key string) string { + s.RLock() + defer s.RUnlock() + return s.SpanContext.Baggage[key] +} + // Finish belongs to the Span interface func (s *MockSpan) Finish() { - s.SpanContext.Lock() + s.Lock() s.FinishTime = time.Now() - s.SpanContext.Unlock() + s.Unlock() s.tracer.recordSpan(s) } // FinishWithOptions belongs to the Span interface func (s *MockSpan) FinishWithOptions(opts opentracing.FinishOptions) { - s.SpanContext.Lock() + s.Lock() s.FinishTime = opts.FinishTime s.logs = append(s.logs, opts.BulkLogData...) - s.SpanContext.Unlock() + s.Unlock() s.tracer.recordSpan(s) } @@ -227,15 +213,15 @@ func (s *MockSpan) LogEventWithPayload(event string, payload interface{}) { // Log belongs to the Span interface func (s *MockSpan) Log(data opentracing.LogData) { - s.SpanContext.Lock() - defer s.SpanContext.Unlock() + s.Lock() + defer s.Unlock() s.logs = append(s.logs, data) } // SetOperationName belongs to the Span interface func (s *MockSpan) SetOperationName(operationName string) opentracing.Span { - s.SpanContext.Lock() - defer s.SpanContext.Unlock() + s.Lock() + defer s.Unlock() s.OperationName = operationName return s } diff --git a/mocktracer/mocktracer.go b/mocktracer/mocktracer.go index 7b81d3e..61112c1 100644 --- a/mocktracer/mocktracer.go +++ b/mocktracer/mocktracer.go @@ -76,7 +76,7 @@ func (t *MockTracer) RegisterExtractor(format interface{}, extractor Extractor) // Inject belongs to the Tracer interface. func (t *MockTracer) Inject(sm opentracing.SpanContext, format interface{}, carrier interface{}) error { - spanContext, ok := sm.(*MockSpanContext) + spanContext, ok := sm.(MockSpanContext) if !ok { return opentracing.ErrInvalidCarrier } diff --git a/mocktracer/mocktracer_test.go b/mocktracer/mocktracer_test.go index ddbf99a..d78cca8 100644 --- a/mocktracer/mocktracer_test.go +++ b/mocktracer/mocktracer_test.go @@ -26,7 +26,7 @@ func TestMockTracer_StartSpan(t *testing.T) { parent := spans[1] child := spans[0] assert.Equal(t, map[string]interface{}{"x": "y"}, parent.Tags()) - assert.Equal(t, child.ParentID, parent.Context().(*MockSpanContext).SpanID) + assert.Equal(t, child.ParentID, parent.Context().(MockSpanContext).SpanID) } func TestMockSpan_SetOperationName(t *testing.T) { @@ -39,9 +39,9 @@ func TestMockSpan_SetOperationName(t *testing.T) { func TestMockSpanContext_Baggage(t *testing.T) { tracer := New() span := tracer.StartSpan("x") - span.Context().SetBaggageItem("x", "y") - assert.Equal(t, "y", span.Context().BaggageItem("x")) - assert.Equal(t, map[string]string{"x": "y"}, span.Context().(*MockSpanContext).Baggage()) + span.SetBaggageItem("x", "y") + assert.Equal(t, "y", span.BaggageItem("x")) + assert.Equal(t, map[string]string{"x": "y"}, span.Context().(MockSpanContext).Baggage) baggage := make(map[string]string) span.Context().ForeachBaggageItem(func(k, v string) bool { @@ -50,13 +50,13 @@ func TestMockSpanContext_Baggage(t *testing.T) { }) 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 return false // exit early }) - assert.Equal(t, 2, len(span.Context().(*MockSpanContext).Baggage())) + assert.Equal(t, 2, len(span.Context().(MockSpanContext).Baggage)) assert.Equal(t, 1, len(baggage)) } @@ -116,7 +116,7 @@ func TestMockTracer_Propagation(t *testing.T) { for _, test := range tests { tracer := New() span := tracer.StartSpan("x") - span.Context().SetBaggageItem("x", "y") + span.SetBaggageItem("x", "y") if !test.sampled { ext.SamplingPriority.Set(span, 0) } @@ -140,9 +140,9 @@ func TestMockTracer_Propagation(t *testing.T) { extractedContext, err := tracer.Extract(opentracing.TextMap, opentracing.TextMapCarrier(carrier)) require.NoError(t, err) - assert.Equal(t, mSpan.SpanContext.TraceID, extractedContext.(*MockSpanContext).TraceID) - assert.Equal(t, mSpan.SpanContext.SpanID, extractedContext.(*MockSpanContext).SpanID) - assert.Equal(t, test.sampled, extractedContext.(*MockSpanContext).Sampled) - assert.Equal(t, "y", extractedContext.BaggageItem("x")) + assert.Equal(t, mSpan.SpanContext.TraceID, extractedContext.(MockSpanContext).TraceID) + assert.Equal(t, mSpan.SpanContext.SpanID, extractedContext.(MockSpanContext).SpanID) + assert.Equal(t, test.sampled, extractedContext.(MockSpanContext).Sampled) + assert.Equal(t, "y", extractedContext.(MockSpanContext).Baggage["x"]) } } diff --git a/mocktracer/propagation.go b/mocktracer/propagation.go index e75337e..be07021 100644 --- a/mocktracer/propagation.go +++ b/mocktracer/propagation.go @@ -11,6 +11,8 @@ import ( const mockTextMapIdsPrefix = "mockpfx-ids-" const mockTextMapBaggagePrefix = "mockpfx-baggage-" +var emptyContext = MockSpanContext{} + // Injector is responsible for injecting SpanContext instances in a manner suitable // for propagation via a format-specific "carrier" object. Typically the // injection will take place across an RPC boundary, but message queues and @@ -21,7 +23,7 @@ type Injector interface { // // Implementations may return opentracing.ErrInvalidCarrier or any other // implementation-specific error if injection fails. - Inject(ctx *MockSpanContext, carrier interface{}) error + Inject(ctx MockSpanContext, carrier interface{}) error } // Extractor is responsible for extracting SpanContext instances from a @@ -32,16 +34,14 @@ 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{}) (*MockSpanContext, error) + Extract(carrier interface{}) (MockSpanContext, error) } // TextMapPropagator implements Injector/Extractor for TextMap format. type TextMapPropagator struct{} // Inject implements the Injector interface -func (t *TextMapPropagator) Inject(spanContext *MockSpanContext, carrier interface{}) error { - spanContext.RLock() - defer spanContext.RUnlock() +func (t *TextMapPropagator) Inject(spanContext MockSpanContext, carrier interface{}) error { writer, ok := carrier.(opentracing.TextMapWriter) if !ok { return opentracing.ErrInvalidCarrier @@ -51,19 +51,19 @@ func (t *TextMapPropagator) Inject(spanContext *MockSpanContext, carrier interfa writer.Set(mockTextMapIdsPrefix+"spanid", strconv.Itoa(spanContext.SpanID)) writer.Set(mockTextMapIdsPrefix+"sampled", fmt.Sprint(spanContext.Sampled)) // Baggage: - for baggageKey, baggageVal := range spanContext.baggage { + for baggageKey, baggageVal := range spanContext.Baggage { writer.Set(mockTextMapBaggagePrefix+baggageKey, baggageVal) } return nil } // Extract implements the Extractor interface -func (t *TextMapPropagator) Extract(carrier interface{}) (*MockSpanContext, error) { +func (t *TextMapPropagator) Extract(carrier interface{}) (MockSpanContext, error) { reader, ok := carrier.(opentracing.TextMapReader) if !ok { - return nil, opentracing.ErrInvalidCarrier + return emptyContext, opentracing.ErrInvalidCarrier } - rval := newMockSpanContext(0, 0, true, nil) + rval := MockSpanContext{0, 0, true, nil} err := reader.ForeachKey(func(key, val string) error { lowerKey := strings.ToLower(key) switch { @@ -89,15 +89,18 @@ func (t *TextMapPropagator) Extract(carrier interface{}) (*MockSpanContext, erro rval.Sampled = b case strings.HasPrefix(lowerKey, mockTextMapBaggagePrefix): // Baggage: - rval.SetBaggageItem(lowerKey[len(mockTextMapBaggagePrefix):], val) + if rval.Baggage == nil { + rval.Baggage = make(map[string]string) + } + rval.Baggage[lowerKey[len(mockTextMapBaggagePrefix):]] = val } return nil }) if rval.TraceID == 0 || rval.SpanID == 0 { - return nil, opentracing.ErrSpanContextNotFound + return emptyContext, opentracing.ErrSpanContextNotFound } if err != nil { - return nil, err + return emptyContext, err } return rval, nil } diff --git a/noop.go b/noop.go index 3fbeeaf..270824e 100644 --- a/noop.go +++ b/noop.go @@ -18,12 +18,12 @@ const ( ) // noopSpanContext: -func (n noopSpanContext) SetBaggageItem(key, val string) SpanContext { return n } -func (n noopSpanContext) BaggageItem(key string) string { return emptyString } func (n noopSpanContext) ForeachBaggageItem(handler func(k, v string) bool) {} // noopSpan: func (n noopSpan) Context() SpanContext { return defaultNoopSpanContext } +func (n noopSpan) SetBaggageItem(key, val string) Span { return defaultNoopSpan } +func (n noopSpan) BaggageItem(key string) string { return emptyString } func (n noopSpan) SetTag(key string, value interface{}) Span { return n } func (n noopSpan) Finish() {} func (n noopSpan) FinishWithOptions(opts FinishOptions) {} diff --git a/span.go b/span.go index 184d176..cc5579d 100644 --- a/span.go +++ b/span.go @@ -5,30 +5,6 @@ import "time" // SpanContext represents Span state that must propagate to descendant Spans and across process // boundaries (e.g., a tuple). type SpanContext interface { - // SetBaggageItem sets a key:value pair on this SpanContext that also - // propagates to future children of the associated Span. - // - // SetBaggageItem() enables powerful functionality given a full-stack - // opentracing integration (e.g., arbitrary application data from a mobile - // app can make it, transparently, all the way into the depths of a storage - // system), and with it some powerful costs: use this feature with care. - // - // IMPORTANT NOTE #1: SetBaggageItem() will only propagate baggage items to - // *future* causal descendants of the associated Span. - // - // IMPORTANT NOTE #2: Use this thoughtfully and with care. Every key and - // value is copied into every local *and remote* child of the associated - // Span, and that can add up to a lot of network and cpu overhead. - // - // Returns a reference to this SpanContext for chaining, etc. - SetBaggageItem(restrictedKey, value string) SpanContext - - // Gets the value for a baggage item given its key. Returns the empty string - // if the value isn't found in this SpanContext. - // - // See the `SetBaggageItem` notes about `restrictedKey`. - BaggageItem(restrictedKey string) string - // ForeachBaggageItem grants access to all baggage items stored in the // SpanContext. // The handler function will be called for each baggage key/value pair. @@ -90,6 +66,28 @@ type Span interface { // See LogData for semantic details. Log(data LogData) + // SetBaggageItem sets a key:value pair on this Span and its SpanContext + // that also propagates to descendants of this Span. + // + // SetBaggageItem() enables powerful functionality given a full-stack + // opentracing integration (e.g., arbitrary application data from a mobile + // app can make it, transparently, all the way into the depths of a storage + // system), and with it some powerful costs: use this feature with care. + // + // IMPORTANT NOTE #1: SetBaggageItem() will only propagate baggage items to + // *future* causal descendants of the associated Span. + // + // IMPORTANT NOTE #2: Use this thoughtfully and with care. Every key and + // value is copied into every local *and remote* child of the associated + // Span, and that can add up to a lot of network and cpu overhead. + // + // Returns a reference to this Span for chaining. + SetBaggageItem(restrictedKey, value string) Span + + // Gets the value for a baggage item given its key. Returns the empty string + // if the value isn't found in this Span. + BaggageItem(restrictedKey string) string + // Provides access to the Tracer that created this Span. Tracer() Tracer } diff --git a/testtracer_test.go b/testtracer_test.go index ce9e92f..cf0b0b0 100644 --- a/testtracer_test.go +++ b/testtracer_test.go @@ -23,8 +23,6 @@ type testSpanContext struct { FakeID int } -func (n testSpanContext) SetBaggageItem(key, val string) SpanContext { return n } -func (n testSpanContext) BaggageItem(key string) string { return "" } func (n testSpanContext) ForeachBaggageItem(handler func(k, v string) bool) {} type testSpan struct { @@ -42,6 +40,8 @@ func (n testSpan) LogEventWithPayload(event string, payload interface{}) {} func (n testSpan) Log(data LogData) {} func (n testSpan) SetOperationName(operationName string) Span { return n } func (n testSpan) Tracer() Tracer { return testTracer{} } +func (n testSpan) SetBaggageItem(key, val string) Span { return n } +func (n testSpan) BaggageItem(key string) string { return "" } // StartSpan belongs to the Tracer interface. func (n testTracer) StartSpan(operationName string, opts ...StartSpanOption) Span {