Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Span Link support for compound header tag extractions with conflicting trace IDs #2948

Merged
merged 32 commits into from
Dec 9, 2024
Merged
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
17fef62
initial code for adding spanlinks for span context
mhlidd Oct 24, 2024
6b1593e
adding span links in startspan
mhlidd Oct 24, 2024
6c2b25c
printing flags
mhlidd Oct 25, 2024
eec17a2
editing flags
mhlidd Oct 25, 2024
92b6415
omitting empty spanlinks in spans
mhlidd Oct 25, 2024
fba3190
changing lclctx to localCtx
mhlidd Oct 29, 2024
452d4d7
creating function to create startSpanOption and update config links f…
mhlidd Oct 30, 2024
dc6c762
updating WithExtractedSpanLinks
mhlidd Oct 31, 2024
cab4ff2
adding copyright
mhlidd Nov 4, 2024
6968211
removing stats files
mhlidd Nov 4, 2024
06f893e
removing unwanted files on remote
mhlidd Nov 4, 2024
3b3968f
addressing PR comments
mhlidd Nov 6, 2024
3487509
refactoring extract and instrumentations
mhlidd Nov 7, 2024
7701611
updating PR comments and unit tests
mhlidd Nov 8, 2024
538d25a
logging
mhlidd Nov 11, 2024
cdc6f40
updating PR comments
mhlidd Nov 19, 2024
279246e
Update ddtrace/tracer/textmap.go
mtoffl01 Nov 20, 2024
14a69f2
Restructure Extract method and clean up tests for idiomiatc Golang TDD
mtoffl01 Nov 20, 2024
6936c46
cleanup overrideDatadogParentID case
mtoffl01 Nov 20, 2024
c7eb900
revert recent changes to overridedatadogparentid case
mtoffl01 Nov 20, 2024
44ff629
updating code to correctly add span links as start span option
mhlidd Nov 20, 2024
990a98a
Make spanLink.context_headers more dynamic and refactor tests
mtoffl01 Nov 21, 2024
1574d11
small change to propagator names for cross-library consistency
mhlidd Nov 21, 2024
7584ed7
updating test
mhlidd Nov 22, 2024
2847f4c
adding valid SpanID check to make consistent with other libraries
mhlidd Nov 22, 2024
a1fce54
Update ddtrace/tracer/textmap.go
mtoffl01 Nov 22, 2024
59e1504
Update ddtrace/tracer/textmap.go
mtoffl01 Nov 22, 2024
036efda
nits: clean up comments and simplify code
mtoffl01 Nov 25, 2024
cf1c426
reverting spanid check to be consistent and keep support for synthetics
mhlidd Dec 5, 2024
ec903d8
Merge branch 'main' into mhlidd/tracecontext_span_links
mhlidd Dec 5, 2024
08f578a
Merge branch 'main' into mhlidd/tracecontext_span_links
darccio Dec 9, 2024
cb7141d
Merge branch 'main' into mhlidd/tracecontext_span_links
mtoffl01 Dec 9, 2024
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
Prev Previous commit
Next Next commit
updating PR comments and unit tests
  • Loading branch information
