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

Add inferred proxy spans #3052

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 3 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
3 changes: 3 additions & 0 deletions contrib/internal/httptrace/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ const (
envTraceClientIPEnabled = "DD_TRACE_CLIENT_IP_ENABLED"
// envServerErrorStatuses is the name of the env var used to specify error status codes on http server spans
envServerErrorStatuses = "DD_TRACE_HTTP_SERVER_ERROR_STATUSES"

//used for enabling inferred span tracing
inferredProxyServicesEnabled = "DD_TRACE_INFERRED_PROXY_SERVICES_ENABLED"
)

// defaultQueryStringRegexp is the regexp used for query string obfuscation if `envQueryStringRegexp` is empty.
Expand Down
17 changes: 16 additions & 1 deletion contrib/internal/httptrace/httptrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer.
if cfg.Tags == nil {
cfg.Tags = make(map[string]interface{})
}

cfg.Tags[ext.SpanType] = ext.SpanTypeWeb
cfg.Tags[ext.HTTPMethod] = r.Method
cfg.Tags[ext.HTTPURL] = urlFromRequest(r)
Expand All @@ -50,9 +51,23 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer.
if r.Host != "" {
cfg.Tags["http.host"] = r.Host
}
if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)); err == nil {

spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header))
is_inferred_proxy_set := false
println("IN HERE inferredProxyEnabled is:")
jordan-wong marked this conversation as resolved.
Show resolved Hide resolved
println(internal.BoolEnv(inferredProxyServicesEnabled, false))
if internal.BoolEnv(inferredProxyServicesEnabled, false) {
if inferred_proxy_span_ctx := tryCreateInferredProxySpan(r.Header, spanctx); inferred_proxy_span_ctx != nil {
println("IN HERE")
cfg.Parent = inferred_proxy_span_ctx
is_inferred_proxy_set = true
}
}

if err != nil && !is_inferred_proxy_set {
cfg.Parent = spanctx
}

for k, v := range ipTags {
cfg.Tags[k] = v
}
Expand Down
196 changes: 196 additions & 0 deletions contrib/internal/httptrace/httptrace_api_gateway_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
package httptrace

import (
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer"
"gopkg.in/DataDog/dd-trace-go.v1/internal/normalizer"

"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",
"x-dd-proxy-path": "/test",
"x-dd-proxy-httpmethod": "GET",
"x-dd-proxy-domain-name": "example.com",
"x-dd-proxy-stage": "dev",
}

// mock the aws server
func loadTest(t *testing.T) {
// Set environment variables
t.Setenv("DD_SERVICE", "aws-server")
t.Setenv("DD_TRACE_INFERRED_PROXY_SERVICES_ENABLED", "true")

// set up http server
mux := http.NewServeMux()

// set routes
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/error" {
w.WriteHeader(http.StatusInternalServerError)
_ = json.NewEncoder(w).Encode(map[string]string{"message": "ERROR"})
} else {
w.WriteHeader(http.StatusOK)
_ = json.NewEncoder(w).Encode(map[string]string{"message": "OK"})
jordan-wong marked this conversation as resolved.
Show resolved Hide resolved
}
})
appListener = 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()

client := &http.Client{}
req, err := http.NewRequest("GET", fmt.Sprintf("%s/", appListener.URL), nil)

assert := assert.New(t)
assert.NoError(err)

for k, v := range inferredHeaders {
req.Header.Set(k, v)
}

sp, _ := StartRequestSpan(req)
resp, err := client.Do(req)
FinishRequestSpan(sp, resp.StatusCode, nil)

spans := mt.FinishedSpans()

assert.NoError(err)
assert.Equal(http.StatusOK, resp.StatusCode)

assert.Equal(2, len(spans))
gateway_span := spans[0]
web_req_span := spans[1]
assert.Equal("aws.apigateway", gateway_span.OperationName())
Copy link

Choose a reason for hiding this comment

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

Can you add tests to make sure the other aspects of the span metadata is present? Some examples can be found in the JS version: https://github.com/DataDog/dd-trace-js/pull/4837/files#diff-30cfe20da922154edf4288007e440ef1fc0873c26813eb2cc7c78e064024ebd0 .

Namely, span metadata like:

  • component
  • http.* (like http.url, http.status_codes)

I'm curious if you can assert that the inferred span in Go follows the underlying span created by httptrace.

Copy link
Author

@jordan-wong jordan-wong Jan 7, 2025

Choose a reason for hiding this comment

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

Currently working through a bug I found related to this in expected tag assertion

I'm curious if you can assert that the inferred span in Go follows the underlying span created by httptrace.

I am assuming you mean asserting that httptrace is a parent of the inferredSpan? - I have a line that checks this https://github.com/DataDog/dd-trace-go/pull/3052/files#diff-5c136a85a92fba2b6b3bdb1a3b8f5c43ed87d3dcb3d65a4bab64d68df68b769cR88
assert.True(web_req_span.ParentID() == gateway_span.SpanID())

