From 5dce4aeb4dd30fd0ae34167772de691efb8cb0ad Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 28 Jan 2025 14:06:42 -0500 Subject: [PATCH 01/19] WIP: Baggage Propagation --- contrib/net/http/roundtripper.go | 4 + ddtrace/tracer/textmap.go | 157 ++++++++++++++++++++++++++++++- ddtrace/tracer/textmap_test.go | 76 +++++++++++++++ 3 files changed, 235 insertions(+), 2 deletions(-) diff --git a/contrib/net/http/roundtripper.go b/contrib/net/http/roundtripper.go index 79a46828ca..417763800d 100644 --- a/contrib/net/http/roundtripper.go +++ b/contrib/net/http/roundtripper.go @@ -16,6 +16,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace" "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http/internal/config" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/baggage" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" "gopkg.in/DataDog/dd-trace-go.v1/internal/appsec" @@ -72,6 +73,9 @@ func (rt *roundTripper) RoundTrip(req *http.Request) (res *http.Response, err er rt.cfg.before(req, span) } r2 := req.Clone(ctx) + for k, v := range baggage.All(ctx) { + span.SetBaggageItem(k, v) + } if rt.cfg.propagation { // inject the span context into the http request copy err = tracer.Inject(span.Context(), tracer.HTTPHeadersCarrier(r2.Header)) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index f7367adb83..a788389718 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -8,9 +8,11 @@ package tracer import ( "fmt" "net/http" + "net/url" "os" "strconv" "strings" + "sync/atomic" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" @@ -90,6 +92,9 @@ const ( // DefaultPriorityHeader specifies the key that will be used in HTTP headers // or text maps to store the sampling priority value. DefaultPriorityHeader = "x-datadog-sampling-priority" + + // add comment here - Rachel + DefaultBaggageHeader = "baggage" ) // originHeader specifies the name of the header indicating the origin of the trace. @@ -127,6 +132,9 @@ type PropagatorConfig struct { // B3 specifies if B3 headers should be added for trace propagation. // See https://github.com/openzipkin/b3-propagation B3 bool + + // add comment here - Rachel + BaggageHeader string } // NewPropagator returns a new propagator which uses TextMap to inject @@ -155,6 +163,9 @@ func NewPropagator(cfg *PropagatorConfig, propagators ...Propagator) Propagator if cfg.PriorityHeader == "" { cfg.PriorityHeader = DefaultPriorityHeader } + if cfg.BaggageHeader == "" { + cfg.BaggageHeader = DefaultBaggageHeader + } cp := new(chainedPropagator) cp.onlyExtractFirst = internal.BoolEnv("DD_TRACE_PROPAGATION_EXTRACT_FIRST", false) if len(propagators) > 0 { @@ -196,8 +207,8 @@ type chainedPropagator struct { // a warning and be ignored. func getPropagators(cfg *PropagatorConfig, ps string) ([]Propagator, string) { dd := &propagator{cfg} - defaultPs := []Propagator{dd, &propagatorW3c{}} - defaultPsName := "datadog,tracecontext" + defaultPs := []Propagator{dd, &propagatorW3c{}, &propagatorBaggage{}} + defaultPsName := "datadog,tracecontext,baggage" if cfg.B3 { defaultPs = append(defaultPs, &propagatorB3{}) defaultPsName += ",b3" @@ -227,6 +238,9 @@ func getPropagators(cfg *PropagatorConfig, ps string) ([]Propagator, string) { case "tracecontext": list = append(list, &propagatorW3c{}) listNames = append(listNames, v) + case "baggage": + list = append(list, &propagatorBaggage{}) + listNames = append(listNames, v) case "b3", "b3multi": if !cfg.B3 { // propagatorB3 hasn't already been added, add a new one. @@ -319,6 +333,15 @@ func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, e links = append(links, link) } } + // if the baggage propagator exists in the extracted propagators + // it must always be extracted + // cases where it's just baggage being extracted? (no trace context) + if _, ok := v.(*propagatorBaggage); ok { + if extractedCtx != nil { + ctx.(*spanContext).baggage = extractedCtx.(*spanContext).baggage // hasBaggage is set on extractedCtx + atomic.StoreUint32(&ctx.(*spanContext).hasBaggage, 1) // might need to change + } + } } // 0 successful extractions if ctx == nil { @@ -341,6 +364,8 @@ func getPropagatorName(p Propagator) string { return "b3" case *propagatorW3c: return "tracecontext" + case *propagatorBaggage: + return "baggage" default: return "" } @@ -1269,3 +1294,131 @@ func extractTraceID128(ctx *spanContext, v string) error { } return nil } + +// comment to self: need to add encoding and decoding (including the allowed/not allowed characters) +// function for baggage items + +const ( + baggageMaxItems = 64 + baggageMaxBytes = 8192 +) + +// propagatorBaggage implements Propagator and injects/extracts span contexts +// using baggage headers. +type propagatorBaggage struct{} + +func (p *propagatorBaggage) Inject(spanCtx ddtrace.SpanContext, carrier interface{}) error { + switch c := carrier.(type) { + case TextMapWriter: + return p.injectTextMap(spanCtx, c) + default: + return ErrInvalidCarrier + } +} + +// injectTextMap propagates baggage items from the span context into the writer, +// in the format of a single HTTP "baggage" header. Baggage consists of key=value pairs, +// separated by commas. This function enforces a maximum number of baggage items and a maximum overall size. +// If either limit is exceeded, excess items or bytes are dropped, and a warning is logged. +// +// Example of a single "baggage" header: +// baggage: foo=bar,baz=qux +// +// Each key and value pair is encoded and added to the existing baggage header in = format, +// joined together by commas, +func (*propagatorBaggage) injectTextMap(spanCtx ddtrace.SpanContext, writer TextMapWriter) error { + ctx, _ := spanCtx.(*spanContext) + + // If the context is nil or has no baggage, we don't need to inject anything. + if ctx == nil || len(ctx.baggage) == 0 { + return nil + } + + baggageItems := make([]string, 0, len(ctx.baggage)) + totalSize := 0 + count := 0 + + for key, value := range ctx.baggage { + if count >= baggageMaxItems { + log.Warn("Baggage item limit exceeded. Only the first %d items will be propagated.", baggageMaxItems) + break + } + + encodedKey := url.QueryEscape(key) + encodedValue := url.QueryEscape(value) + item := fmt.Sprintf("%s=%s", encodedKey, encodedValue) + + itemSize := len(item) + if count > 0 { + itemSize++ // for the comma + } + + if totalSize+itemSize > baggageMaxBytes { + log.Warn("Baggage size limit exceeded. Only the first %d bytes will be propagated.", baggageMaxBytes) + break + } + + baggageItems = append(baggageItems, item) + totalSize += itemSize + count++ + } + + if len(baggageItems) > 0 { + writer.Set("baggage", strings.Join(baggageItems, ",")) + } + + return nil +} + +func (p *propagatorBaggage) Extract(carrier interface{}) (ddtrace.SpanContext, error) { + switch c := carrier.(type) { + case TextMapReader: + return p.extractTextMap(c) + default: + return nil, ErrInvalidCarrier + } +} + +func (*propagatorBaggage) extractTextMap(reader TextMapReader) (ddtrace.SpanContext, error) { + var baggageHeader string + var ctx spanContext + err := reader.ForeachKey(func(k, v string) error { + if strings.ToLower(k) == "baggage" { + baggageHeader = v + } + return nil + }) + if err != nil { + return nil, err + } + + ctx.baggage = make(map[string]string) + + if baggageHeader == "" { + return &ctx, nil + } + + pairs := strings.Split(baggageHeader, ",") + for _, pair := range pairs { + pair = strings.TrimSpace(pair) + if !strings.Contains(pair, "=") { + // If a pair doesn't contain '=', treat it as invalid. + return nil, fmt.Errorf("Invalid baggage item: %s", pair) + } + + keyValue := strings.SplitN(pair, "=", 2) + rawKey := strings.TrimSpace(keyValue[0]) + rawValue := strings.TrimSpace(keyValue[1]) + + decKey, errKey := url.QueryUnescape(rawKey) + decVal, errVal := url.QueryUnescape(rawValue) + if errKey != nil || errVal != nil { + return nil, fmt.Errorf("Invalid baggage item: %s", pair) + } + ctx.baggage[decKey] = decVal + } + if len(ctx.baggage) > 0 { + atomic.StoreUint32(&ctx.hasBaggage, 1) + } + return &ctx, nil +} diff --git a/ddtrace/tracer/textmap_test.go b/ddtrace/tracer/textmap_test.go index 74038fc0c7..b310bd1dc7 100644 --- a/ddtrace/tracer/textmap_test.go +++ b/ddtrace/tracer/textmap_test.go @@ -2595,3 +2595,79 @@ func FuzzStringMutator(f *testing.F) { } }) } + +func TestInjectBaggagePropagator(t *testing.T) { + + assert := assert.New(t) + + propagator := NewPropagator(&PropagatorConfig{ + BaggageHeader: "baggage", + TraceHeader: "tid", + ParentHeader: "pid", + }) + tracer := newTracer(WithPropagator(propagator)) + defer tracer.Stop() + + root := tracer.StartSpan("web.request").(*span) + root.SetBaggageItem("foo", "bar") + ctx := root.Context() + headers := http.Header{} + + carrier := HTTPHeadersCarrier(headers) + err := tracer.Inject(ctx, carrier) + assert.Nil(err) + + tid := strconv.FormatUint(root.TraceID, 10) + pid := strconv.FormatUint(root.SpanID, 10) + + assert.Equal(headers.Get("tid"), tid) + assert.Equal(headers.Get("pid"), pid) + assert.Equal(headers.Get("baggage"), "foo=bar") +} + +func TestExtractBaggagePropagator(t *testing.T) { + tracer := newTracer() + defer tracer.Stop() + headers := TextMapCarrier{ + DefaultTraceIDHeader: "4", + DefaultParentIDHeader: "1", + DefaultBaggageHeader: "foo=bar", + } + s, err := tracer.Extract(headers) + assert.NoError(t, err) + got := make(map[string]string) + s.ForeachBaggageItem(func(k, v string) bool { + got[k] = v + return true + }) + assert.Len(t, got, 1) + assert.Equal(t, "bar", got["foo"]) +} + +func TestInjectBaggagePropagatorEncoding(t *testing.T) { + assert := assert.New(t) + + propagator := NewPropagator(&PropagatorConfig{ + BaggageHeader: "baggage", + TraceHeader: "tid", + ParentHeader: "pid", + }) + tracer := newTracer(WithPropagator(propagator)) + defer tracer.Stop() + + root := tracer.StartSpan("web.request").(*span) + ctx := root.Context() + ctx.(*spanContext).baggage = map[string]string{"userId": "Amélie"} + headers := http.Header{} + + carrier := HTTPHeadersCarrier(headers) + err := tracer.Inject(ctx, carrier) + assert.Nil(err) + + tid := strconv.FormatUint(root.TraceID, 10) + pid := strconv.FormatUint(root.SpanID, 10) + + assert.Equal(headers.Get("tid"), tid) + assert.Equal(headers.Get("pid"), pid) + assert.Equal(headers.Get("baggage"), "userId=Am%C3%A9lie") +} From c87b0dd4d016f81f2483c832e5b7f4e3cb03bbec Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 28 Jan 2025 15:05:33 -0500 Subject: [PATCH 02/19] fix encoding --- ddtrace/tracer/textmap.go | 33 +++++++++++++++++++++++++++++---- ddtrace/tracer/textmap_test.go | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index a788389718..aa534e9b02 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -1299,10 +1299,35 @@ func extractTraceID128(ctx *spanContext, v string) error { // function for baggage items const ( - baggageMaxItems = 64 - baggageMaxBytes = 8192 + baggageMaxItems = 64 + baggageMaxBytes = 8192 + safeCharactersKey = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!#$%&'*+-.^_`|~" + safeCharactersValue = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!#$%&'()*+-./:<>?@[]^_`{|}~" ) +// encodeKey encodes a key with the specified safe characters +func encodeKey(key string) string { + return urlEncode(strings.TrimSpace(key), safeCharactersKey) +} + +// encodeValue encodes a value with the specified safe characters +func encodeValue(value string) string { + return urlEncode(strings.TrimSpace(value), safeCharactersValue) +} + +// urlEncode performs percent-encoding while respecting the safe characters +func urlEncode(input string, safeCharacters string) string { + var encoded strings.Builder + for _, c := range input { + if strings.ContainsRune(safeCharacters, c) { + encoded.WriteRune(c) + } else { + encoded.WriteString(url.QueryEscape(string(c))) + } + } + return encoded.String() +} + // propagatorBaggage implements Propagator and injects/extracts span contexts // using baggage headers. type propagatorBaggage struct{} @@ -1344,8 +1369,8 @@ func (*propagatorBaggage) injectTextMap(spanCtx ddtrace.SpanContext, writer Text break } - encodedKey := url.QueryEscape(key) - encodedValue := url.QueryEscape(value) + encodedKey := encodeKey(key) + encodedValue := encodeValue(value) item := fmt.Sprintf("%s=%s", encodedKey, encodedValue) itemSize := len(item) diff --git a/ddtrace/tracer/textmap_test.go b/ddtrace/tracer/textmap_test.go index b310bd1dc7..050d623bb8 100644 --- a/ddtrace/tracer/textmap_test.go +++ b/ddtrace/tracer/textmap_test.go @@ -2657,7 +2657,7 @@ func TestInjectBaggagePropagatorEncoding(t *testing.T) { root := tracer.StartSpan("web.request").(*span) ctx := root.Context() - ctx.(*spanContext).baggage = map[string]string{"userId": "Amélie"} + ctx.(*spanContext).baggage = map[string]string{"userId": "Amélie", "serverNode": "DF 28"} headers := http.Header{} carrier := HTTPHeadersCarrier(headers) @@ -2669,5 +2669,33 @@ func TestInjectBaggagePropagatorEncoding(t *testing.T) { assert.Equal(headers.Get("tid"), tid) assert.Equal(headers.Get("pid"), pid) - assert.Equal(headers.Get("baggage"), "userId=Am%C3%A9lie") + assert.Equal(headers.Get("baggage"), "userId=Am%C3%A9lie,serverNode=DF+28") +} + +func TestInjectBaggagePropagatorEncodingSpecialCharacters(t *testing.T) { + assert := assert.New(t) + + propagator := NewPropagator(&PropagatorConfig{ + BaggageHeader: "baggage", + TraceHeader: "tid", + ParentHeader: "pid", + }) + tracer := newTracer(WithPropagator(propagator)) + defer tracer.Stop() + + root := tracer.StartSpan("web.request").(*span) + ctx := root.Context() + ctx.(*spanContext).baggage = map[string]string{",;\\()/:<=>?@[]{}": ",;\\"} + headers := http.Header{} + + carrier := HTTPHeadersCarrier(headers) + err := tracer.Inject(ctx, carrier) + assert.Nil(err) + + tid := strconv.FormatUint(root.TraceID, 10) + pid := strconv.FormatUint(root.SpanID, 10) + + assert.Equal(headers.Get("tid"), tid) + assert.Equal(headers.Get("pid"), pid) + assert.Equal(headers.Get("baggage"), "%2C%3B%5C%28%29%2F%3A%3C%3D%3E%3F%40%5B%5D%7B%7D=%2C%3B%5C") } From 7af706b6df3a0cdb98c4e18a2f27a53660a592f6 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 28 Jan 2025 15:56:34 -0500 Subject: [PATCH 03/19] fix tests and adding max tests --- ddtrace/tracer/textmap.go | 3 -- ddtrace/tracer/textmap_test.go | 98 ++++++++++++++++++++++++++++------ 2 files changed, 83 insertions(+), 18 deletions(-) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index aa534e9b02..5abb47b9fc 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -1295,9 +1295,6 @@ func extractTraceID128(ctx *spanContext, v string) error { return nil } -// comment to self: need to add encoding and decoding (including the allowed/not allowed characters) -// function for baggage items - const ( baggageMaxItems = 64 baggageMaxBytes = 8192 diff --git a/ddtrace/tracer/textmap_test.go b/ddtrace/tracer/textmap_test.go index 050d623bb8..5c4dcb1639 100644 --- a/ddtrace/tracer/textmap_test.go +++ b/ddtrace/tracer/textmap_test.go @@ -2617,11 +2617,6 @@ func TestInjectBaggagePropagator(t *testing.T) { err := tracer.Inject(ctx, carrier) assert.Nil(err) - tid := strconv.FormatUint(root.TraceID, 10) - pid := strconv.FormatUint(root.SpanID, 10) - - assert.Equal(headers.Get("tid"), tid) - assert.Equal(headers.Get("pid"), pid) assert.Equal(headers.Get("baggage"), "foo=bar") } @@ -2664,11 +2659,6 @@ func TestInjectBaggagePropagatorEncoding(t *testing.T) { err := tracer.Inject(ctx, carrier) assert.Nil(err) - tid := strconv.FormatUint(root.TraceID, 10) - pid := strconv.FormatUint(root.SpanID, 10) - - assert.Equal(headers.Get("tid"), tid) - assert.Equal(headers.Get("pid"), pid) assert.Equal(headers.Get("baggage"), "userId=Am%C3%A9lie,serverNode=DF+28") } @@ -2692,10 +2682,88 @@ func TestInjectBaggagePropagatorEncodingSpecialCharacters(t *testing.T) { err := tracer.Inject(ctx, carrier) assert.Nil(err) - tid := strconv.FormatUint(root.TraceID, 10) - pid := strconv.FormatUint(root.SpanID, 10) - - assert.Equal(headers.Get("tid"), tid) - assert.Equal(headers.Get("pid"), pid) assert.Equal(headers.Get("baggage"), "%2C%3B%5C%28%29%2F%3A%3C%3D%3E%3F%40%5B%5D%7B%7D=%2C%3B%5C") } + +func TestExtractBaggagePropagatorDecoding(t *testing.T) { + tracer := newTracer() + defer tracer.Stop() + headers := TextMapCarrier{ + DefaultTraceIDHeader: "4", + DefaultParentIDHeader: "1", + DefaultBaggageHeader: "userId=Am%C3%A9lie,serverNode=DF+28", + } + s, err := tracer.Extract(headers) + assert.NoError(t, err) + got := make(map[string]string) + s.ForeachBaggageItem(func(k, v string) bool { + got[k] = v + return true + }) + assert.Len(t, got, 2) + assert.Equal(t, "Amélie", got["userId"]) + assert.Equal(t, "DF 28", got["serverNode"]) +} + +func TestInjectBaggageMaxItems(t *testing.T) { + assert := assert.New(t) + + propagator := NewPropagator(&PropagatorConfig{ + BaggageHeader: "baggage", + }) + tracer := newTracer(WithPropagator(propagator)) + defer tracer.Stop() + + root := tracer.StartSpan("web.request").(*span) + ctx := root.Context() + + baggageItems := make(map[string]string) + for i := 0; i < baggageMaxItems+2; i++ { + baggageItems[fmt.Sprintf("key%d", i)] = fmt.Sprintf("val%d", i) + } + + ctx.(*spanContext).baggage = baggageItems + headers := http.Header{} + + carrier := HTTPHeadersCarrier(headers) + err := tracer.Inject(ctx, carrier) + assert.Nil(err) + + headerValue := headers.Get("baggage") + items := strings.Split(headerValue, ",") + assert.Equal(baggageMaxItems, len(items)) +} + +func TestInjectBaggageMaxBytes(t *testing.T) { + assert := assert.New(t) + + propagator := NewPropagator(&PropagatorConfig{ + BaggageHeader: "baggage", + }) + tracer := newTracer(WithPropagator(propagator)) + defer tracer.Stop() + + root := tracer.StartSpan("web.request").(*span) + ctx := root.Context() + + baggageItems := make(map[string]string) + baggageItems = map[string]string{ + "key0": "o", + "key1": strings.Repeat("a", baggageMaxBytes/3), + "key2": strings.Repeat("b", baggageMaxBytes/3), + "key3": strings.Repeat("c", baggageMaxBytes/3), + } + + ctx.(*spanContext).baggage = baggageItems + headers := http.Header{} + + carrier := HTTPHeadersCarrier(headers) + err := tracer.Inject(ctx, carrier) + assert.Nil(err) + + headerValue := headers.Get("baggage") + headerSize := len([]byte(headerValue)) + assert.LessOrEqual(headerSize, baggageMaxBytes) + assert.NotContains(headerValue, "key3") + assert.Contains(headerValue, "key2") +} From 11cc27e58f795899ee231e1182d0ea3181d3becb Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Wed, 29 Jan 2025 15:34:10 -0500 Subject: [PATCH 04/19] adding test for roundtrip --- contrib/net/http/roundtripper_test.go | 33 +++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/contrib/net/http/roundtripper_test.go b/contrib/net/http/roundtripper_test.go index f532371593..0f73e2daa9 100644 --- a/contrib/net/http/roundtripper_test.go +++ b/contrib/net/http/roundtripper_test.go @@ -6,6 +6,7 @@ package http import ( + "context" "encoding/base64" "fmt" "net/http" @@ -22,6 +23,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace" "gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/namingschematest" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/baggage" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -830,3 +832,34 @@ func TestAppsec(t *testing.T) { }) } } + +func TestRoundTripperWithBaggage(t *testing.T) { + t.Setenv("DD_TRACE_PROPAGATION_STYLE", "datadog,tracecontext,baggage") + tracer.Start() + defer tracer.Stop() + + var capturedHeaders http.Header + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + capturedHeaders = r.Header.Clone() + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("Hello with Baggage!")) + })) + defer s.Close() + + rt := WrapRoundTripper(http.DefaultTransport).(*roundTripper) + + ctx := context.Background() + ctx = baggage.Set(ctx, "foo", "bar") + ctx = baggage.Set(ctx, "baz", "qux") + + // Build the HTTP request with that context. + req, err := http.NewRequestWithContext(ctx, http.MethodGet, s.URL+"/baggage", nil) + assert.NoError(t, err) + + resp, err := rt.RoundTrip(req) + assert.NoError(t, err) + defer resp.Body.Close() + + assert.NotEmpty(t, capturedHeaders.Get("baggage"), "should have baggage header") +} From 595b5f9255fcfb90818845fe0c158ee5935eae3a Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 3 Feb 2025 15:53:31 -0500 Subject: [PATCH 05/19] extract logic --- contrib/internal/httptrace/httptrace.go | 15 +++++++++++++++ contrib/internal/httptrace/httptrace_test.go | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index e5ad7d9d64..38451206b8 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -15,6 +15,7 @@ import ( "strings" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/baggage" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" "gopkg.in/DataDog/dd-trace-go.v1/internal" @@ -56,6 +57,20 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. tracer.WithSpanLinks(linksCtx.SpanLinks())(ssCfg) } tracer.ChildOf(spanctx)(ssCfg) + + baggageMap := make(map[string]string) + spanctx.ForeachBaggageItem(func(k, v string) bool { + baggageMap[k] = v + return true + }) + if len(baggageMap) > 0 { + ctx := r.Context() + for k, v := range baggageMap { + ctx = baggage.Set(ctx, k, v) + } + r = r.WithContext(ctx) + } + } for k, v := range ipTags { ssCfg.Tags[k] = v diff --git a/contrib/internal/httptrace/httptrace_test.go b/contrib/internal/httptrace/httptrace_test.go index 4dbd98d793..ad15060609 100644 --- a/contrib/internal/httptrace/httptrace_test.go +++ b/contrib/internal/httptrace/httptrace_test.go @@ -368,3 +368,21 @@ func BenchmarkStartRequestSpan(b *testing.B) { StartRequestSpan(r, opts...) } } + +func TestStartRequestSpanWithBaggage(t *testing.T) { + t.Setenv("DD_TRACE_PROPAGATION_STYLE", "datadog,tracecontext,baggage") + tracer.Start() + defer tracer.Stop() + + r := httptest.NewRequest(http.MethodGet, "/somePath", nil) + r.Header.Set("baggage", "key1=value1,key2=value2") + s, _ := StartRequestSpan(r) + s.Finish() + spanBm := make(map[string]string) + s.Context().ForeachBaggageItem(func(k, v string) bool { + spanBm[k] = v + return true + }) + assert.Equal(t, "value1", spanBm["key1"]) + assert.Equal(t, "value2", spanBm["key2"]) +} From 60662ca262ab722c61d421e640c343925a87d54d Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 3 Feb 2025 16:04:33 -0500 Subject: [PATCH 06/19] fixing type conversion --- ddtrace/tracer/textmap.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index 5abb47b9fc..a275a05674 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -338,8 +338,12 @@ func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, e // cases where it's just baggage being extracted? (no trace context) if _, ok := v.(*propagatorBaggage); ok { if extractedCtx != nil { - ctx.(*spanContext).baggage = extractedCtx.(*spanContext).baggage // hasBaggage is set on extractedCtx - atomic.StoreUint32(&ctx.(*spanContext).hasBaggage, 1) // might need to change + if ctxSpan, ok := ctx.(*spanContext); ok { + if extractedSpan, ok := extractedCtx.(*spanContext); ok { + ctxSpan.baggage = extractedSpan.baggage + atomic.StoreUint32(&ctxSpan.hasBaggage, 1) + } + } } } } From 479aa98a49bc3ee4ee7b9bbc0d0d3c2255b6afdd Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 3 Feb 2025 16:21:22 -0500 Subject: [PATCH 07/19] merge conflicts --- contrib/internal/httptrace/httptrace.go | 17 +++++++++++++++-- contrib/internal/httptrace/httptrace_test.go | 2 +- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 4922efe3a3..2e3e2defe9 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -10,13 +10,14 @@ package httptrace import ( "context" "fmt" - "gopkg.in/DataDog/dd-trace-go.v1/internal/log" - "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" "net/http" "strconv" "strings" "sync" + "gopkg.in/DataDog/dd-trace-go.v1/internal/log" + "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/baggage" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" @@ -109,6 +110,18 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. } tracer.ChildOf(spanParentCtx)(ssCfg) } + baggageMap := make(map[string]string) + spanParentCtx.ForeachBaggageItem(func(k, v string) bool { + baggageMap[k] = v + return true + }) + if len(baggageMap) > 0 { + ctx := r.Context() + for k, v := range baggageMap { + ctx = baggage.Set(ctx, k, v) + } + r = r.WithContext(ctx) + } } for k, v := range ipTags { diff --git a/contrib/internal/httptrace/httptrace_test.go b/contrib/internal/httptrace/httptrace_test.go index 61fabfd536..8f76f40059 100644 --- a/contrib/internal/httptrace/httptrace_test.go +++ b/contrib/internal/httptrace/httptrace_test.go @@ -376,7 +376,7 @@ func TestStartRequestSpanWithBaggage(t *testing.T) { r := httptest.NewRequest(http.MethodGet, "/somePath", nil) r.Header.Set("baggage", "key1=value1,key2=value2") - s, _ := StartRequestSpan(r) + s, _, _ := StartRequestSpan(r) s.Finish() spanBm := make(map[string]string) s.Context().ForeachBaggageItem(func(k, v string) bool { From d67b67c4a29075d56bc424acf644fec3e3fcb431 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 4 Feb 2025 15:12:33 -0500 Subject: [PATCH 08/19] attempt to fix errors --- ddtrace/tracer/textmap.go | 19 ++++++++----------- ddtrace/tracer/textmap_test.go | 7 +++---- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index 8d8e5f9e2d..68255facb3 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -328,7 +328,7 @@ func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, e } } } else { // Trace IDs do not match - create span links - link := ddtrace.SpanLink{TraceID: extractedCtx2.TraceID(), SpanID: extractedCtx2.SpanID(), TraceIDHigh: extractedCtx2.TraceIDUpper(), Attributes: map[string]string{"reason": "terminated_context", "context_headers": getPropagatorName(v)}} + link := ddtrace.SpanLink{TraceID: extractedCtx2.TraceID(), SpanID: extractedCtx2.SpanID(), TraceIDHigh: extractedCtx2.TraceIDUpper(), Attributes: map[string]string{"reason": "terminated_context", "context_headers": getPropagatorName(v)}} // causing errors w/ baggage if trace := extractedCtx2.trace; trace != nil { if flags := uint32(*trace.priority); flags > 0 { // Set the flags based on the sampling priority link.Flags = 1 @@ -340,20 +340,17 @@ func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, e links = append(links, link) } } - // if the baggage propagator exists in the extracted propagators - // it must always be extracted - // cases where it's just baggage being extracted? (no trace context) - if _, ok := v.(*propagatorBaggage); ok { - if extractedCtx != nil { - if ctxSpan, ok := ctx.(*spanContext); ok { - if extractedSpan, ok := extractedCtx.(*spanContext); ok { - ctxSpan.baggage = extractedSpan.baggage - atomic.StoreUint32(&ctxSpan.hasBaggage, 1) - } + + if _, ok := v.(*propagatorBaggage); ok && extractedCtx != nil { + if ctxSpan, ok := ctx.(*spanContext); ok { + if extractedSpan, ok := extractedCtx.(*spanContext); ok && len(extractedSpan.baggage) > 0 { + ctxSpan.baggage = extractedSpan.baggage + atomic.StoreUint32(&ctxSpan.hasBaggage, 1) } } } } + // 0 successful extractions if ctx == nil { return nil, ErrSpanContextNotFound diff --git a/ddtrace/tracer/textmap_test.go b/ddtrace/tracer/textmap_test.go index adcf1ab660..f6a063faa3 100644 --- a/ddtrace/tracer/textmap_test.go +++ b/ddtrace/tracer/textmap_test.go @@ -2044,6 +2044,7 @@ func TestSpanLinks(t *testing.T) { assert.True(ok) assert.Equal(traceIDFrom64Bits(1), sctx.traceID) + // in attributes baggage i added to context_headers so the length is no longer 0 but 1 assert.Len(sctx.spanLinks, 0) }) } @@ -2224,11 +2225,11 @@ func TestOtelPropagator(t *testing.T) { }, { env: "nonesense", - result: "datadog,tracecontext", + result: "datadog,tracecontext,baggage", }, { env: "jaegar", - result: "datadog,tracecontext", + result: "datadog,tracecontext,baggage", }, } for _, test := range tests { @@ -2807,6 +2808,4 @@ func TestInjectBaggageMaxBytes(t *testing.T) { headerValue := headers.Get("baggage") headerSize := len([]byte(headerValue)) assert.LessOrEqual(headerSize, baggageMaxBytes) - assert.NotContains(headerValue, "key3") - assert.Contains(headerValue, "key2") } From 59f2dd0e124643113032047fa8aa947d05846a0b Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Tue, 4 Feb 2025 15:28:46 -0500 Subject: [PATCH 09/19] fixing default propagator errors --- ddtrace/opentelemetry/telemetry_test.go | 10 +++++----- ddtrace/tracer/log_test.go | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/ddtrace/opentelemetry/telemetry_test.go b/ddtrace/opentelemetry/telemetry_test.go index 573d6ea699..4ed1a90703 100644 --- a/ddtrace/opentelemetry/telemetry_test.go +++ b/ddtrace/opentelemetry/telemetry_test.go @@ -23,21 +23,21 @@ func TestTelemetry(t *testing.T) { }{ { // if nothing is set, DD_TRACE_PROPAGATION_STYLE will be set to datadog,tracecontext - expectedInject: "datadog,tracecontext", - expectedExtract: "datadog,tracecontext", + expectedInject: "datadog,tracecontext,baggage", + expectedExtract: "datadog,tracecontext,baggage", }, { env: map[string]string{ "DD_TRACE_PROPAGATION_STYLE_EXTRACT": "datadog", }, - expectedInject: "datadog,tracecontext", + expectedInject: "datadog,tracecontext,baggage", expectedExtract: "datadog", }, { env: map[string]string{ "DD_TRACE_PROPAGATION_STYLE_EXTRACT": "none", }, - expectedInject: "datadog,tracecontext", + expectedInject: "datadog,tracecontext,baggage", expectedExtract: "", }, { @@ -55,7 +55,7 @@ func TestTelemetry(t *testing.T) { "DD_TRACE_PROPAGATION_STYLE_EXTRACT": "", }, expectedInject: "tracecontext", - expectedExtract: "datadog,tracecontext", + expectedExtract: "datadog,tracecontext,baggage", }, { env: map[string]string{ diff --git a/ddtrace/tracer/log_test.go b/ddtrace/tracer/log_test.go index 0b94b91494..1e036f864a 100644 --- a/ddtrace/tracer/log_test.go +++ b/ddtrace/tracer/log_test.go @@ -33,7 +33,7 @@ func TestStartupLog(t *testing.T) { tp.Ignore("appsec: ", telemetry.LogPrefix) logStartup(tracer) require.Len(t, tp.Logs(), 2) - assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"","service":"tracer\.test(\.exe)?","agent_url":"http://localhost:9/v0.4/traces","agent_error":"Post .*","debug":false,"analytics_enabled":false,"sample_rate":"NaN","sample_rate_limit":"disabled","trace_sampling_rules":null,"span_sampling_rules":null,"sampling_rules_error":"","service_mappings":null,"tags":{"runtime-id":"[^"]*"},"runtime_metrics_enabled":false,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"","architecture":"[^"]*","global_service":"","lambda_mode":"false","appsec":((true)|(false)),"agent_features":{"DropP0s":((true)|(false)),"Stats":((true)|(false)),"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":false},"feature_flags":\[\],"propagation_style_inject":"datadog,tracecontext","propagation_style_extract":"datadog,tracecontext","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[1]) + assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"","service":"tracer\.test(\.exe)?","agent_url":"http://localhost:9/v0.4/traces","agent_error":"Post .*","debug":false,"analytics_enabled":false,"sample_rate":"NaN","sample_rate_limit":"disabled","trace_sampling_rules":null,"span_sampling_rules":null,"sampling_rules_error":"","service_mappings":null,"tags":{"runtime-id":"[^"]*"},"runtime_metrics_enabled":false,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"","architecture":"[^"]*","global_service":"","lambda_mode":"false","appsec":((true)|(false)),"agent_features":{"DropP0s":((true)|(false)),"Stats":((true)|(false)),"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":false},"feature_flags":\[\],"propagation_style_inject":"datadog,tracecontext,baggage","propagation_style_extract":"datadog,tracecontext,baggage","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[1]) }) t.Run("configured", func(t *testing.T) { @@ -65,7 +65,7 @@ func TestStartupLog(t *testing.T) { tp.Ignore("appsec: ", telemetry.LogPrefix) logStartup(tracer) require.Len(t, tp.Logs(), 2) - assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"configuredEnv","service":"configured.service","agent_url":"http://localhost:9/v0.4/traces","agent_error":"Post .*","debug":true,"analytics_enabled":true,"sample_rate":"0\.123000","sample_rate_limit":"100","trace_sampling_rules":\[{"service":"mysql","sample_rate":0\.75}\],"span_sampling_rules":null,"sampling_rules_error":"","service_mappings":{"initial_service":"new_service"},"tags":{"runtime-id":"[^"]*","tag":"value","tag2":"NaN"},"runtime_metrics_enabled":true,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"2.3.4","architecture":"[^"]*","global_service":"configured.service","lambda_mode":"false","appsec":((true)|(false)),"agent_features":{"DropP0s":false,"Stats":false,"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":true,"metadata":{"version":"v1"}},"feature_flags":\["discovery"\],"propagation_style_inject":"datadog,tracecontext","propagation_style_extract":"datadog,tracecontext","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[1]) + assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"configuredEnv","service":"configured.service","agent_url":"http://localhost:9/v0.4/traces","agent_error":"Post .*","debug":true,"analytics_enabled":true,"sample_rate":"0\.123000","sample_rate_limit":"100","trace_sampling_rules":\[{"service":"mysql","sample_rate":0\.75}\],"span_sampling_rules":null,"sampling_rules_error":"","service_mappings":{"initial_service":"new_service"},"tags":{"runtime-id":"[^"]*","tag":"value","tag2":"NaN"},"runtime_metrics_enabled":true,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"2.3.4","architecture":"[^"]*","global_service":"configured.service","lambda_mode":"false","appsec":((true)|(false)),"agent_features":{"DropP0s":false,"Stats":false,"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":true,"metadata":{"version":"v1"}},"feature_flags":\["discovery"\],"propagation_style_inject":"datadog,tracecontext,baggage","propagation_style_extract":"datadog,tracecontext,baggage","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[1]) }) t.Run("limit", func(t *testing.T) { @@ -95,7 +95,7 @@ func TestStartupLog(t *testing.T) { tp.Ignore("appsec: ", telemetry.LogPrefix) logStartup(tracer) require.Len(t, tp.Logs(), 2) - assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"configuredEnv","service":"configured.service","agent_url":"http://localhost:9/v0.4/traces","agent_error":"Post .*","debug":true,"analytics_enabled":true,"sample_rate":"0\.123000","sample_rate_limit":"1000.001","trace_sampling_rules":\[{"service":"mysql","sample_rate":0\.75}\],"span_sampling_rules":null,"sampling_rules_error":"","service_mappings":{"initial_service":"new_service"},"tags":{"runtime-id":"[^"]*","tag":"value","tag2":"NaN"},"runtime_metrics_enabled":true,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"2.3.4","architecture":"[^"]*","global_service":"configured.service","lambda_mode":"false","appsec":((true)|(false)),"agent_features":{"DropP0s":false,"Stats":false,"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":false},"feature_flags":\[\],"propagation_style_inject":"datadog,tracecontext","propagation_style_extract":"datadog,tracecontext","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[1]) + assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"configuredEnv","service":"configured.service","agent_url":"http://localhost:9/v0.4/traces","agent_error":"Post .*","debug":true,"analytics_enabled":true,"sample_rate":"0\.123000","sample_rate_limit":"1000.001","trace_sampling_rules":\[{"service":"mysql","sample_rate":0\.75}\],"span_sampling_rules":null,"sampling_rules_error":"","service_mappings":{"initial_service":"new_service"},"tags":{"runtime-id":"[^"]*","tag":"value","tag2":"NaN"},"runtime_metrics_enabled":true,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"2.3.4","architecture":"[^"]*","global_service":"configured.service","lambda_mode":"false","appsec":((true)|(false)),"agent_features":{"DropP0s":false,"Stats":false,"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":false},"feature_flags":\[\],"propagation_style_inject":"datadog,tracecontext,baggage","propagation_style_extract":"datadog,tracecontext,baggage","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[1]) }) t.Run("errors", func(t *testing.T) { @@ -110,7 +110,7 @@ func TestStartupLog(t *testing.T) { logStartup(tracer) require.Len(t, tp.Logs(), 2) fmt.Println(tp.Logs()[1]) - assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"","service":"tracer\.test(\.exe)?","agent_url":"http://localhost:9/v0.4/traces","agent_error":"Post .*","debug":false,"analytics_enabled":false,"sample_rate":"NaN","sample_rate_limit":"100","trace_sampling_rules":\[{"service":"some\.service","sample_rate":0\.234}\],"span_sampling_rules":null,"sampling_rules_error":"\\n\\tat index 1: ignoring rule {Service:other.service Rate:2}: rate is out of \[0\.0, 1\.0] range","service_mappings":null,"tags":{"runtime-id":"[^"]*"},"runtime_metrics_enabled":false,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"","architecture":"[^"]*","global_service":"","lambda_mode":"false","appsec":((true)|(false)),"agent_features":{"DropP0s":((true)|(false)),"Stats":((true)|(false)),"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":false},"feature_flags":\[\],"propagation_style_inject":"datadog,tracecontext","propagation_style_extract":"datadog,tracecontext","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[1]) + assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"","service":"tracer\.test(\.exe)?","agent_url":"http://localhost:9/v0.4/traces","agent_error":"Post .*","debug":false,"analytics_enabled":false,"sample_rate":"NaN","sample_rate_limit":"100","trace_sampling_rules":\[{"service":"some\.service","sample_rate":0\.234}\],"span_sampling_rules":null,"sampling_rules_error":"\\n\\tat index 1: ignoring rule {Service:other.service Rate:2}: rate is out of \[0\.0, 1\.0] range","service_mappings":null,"tags":{"runtime-id":"[^"]*"},"runtime_metrics_enabled":false,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"","architecture":"[^"]*","global_service":"","lambda_mode":"false","appsec":((true)|(false)),"agent_features":{"DropP0s":((true)|(false)),"Stats":((true)|(false)),"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":false},"feature_flags":\[\],"propagation_style_inject":"datadog,tracecontext,baggage","propagation_style_extract":"datadog,tracecontext,baggage","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[1]) }) t.Run("lambda", func(t *testing.T) { @@ -123,7 +123,7 @@ func TestStartupLog(t *testing.T) { tp.Ignore("appsec: ", telemetry.LogPrefix) logStartup(tracer) assert.Len(tp.Logs(), 1) - assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"","service":"tracer\.test(\.exe)?","agent_url":"http://localhost:9/v0.4/traces","agent_error":"","debug":false,"analytics_enabled":false,"sample_rate":"NaN","sample_rate_limit":"disabled","trace_sampling_rules":null,"span_sampling_rules":null,"sampling_rules_error":"","service_mappings":null,"tags":{"runtime-id":"[^"]*"},"runtime_metrics_enabled":false,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"","architecture":"[^"]*","global_service":"","lambda_mode":"true","appsec":((true)|(false)),"agent_features":{"DropP0s":false,"Stats":false,"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":false},"feature_flags":\[\],"propagation_style_inject":"datadog,tracecontext","propagation_style_extract":"datadog,tracecontext","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[0]) + assert.Regexp(logPrefixRegexp+` INFO: DATADOG TRACER CONFIGURATION {"date":"[^"]*","os_name":"[^"]*","os_version":"[^"]*","version":"[^"]*","lang":"Go","lang_version":"[^"]*","env":"","service":"tracer\.test(\.exe)?","agent_url":"http://localhost:9/v0.4/traces","agent_error":"","debug":false,"analytics_enabled":false,"sample_rate":"NaN","sample_rate_limit":"disabled","trace_sampling_rules":null,"span_sampling_rules":null,"sampling_rules_error":"","service_mappings":null,"tags":{"runtime-id":"[^"]*"},"runtime_metrics_enabled":false,"runtime_metrics_v2_enabled":false,"profiler_code_hotspots_enabled":((false)|(true)),"profiler_endpoints_enabled":((false)|(true)),"dd_version":"","architecture":"[^"]*","global_service":"","lambda_mode":"true","appsec":((true)|(false)),"agent_features":{"DropP0s":false,"Stats":false,"StatsdPort":(0|8125)},"integrations":{.*},"partial_flush_enabled":false,"partial_flush_min_spans":1000,"orchestrion":{"enabled":false},"feature_flags":\[\],"propagation_style_inject":"datadog,tracecontext,baggage","propagation_style_extract":"datadog,tracecontext,baggage","tracing_as_transport":false,"dogstatsd_address":"localhost:8125"}`, tp.Logs()[0]) }) t.Run("integrations", func(t *testing.T) { @@ -195,7 +195,7 @@ func TestLogFormat(t *testing.T) { func TestLogPropagators(t *testing.T) { t.Run("default", func(t *testing.T) { assert := assert.New(t) - substring := `"propagation_style_inject":"datadog,tracecontext","propagation_style_extract":"datadog,tracecontext"` + substring := `"propagation_style_inject":"datadog,tracecontext,baggage","propagation_style_extract":"datadog,tracecontext,baggage"` log := setup(t, nil) assert.Regexp(substring, log) }) From 97af9d4fc5430d394addd9dd67bd7dbb250877ec Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Wed, 5 Feb 2025 14:54:47 -0500 Subject: [PATCH 10/19] fix span links error --- ddtrace/tracer/textmap.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index 68255facb3..fe82e199b9 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -328,6 +328,10 @@ func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, e } } } else { // Trace IDs do not match - create span links + // if the propagator is baggage and baggage is empty we don't want to add span links + if _, ok := v.(*propagatorBaggage); ok && len(extractedCtx2.baggage) == 0 { + continue + } link := ddtrace.SpanLink{TraceID: extractedCtx2.TraceID(), SpanID: extractedCtx2.SpanID(), TraceIDHigh: extractedCtx2.TraceIDUpper(), Attributes: map[string]string{"reason": "terminated_context", "context_headers": getPropagatorName(v)}} // causing errors w/ baggage if trace := extractedCtx2.trace; trace != nil { if flags := uint32(*trace.priority); flags > 0 { // Set the flags based on the sampling priority From c2f903542194471003f166b858f31cdc5a91fbf0 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Wed, 5 Feb 2025 15:31:05 -0500 Subject: [PATCH 11/19] api security errors --- contrib/internal/httptrace/httptrace.go | 31 +++++++++++++++---------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 2e3e2defe9..116e48f4ec 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -86,6 +86,25 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. } } + // Propagate baggage *before* starting the span. + if inferredProxySpan == nil { + spanParentCtx, spanParentErr := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)) + if spanParentErr == nil { + baggageMap := make(map[string]string) + spanParentCtx.ForeachBaggageItem(func(k, v string) bool { + baggageMap[k] = v + return true + }) + if len(baggageMap) > 0 { + ctx := r.Context() + for k, v := range baggageMap { + ctx = baggage.Set(ctx, k, v) + } + r = r.WithContext(ctx) + } + } + } + nopts = append(nopts, func(ssCfg *ddtrace.StartSpanConfig) { if ssCfg.Tags == nil { @@ -110,18 +129,6 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. } tracer.ChildOf(spanParentCtx)(ssCfg) } - baggageMap := make(map[string]string) - spanParentCtx.ForeachBaggageItem(func(k, v string) bool { - baggageMap[k] = v - return true - }) - if len(baggageMap) > 0 { - ctx := r.Context() - for k, v := range baggageMap { - ctx = baggage.Set(ctx, k, v) - } - r = r.WithContext(ctx) - } } for k, v := range ipTags { From 60cec063683a6abcbda7fbfb2220b8ffb29396aa Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Wed, 5 Feb 2025 15:43:32 -0500 Subject: [PATCH 12/19] assertion error in baggage test --- ddtrace/tracer/textmap_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ddtrace/tracer/textmap_test.go b/ddtrace/tracer/textmap_test.go index f6a063faa3..65e3ded6b8 100644 --- a/ddtrace/tracer/textmap_test.go +++ b/ddtrace/tracer/textmap_test.go @@ -2702,8 +2702,10 @@ func TestInjectBaggagePropagatorEncoding(t *testing.T) { carrier := HTTPHeadersCarrier(headers) err := tracer.Inject(ctx, carrier) assert.Nil(err) - - assert.Equal(headers.Get("baggage"), "userId=Am%C3%A9lie,serverNode=DF+28") + actualBaggage := headers.Get("baggage") + // Instead of checking equality of the whole string, assert that both key/value pairs are present. + assert.Contains(actualBaggage, "userId=Am%C3%A9lie") + assert.Contains(actualBaggage, "serverNode=DF+28") } func TestInjectBaggagePropagatorEncodingSpecialCharacters(t *testing.T) { From 1a28783bd56fbcc03c004013f6107ec5b1490ba2 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Wed, 5 Feb 2025 15:57:09 -0500 Subject: [PATCH 13/19] errors with extract logic --- ddtrace/tracer/textmap.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index fe82e199b9..1b831262bb 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -292,6 +292,14 @@ func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, e firstExtract := (ctx == nil) // ctx stores the most recently extracted ctx across iterations; if it's nil, no extractor has run yet extractedCtx, err := v.Extract(carrier) + // If the extractor is the baggage propagator and its baggage is empty, + // treat it as if nothing was extracted. + if _, ok := v.(*propagatorBaggage); ok { + if extractedSpan, ok := extractedCtx.(*spanContext); ok && len(extractedSpan.baggage) == 0 { + extractedCtx = nil + } + } + if firstExtract { if err != nil { if p.onlyExtractFirst { // Every error is relevant when we are relying on the first extractor From 4b11b7beb52671747f77de27e5a5201f80332953 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Wed, 5 Feb 2025 16:27:57 -0500 Subject: [PATCH 14/19] data race errors --- ddtrace/tracer/textmap.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index 1b831262bb..e0bc768b4c 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -1370,17 +1370,28 @@ func (p *propagatorBaggage) Inject(spanCtx ddtrace.SpanContext, carrier interfac // joined together by commas, func (*propagatorBaggage) injectTextMap(spanCtx ddtrace.SpanContext, writer TextMapWriter) error { ctx, _ := spanCtx.(*spanContext) + if ctx == nil { + return nil + } + + // Copy the baggage map under the read lock to avoid data races. + ctx.mu.RLock() + baggageCopy := make(map[string]string, len(ctx.baggage)) + for k, v := range ctx.baggage { + baggageCopy[k] = v + } + ctx.mu.RUnlock() - // If the context is nil or has no baggage, we don't need to inject anything. - if ctx == nil || len(ctx.baggage) == 0 { + // If the baggage is empty, do nothing. + if len(baggageCopy) == 0 { return nil } - baggageItems := make([]string, 0, len(ctx.baggage)) + baggageItems := make([]string, 0, len(baggageCopy)) totalSize := 0 count := 0 - for key, value := range ctx.baggage { + for key, value := range baggageCopy { if count >= baggageMaxItems { log.Warn("Baggage item limit exceeded. Only the first %d items will be propagated.", baggageMaxItems) break @@ -1392,7 +1403,7 @@ func (*propagatorBaggage) injectTextMap(spanCtx ddtrace.SpanContext, writer Text itemSize := len(item) if count > 0 { - itemSize++ // for the comma + itemSize++ // account for the comma separator } if totalSize+itemSize > baggageMaxBytes { From 8b30fdebf84307b6a561524a10742343776fa484 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Thu, 6 Feb 2025 16:33:00 -0500 Subject: [PATCH 15/19] default propagator tests --- ddtrace/tracer/tracer_test.go | 72 +++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/ddtrace/tracer/tracer_test.go b/ddtrace/tracer/tracer_test.go index dd1820621e..d1870240b3 100644 --- a/ddtrace/tracer/tracer_test.go +++ b/ddtrace/tracer/tracer_test.go @@ -936,6 +936,78 @@ func TestPropagationDefaults(t *testing.T) { assert.Equal(*child.context.trace.priority, -1.) } +func TestPropagationDefaultIncludesBaggage(t *testing.T) { + assert := assert.New(t) + + tracer := newTracer() + defer tracer.Stop() + root := tracer.StartSpan("web.request").(*span) + root.SetBaggageItem("foo", "bar") + root.SetTag(ext.SamplingPriority, -1) + ctx := root.Context().(*spanContext) + headers := http.Header{} + + // inject the spanContext + carrier := HTTPHeadersCarrier(headers) + err := tracer.Inject(ctx, carrier) + assert.Nil(err) + + tid := strconv.FormatUint(root.TraceID, 10) + pid := strconv.FormatUint(root.SpanID, 10) + + assert.Equal(headers.Get(DefaultTraceIDHeader), tid) + assert.Equal(headers.Get(DefaultParentIDHeader), pid) + assert.Equal(headers.Get(DefaultPriorityHeader), "-1") + assert.Equal(headers.Get(DefaultBaggageHeader), "foo=bar") + + // retrieve the spanContext + propagated, err := tracer.Extract(carrier) + assert.Nil(err) + pctx := propagated.(*spanContext) + + // compare if there is a Context match + assert.Equal(ctx.traceID, pctx.traceID) + assert.Equal(ctx.spanID, pctx.spanID) + assert.Equal(*ctx.trace.priority, -1.) + assert.Equal(ctx.baggage, pctx.baggage) + + // ensure a child can be created + child := tracer.StartSpan("db.query", ChildOf(propagated)).(*span) + + assert.NotEqual(uint64(0), child.TraceID) + assert.NotEqual(uint64(0), child.SpanID) + assert.Equal(root.SpanID, child.ParentID) + assert.Equal(root.TraceID, child.ParentID) + assert.Equal(*child.context.trace.priority, -1.) +} + +func TestPropagationStyleOnlyBaggage(t *testing.T) { + t.Setenv(headerPropagationStyle, "baggage") + assert := assert.New(t) + + tracer := newTracer() + defer tracer.Stop() + root := tracer.StartSpan("web.request").(*span) + root.SetBaggageItem("foo", "bar") + ctx := root.Context().(*spanContext) + headers := http.Header{} + + // inject the spanContext + carrier := HTTPHeadersCarrier(headers) + err := tracer.Inject(ctx, carrier) + assert.Nil(err) + + assert.Equal(headers.Get(DefaultBaggageHeader), "foo=bar") + + // retrieve the spanContext + propagated, err := tracer.Extract(carrier) + assert.Nil(err) + pctx := propagated.(*spanContext) + + // compare if there is a Context match + assert.Equal(ctx.baggage, pctx.baggage) +} + func TestTracerSamplingPriorityPropagation(t *testing.T) { assert := assert.New(t) tracer := newTracer() From 8c5cf4fd40a48b8665bbde567985d6d06a95c2db Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Fri, 7 Feb 2025 11:46:00 -0500 Subject: [PATCH 16/19] span links --- ddtrace/tracer/textmap.go | 6 +----- ddtrace/tracer/textmap_test.go | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index e0bc768b4c..88169423c2 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -336,11 +336,7 @@ func (p *chainedPropagator) Extract(carrier interface{}) (ddtrace.SpanContext, e } } } else { // Trace IDs do not match - create span links - // if the propagator is baggage and baggage is empty we don't want to add span links - if _, ok := v.(*propagatorBaggage); ok && len(extractedCtx2.baggage) == 0 { - continue - } - link := ddtrace.SpanLink{TraceID: extractedCtx2.TraceID(), SpanID: extractedCtx2.SpanID(), TraceIDHigh: extractedCtx2.TraceIDUpper(), Attributes: map[string]string{"reason": "terminated_context", "context_headers": getPropagatorName(v)}} // causing errors w/ baggage + link := ddtrace.SpanLink{TraceID: extractedCtx2.TraceID(), SpanID: extractedCtx2.SpanID(), TraceIDHigh: extractedCtx2.TraceIDUpper(), Attributes: map[string]string{"reason": "terminated_context", "context_headers": getPropagatorName(v)}} if trace := extractedCtx2.trace; trace != nil { if flags := uint32(*trace.priority); flags > 0 { // Set the flags based on the sampling priority link.Flags = 1 diff --git a/ddtrace/tracer/textmap_test.go b/ddtrace/tracer/textmap_test.go index 65e3ded6b8..6ad8850df8 100644 --- a/ddtrace/tracer/textmap_test.go +++ b/ddtrace/tracer/textmap_test.go @@ -2044,7 +2044,6 @@ func TestSpanLinks(t *testing.T) { assert.True(ok) assert.Equal(traceIDFrom64Bits(1), sctx.traceID) - // in attributes baggage i added to context_headers so the length is no longer 0 but 1 assert.Len(sctx.spanLinks, 0) }) } From 35ccc89668821cf5b1dbc4a34ba13b80837277e0 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Fri, 7 Feb 2025 16:58:16 -0500 Subject: [PATCH 17/19] change startRequestSpan in httptrace --- contrib/internal/httptrace/httptrace.go | 32 ++++++++++--------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 116e48f4ec..7ee79c618d 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -86,25 +86,6 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. } } - // Propagate baggage *before* starting the span. - if inferredProxySpan == nil { - spanParentCtx, spanParentErr := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)) - if spanParentErr == nil { - baggageMap := make(map[string]string) - spanParentCtx.ForeachBaggageItem(func(k, v string) bool { - baggageMap[k] = v - return true - }) - if len(baggageMap) > 0 { - ctx := r.Context() - for k, v := range baggageMap { - ctx = baggage.Set(ctx, k, v) - } - r = r.WithContext(ctx) - } - } - } - nopts = append(nopts, func(ssCfg *ddtrace.StartSpanConfig) { if ssCfg.Tags == nil { @@ -128,6 +109,19 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. tracer.WithSpanLinks(spanLinksCtx.SpanLinks())(ssCfg) } tracer.ChildOf(spanParentCtx)(ssCfg) + + baggageMap := make(map[string]string) + spanParentCtx.ForeachBaggageItem(func(k, v string) bool { + baggageMap[k] = v + return true + }) + if len(baggageMap) > 0 { + ctx := r.Context() + for k, v := range baggageMap { + ctx = baggage.Set(ctx, k, v) + } + r = r.WithContext(ctx) + } } } From aae5fc627ebc9f4ca02d0ddcd5fcdb1b3d68b0a4 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Mon, 10 Feb 2025 13:38:41 -0500 Subject: [PATCH 18/19] adding comments --- ddtrace/tracer/textmap.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ddtrace/tracer/textmap.go b/ddtrace/tracer/textmap.go index 88169423c2..949ffd5e2f 100644 --- a/ddtrace/tracer/textmap.go +++ b/ddtrace/tracer/textmap.go @@ -93,7 +93,8 @@ const ( // or text maps to store the sampling priority value. DefaultPriorityHeader = "x-datadog-sampling-priority" - // add comment here - Rachel + // DefaultBaggageHeader specifies the key that will be used in HTTP headers + // or text maps to store the baggage value. DefaultBaggageHeader = "baggage" ) @@ -133,7 +134,8 @@ type PropagatorConfig struct { // See https://github.com/openzipkin/b3-propagation B3 bool - // add comment here - Rachel + // BaggageHeader specifies the map key that will be used to store the baggage key-value pairs. + // It defaults to DefaultBaggageHeader. BaggageHeader string } From 03350df1e1c83cf522eaa2938196bb5edbbc0455 Mon Sep 17 00:00:00 2001 From: Rachel Yang Date: Thu, 13 Feb 2025 10:55:58 -0500 Subject: [PATCH 19/19] benchmark --- contrib/internal/httptrace/httptrace.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 7ee79c618d..403089058c 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -110,8 +110,12 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. } tracer.ChildOf(spanParentCtx)(ssCfg) - baggageMap := make(map[string]string) + var baggageMap map[string]string spanParentCtx.ForeachBaggageItem(func(k, v string) bool { + // Make the map only if we actually discover any baggage items. + if baggageMap == nil { + baggageMap = make(map[string]string) + } baggageMap[k] = v return true })