mhlidd committed Nov 18, 2024
commit 77016110bd0133e08b50436480c91cb05b03022d
8 changes: 6 additions & 2 deletions contrib/IBM/sarama.v1/sarama.go
Original file line number Diff line number Diff line change
@@ -74,7 +74,9 @@ func WrapPartitionConsumer(pc sarama.PartitionConsumer, opts ...Option) sarama.P
// kafka supports headers, so try to extract a span context
carrier := NewConsumerMessageCarrier(msg)
if spanctx, err := tracer.Extract(carrier); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
@@ -302,7 +304,9 @@ func startProducerSpan(cfg *config, version sarama.KafkaVersion, msg *sarama.Pro
}
// if there's a span context in the headers, use that as the parent
if spanctx, err := tracer.Extract(carrier); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
8 changes: 6 additions & 2 deletions contrib/Shopify/sarama/sarama.go
Original file line number Diff line number Diff line change
@@ -77,7 +77,9 @@ func WrapPartitionConsumer(pc sarama.PartitionConsumer, opts ...Option) sarama.P
// kafka supports headers, so try to extract a span context
carrier := NewConsumerMessageCarrier(msg)
if spanctx, err := tracer.Extract(carrier); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
@@ -305,7 +307,9 @@ func startProducerSpan(cfg *config, version sarama.KafkaVersion, msg *sarama.Pro
}
// if there's a span context in the headers, use that as the parent
if spanctx, err := tracer.Extract(carrier); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
Original file line number Diff line number Diff line change
@@ -118,7 +118,9 @@ func TraceReceiveFunc(s Subscription, opts ...Option) func(ctx context.Context,
if cfg.measured {
opts = append(opts, tracer.Measured())
}
if linksCtx, err := parentSpanCtx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := parentSpanCtx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
Original file line number Diff line number Diff line change
@@ -77,7 +77,9 @@ func (tr *KafkaTracer) StartConsumeSpan(msg Message) ddtrace.Span {
// kafka supports headers, so try to extract a span context
carrier := MessageCarrier{msg: msg}
if spanctx, err := tracer.Extract(carrier); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
Original file line number Diff line number Diff line change
@@ -67,7 +67,9 @@ func (tr *KafkaTracer) StartProduceSpan(msg Message) ddtrace.Span {
// if there's a span context in the headers, use that as the parent
carrier := NewMessageCarrier(msg)
if spanctx, err := tracer.Extract(carrier); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
4 changes: 3 additions & 1 deletion contrib/gofiber/fiber.v2/fiber.go
Original file line number Diff line number Diff line change
@@ -62,7 +62,9 @@ func Middleware(opts ...Option) func(c *fiber.Ctx) error {
}
}
if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(h)); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
4 changes: 3 additions & 1 deletion contrib/google.golang.org/grpc/grpc.go
Original file line number Diff line number Diff line change
@@ -71,7 +71,9 @@ func startSpanFromContext(
)
md, _ := metadata.FromIncomingContext(ctx) // nil is ok
if sctx, err := tracer.Extract(grpcutil.MDCarrier(md)); err == nil {
if linksCtx, err := sctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := sctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
4 changes: 3 additions & 1 deletion contrib/internal/httptrace/httptrace.go
Original file line number Diff line number Diff line change
@@ -51,7 +51,9 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer.
cfg.Tags["http.host"] = r.Host
}
if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
4 changes: 3 additions & 1 deletion contrib/segmentio/kafka.go.v0/internal/tracing/tracing.go
Original file line number Diff line number Diff line change
@@ -49,7 +49,9 @@ func (tr *Tracer) StartConsumeSpan(ctx context.Context, msg Message) ddtrace.Spa
// kafka supports headers, so try to extract a span context
carrier := NewMessageCarrier(msg)
if spanctx, err := tracer.Extract(carrier); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
8 changes: 6 additions & 2 deletions contrib/twitchtv/twirp/twirp.go
Original file line number Diff line number Diff line change
@@ -91,7 +91,9 @@ func (wc *wrappedClient) Do(req *http.Request) (*http.Response, error) {
opts = append(opts, tracer.Tag(ext.EventSampleRate, wc.cfg.analyticsRate))
}
if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(req.Header)); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
opts = append(opts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
@@ -144,7 +146,9 @@ func WrapServer(h http.Handler, opts ...Option) http.Handler {
spanOpts = append(spanOpts, tracer.Tag(ext.EventSampleRate, cfg.analyticsRate))
}
if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)); err == nil {
if linksCtx, err := spanctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := spanctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
spanOpts = append(spanOpts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
4 changes: 3 additions & 1 deletion contrib/valyala/fasthttp.v1/fasthttp.go
Original file line number Diff line number Diff line change
@@ -46,7 +46,9 @@ func WrapHandler(h fasthttp.RequestHandler, opts ...Option) fasthttp.RequestHand
ReqHeader: &fctx.Request.Header,
}
if sctx, err := tracer.Extract(fcc); err == nil {
if linksCtx, err := sctx.(ddtrace.SpanContextWithLinks); err && linksCtx.SpanLinks() != nil {
// If there are span links as a result of context extraction, add them as a StartSpanOption
// and remove from the extracted context as they belong to the span being created, not the parent span
if linksCtx, ok := sctx.(ddtrace.SpanContextWithLinks); ok && linksCtx.SpanLinks() != nil {
spanOpts = append(spanOpts, tracer.WithSpanLinks(linksCtx.SpanLinks()))
linksCtx.SetLinks(nil)
}
8 changes: 4 additions & 4 deletions ddtrace/tracer/textmap.go
Original file line number Diff line number Diff line change
@@ -270,13 +270,13 @@ func (p *chainedPropagator) Inject(spanCtx ddtrace.SpanContext, carrier interfac
// a previous propagator has already succeeded so long as the trace-ids match.
// Furthermore, if we have already successfully extracted a trace context and a
// subsequent trace context has conflicting trace information, such information will
// be relayed in the original trace context with a SpanLink.
// be relayed in the returned SpanContext with a SpanLink.
func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, error) {
var ctx ddtrace.SpanContext // First valid trace context will be stored here
var links []ddtrace.SpanLink
for _, v := range p.extractors {
extractedCtx, err := v.Extract(carrier)
// Handling extraction errors. If it is the first extraction, return error. Else, ignore
// Handling extraction errors. If it is the first extraction, distributed tracing breaks and return error. Else, ignore
if err != nil {
if ctx == nil && err != ErrSpanContextNotFound {
return nil, err
@@ -287,7 +287,7 @@ func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, e
if ctx != nil { // A local trace context has already been extracted
propagatorType, err := getPropagatorType(v)
if err != nil { // Propagator is not one of the known, supported types. Ignore this extraction
log.Debug("Propagator type is unknown. Skipping extraction")
log.Debug("Trace propagator type %T is unknown. Skipping trace extraction", v)
continue
}
if extractedCtx.(ddtrace.SpanContextW3C).TraceID128() == ctx.(ddtrace.SpanContextW3C).TraceID128() {
@@ -303,7 +303,7 @@ func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, e
overrideDatadogParentID(ctx.(*spanContext), extractedCtx.(*spanContext), ddCtx)
}
}
} else {
} else { //handle non-w3c propagators
var flags uint32
if tracer := extractedCtx.(*spanContext).trace; tracer != nil {
if flags = uint32(*tracer.priority); flags > 0 { // Set the flags based on the sampling priority
76 changes: 48 additions & 28 deletions ddtrace/tracer/textmap_test.go
Original file line number Diff line number Diff line change
@@ -1972,63 +1972,83 @@ func TestInvalidTraceSpanLinkCreation(t *testing.T) {
for k, v := range testEnv {
t.Setenv(k, v)
}
var test = []struct {
var test = struct {
in TextMapCarrier
out []ddtrace.SpanLink
tid []traceID
}{
{
in: TextMapCarrier{
DefaultTraceIDHeader: "1",
DefaultParentIDHeader: "1",
DefaultPriorityHeader: "3",
traceparentHeader: "00-00000000000000000000000000000002-0000000000000002-01",
tracestateHeader: "dd=s:1;o:rum;t.usr.id:baz64~~",
},
out: []ddtrace.SpanLink{{TraceID: 2, TraceIDHigh: 0, SpanID: 2, Tracestate: "dd=s:1;o:rum;t.usr.id:baz64~~", Flags: 1, Attributes: map[string]string{"reason": "terminated_context", "context_headers": "tracecontext"}}, {TraceID: 1, TraceIDHigh: 0, SpanID: 1, Flags: 1, Attributes: map[string]string{"reason": "terminated_context", "context_headers": "datadog"}}},
tid: []traceID{traceIDFrom64Bits(1), traceIDFrom64Bits(2)},
},
{
in: TextMapCarrier{
DefaultTraceIDHeader: "1",
DefaultParentIDHeader: "1",
DefaultPriorityHeader: "3",
traceparentHeader: "00-00000000000000000000000000000001-0000000000000002-01",
tracestateHeader: "dd=s:1;o:rum;t.usr.id:baz64~~",
},
out: nil,
tid: []traceID{traceIDFrom64Bits(1), traceIDFrom64Bits(1)},
in: TextMapCarrier{
DefaultTraceIDHeader: "1",
DefaultParentIDHeader: "1",
DefaultPriorityHeader: "3",
traceparentHeader: "00-00000000000000000000000000000002-0000000000000002-01",
tracestateHeader: "dd=s:1;o:rum;t.usr.id:baz64~~",
},
out: []ddtrace.SpanLink{{TraceID: 2, TraceIDHigh: 0, SpanID: 2, Tracestate: "dd=s:1;o:rum;t.usr.id:baz64~~", Flags: 1, Attributes: map[string]string{"reason": "terminated_context", "context_headers": "tracecontext"}}, {TraceID: 1, TraceIDHigh: 0, SpanID: 1, Flags: 1, Attributes: map[string]string{"reason": "terminated_context", "context_headers": "datadog"}}},
tid: []traceID{traceIDFrom64Bits(1), traceIDFrom64Bits(2)},
}
t.Run("InvalidTraceSpanLinkCreation", func(t *testing.T) {
tracer := newTracer(WithHTTPClient(c))
defer tracer.Stop()
assert := assert.New(t)
ctx, err := tracer.Extract(test[0].in)
ctx, err := tracer.Extract(test.in)
if err != nil {
t.Fatal(err)
}
sctx, ok := ctx.(*spanContext)
assert.True(ok)

assert.Equal(test[0].tid[i], sctx.traceID)
assert.Equal(test[0].out[i], sctx.spanLinks[0])
assert.Equal(test.tid[i], sctx.traceID)
assert.Equal(test.out[i], sctx.spanLinks[0])
assert.True(ok)
})
}
}

func TestValidTraceSpanLinkCreation(t *testing.T) {
var testEnvs []map[string]string
s, c := httpmem.ServerAndClient(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(404)
}))
defer s.Close()
testEnvs = []map[string]string{
{headerPropagationStyleExtract: "datadog,tracecontext"},
{headerPropagationStyleExtract: "tracecontext,datadog"},
}

for i, testEnv := range testEnvs {
for k, v := range testEnv {
t.Setenv(k, v)
}
var test = struct {
in TextMapCarrier
out []ddtrace.SpanLink
tid []traceID
}{
in: TextMapCarrier{
DefaultTraceIDHeader: "1",
DefaultParentIDHeader: "1",
DefaultPriorityHeader: "3",
traceparentHeader: "00-00000000000000000000000000000001-0000000000000002-01",
tracestateHeader: "dd=s:1;o:rum;t.usr.id:baz64~~",
},
out: nil,
tid: []traceID{traceIDFrom64Bits(1), traceIDFrom64Bits(1)},
}

t.Run("ValidTraceSpanLinkCreation", func(t *testing.T) {
tracer := newTracer(WithHTTPClient(c))
defer tracer.Stop()
assert := assert.New(t)
ctx, err := tracer.Extract(test[1].in)
ctx, err := tracer.Extract(test.in)
if err != nil {
t.Fatal(err)
}
sctx, ok := ctx.(*spanContext)
assert.True(ok)

assert.Equal(test[1].tid[i], sctx.traceID)
assert.Equal(test[1].out, sctx.spanLinks)
assert.Equal(test.tid[i], sctx.traceID)
assert.Equal(test.out, sctx.spanLinks)
assert.True(ok)
})
}