From 8145bd01c40a7da774d89a91cb51759a8af6d283 Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Fri, 2 Feb 2024 12:22:59 -0500 Subject: [PATCH 1/2] opentelemetry: span links refactor --- ddtrace/opentelemetry/span_test.go | 39 ++++++++++++--------------- ddtrace/opentelemetry/tracer.go | 43 +++++++++++++----------------- 2 files changed, 36 insertions(+), 46 deletions(-) diff --git a/ddtrace/opentelemetry/span_test.go b/ddtrace/opentelemetry/span_test.go index 221bce8be6..6fd1531321 100644 --- a/ddtrace/opentelemetry/span_test.go +++ b/ddtrace/opentelemetry/span_test.go @@ -135,59 +135,54 @@ func TestSpanSetName(t *testing.T) { func TestSpanLink(t *testing.T) { assert := assert.New(t) - - _, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + _, payloads, cleanup := mockTracerProvider(t) + tr := otel.Tracer("") + defer cleanup() // Use traceID, spanID, and traceflags that can be unmarshalled from unint64 to float64 without loss of precision traceID, _ := oteltrace.TraceIDFromHex("00000000000001c8000000000000007b") spanID, _ := oteltrace.SpanIDFromHex("000000000000000f") - traceState, _ := oteltrace.ParseTraceState("dd_origin=ci") + traceStateVal := "dd_origin=ci" + traceState, _ := oteltrace.ParseTraceState(traceStateVal) remoteSpanContext := oteltrace.NewSpanContext( oteltrace.SpanContextConfig{ TraceID: traceID, SpanID: spanID, - TraceFlags: 0x0, + TraceFlags: oteltrace.FlagsSampled, TraceState: traceState, Remote: true, }, ) - _, payloads, cleanup := mockTracerProvider(t) - tr := otel.Tracer("") - defer cleanup() - // Create a span with a link to a remote span - _, decoratedSpan := tr.Start(context.Background(), "span_with_link", + _, span := tr.Start(context.Background(), "span_with_link", oteltrace.WithLinks(oteltrace.Link{ SpanContext: remoteSpanContext, Attributes: []attribute.KeyValue{attribute.String("link.name", "alpha_transaction")}, })) - decoratedSpan.End() + span.End() - // Flush span with link and unmarshal the payload to a map tracer.Flush() payload, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } assert.NotNil(payload) - // Ensure payload contains one trace and one span - assert.Len(payload[0], 1) + assert.Len(payload, 1) // only one trace + assert.Len(payload[0], 1) // only one span - // Convert the span_links field from type []map[string]interface{} to a struct var spanLinks []ddtrace.SpanLink spanLinkBytes, _ := json.Marshal(payload[0][0]["span_links"]) json.Unmarshal(spanLinkBytes, &spanLinks) - assert.Len(spanLinks, 1) + assert.Len(spanLinks, 1) // only one span link // Ensure the span link has the correct values - assert.Equal(spanLinks[0].TraceID, uint64(123)) - assert.Equal(spanLinks[0].TraceIDHigh, uint64(456)) - assert.Equal(spanLinks[0].SpanID, uint64(15)) - assert.Equal(spanLinks[0].Attributes, map[string]string{"link.name": "alpha_transaction"}) - assert.Equal(spanLinks[0].Tracestate, "dd_origin=ci") - assert.Equal(spanLinks[0].Flags, uint32(0x80000000)) + assert.Equal(uint64(123), spanLinks[0].TraceID) + assert.Equal(uint64(456), spanLinks[0].TraceIDHigh) + assert.Equal(uint64(15), spanLinks[0].SpanID) + assert.Equal(map[string]string{"link.name": "alpha_transaction"}, spanLinks[0].Attributes) + assert.Equal(traceStateVal, spanLinks[0].Tracestate) + assert.Equal(uint32(oteltrace.FlagsSampled), spanLinks[0].Flags) } func TestSpanEnd(t *testing.T) { diff --git a/ddtrace/opentelemetry/tracer.go b/ddtrace/opentelemetry/tracer.go index bad2194409..9450d9b326 100644 --- a/ddtrace/opentelemetry/tracer.go +++ b/ddtrace/opentelemetry/tracer.go @@ -62,33 +62,23 @@ func (t *oteltracer) Start(ctx context.Context, spanName string, opts ...oteltra o(&cfg) } } - // Add span links to underlying Datadog span + // Add provide OTel Span Links to the underlying Datadog span. if len(ssConfig.Links()) > 0 { links := make([]ddtrace.SpanLink, 0, len(ssConfig.Links())) - for _, otelLink := range ssConfig.Links() { - var link ddtrace.SpanLink - - traceIDbytes := otelLink.SpanContext.TraceID() - link.TraceIDHigh = binary.BigEndian.Uint64(traceIDbytes[:8]) - link.TraceID = binary.BigEndian.Uint64(traceIDbytes[8:]) - - spanIDbytes := otelLink.SpanContext.SpanID() - link.SpanID = binary.BigEndian.Uint64(spanIDbytes[:]) - - link.Tracestate = otelLink.SpanContext.TraceState().String() - - if otelLink.SpanContext.IsSampled() { - link.Flags = uint32(0x80000001) - } else { - link.Flags = uint32(0x80000000) - } - - link.Attributes = make(map[string]string) - for _, attr := range otelLink.Attributes { - link.Attributes[string(attr.Key)] = attr.Value.Emit() + for _, link := range ssConfig.Links() { + attrs := make(map[string]string, len(link.Attributes)) + for _, attr := range link.Attributes { + attrs[string(attr.Key)] = attr.Value.Emit() } - - links = append(links, link) + ctx := otelCtxToDDCtx{link.SpanContext} + links = append(links, ddtrace.SpanLink{ + TraceID: ctx.TraceID(), + TraceIDHigh: ctx.TraceIDUpper(), + SpanID: ctx.SpanID(), + Attributes: attrs, + Tracestate: link.SpanContext.TraceState().String(), + Flags: uint32(link.SpanContext.TraceFlags()), + }) } ddopts = append(ddopts, tracer.WithSpanLinks(links)) } @@ -118,6 +108,11 @@ func (c *otelCtxToDDCtx) TraceID() uint64 { return binary.BigEndian.Uint64(id[8:]) } +func (c *otelCtxToDDCtx) TraceIDUpper() uint64 { + id := c.oc.TraceID() + return binary.BigEndian.Uint64(id[:8]) +} + func (c *otelCtxToDDCtx) SpanID() uint64 { id := c.oc.SpanID() return binary.BigEndian.Uint64(id[:]) From 19cfab9677e85a01029f6593a5b629f9c3ff2073 Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Fri, 2 Feb 2024 12:57:42 -0500 Subject: [PATCH 2/2] clarify trace flags --- ddtrace/opentelemetry/span_test.go | 2 +- ddtrace/opentelemetry/tracer.go | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ddtrace/opentelemetry/span_test.go b/ddtrace/opentelemetry/span_test.go index 6fd1531321..66fbb63e5e 100644 --- a/ddtrace/opentelemetry/span_test.go +++ b/ddtrace/opentelemetry/span_test.go @@ -182,7 +182,7 @@ func TestSpanLink(t *testing.T) { assert.Equal(uint64(15), spanLinks[0].SpanID) assert.Equal(map[string]string{"link.name": "alpha_transaction"}, spanLinks[0].Attributes) assert.Equal(traceStateVal, spanLinks[0].Tracestate) - assert.Equal(uint32(oteltrace.FlagsSampled), spanLinks[0].Flags) + assert.Equal(uint32(0x80000001), spanLinks[0].Flags) // sampled and set } func TestSpanEnd(t *testing.T) { diff --git a/ddtrace/opentelemetry/tracer.go b/ddtrace/opentelemetry/tracer.go index 9450d9b326..42df8e2b6f 100644 --- a/ddtrace/opentelemetry/tracer.go +++ b/ddtrace/opentelemetry/tracer.go @@ -66,18 +66,21 @@ func (t *oteltracer) Start(ctx context.Context, spanName string, opts ...oteltra if len(ssConfig.Links()) > 0 { links := make([]ddtrace.SpanLink, 0, len(ssConfig.Links())) for _, link := range ssConfig.Links() { + ctx := otelCtxToDDCtx{link.SpanContext} attrs := make(map[string]string, len(link.Attributes)) for _, attr := range link.Attributes { attrs[string(attr.Key)] = attr.Value.Emit() } - ctx := otelCtxToDDCtx{link.SpanContext} links = append(links, ddtrace.SpanLink{ TraceID: ctx.TraceID(), TraceIDHigh: ctx.TraceIDUpper(), SpanID: ctx.SpanID(), - Attributes: attrs, Tracestate: link.SpanContext.TraceState().String(), - Flags: uint32(link.SpanContext.TraceFlags()), + Attributes: attrs, + // To distinguish between "not sampled" and "not set", Datadog + // will rely on the highest bit being set. The OTel API doesn't + // differentiate this, so we will just always mark it as set. + Flags: uint32(link.SpanContext.TraceFlags()) | (1 << 31), }) } ddopts = append(ddopts, tracer.WithSpanLinks(links))