From 04f9b8c19ff3b7a3e94c5f3b5a711743f294fa6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Fri, 17 Jun 2022 16:52:32 +0200 Subject: [PATCH 1/6] contrib/internal/httptrace: setup multiple-ip-headers and http.request.headers.* tags --- contrib/internal/httptrace/httptrace.go | 44 +++++++++++++++++--- contrib/internal/httptrace/httptrace_test.go | 44 ++++++++++++-------- ddtrace/ext/tags.go | 10 +++++ 3 files changed, 74 insertions(+), 24 deletions(-) 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. From 6d1c0dc4ecd5ea99644791c790c8ff1bfb7e4723 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Mon, 20 Jun 2022 14:36:43 +0200 Subject: [PATCH 2/6] contrib/internal/httptrace: simplify getClientIP prototype and generate span tags directly --- contrib/internal/httptrace/httptrace.go | 31 ++++++++++---------- contrib/internal/httptrace/httptrace_test.go | 11 ++++--- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index c500b88d46..b776ccfd20 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -39,6 +39,7 @@ var ( "true-client-ip", } clientIPHeader = os.Getenv("DD_TRACE_CLIENT_IP_HEADER") + collectIP = os.Getenv("DD_TRACE_CLIENT_IP_HEADER_DISABLED") != "true" ) // StartRequestSpan starts an HTTP request span with the standard list of HTTP request span tags (http.method, http.url, @@ -57,8 +58,8 @@ func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer. tracer.Tag("http.host", r.Host), }, opts...) } - for k, v := range genClientIPSpanTags(r) { - opts = append([]ddtrace.StartSpanOption{tracer.Tag(k, v)}, opts...) + if collectIP { + opts = append(genClientIPSpanTags(r), opts...) } if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)); err == nil { opts = append(opts, tracer.ChildOf(spanctx)) @@ -90,31 +91,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{} +// genClientIPSpanTags generates the client IP related span tags. +func genClientIPSpanTags(r *http.Request) []ddtrace.StartSpanOption { + tags := []ddtrace.StartSpanOption{} ip, matches := getClientIP(r) if matches == nil { if ip.IsValid() { - tags[ext.HTTPClientIP] = ip.String() + tags = append(tags, tracer.Tag(ext.HTTPClientIP, ip.String())) } return tags } sb := strings.Builder{} - for hdr, ip := range matches { - tags[ext.HTTPRequestHeaders+"."+hdr] = ip - sb.WriteString(hdr + ":" + ip + ",") + for _, hdr := range matches { + tags = append(tags, tracer.Tag(ext.HTTPRequestHeaders+"."+hdr, ip)) + sb.WriteString(hdr + ",") } hdrList := sb.String() - tags[ext.MultipleIPHeaders] = hdrList[:len(hdrList)-1] + tags = append(tags, tracer.Tag(ext.MultipleIPHeaders, hdrList[:len(hdrList)-1])) return tags } // getClientIP attempts to find the client IP address in the given request r. -// 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. +// If several IP headers are present in the request, the returned IP is invalid and the list of all IP headers is +// returned. Otherwise, the returned list 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) { +func getClientIP(r *http.Request) (netaddr.IP, []string) { ipHeaders := defaultIPHeaders if len(clientIPHeader) > 0 { ipHeaders = []string{clientIPHeader} @@ -131,11 +132,11 @@ func getClientIP(r *http.Request) (netaddr.IP, map[string]string) { } return netaddr.IP{} } - matches := map[string]string{} + matches := []string{} var matchedIP netaddr.IP for _, hdr := range ipHeaders { if v := r.Header.Get(hdr); v != "" { - matches[hdr] = v + matches = append(matches, hdr) if ip := check(v); ip.IsValid() { matchedIP = ip } diff --git a/contrib/internal/httptrace/httptrace_test.go b/contrib/internal/httptrace/httptrace_test.go index 28c6caced8..e6df6282c7 100644 --- a/contrib/internal/httptrace/httptrace_test.go +++ b/contrib/internal/httptrace/httptrace_test.go @@ -36,7 +36,7 @@ type IPTestCase struct { remoteAddr string headers map[string]string expectedIP netaddr.IP - expectedMatches map[string]string + expectedMatches []string clientIPHeader string } @@ -111,13 +111,13 @@ func genIPTestCases() []IPTestCase { 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}, + expectedMatches: []string{"x-forwarded-for", "forwarded-for"}, }, { 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}, + expectedMatches: []string{"x-forwarded-for", "forwarded-for"}, }, { name: "invalid-ipv6", @@ -133,13 +133,13 @@ func genIPTestCases() []IPTestCase { 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}, + expectedMatches: []string{"x-forwarded-for", "forwarded-for"}, }, { 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}, + expectedMatches: []string{"x-forwarded-for", "forwarded-for"}, }, }, tcs...) tcs = append([]IPTestCase{ @@ -183,7 +183,6 @@ func TestIPHeaders(t *testing.T) { ip, matches := getClientIP(&r) require.Equal(t, tc.expectedIP, ip) require.Equal(t, tc.expectedMatches, matches) - genClientIPSpanTags(&http.Request{Header: header, RemoteAddr: tc.remoteAddr}) }) } } From 165f5a59ec2df6cecc5d19b5e7eef824c03118e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Wed, 22 Jun 2022 15:08:06 +0200 Subject: [PATCH 3/6] contrib/internal/httptrace: use strings.Join to build multiple-ip tag --- contrib/internal/httptrace/httptrace.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index b776ccfd20..76ff95ec55 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -101,13 +101,10 @@ func genClientIPSpanTags(r *http.Request) []ddtrace.StartSpanOption { } return tags } - sb := strings.Builder{} for _, hdr := range matches { tags = append(tags, tracer.Tag(ext.HTTPRequestHeaders+"."+hdr, ip)) - sb.WriteString(hdr + ",") } - hdrList := sb.String() - tags = append(tags, tracer.Tag(ext.MultipleIPHeaders, hdrList[:len(hdrList)-1])) + tags = append(tags, tracer.Tag(ext.MultipleIPHeaders, strings.Join(matches, ","))) return tags } From 3eded75cf867baa23fa76f28ba709d56b22ef904 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Wed, 22 Jun 2022 17:25:10 +0200 Subject: [PATCH 4/6] contrib/internal/httptrace: add TODO to track span start options refactoring --- contrib/internal/httptrace/httptrace.go | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 76ff95ec55..18bcd7c84a 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -46,6 +46,7 @@ var ( // http.useragent). Any further span start option can be added with opts. func StartRequestSpan(r *http.Request, opts ...ddtrace.StartSpanOption) (tracer.Span, context.Context) { // Append our span options before the given ones so that the caller can "overwrite" them. + // TODO(): rework span start option handling (https://github.com/DataDog/dd-trace-go/issues/1352) opts = append([]ddtrace.StartSpanOption{ tracer.SpanType(ext.SpanTypeWeb), tracer.Tag(ext.HTTPMethod, r.Method), From 9c3ff4b07b925c07dcc437c95f7ab01e5eabe598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Wed, 22 Jun 2022 20:47:57 +0200 Subject: [PATCH 5/6] contrib/internal/httptrace: simplify client ip tag generation down to one function --- contrib/internal/httptrace/httptrace.go | 73 +++++++------------- contrib/internal/httptrace/httptrace_test.go | 62 ++++++++++------- 2 files changed, 62 insertions(+), 73 deletions(-) diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index 18bcd7c84a..1e3ae32d3e 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -92,64 +92,41 @@ func ippref(s string) *netaddr.IPPrefix { return nil } -// genClientIPSpanTags generates the client IP related span tags. -func genClientIPSpanTags(r *http.Request) []ddtrace.StartSpanOption { - tags := []ddtrace.StartSpanOption{} - ip, matches := getClientIP(r) - if matches == nil { - if ip.IsValid() { - tags = append(tags, tracer.Tag(ext.HTTPClientIP, ip.String())) - } - return tags - } - for _, hdr := range matches { - tags = append(tags, tracer.Tag(ext.HTTPRequestHeaders+"."+hdr, ip)) - } - tags = append(tags, tracer.Tag(ext.MultipleIPHeaders, strings.Join(matches, ","))) - return tags -} - -// getClientIP attempts to find the client IP address in the given request r. -// If several IP headers are present in the request, the returned IP is invalid and the list of all IP headers is -// returned. Otherwise, the returned list is nil. +// genClientIPSpanTags generates the client IP related tags that need to be added to the span. // See https://datadoghq.atlassian.net/wiki/spaces/APS/pages/2118779066/Client+IP+addresses+resolution -func getClientIP(r *http.Request) (netaddr.IP, []string) { +func genClientIPSpanTags(r *http.Request) []ddtrace.StartSpanOption { ipHeaders := defaultIPHeaders if len(clientIPHeader) > 0 { ipHeaders = []string{clientIPHeader} } - check := func(s string) netaddr.IP { - for _, ipstr := range strings.Split(s, ",") { - ip := parseIP(strings.TrimSpace(ipstr)) - if !ip.IsValid() { - continue - } - if isGlobal(ip) { - return ip - } - } - return netaddr.IP{} - } - matches := []string{} - var matchedIP netaddr.IP + var headers []string + var ips []string + var opts []ddtrace.StartSpanOption for _, hdr := range ipHeaders { if v := r.Header.Get(hdr); v != "" { - matches = append(matches, hdr) - if ip := check(v); ip.IsValid() { - matchedIP = ip - } + headers = append(headers, hdr) + ips = append(ips, v) } } - 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, nil + if len(ips) == 0 { + if remoteIP := parseIP(r.RemoteAddr); remoteIP.IsValid() && isGlobal(remoteIP) { + opts = append(opts, tracer.Tag(ext.HTTPClientIP, remoteIP.String())) + } + } else if len(ips) == 1 { + for _, ipstr := range strings.Split(ips[0], ",") { + ip := parseIP(strings.TrimSpace(ipstr)) + if ip.IsValid() && isGlobal(ip) { + opts = append(opts, tracer.Tag(ext.HTTPClientIP, ip.String())) + break + } + } + } else { + for i := range ips { + opts = append(opts, tracer.Tag(ext.HTTPRequestHeaders+"."+headers[i], ips[i])) + } + opts = append(opts, tracer.Tag(ext.MultipleIPHeaders, strings.Join(headers, ","))) } - return netaddr.IP{}, nil + return opts } func parseIP(s string) netaddr.IP { diff --git a/contrib/internal/httptrace/httptrace_test.go b/contrib/internal/httptrace/httptrace_test.go index e6df6282c7..a16d482553 100644 --- a/contrib/internal/httptrace/httptrace_test.go +++ b/contrib/internal/httptrace/httptrace_test.go @@ -16,6 +16,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" ) @@ -32,12 +34,12 @@ func TestStartRequestSpan(t *testing.T) { } type IPTestCase struct { - name string - remoteAddr string - headers map[string]string - expectedIP netaddr.IP - expectedMatches []string - clientIPHeader string + name string + remoteAddr string + headers map[string]string + expectedIP netaddr.IP + multiHeaders string + clientIPHeader string } func genIPTestCases() []IPTestCase { @@ -108,16 +110,16 @@ func genIPTestCases() []IPTestCase { 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: []string{"x-forwarded-for", "forwarded-for"}, + name: "ipv4-multi-header-1", + headers: map[string]string{"x-forwarded-for": "127.0.0.1", "forwarded-for": ipv4Global}, + expectedIP: netaddr.IP{}, + multiHeaders: "x-forwarded-for,forwarded-for", }, { - name: "ipv4-multi-header-2", - headers: map[string]string{"forwarded-for": ipv4Global, "x-forwarded-for": "127.0.0.1"}, - expectedIP: netaddr.IP{}, - expectedMatches: []string{"x-forwarded-for", "forwarded-for"}, + name: "ipv4-multi-header-2", + headers: map[string]string{"forwarded-for": ipv4Global, "x-forwarded-for": "127.0.0.1"}, + expectedIP: netaddr.IP{}, + multiHeaders: "x-forwarded-for,forwarded-for", }, { name: "invalid-ipv6", @@ -130,16 +132,16 @@ func genIPTestCases() []IPTestCase { 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: []string{"x-forwarded-for", "forwarded-for"}, + name: "ipv6-multi-header-1", + headers: map[string]string{"x-forwarded-for": "2001:0db8:2001:zzzz::", "forwarded-for": ipv6Global}, + expectedIP: netaddr.IP{}, + multiHeaders: "x-forwarded-for,forwarded-for", }, { - name: "ipv6-multi-header-2", - headers: map[string]string{"forwarded-for": ipv6Global, "x-forwarded-for": "2001:0db8:2001:zzzz::"}, - expectedIP: netaddr.IP{}, - expectedMatches: []string{"x-forwarded-for", "forwarded-for"}, + name: "ipv6-multi-header-2", + headers: map[string]string{"forwarded-for": ipv6Global, "x-forwarded-for": "2001:0db8:2001:zzzz::"}, + expectedIP: netaddr.IP{}, + multiHeaders: "x-forwarded-for,forwarded-for", }, }, tcs...) tcs = append([]IPTestCase{ @@ -180,9 +182,19 @@ func TestIPHeaders(t *testing.T) { } r := http.Request{Header: header, RemoteAddr: tc.remoteAddr} clientIPHeader = tc.clientIPHeader - ip, matches := getClientIP(&r) - require.Equal(t, tc.expectedIP, ip) - require.Equal(t, tc.expectedMatches, matches) + cfg := ddtrace.StartSpanConfig{} + for _, opt := range genClientIPSpanTags(&r) { + opt(&cfg) + } + if tc.expectedIP.IsValid() { + require.Equal(t, tc.expectedIP.String(), cfg.Tags[ext.HTTPClientIP]) + require.Nil(t, cfg.Tags[ext.MultipleIPHeaders]) + } else { + require.Nil(t, cfg.Tags[ext.HTTPClientIP]) + if tc.multiHeaders != "" { + require.Equal(t, tc.multiHeaders, cfg.Tags[ext.MultipleIPHeaders]) + } + } }) } } From 3197be8928bd9a5c5b638798f217b7418f48f66b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mazeau?= Date: Wed, 22 Jun 2022 22:41:28 +0200 Subject: [PATCH 6/6] contrib/internal/httptrace: add check for multiple ip headers --- contrib/internal/httptrace/httptrace_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/internal/httptrace/httptrace_test.go b/contrib/internal/httptrace/httptrace_test.go index a16d482553..d1dce63be6 100644 --- a/contrib/internal/httptrace/httptrace_test.go +++ b/contrib/internal/httptrace/httptrace_test.go @@ -193,6 +193,9 @@ func TestIPHeaders(t *testing.T) { require.Nil(t, cfg.Tags[ext.HTTPClientIP]) if tc.multiHeaders != "" { require.Equal(t, tc.multiHeaders, cfg.Tags[ext.MultipleIPHeaders]) + for hdr, ip := range tc.headers { + require.Equal(t, ip, cfg.Tags[ext.HTTPRequestHeaders+"."+hdr]) + } } } })