diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 8d9fdca727..c500b88d46 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -57,8 +57,8 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. tracer.Tag("http.host", r.Host), }, opts...) } - if ip := getClientIP(r); ip.IsValid() { - opts = append(opts, tracer.Tag(ext.HTTPClientIP, ip.String())) + for k, v := range genClientIPSpanTags(r) { + opts = append([]ddtrace.StartSpanOption{tracer.Tag(k, v)}, opts...) } if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)); err == nil { opts = append(opts, tracer.ChildOf(spanctx)) @@ -90,8 +90,31 @@ func ippref(s string) *netaddr.IPPrefix { return nil } +// genClientIPSpanTags generates the client IP related key/value map of tags that need to be added to the span. +func genClientIPSpanTags(r *http.Request) map[string]string { + tags := map[string]string{} + ip, matches := getClientIP(r) + if matches == nil { + if ip.IsValid() { + tags[ext.HTTPClientIP] = ip.String() + } + return tags + } + sb := strings.Builder{} + for hdr, ip := range matches { + tags[ext.HTTPRequestHeaders+"."+hdr] = ip + sb.WriteString(hdr + ":" + ip + ",") + } + hdrList := sb.String() + tags[ext.MultipleIPHeaders] = hdrList[:len(hdrList)-1] + return tags +} + // getClientIP attempts to find the client IP address in the given request r. -func getClientIP(r *http.Request) netaddr.IP { +// If several IP headers are present in the request, the returned IP is invalid and the map gets filled with the +// header/ip pairs for all IP headers found in the request. Otherwise, the returned map is nil. +// See https://datadoghq.atlassian.net/wiki/spaces/APS/pages/2118779066/Client+IP+addresses+resolution +func getClientIP(r *http.Request) (netaddr.IP, map[string]string) { ipHeaders := defaultIPHeaders if len(clientIPHeader) > 0 { ipHeaders = []string{clientIPHeader} @@ -108,17 +131,26 @@ func getClientIP(r *http.Request) netaddr.IP { } return netaddr.IP{} } + matches := map[string]string{} + var matchedIP netaddr.IP for _, hdr := range ipHeaders { if v := r.Header.Get(hdr); v != "" { + matches[hdr] = v if ip := check(v); ip.IsValid() { - return ip + matchedIP = ip } } } + if len(matches) == 1 { + return matchedIP, nil + } + if len(matches) > 1 { + return netaddr.IP{}, matches + } if remoteIP := parseIP(r.RemoteAddr); remoteIP.IsValid() && isGlobal(remoteIP) { - return remoteIP + return remoteIP, nil } - return netaddr.IP{} + return netaddr.IP{}, nil } func parseIP(s string) netaddr.IP { diff --git a/contrib/internal/httptrace/httptrace_test.go b/contrib/internal/httptrace/httptrace_test.go index 391d5f8dfa..28c6caced8 100644 --- a/contrib/internal/httptrace/httptrace_test.go +++ b/contrib/internal/httptrace/httptrace_test.go @@ -32,11 +32,12 @@ func TestStartRequestSpan(t *testing.T) { } type IPTestCase struct { - name string - remoteAddr string - headers map[string]string - expectedIP netaddr.IP - clientIPHeader string + name string + remoteAddr string + headers map[string]string + expectedIP netaddr.IP + expectedMatches map[string]string + clientIPHeader string } func genIPTestCases() []IPTestCase { @@ -107,14 +108,16 @@ func genIPTestCases() []IPTestCase { expectedIP: netaddr.MustParseIP(ipv4Global), }, { - name: "invalid-ipv4-recover-multi-header-1", - headers: map[string]string{"x-forwarded-for": "127..0.0.1", "forwarded-for": ipv4Global}, - expectedIP: netaddr.MustParseIP(ipv4Global), + name: "ipv4-multi-header-1", + headers: map[string]string{"x-forwarded-for": "127.0.0.1", "forwarded-for": ipv4Global}, + expectedIP: netaddr.IP{}, + expectedMatches: map[string]string{"x-forwarded-for": "127.0.0.1", "forwarded-for": ipv4Global}, }, { - name: "invalid-ipv4-recover-multi-header-2", - headers: map[string]string{"forwarded-for": ipv4Global, "x-forwarded-for": "127..0.0.1"}, - expectedIP: netaddr.MustParseIP(ipv4Global), + name: "ipv4-multi-header-2", + headers: map[string]string{"forwarded-for": ipv4Global, "x-forwarded-for": "127.0.0.1"}, + expectedIP: netaddr.IP{}, + expectedMatches: map[string]string{"x-forwarded-for": "127.0.0.1", "forwarded-for": ipv4Global}, }, { name: "invalid-ipv6", @@ -127,14 +130,16 @@ func genIPTestCases() []IPTestCase { expectedIP: netaddr.MustParseIP(ipv6Global), }, { - name: "invalid-ipv6-recover-multi-header-1", - headers: map[string]string{"x-forwarded-for": "2001:0db8:2001:zzzz::", "forwarded-for": ipv6Global}, - expectedIP: netaddr.MustParseIP(ipv6Global), + name: "ipv6-multi-header-1", + headers: map[string]string{"x-forwarded-for": "2001:0db8:2001:zzzz::", "forwarded-for": ipv6Global}, + expectedIP: netaddr.IP{}, + expectedMatches: map[string]string{"x-forwarded-for": "2001:0db8:2001:zzzz::", "forwarded-for": ipv6Global}, }, { - name: "invalid-ipv6-recover-multi-header-2", - headers: map[string]string{"forwarded-for": ipv6Global, "x-forwarded-for": "2001:0db8:2001:zzzz::"}, - expectedIP: netaddr.MustParseIP(ipv6Global), + name: "ipv6-multi-header-2", + headers: map[string]string{"forwarded-for": ipv6Global, "x-forwarded-for": "2001:0db8:2001:zzzz::"}, + expectedIP: netaddr.IP{}, + expectedMatches: map[string]string{"x-forwarded-for": "2001:0db8:2001:zzzz::", "forwarded-for": ipv6Global}, }, }, tcs...) tcs = append([]IPTestCase{ @@ -175,7 +180,10 @@ func TestIPHeaders(t *testing.T) { } r := http.Request{Header: header, RemoteAddr: tc.remoteAddr} clientIPHeader = tc.clientIPHeader - require.Equal(t, tc.expectedIP.String(), getClientIP(&r).String()) + ip, matches := getClientIP(&r) + require.Equal(t, tc.expectedIP, ip) + require.Equal(t, tc.expectedMatches, matches) + genClientIPSpanTags(&http.Request{Header: header, RemoteAddr: tc.remoteAddr}) }) } } diff --git a/ddtrace/ext/tags.go b/ddtrace/ext/tags.go index b76024f54a..5799c79503 100644 --- a/ddtrace/ext/tags.go +++ b/ddtrace/ext/tags.go @@ -42,6 +42,16 @@ const ( // HTTPClientIP sets the HTTP client IP tag. HTTPClientIP = "http.client_ip" + // MultipleIPHeaders sets the multiple ip header tag used internally to tell the backend an error occurred when + // retrieving an HTTP request client IP. + // See https://datadoghq.atlassian.net/wiki/spaces/APS/pages/2118779066/Client+IP+addresses+resolution + MultipleIPHeaders = "_dd.multiple-ip-headers" + + // HTTPRequestHeaders sets the HTTP request headers partial tag + // This tag is meant to be composed, i.e http.request.headers.headerX, http.request.headers.headerY, etc... + // See https://datadoghq.atlassian.net/wiki/spaces/APMINT/pages/2302444638/DD+TRACE+HEADER+TAGS + HTTPRequestHeaders = "http.request.headers" + // SpanName is a pseudo-key for setting a span's operation name by means of // a tag. It is mostly here to facilitate vendor-agnostic frameworks like Opentracing // and OpenCensus.