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

Span links improvements and bug fix #3123

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ variables:
INDEX_FILE: index.txt
KUBERNETES_SERVICE_ACCOUNT_OVERWRITE: dd-trace-go
FF_USE_LEGACY_KUBERNETES_EXECUTION_STRATEGY: "true"
BENCHMARK_TARGETS: "BenchmarkStartRequestSpan|BenchmarkHttpServeTrace|BenchmarkTracerAddSpans|BenchmarkStartSpan|BenchmarkSingleSpanRetention|BenchmarkOTelApiWithCustomTags|BenchmarkInjectW3C|BenchmarkExtractW3C|BenchmarkPartialFlushing|BenchmarkGraphQL|BenchmarkSampleWAFContext|BenchmarkCaptureStackTrace|BenchmarkSetTagString|BenchmarkSetTagStringPtr|BenchmarkSetTagMetric|BenchmarkSetTagStringer"
BENCHMARK_TARGETS: "BenchmarkStartRequestSpan|BenchmarkHttpServeTrace|BenchmarkTracerAddSpans|BenchmarkStartSpan|BenchmarkSingleSpanRetention|BenchmarkOTelApiWithCustomTags|BenchmarkInjectW3C|BenchmarkExtractW3C|BenchmarkPartialFlushing|BenchmarkGraphQL|BenchmarkSampleWAFContext|BenchmarkCaptureStackTrace|BenchmarkSetTagString|BenchmarkSetTagStringPtr|BenchmarkSetTagMetric|BenchmarkSetTagStringer|BenchmarkSerializeSpanLinksInMeta"

include:
- ".gitlab/benchmarks.yml"
Expand Down
19 changes: 0 additions & 19 deletions ddtrace/ddtrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,25 +103,6 @@ type SpanContext interface {
ForeachBaggageItem(handler func(k, v string) bool)
}

// SpanLink represents a reference to a span that exists outside of the trace.
//
//go:generate msgp -unexported -marshal=false -o=span_link_msgp.go -tests=false

type SpanLink struct {
// TraceID represents the low 64 bits of the linked span's trace id. This field is required.
TraceID uint64 `msg:"trace_id" json:"trace_id"`
// TraceIDHigh represents the high 64 bits of the linked span's trace id. This field is only set if the linked span's trace id is 128 bits.
TraceIDHigh uint64 `msg:"trace_id_high,omitempty" json:"trace_id_high"`
// SpanID represents the linked span's span id.
SpanID uint64 `msg:"span_id" json:"span_id"`
// Attributes is a mapping of keys to string values. These values are used to add additional context to the span link.
Attributes map[string]string `msg:"attributes,omitempty" json:"attributes"`
// Tracestate is the tracestate of the linked span. This field is optional.
Tracestate string `msg:"tracestate,omitempty" json:"tracestate"`
// Flags represents the W3C trace flags of the linked span. This field is optional.
Flags uint32 `msg:"flags,omitempty" json:"flags"`
}

// StartSpanOption is a configuration option that can be used with a Tracer's StartSpan method.
type StartSpanOption func(cfg *StartSpanConfig)

Expand Down
25 changes: 25 additions & 0 deletions ddtrace/span_link.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2016 Datadog, Inc.

package ddtrace

// SpanLink represents a reference to a span that exists outside of the trace.
//
//go:generate msgp -unexported -marshal=false -o=span_link_msgp.go -tests=false

type SpanLink struct {
// TraceID represents the low 64 bits of the linked span's trace id. This field is required.
TraceID uint64 `msg:"trace_id" json:"trace_id"`
// TraceIDHigh represents the high 64 bits of the linked span's trace id. This field is only set if the linked span's trace id is 128 bits.
TraceIDHigh uint64 `msg:"trace_id_high" json:"trace_id_high"`
// SpanID represents the linked span's span id.
SpanID uint64 `msg:"span_id" json:"span_id"`
// Attributes is a mapping of keys to string values. These values are used to add additional context to the span link.
Attributes map[string]string `msg:"attributes" json:"attributes"`
// Tracestate is the tracestate of the linked span. This field is optional.
Tracestate string `msg:"tracestate" json:"tracestate"`
// Flags represents the W3C trace flags of the linked span. This field is optional.
Flags uint32 `msg:"flags" json:"flags"`
}
120 changes: 43 additions & 77 deletions ddtrace/span_link_msgp.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions ddtrace/tracer/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ package tracer
import (
"context"
"encoding/base64"
"encoding/json"
"fmt"
"os"
"reflect"
Expand Down Expand Up @@ -91,6 +92,13 @@ type span struct {
taskEnd func() // ends execution tracer (runtime/trace) task, if started
}

type SpanWithLinks interface {
ddtrace.Span

// AddSpanLink appends the given link to span's span links.
AddSpanLink(link ddtrace.SpanLink)
}

// Context yields the SpanContext for this Span. Note that the return
// value of Context() is still valid after a call to Finish(). This is
// called the span context and it is different from Go's context.
Expand Down Expand Up @@ -464,6 +472,27 @@ func (s *span) setMetric(key string, v float64) {
}
}

// AddSpanLink appends the given link to the span's span links.
func (s *span) AddSpanLink(link ddtrace.SpanLink) {
s.SpanLinks = append(s.SpanLinks, link)
}

