Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Save baggage in span #153

Merged
merged 4 commits into from
May 24, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions jaeger_span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/opentracing/opentracing-go/ext"
"github.com/opentracing/opentracing-go/log"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

j "github.com/uber/jaeger-client-go/thrift-gen/jaeger"
"github.com/uber/jaeger-client-go/utils"
Expand Down Expand Up @@ -295,6 +296,32 @@ func TestBuildReferences(t *testing.T) {
assert.EqualValues(t, 2, spanRefs[1].TraceIdLow)
}

func TestJaegerSpanBaggageLogs(t *testing.T) {
tracer, closer := NewTracer("DOOP",
NewConstSampler(true),
NewNullReporter())
defer closer.Close()

sp := tracer.StartSpan("s1").(*Span)
sp.SetBaggageItem("auth.token", "token")
ext.SpanKindRPCServer.Set(sp)
sp.Finish()

jaegerSpan := buildJaegerSpan(sp)
require.Len(t, jaegerSpan.Logs, 1)
fields := jaegerSpan.Logs[0].Fields
require.Len(t, fields, 3)
assertJaegerTag(t, fields, "event", "baggage")
assertJaegerTag(t, fields, "key", "auth.token")
assertJaegerTag(t, fields, "value", "token")
}

func assertJaegerTag(t *testing.T, tags []*j.Tag, key string, value string) {
tag := findJaegerTag(key, tags)
require.NotNil(t, tag)
assert.Equal(t, value, tag.GetVStr())
}

func getStringPtr(s string) *string {
return &s
}
Expand Down
2 changes: 1 addition & 1 deletion propagation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestSpanPropagator(t *testing.T) {
assert.Equal(t, exp.context, sp.context, formatName)
assert.Equal(t, "span.kind", sp.tags[0].key)
assert.Equal(t, expTags, sp.tags[1:] /*skip span.kind tag*/, formatName)
assert.Equal(t, exp.logs, sp.logs, formatName)
assert.NotEqual(t, exp.logs, sp.logs, formatName) // Only the parent span should have baggage logs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not equal is a very weak test, it may succeed for other reasons

// Override collections to avoid tripping comparison on different pointers
sp.context = exp.context
sp.tags = exp.tags
Expand Down
16 changes: 15 additions & 1 deletion span.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ type Span struct {
observer SpanObserver
}

// Tag a simple key value wrapper
// Tag is a simple key value wrapper
type Tag struct {
key string
value interface{}
Expand Down Expand Up @@ -116,6 +116,11 @@ func (s *Span) LogFields(fields ...log.Field) {
if !s.context.IsSampled() {
return
}
s.logFieldsNoLocking(fields...)
}

// this function should only be called while holding a Write lock
func (s *Span) logFieldsNoLocking(fields ...log.Field) {
lr := opentracing.LogRecord{
Fields: fields,
Timestamp: time.Now(),
Expand Down Expand Up @@ -173,6 +178,15 @@ func (s *Span) SetBaggageItem(key, value string) opentracing.Span {
s.Lock()
defer s.Unlock()
s.context = s.context.WithBaggageItem(key, value)
if !s.context.IsSampled() {
return s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this shortcut makes the assumption that the baggage log is always the last thing, which may not be the correct assumption in the future. It's better to include the log statement inside the if - it does not introduce significant nesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not completely following, if the span is not sampled, we shouldn't log the span so it should fall outside of the id stmt. Not sure what you mean by "the baggage log is always the last thing"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simply meant

If sampled then log baggage.

}
// If sampled, record the baggage in the span
s.logFieldsNoLocking(
log.String("event", "baggage"),
log.String("key", key),
log.String("value", value),
)
return s
}

Expand Down
21 changes: 19 additions & 2 deletions span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,40 @@ import (

"github.com/opentracing/opentracing-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestBaggageIterator(t *testing.T) {
tracer, closer := NewTracer("DOOP", NewConstSampler(true), NewNullReporter())
service := "DOOP"
tracer, closer := NewTracer(service, NewConstSampler(true), NewNullReporter())
defer closer.Close()

sp1 := tracer.StartSpan("s1").(*Span)
sp1.SetBaggageItem("Some_Key", "12345")
sp1.SetBaggageItem("Some-other-key", "42")
expectedBaggage := map[string]string{"some-key": "12345", "some-other-key": "42"}
assertBaggage(t, sp1, expectedBaggage)
assertBaggageRecords(t, sp1, expectedBaggage)

b := extractBaggage(sp1, false) // break out early
assert.Equal(t, 1, len(b), "only one baggage item should be extracted")

sp2 := tracer.StartSpan("s2", opentracing.ChildOf(sp1.Context()))
sp2 := tracer.StartSpan("s2", opentracing.ChildOf(sp1.Context())).(*Span)
assertBaggage(t, sp2, expectedBaggage) // child inherits the same baggage
require.Len(t, sp2.logs, 0) // child doesn't inherit the baggage logs
}

func assertBaggageRecords(t *testing.T, sp *Span, expected map[string]string) {
require.Len(t, sp.logs, len(expected))
for _, logRecord := range sp.logs {
require.Len(t, logRecord.Fields, 3)
require.Equal(t, "event:baggage", logRecord.Fields[0].String())
key := logRecord.Fields[1].Value().(string)
value := logRecord.Fields[2].Value().(string)

require.Contains(t, expected, key)
assert.Equal(t, expected[key], value)
}
}

func assertBaggage(t *testing.T, sp opentracing.Span, expected map[string]string) {
Expand Down
8 changes: 7 additions & 1 deletion testutils/mock_agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,14 @@ func TestMockAgentSpanServer(t *testing.T) {

err = client.EmitZipkinBatch(spans)
assert.NoError(t, err)
time.Sleep(5 * time.Millisecond)

for k := 0; k < 100; k++ {
time.Sleep(time.Millisecond)
spans = mockAgent.GetZipkinSpans()
if len(spans) == i {
break
}
}
spans = mockAgent.GetZipkinSpans()
require.Equal(t, i, len(spans))
for j := 0; j < i; j++ {
Expand Down
15 changes: 15 additions & 0 deletions thrift_span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,21 @@ func TestSpecialTags(t *testing.T) {
assert.NotNil(t, findAnnotation(thriftSpan, "ss"))
}

func TestBaggageLogs(t *testing.T) {
tracer, closer := NewTracer("DOOP",
NewConstSampler(true),
NewNullReporter())
defer closer.Close()

sp := tracer.StartSpan("s1").(*Span)
sp.SetBaggageItem("auth.token", "token")
ext.SpanKindRPCServer.Set(sp)
sp.Finish()

thriftSpan := buildThriftSpan(sp)
assert.NotNil(t, findAnnotation(thriftSpan, `{"event":"baggage","key":"auth.token","value":"token"}`))
}

func findAnnotation(span *zipkincore.Span, name string) *zipkincore.Annotation {
for _, a := range span.Annotations {
if a.Value == name {
Expand Down