Copy link

Choose a reason for hiding this comment

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

Sorry, to clarify, I mean asserting that the httptrace span and the inferred span have matching:

  • status codes
  • http urls
  • type

assert.Equal("http.request", web_req_span.OperationName())
assert.True(web_req_span.ParentID() == gateway_span.SpanID())
for _, arg := range inferredHeaders {
header, tag := normalizer.HeaderTag(arg)
gateway_span_tags, ok := gateway_span.Tags()[tag]
if !ok {
gateway_span_tags = ""
}
assert.Equal(strings.Join(req.Header.Values(header), ","), gateway_span_tags)
jordan-wong marked this conversation as resolved.
Show resolved Hide resolved
}

assert.Equal(2, len(spans))

})

t.Run("should create parent and child spans for error", 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/error", appListener.URL), nil)
assert := assert.New(t)
assert.NoError(err)
for k, v := range inferredHeaders {
req.Header.Set(k, v)
}

sp, _ := StartRequestSpan(req)
resp, err := client.Do(req)
FinishRequestSpan(sp, resp.StatusCode, nil)

assert.NoError(err)
assert.Equal(http.StatusInternalServerError, resp.StatusCode)

spans := mt.FinishedSpans()
assert.Equal(2, len(spans))
gateway_span := spans[0]
web_req_span := spans[1]
assert.Equal("aws.apigateway", gateway_span.OperationName())
assert.Equal("http.request", web_req_span.OperationName())
assert.True(web_req_span.ParentID() == gateway_span.SpanID())
for _, arg := range inferredHeaders {
header, tag := normalizer.HeaderTag(arg)
gateway_span_tags, ok := gateway_span.Tags()[tag]
if !ok {
gateway_span_tags = ""
}
assert.Equal(strings.Join(req.Header.Values(header), ","), gateway_span_tags)
}
assert.Equal(2, len(spans))

})

t.Run("should not create API Gateway spanif headers are missing", 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/no-aws-headers", appListener.URL), nil)
assert := assert.New(t)
assert.NoError(err)

sp, _ := StartRequestSpan(req)
resp, err := client.Do(req)
FinishRequestSpan(sp, resp.StatusCode, nil)
assert.NoError(err)
assert.Equal(http.StatusOK, resp.StatusCode)

spans := mt.FinishedSpans()
assert.Equal(1, len(spans))
assert.Equal("http.request", spans[0].OperationName())

})
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()

client := &http.Client{}
req, err := http.NewRequest("GET", fmt.Sprintf("%s/no-aws-headers", appListener.URL), nil)
assert := assert.New(t)
assert.NoError(err)

for k, v := range inferredHeaders {
if k != "x-dd-proxy" {
req.Header.Set(k, v)
}
}

sp, _ := StartRequestSpan(req)
resp, err := client.Do(req)
FinishRequestSpan(sp, resp.StatusCode, nil)

assert.NoError(err)
assert.Equal(http.StatusOK, resp.StatusCode)

spans := mt.FinishedSpans()
assert.Equal(1, len(spans))
assert.Equal("http.request", spans[0].OperationName())

})

//loadTest(nil)

}
143 changes: 143 additions & 0 deletions contrib/internal/httptrace/inferred_proxy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
package httptrace

import (
"fmt"
"net/http"
"strconv"
"time"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
"gopkg.in/DataDog/dd-trace-go.v1/internal"
)

const (
PROXY_HEADER_SYSTEM = "X-Dd-Proxy"
//PROXY_HEADER_START_TIME_MS = "x-dd-proxy-request-time-ms"
PROXY_HEADER_START_TIME_MS = "X-Dd-Proxy-Request-Time-Ms"
PROXY_HEADER_PATH = "X-Dd-Proxy-Path"
PROXY_HEADER_HTTPMETHOD = "X-Dd-Proxy-Httpmethod"
PROXY_HEADER_DOMAIN = "X-Dd-Proxy-Domain-Name"
PROXY_HEADER_STAGE = "X-Dd-Proxy-Stage"
Comment on lines +16 to +22
Copy link

Choose a reason for hiding this comment

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

Just a question: Is this casing Go syntax specific? I noticed in the test that it'd work with x-dd-proxy-path.

Copy link
Author

Choose a reason for hiding this comment

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

Golang's http library parses and returns http request header content in "Canonical Form" with each word capitalized, so when accessing headers from a request we need to reference the exact spelling. https://ron-liu.medium.com/what-canonical-http-header-mean-in-golang-2e97f854316d

In the tests we are passing in the headers to create a new Request, so the http library automatically formats/capitalizes our headers to canonical form for us even if we pass in lowercased strings.

)