// serializeSpanLinksInMeta saves span links as a JSON string under `Span[meta][_dd.span_links]`.
func (s *span) serializeSpanLinksInMeta() {
if len(s.SpanLinks) == 0 {
return
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
}
spanLinkBytes, err := json.Marshal(s.SpanLinks)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A little concerned about performance, but not sure if there's any other way to do this. We have to serialize the links into a JSON string such as "[{\"trace_id\":0,\"trace_id_high\":0,\"span_id\":0,\"attributes\":{\"link.kind\":\"span-pointer\",\"ptr.dir\":\"d\",\"ptr.hash\":\"eb29cb7d923f904f02bd8b3d85e228ed\",\"ptr.kind\":\"aws.s3.object\"},\"tracestate\":\"\",\"flags\":0}]"

Copy link
Contributor

Choose a reason for hiding this comment

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

You could always add a benchmark for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually must faster than I expected

cpu: Apple M3 Max
BenchmarkSerializeSpanLinksInMeta
BenchmarkSerializeSpanLinksInMeta-16    	 1253884	       964.4 ns/op

if err != nil {
log.Debug("Unable to marshal span links. Not adding span links to span meta.")
return
mtoffl01 marked this conversation as resolved.
Show resolved Hide resolved
}
if s.Meta == nil {
s.Meta = make(map[string]string)
}
s.Meta["_dd.span_links"] = string(spanLinkBytes)
}

// Finish closes this Span (but not its children) providing the duration
// of its part of the tracing session.
func (s *span) Finish(opts ...ddtrace.FinishOption) {
Expand Down Expand Up @@ -514,6 +543,8 @@ func (s *span) Finish(opts ...ddtrace.FinishOption) {
}
}

s.serializeSpanLinksInMeta()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you state explicitly why you were not able to rely on json.Marshal and we have to call this custom serializer? You may be correct, I just want to make sure I'm following

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The serializeSpanLinksInMeta func does use json.Marshal; I just extracted the method for easier refactoring in the future. We could remove the serializeSpanLinksInMeta() function and put the logic directly in Finish(), wdyt?


s.finish(t)
orchestrion.GLSPopValue(sharedinternal.ActiveSpanKey)
}
Expand Down
63 changes: 63 additions & 0 deletions ddtrace/tracer/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
package tracer

import (
"encoding/json"
"errors"
"fmt"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"runtime"
"strings"
"sync"
Expand Down Expand Up @@ -1051,6 +1053,27 @@ func BenchmarkSetTagField(b *testing.B) {
}
}

func BenchmarkSerializeSpanLinksInMeta(b *testing.B) {
span := newBasicSpan("bench.span")

span.AddSpanLink(ddtrace.SpanLink{SpanID: 123, TraceID: 456})
span.AddSpanLink(ddtrace.SpanLink{SpanID: 789, TraceID: 101})

// Sample span pointer
attributes := map[string]string{
"link.kind": "span-pointer",
"ptr.dir": "d",
"ptr.hash": "eb29cb7d923f904f02bd8b3d85e228ed",
"ptr.kind": "aws.s3.object",
}
span.AddSpanLink(ddtrace.SpanLink{TraceID: 0, SpanID: 0, Attributes: attributes})

b.ResetTimer()
for i := 0; i < b.N; i++ {
span.serializeSpanLinksInMeta()
}
}

type boomError struct{}

func (e *boomError) Error() string { return "boom" }
Expand Down Expand Up @@ -1092,3 +1115,43 @@ func testConcurrentSpanSetTag(t *testing.T) {
}
wg.Wait()
}

func TestSpanLinksInMeta(t *testing.T) {
t.Run("no_links", func(t *testing.T) {
tracer := newTracer()
defer tracer.Stop()

sp := tracer.StartSpan("test-no-links")
sp.Finish()

internalSpan := sp.(*span)
_, ok := internalSpan.Meta["_dd.span_links"]
assert.False(t, ok, "Expected no _dd.span_links in Meta.")
})

t.Run("with_links", func(t *testing.T) {
tracer := newTracer()
defer tracer.Stop()

sp, ok := tracer.StartSpan("test-with-links").(SpanWithLinks)
require.True(t, ok, "Span does not implement SpanWithLinks interface")

sp.AddSpanLink(ddtrace.SpanLink{SpanID: 123, TraceID: 456})
sp.AddSpanLink(ddtrace.SpanLink{SpanID: 789, TraceID: 012})
sp.Finish()

internalSpan := sp.(*span)
raw, ok := internalSpan.Meta["_dd.span_links"]
require.True(t, ok, "Expected _dd.span_links in Meta after adding links.")

var links []ddtrace.SpanLink
err := json.Unmarshal([]byte(raw), &links)
require.NoError(t, err, "Failed to unmarshal links JSON")
require.Len(t, links, 2, "Expected 2 links in _dd.span_links JSON")

assert.Equal(t, uint64(123), links[0].SpanID)
assert.Equal(t, uint64(456), links[0].TraceID)
assert.Equal(t, uint64(789), links[1].SpanID)
assert.Equal(t, uint64(012), links[1].TraceID)
})
}
Loading