diff --git a/contrib/internal/httptrace/httptrace.go b/contrib/internal/httptrace/httptrace.go index c500b88d46..afd56c3744 100644 --- a/contrib/internal/httptrace/httptrace.go +++ b/contrib/internal/httptrace/httptrace.go @@ -57,9 +57,7 @@ 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...) - } + opts = append(genClientIPSpanTags(r), opts...) if spanctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(r.Header)); err == nil { opts = append(opts, tracer.ChildOf(spanctx)) } @@ -90,31 +88,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 +129,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}) }) } }