From ce40b4326c96dd5b16be88a2076d4dc532411ff4 Mon Sep 17 00:00:00 2001 From: Zarir Hamza Date: Wed, 15 Jan 2025 00:32:41 -0500 Subject: [PATCH] adds span links for inferred spans --- contrib/internal/httptrace/httptrace.go | 16 +++- .../httptrace/httptrace_api_gateway_test.go | 73 ++----------------- contrib/internal/httptrace/inferred_proxy.go | 43 ++++++----- 3 files changed, 47 insertions(+), 85 deletions(-) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index dd4441184c..39474fabb0 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -42,8 +42,18 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. } nopts := make([]ddtrace.StartSpanOption, 0, len(opts)+1+len(ipTags)) + inferredStartSpanOpts := make([]ddtrace.StartSpanOption, 0, 1) + spanParentCtx, spanParentErr := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)) + var spanLinksCtx ddtrace.SpanContextWithLinks + if spanParentErr == nil && spanParentCtx.(ddtrace.SpanContextWithLinks) != nil { + // If there are span links as a result of context extraction, add them as a StartSpanOption + if spanLinksCtx = spanParentCtx.(ddtrace.SpanContextWithLinks); spanLinksCtx.SpanLinks() != nil { + inferredStartSpanOpts = append(inferredStartSpanOpts, tracer.WithSpanLinks(spanLinksCtx.SpanLinks())) + } + } + var inferredProxySpan tracer.Span inferredProxySpanCreated := false @@ -52,7 +62,7 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. } if cfg.inferredProxyServicesEnabled && !inferredProxySpanCreated { - if inferredProxySpan = tryCreateInferredProxySpan(r.Header, spanParentCtx); inferredProxySpan != nil { + if inferredProxySpan = tryCreateInferredProxySpan(r.Header, spanParentCtx, inferredStartSpanOpts...); inferredProxySpan != nil { inferredProxySpanCreated = true spanParentCtx = inferredProxySpan.Context() } @@ -77,6 +87,10 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. cfg.Parent = spanParentCtx } + if spanParentErr != nil && !inferredProxySpanCreated { + cfg.SpanLinks = spanLinksCtx.SpanLinks() + } + for k, v := range ipTags { cfg.Tags[k] = v } diff --git a/contrib/internal/httptrace/httptrace_api_gateway_test.go b/contrib/internal/httptrace/httptrace_api_gateway_test.go index 1adc671ea9..8694798077 100644 --- a/contrib/internal/httptrace/httptrace_api_gateway_test.go +++ b/contrib/internal/httptrace/httptrace_api_gateway_test.go @@ -14,7 +14,6 @@ import ( as "github.com/stretchr/testify/assert" ) -var appListener *httptest.Server var inferredHeaders = map[string]string{ "x-dd-proxy": "aws-apigateway", "x-dd-proxy-request-time-ms": "1729780025473", @@ -25,7 +24,7 @@ var inferredHeaders = map[string]string{ } // mock the aws server -func loadTest(t *testing.T) { +func loadTest(t *testing.T) *httptest.Server { // Set environment variables t.Setenv("DD_SERVICE", "aws-server") t.Setenv("DD_TRACE_INFERRED_PROXY_SERVICES_ENABLED", "true") @@ -50,24 +49,16 @@ func loadTest(t *testing.T) { } }) - appListener = httptest.NewServer(mux) + return httptest.NewServer(mux) } -func cleanupTest() { - // close server - if appListener != nil { - appListener.Close() - } -} - func TestInferredProxySpans(t *testing.T) { t.Run("should create parent and child spans for a 200", func(t *testing.T) { mt := mocktracer.Start() defer mt.Stop() - loadTest(t) - defer cleanupTest() + appListener := loadTest(t) client := &http.Client{} req, err := http.NewRequest("GET", fmt.Sprintf("%s/", appListener.URL), nil) @@ -115,8 +106,7 @@ func TestInferredProxySpans(t *testing.T) { t.Run("should create parent and child spans for error", func(t *testing.T) { mt := mocktracer.Start() defer mt.Stop() - loadTest(t) - defer cleanupTest() + appListener := loadTest(t) client := &http.Client{} req, err := http.NewRequest("GET", fmt.Sprintf("%s/error", appListener.URL), nil) @@ -160,8 +150,7 @@ func TestInferredProxySpans(t *testing.T) { t.Run("should not create API Gateway span if headers are missing", func(t *testing.T) { mt := mocktracer.Start() defer mt.Stop() - loadTest(t) - defer cleanupTest() + appListener := loadTest(t) client := &http.Client{} req, err := http.NewRequest("GET", fmt.Sprintf("%s/no-aws-headers", appListener.URL), nil) @@ -172,6 +161,7 @@ func TestInferredProxySpans(t *testing.T) { _, _, finishSpans := StartRequestSpan(req) resp, err := client.Do(req) finishSpans(resp.StatusCode, nil) + assert.NoError(err) assert.Equal(http.StatusOK, resp.StatusCode) @@ -184,8 +174,7 @@ func TestInferredProxySpans(t *testing.T) { t.Run("should not create API Gateway span if x-dd-proxy is missing", func(t *testing.T) { mt := mocktracer.Start() defer mt.Stop() - loadTest(t) - defer cleanupTest() + appListener := loadTest(t) client := &http.Client{} req, err := http.NewRequest("GET", fmt.Sprintf("%s/no-aws-headers", appListener.URL), nil) @@ -211,52 +200,4 @@ func TestInferredProxySpans(t *testing.T) { assert.Equal("http.request", spans[0].OperationName()) }) - - t.Run("should create only 1 API Gateway span", func(t *testing.T) { - mt := mocktracer.Start() - defer mt.Stop() - loadTest(t) - defer cleanupTest() - - client := &http.Client{} - req, err := http.NewRequest("GET", fmt.Sprintf("%s/", appListener.URL), nil) - - assert := as.New(t) - assert.NoError(err) - - for k, v := range inferredHeaders { - req.Header.Set(k, v) - } - - cfg = newConfig() - _, _, finishSpans := StartRequestSpan(req) - resp, err := client.Do(req) - finishSpans(resp.StatusCode, nil) - - spans := mt.FinishedSpans() - - assert.NoError(err) - assert.Equal(http.StatusOK, resp.StatusCode) - - assert.Equal(2, len(spans)) - gatewaySpan := spans[1] - webReqSpan := spans[0] - assert.Equal("aws.apigateway", gatewaySpan.OperationName()) - assert.Equal("http.request", webReqSpan.OperationName()) - assert.True(webReqSpan.ParentID() == gatewaySpan.SpanID()) - for _, arg := range inferredHeaders { - header, tag := normalizer.HeaderTag(arg) - - // Default to an empty string if the tag does not exist - gatewaySpanTags, exists := gatewaySpan.Tags()[tag] - if !exists { - gatewaySpanTags = "" - } - expectedTags := strings.Join(req.Header.Values(header), ",") - // compare expected and actual values - assert.Equal(expectedTags, gatewaySpanTags) - } - - assert.Equal(2, len(spans)) - }) } diff --git a/contrib/internal/httptrace/inferred_proxy.go b/contrib/internal/httptrace/inferred_proxy.go index 910145d55e..e8e890b686 100644 --- a/contrib/internal/httptrace/inferred_proxy.go +++ b/contrib/internal/httptrace/inferred_proxy.go @@ -13,6 +13,8 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/log" ) +type StartSpanOption = ddtrace.StartSpanOption + const ( ProxyHeaderSystem = "X-Dd-Proxy" ProxyHeaderStartTimeMs = "X-Dd-Proxy-Request-Time-Ms" @@ -74,7 +76,7 @@ func extractInferredProxyContext(headers http.Header) *ProxyContext { } -func tryCreateInferredProxySpan(headers http.Header, parent ddtrace.SpanContext) tracer.Span { +func tryCreateInferredProxySpan(headers http.Header, parent ddtrace.SpanContext, opts ...StartSpanOption) tracer.Span { if headers == nil { log.Debug("Headers do not exist") return nil @@ -109,25 +111,30 @@ func tryCreateInferredProxySpan(headers http.Header, parent ddtrace.SpanContext) configService = globalconfig.ServiceName() } - config := ddtrace.StartSpanConfig{ - Parent: parent, - StartTime: parsedTime, - Tags: map[string]interface{}{ - ext.SpanType: ext.SpanTypeWeb, - ext.ServiceName: configService, - ext.Component: proxySpanInfo.Component, - ext.HTTPMethod: requestProxyContext.Method, - ext.HTTPURL: requestProxyContext.DomainName + requestProxyContext.Path, - ext.HTTPRoute: requestProxyContext.Path, - ext.ResourceName: fmt.Sprintf("%s %s", requestProxyContext.Method, requestProxyContext.Path), - "stage": requestProxyContext.Stage, + optsLocal := make([]StartSpanOption, len(opts), len(opts)+1) + copy(optsLocal, opts) + + optsLocal = append(optsLocal, + func(cfg *ddtrace.StartSpanConfig) { + if cfg.Tags == nil { + cfg.Tags = make(map[string]interface{}) + } + + cfg.Parent = parent + cfg.StartTime = parsedTime + + cfg.Tags[ext.SpanType] = ext.SpanTypeWeb + cfg.Tags[ext.ServiceName] = configService + cfg.Tags[ext.Component] = proxySpanInfo.Component + cfg.Tags[ext.HTTPMethod] = requestProxyContext.Method + cfg.Tags[ext.HTTPURL] = requestProxyContext.DomainName + requestProxyContext.Path + cfg.Tags[ext.HTTPRoute] = requestProxyContext.Path + cfg.Tags[ext.ResourceName] = fmt.Sprintf("%s %s", requestProxyContext.Method, requestProxyContext.Path) + cfg.Tags["stage"] = requestProxyContext.Stage }, - } + ) - span := tracer.StartSpan(proxySpanInfo.SpanName, tracer.StartTime(config.StartTime), tracer.ChildOf(config.Parent), tracer.Tag("service", config.Tags[ext.ServiceName])) - for k, v := range config.Tags { - span.SetTag(k, v) - } + span := tracer.StartSpan(proxySpanInfo.SpanName, optsLocal...) return span }