-
Notifications
You must be signed in to change notification settings - Fork 443
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
base: main
Are you sure you want to change the base?
Add inferred proxy spans #3052
Changes from all commits
6fb5400
9762bfd
63c8418
5f16ca4
10ed7f7
923d28b
b356a70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,201 @@ | ||
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"}) | ||
} | ||
}) | ||
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()) | ||
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) | ||
|
||
// Default to an empty string if the tag does not exist | ||
gateway_span_tags, exists := gateway_span.Tags()[tag] | ||
if !exists { | ||
gateway_span_tags = "" | ||
} | ||
expected_tags := strings.Join(req.Header.Values(header), ",") | ||
// compare expected and actual values | ||
assert.Equal(expected_tags, gateway_span_tags) | ||
} | ||
|
||
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) | ||
|
||
// Default to an empty string if the tag does not exist | ||
gateway_span_tags, exists := gateway_span.Tags()[tag] | ||
if !exists { | ||
gateway_span_tags = "" | ||
} | ||
expected_tags := strings.Join(req.Header.Values(header), ",") | ||
// compare expected and actual values | ||
assert.Equal(expected_tags, 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()) | ||
|
||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
package httptrace | ||
|
||
import ( | ||
"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" | ||
"gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig" | ||
"gopkg.in/DataDog/dd-trace-go.v1/internal/log" | ||
) | ||
|
||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
log.Debug("Proxy header start time does not exist") | ||
return nil | ||
} | ||
|
||
proxyHeaderSystem, exists := headers[PROXY_HEADER_SYSTEM] | ||
if !exists { | ||
log.Debug("Proxy header system does not exist") | ||
return nil | ||
} | ||
if _, ok := supportedProxies[proxyHeaderSystem[0]]; !ok { | ||
log.Debug("Unsupported Proxy header system") | ||
return nil | ||
} | ||
|
||
// Q: is it possible to have multiple values for any of these http headers?? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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 { | ||
if headers == nil { | ||
log.Debug("Headers do not exist") | ||
return nil | ||
|
||
} | ||
if !internal.BoolEnv(inferredProxyServicesEnabled, false) { | ||
log.Debug("The inferred proxy services are not enabled") | ||
return nil | ||
} | ||
|
||
requestProxyContext := extractInferredProxyContext(headers) | ||
if requestProxyContext == nil { | ||
log.Debug("Unabole to extract inferred proxy context") | ||
return nil | ||
} | ||
|
||
proxySpanInfo := supportedProxies[requestProxyContext.ProxySystemName] | ||
log.Debug(`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 { | ||
log.Debug("Error parsing time string: %v", 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) | ||
|
||
configService := requestProxyContext.DomainName | ||
if configService == "" { | ||
configService = globalconfig.ServiceName() | ||
} | ||
config := ddtrace.StartSpanConfig{ | ||
Parent: parent, | ||
StartTime: parsedTime, | ||
Tags: map[string]interface{}{ | ||
"service": configService, | ||
"component": proxySpanInfo.Component, | ||
"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() | ||
} |
There was a problem hiding this comment.
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:
I'm curious if you can assert that the inferred span in Go follows the underlying span created by httptrace.
There was a problem hiding this comment.
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 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())
There was a problem hiding this comment.
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: