Skip to content

Commit

Permalink
ddtrace/tracer: fixed propagator not updating the tracestate header a…
Browse files Browse the repository at this point in the history 
…fter changing sampling decision (#1676)
  • Loading branch information
@dianashevchenko
dianashevchenko committed Jan 19, 2023
1 parent e1d94a7 commit 2dd2f38
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 40 deletions.
4 changes: 4 additions & 0 deletions ddtrace/tracer/span.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,13 @@ func (s *span) SetUser(id string, opts ...UserMonitoringOption) {
delete(root.Meta, keyUserID)
idenc := base64.StdEncoding.EncodeToString([]byte(id))
trace.setPropagatingTag(keyPropagatedUserID, idenc)
s.context.updated = true
} else {
// Unset the propagated user ID so that a propagated user ID coming from upstream won't be propagated anymore.
trace.unsetPropagatingTag(keyPropagatedUserID)
if _, ok := trace.propagatingTags[keyPropagatedUserID]; ok {
s.context.updated = true
}
delete(root.Meta, keyPropagatedUserID)
// setMeta is used since the span is already locked
root.setMeta(keyUserID, id)
Expand Down
7 changes: 7 additions & 0 deletions ddtrace/tracer/spancontext.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ func newSpanContext(span *span, parent *spanContext) *spanContext {
}
// put span in context's trace
context.trace.push(span)
// setting context.updated to false here is necessary to distinguish
// between initializing properties of the span (priority)
// and updating them after extracting context through propagators
context.updated = false
return context
}

Expand Down Expand Up @@ -98,6 +102,9 @@ func (c *spanContext) setSamplingPriority(p int, sampler samplernames.SamplerNam
if c.trace == nil {
c.trace = newTrace()
}
if c.trace.priority != nil && *c.trace.priority != float64(p) {
c.updated = true
}
c.trace.setSamplingPriority(p, sampler)
}

Expand Down
116 changes: 76 additions & 40 deletions ddtrace/tracer/textmap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,6 @@ func TestEnvVars(t *testing.T) {
spanID uint64
priority int
origin string
updated bool
propagatingTags map[string]string
}{
{
Expand All @@ -1120,7 +1119,6 @@ func TestEnvVars(t *testing.T) {
spanID: 2459565876494606882,
priority: 2,
origin: "rum",
updated: true,
propagatingTags: map[string]string{
"_dd.p.usr.id": " baz64 ==",
"tracestate": "othervendor=t61rcWkgMzE,dd=s:2;o:rum;t.dm:-4;t.usr.id:baz64~~",
Expand All @@ -1129,16 +1127,14 @@ func TestEnvVars(t *testing.T) {
{
out: TextMapCarrier{
traceparentHeader: "00-00000000000000001111111111111111-2222222222222222-01",
tracestateHeader: "dd=s:1;o:rum;t.usr.id:baz64~~,othervendor=t61rcWkgMzE",
tracestateHeader: "dd=s:1;o:rum;t.usr.id:baz64~~",
},
traceID: 1229782938247303441,
spanID: 2459565876494606882,
priority: 1,
origin: "rum",
updated: true,
propagatingTags: map[string]string{
"_dd.p.usr.id": "baz64==",
"tracestate": "dd=s:2;o:rum;t.usr.id:baz64~~,othervendor=t61rcWkgMzE",
},
},
{
Expand All @@ -1150,7 +1146,6 @@ func TestEnvVars(t *testing.T) {
spanID: 2459565876494606882,
priority: 2, // tracestate priority takes precedence
origin: "rum:rum",
updated: true,
propagatingTags: map[string]string{
"_dd.p.usr.id": "baz64==",
"tracestate": "dd=s:2;o:rum_rum;t.usr.id:baz64~~,othervendor=t61rcWkgMzE",
Expand All @@ -1164,7 +1159,6 @@ func TestEnvVars(t *testing.T) {
},
traceID: 1229782938247303441,
spanID: 2459565876494606882,
updated: true,
priority: 1, // traceparent priority takes precedence
origin: "rum:rum",
propagatingTags: map[string]string{
Expand All @@ -1179,36 +1173,20 @@ func TestEnvVars(t *testing.T) {
},
traceID: 1229782938247303441,
spanID: 2459565876494606882,
updated: true,
priority: -1, // traceparent priority takes precedence
origin: "rum:rum",
propagatingTags: map[string]string{
"_dd.p.usr.id": "baz:64==",
"tracestate": "dd=s:1;o:rum:rum;t.usr.id:baz64~~,othervendor=t61rcWkgMzE",
},
},
{
out: TextMapCarrier{
traceparentHeader: "00-00000000000000001111111111111111-2222222222222222-00",
tracestateHeader: "dd=s:1;o:old_rum;t.usr.id:baz64~~,oldvendor=t61rcWkgMzE",
},
traceID: 1229782938247303441,
spanID: 2459565876494606882,
updated: false,
origin: "old_tracestate",
propagatingTags: map[string]string{
"_dd.p.usr.id": "baz:64==",
"tracestate": "dd=s:1;o:old_rum;t.usr.id:baz64~~,oldvendor=t61rcWkgMzE",
},
},
{
out: TextMapCarrier{
traceparentHeader: "00-00000000000000001111111111111112-2222222222222222-00",
tracestateHeader: "dd=s:0;o:old_tracestate;t.usr.id:baz:64~~ ,a0=a:1,a1=a:1,a2=a:1,a3=a:1,a4=a:1,a5=a:1,a6=a:1,a7=a:1,a8=a:1,a9=a:1,a10=a:1,a11=a:1,a12=a:1,a13=a:1,a14=a:1,a15=a:1,a16=a:1,a17=a:1,a18=a:1,a19=a:1,a20=a:1,a21=a:1,a22=a:1,a23=a:1,a24=a:1,a25=a:1,a26=a:1,a27=a:1,a28=a:1,a29=a:1,a30=a:1",
},
traceID: 1229782938247303442,
spanID: 2459565876494606882,
updated: true,
origin: "old_tracestate",
propagatingTags: map[string]string{
"_dd.p.usr.id": "baz:64== ",
Expand All @@ -1222,7 +1200,6 @@ func TestEnvVars(t *testing.T) {
},
traceID: 1229782938247303442,
spanID: 2459565876494606882,
updated: true,
origin: "old_tracestate",
propagatingTags: map[string]string{
"_dd.p.usr.id": "baz:64==",
Expand All @@ -1236,7 +1213,6 @@ func TestEnvVars(t *testing.T) {
},
traceID: 1229782938247303442,
spanID: 2459565876494606882,
updated: true,
origin: "old_tracestate",
propagatingTags: map[string]string{
"_dd.p.usr.id": "baz:64==",
Expand All @@ -1250,7 +1226,6 @@ func TestEnvVars(t *testing.T) {
},
traceID: 1229782938247303442,
spanID: 2459565876494606882,
updated: true,
origin: "old_tracestate",
propagatingTags: map[string]string{
"_dd.p.usr.id": "baz:64~~",
Expand All @@ -1268,9 +1243,7 @@ func TestEnvVars(t *testing.T) {
ctx, ok := root.Context().(*spanContext)
ctx.origin = test.origin
ctx.traceID = test.traceID
ctx.updated = test.updated
ctx.spanID = test.spanID
ctx.updated = test.updated
ctx.trace.propagatingTags = test.propagatingTags
headers := TextMapCarrier(map[string]string{})
err := tracer.Inject(ctx, headers)
Expand All @@ -1292,7 +1265,6 @@ func TestEnvVars(t *testing.T) {
ctx, ok := root.Context().(*spanContext)
ctx.origin = "old_tracestate"
ctx.traceID = 1229782938247303442
ctx.updated = true
ctx.spanID = 2459565876494606882
ctx.trace.propagatingTags = map[string]string{
"tracestate": "valid_vendor=a:1",
Expand Down Expand Up @@ -1396,38 +1368,35 @@ func TestEnvVars(t *testing.T) {
out TextMapCarrier
traceID uint64
spanID uint64
priority int
priority float64
origin string
updated bool
}{
{
in: TextMapCarrier{
traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01", //invalid length
tracestateHeader: "dd=s:1;o:rum;t.usr.id:baz64~~",
traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01",
tracestateHeader: "dd=s:2;o:rum;t.usr.id:baz64~~",
},
out: TextMapCarrier{
traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01", //invalid length
tracestateHeader: "dd=s:1;o:rum;t.usr.id:baz64~~",
traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01",
tracestateHeader: "dd=s:2;o:rum;t.usr.id:baz64~~",
},
traceID: 8687463697196027922,
spanID: 1311768467284833366,
priority: 1,
priority: 2,
origin: "rum",
updated: true,
},
{
in: TextMapCarrier{
traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01", //invalid length
traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01",
tracestateHeader: "foo=1",
},
out: TextMapCarrier{
traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01", //invalid length
traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01",
tracestateHeader: "dd=s:1,foo=1",
},
traceID: 8687463697196027922,
spanID: 1311768467284833366,
priority: 1,
updated: true,
},
}
for i, test := range tests {
Expand All @@ -1445,6 +1414,7 @@ func TestEnvVars(t *testing.T) {
assert.Equal(test.traceID, sctx.traceID)
assert.Equal(test.spanID, sctx.spanID)
assert.Equal(test.origin, sctx.origin)
assert.Equal(test.priority, *sctx.trace.priority)

headers := TextMapCarrier(map[string]string{})
err = tracer.Inject(ctx, headers)
Expand All @@ -1458,6 +1428,72 @@ func TestEnvVars(t *testing.T) {
}
}
})

t.Run("w3c extract,update span, inject", func(t *testing.T) {
testEnvs = []map[string]string{
{headerPropagationStyleInject: "tracecontext", headerPropagationStyleExtract: "tracecontext"},
{headerPropagationStyleInject: "datadog,tracecontext", headerPropagationStyleExtract: "datadog,tracecontext"},
{headerPropagationStyleInjectDeprecated: "tracecontext", headerPropagationStyleExtractDeprecated: "tracecontext"},
}
for _, testEnv := range testEnvs {
for k, v := range testEnv {
t.Setenv(k, v)
}
var tests = []struct {
in TextMapCarrier
out TextMapCarrier
traceID uint64
spanID uint64
parentID uint64
priority float64
origin string
}{
{
in: TextMapCarrier{
traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01",
tracestateHeader: "dd=s:2;o:rum;t.usr.id:baz64~~",
},
out: TextMapCarrier{
traceparentHeader: "00-12345678901234567890123456789012-0000000000000001-01",
tracestateHeader: "dd=s:1;o:rum;t.usr.id:baz64~~",
},
traceID: 8687463697196027922,
spanID: 1,
parentID: 1311768467284833366,
priority: 1,
},
}
for i, test := range tests {
t.Run(fmt.Sprintf("#%d w3c inject/extract with env=%q", i, testEnv), func(t *testing.T) {
tracer := newTracer(WithHTTPClient(c), withStatsdClient(&statsd.NoOpClient{}))
defer tracer.Stop()
assert := assert.New(t)
pCtx, err := tracer.Extract(test.in)
if err != nil {
t.FailNow()
}
s := tracer.StartSpan("op", ChildOf(pCtx), WithSpanID(1))
sctx, ok := s.Context().(*spanContext)
assert.True(ok)
// changing priority must set ctx.updated = true
if test.priority != 0 {
sctx.setSamplingPriority(int(test.priority), samplernames.Unknown)
}
assert.Equal(true, sctx.updated)

headers := TextMapCarrier(map[string]string{})
err = tracer.Inject(s.Context(), headers)
assert.Equal(test.traceID, sctx.traceID)
assert.Equal(test.parentID, sctx.span.ParentID)
assert.Equal(test.spanID, sctx.spanID)
assert.Equal(test.out[traceparentHeader], headers[traceparentHeader])
assert.Equal(test.out[tracestateHeader], headers[tracestateHeader])
ddTag := strings.SplitN(headers[tracestateHeader], ",", 2)[0]
assert.LessOrEqual(len(ddTag), 256)
})
}
}
})
}

func TestNonePropagator(t *testing.T) {
Expand Down

0 comments on commit 2dd2f38

Please sign in to comment.