type ProxyDetails struct {
SpanName string `json:"spanName"`
Component string `json:"component"`
}

var (
supportedProxies = map[string]ProxyDetails{
"aws-apigateway": {
SpanName: "aws.apigateway",
Component: "aws-apigateway",
},
}
)

type ProxyContext struct {
RequestTime string `json:"requestTime"`
Method string `json:"method"`
Path string `json:"path"`
Stage string `json:"stage"`
DomainName string `json:"domainName"`
ProxySystemName string `json:"proxySystemName"`
}

func extractInferredProxyContext(headers http.Header) *ProxyContext {
//proxyContent := make(map[string][]string)

_, exists := headers[PROXY_HEADER_START_TIME_MS]
if !exists {
println("no proxy header start time")
return nil
}

proxyHeaderSystem, exists := headers[PROXY_HEADER_SYSTEM]
if !exists {
println("no proxy header system")
return nil
}
if _, ok := supportedProxies[proxyHeaderSystem[0]]; !ok {
println("unsupported Proxy header system")
return nil
}

// Q: is it possible to have multiple values for any of these http headers??
Copy link

Choose a reason for hiding this comment

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

Is this comment asking if it's possible for something like domain to have multiple "domain values"? At the moment, I don't think so.

Copy link
Member

@darccio darccio Jan 9, 2025

Choose a reason for hiding this comment

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

HTTP headers may have multiple values but it's up to specs to treat each header as a concrete value or a list of them, according to RFC 7231:

Authors of specifications defining new header fields are advised to consider documenting:

  • Whether the field is a single value or whether it can be a list (delimited by commas; see Section 3.2 of [RFC7230]). If it does not use the list syntax, document how to treat messages where the field occurs multiple times (a sensible default would be to ignore the field, but this might not always be the right choice).

So, whoever is emitting PROXY_HEADER_DOMAIN need to confirm if it may have multiple values or not. The same for the rest of headers.

return &ProxyContext{
RequestTime: headers[PROXY_HEADER_START_TIME_MS][0],
Method: headers[PROXY_HEADER_HTTPMETHOD][0],
Path: headers[PROXY_HEADER_PATH][0],
Stage: headers[PROXY_HEADER_STAGE][0],
DomainName: headers[PROXY_HEADER_DOMAIN][0],
ProxySystemName: headers[PROXY_HEADER_SYSTEM][0],
}

}

func tryCreateInferredProxySpan(headers http.Header, parent ddtrace.SpanContext) ddtrace.SpanContext {
println("IN TRYCREATE")
jordan-wong marked this conversation as resolved.
Show resolved Hide resolved
println("headers are:")
for key, values := range headers {
fmt.Printf("Key: %s\n", key)
println(key)
for _, value := range values {
fmt.Printf(" Value: %s\n", value)
println(value)
}
}
if headers == nil {
println("headers nil")
jordan-wong marked this conversation as resolved.
Show resolved Hide resolved
return nil

}
if !internal.BoolEnv(inferredProxyServicesEnabled, false) {
println("bool env false")
return nil
}

requestProxyContext := extractInferredProxyContext(headers)
if requestProxyContext == nil {
println("requestProxyContext nil")
return nil
}

proxySpanInfo := supportedProxies[requestProxyContext.ProxySystemName]
fmt.Printf(`Successfully extracted inferred span info ${proxyContext} for proxy: ${proxyContext.proxySystemName}`)

// Parse Time string to Time Type
millis, err := strconv.ParseInt(requestProxyContext.RequestTime, 10, 64)
if err != nil {
fmt.Println("Error parsing time string:", err)
return nil
}

// Convert milliseconds to seconds and nanoseconds
seconds := millis / 1000
nanoseconds := (millis % 1000) * int64(time.Millisecond)

// Create time.Time from Unix timestamp
parsedTime := time.Unix(seconds, nanoseconds)

config := ddtrace.StartSpanConfig{
Parent: parent,
//StartTime: requestProxyContext.RequestTime,
StartTime: parsedTime,
Tags: map[string]interface{}{
"service": requestProxyContext.DomainName,
jordan-wong marked this conversation as resolved.
Show resolved Hide resolved
"HTTP_METHOD": requestProxyContext.Method,
"PATH": requestProxyContext.Path,
"STAGE": requestProxyContext.Stage,
"DOMAIN_NAME": requestProxyContext.DomainName,
"PROXY_SYSTEM_NAME": requestProxyContext.ProxySystemName,
},
}

span := tracer.StartSpan(proxySpanInfo.SpanName, tracer.StartTime(config.StartTime), tracer.ChildOf(config.Parent), tracer.Tag("service", config.Tags["service"]))
defer span.Finish()
for k, v := range config.Tags {
span.SetTag(k, v)
}

return span.Context()
}
Loading