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

contrib/net/http: support for path params from go 1.22 proposal #3079

Merged
merged 8 commits into from
Jan 15, 2025
24 changes: 6 additions & 18 deletions contrib/net/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package http // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http"

import (
"net/http"
"strings"

"gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace"
"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
Expand Down Expand Up @@ -60,26 +59,14 @@ func (mux *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
copy(so, mux.cfg.spanOpts)
so = append(so, httptrace.HeaderTagsFromRequest(r, mux.cfg.headerTags))
TraceAndServe(mux.ServeMux, w, r, &ServeConfig{
Service: mux.cfg.serviceName,
Resource: resource,
SpanOpts: so,
Route: route,
Service: mux.cfg.serviceName,
Resource: resource,
SpanOpts: so,
Route: route,
RouteParams: patternValues(pattern, r),
})
}

// patternRoute returns the route part of a go1.22 style ServeMux pattern. I.e.
// it returns "/foo" for the pattern "/foo" as well as the pattern "GET /foo".
func patternRoute(s string) string {
// Support go1.22 serve mux patterns: [METHOD ][HOST]/[PATH]
// Consider any text before a space or tab to be the method of the pattern.
// See net/http.parsePattern and the link below for more information.
// https://pkg.go.dev/net/http#hdr-Patterns
if i := strings.IndexAny(s, " \t"); i > 0 && len(s) >= i+1 {
return strings.TrimLeft(s[i+1:], " \t")
}
return s
}

// WrapHandler wraps an http.Handler with tracing using the given service and resource.
// If the WithResourceNamer option is provided as part of opts, it will take precedence over the resource argument.
func WrapHandler(h http.Handler, service, resource string, opts ...Option) http.Handler {
Expand All @@ -96,6 +83,7 @@ func WrapHandler(h http.Handler, service, resource string, opts ...Option) http.
h.ServeHTTP(w, req)
return
}
// TODO: add calls to patternRoute and patternValues once 1.23 is the minimum supported version because 1.23 adds an `(*http.Request).Pattern` field
darccio marked this conversation as resolved.
Show resolved Hide resolved
resc := resource
if r := cfg.resourceNamer(req); r != "" {
resc = r
Expand Down
172 changes: 172 additions & 0 deletions contrib/net/http/pattern.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2025 Datadog, Inc.

package http

import (
"errors"
"fmt"
"net/http"
"strings"
"sync"
"unicode"

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

// patternRoute returns the route part of a go1.22 style ServeMux pattern. I.e.
// it returns "/foo" for the pattern "/foo" as well as the pattern "GET /foo".
func patternRoute(s string) string {
// Support go1.22 serve mux patterns: [METHOD ][HOST]/[PATH]
// Consider any text before a space or tab to be the method of the pattern.
// See net/http.parsePattern and the link below for more information.
// https://pkg.go.dev/net/http#hdr-Patterns-ServeMux
if i := strings.IndexAny(s, " \t"); i > 0 && len(s) >= i+1 {
return strings.TrimLeft(s[i+1:], " \t")
}
return s
}

var patternSegmentsCache sync.Map // map[string][]string
darccio marked this conversation as resolved.
Show resolved Hide resolved

// patternValues return the path parameter values and names from the request.
func patternValues(pattern string, request *http.Request) map[string]string {
if pattern == "" { // using <=1.21 serve mux behavior, aborting
return nil
}
names := getPatternNames(pattern)
res := make(map[string]string, len(names))
for _, name := range names {
res[name] = request.PathValue(name)
}
return res
}

func getPatternNames(pattern string) []string {
if v, ok := patternSegmentsCache.Load(pattern); ok {
if v == nil {
return nil
}
return v.([]string)
}

segments, err := patternNames(pattern)
if err != nil {
// Ignore the error: Something as gone wrong, but we are not eager to find out why.
log.Debug("contrib/net/http: failed to parse mux path pattern %q: %v", pattern, err)
// here we fallthrough instead of returning to load a nil value into the cache to avoid reparsing the pattern.
}

patternSegmentsCache.Store(pattern, segments)
return segments
}

// patternNames returns the names of the wildcards in the pattern.
// Based on https://cs.opensource.google/go/go/+/refs/tags/go1.23.4:src/net/http/pattern.go;l=84
// but very simplified as we know that the pattern returned must be valid or `net/http` would have panicked earlier.
//
// The pattern string's syntax is
//
// [METHOD] [HOST]/[PATH]
//
// where:
// - METHOD is an HTTP method
// - HOST is a hostname
// - PATH consists of slash-separated segments, where each segment is either
// a literal or a wildcard of the form "{name}", "{name...}", or "{$}".
//
// METHOD, HOST and PATH are all optional; that is, the string can be "/".
// If METHOD is present, it must be followed by at least one space or tab.
// Wildcard names must be valid Go identifiers.
// The "{$}" and "{name...}" wildcard must occur at the end of PATH.
// PATH may end with a '/'.
// Wildcard names in a path must be distinct.
func patternNames(pattern string) ([]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I had to dig into the implementation and tests to understand what this function returns (since the type []string doesn't give much information). I think it would be nice if this was added to the doc string for future readers to quickly understand what this function does.

Adding something at the end like:

Given a pattern like "GET /foo/{bar}/{baz}", it returns []string{"bar", "baz"}

Also having 2 functions getPatternNames and patternNames with almost the same function signature doesn't help much (the difference is the first one uses the cache first). WDYT about renaming this one to parsePattern or parsePatternNames? (ideally I think it should be parsePattern, because if we want to parse other stuff from it in the future we would likely want to do it only once).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied all your comments (I believe) 👍

if len(pattern) == 0 {
return nil, errors.New("empty pattern")
}
method, rest, found := pattern, "", false
if i := strings.IndexAny(pattern, " \t"); i >= 0 {
method, rest, found = pattern[:i], strings.TrimLeft(pattern[i+1:], " \t"), true
}
if !found {
rest = method
method = ""
}

i := strings.IndexByte(rest, '/')
if i < 0 {
return nil, errors.New("host/path missing /")
}
host := rest[:i]
rest = rest[i:]
if j := strings.IndexByte(host, '{'); j >= 0 {
return nil, errors.New("host contains '{' (missing initial '/'?)")
}

// At this point, rest is the path.
var names []string
seenNames := make(map[string]bool)
for len(rest) > 0 {
// Invariant: rest[0] == '/'.
rest = rest[1:]
if len(rest) == 0 {
// Trailing slash.
break
}
i := strings.IndexByte(rest, '/')
if i < 0 {
i = len(rest)
}
var seg string
seg, rest = rest[:i], rest[i:]
if i := strings.IndexByte(seg, '{'); i >= 0 {
// Wildcard.
if i != 0 {
return nil, errors.New("bad wildcard segment (must start with '{')")
}
if seg[len(seg)-1] != '}' {
return nil, errors.New("bad wildcard segment (must end with '}')")
}
name := seg[1 : len(seg)-1]
if name == "$" {
if len(rest) != 0 {
return nil, errors.New("{$} not at end")
}
break
}
name, multi := strings.CutSuffix(name, "...")
if multi && len(rest) != 0 {
return nil, errors.New("{...} wildcard not at end")
}
if name == "" {
return nil, errors.New("empty wildcard name")
}
if !isValidWildcardName(name) {
return nil, fmt.Errorf("bad wildcard name %q", name)
}
if seenNames[name] {
return nil, fmt.Errorf("duplicate wildcard name %q", name)
}
seenNames[name] = true
names = append(names, name)
}
}

return names, nil
}

func isValidWildcardName(s string) bool {
if s == "" {
return false
}
// Valid Go identifier.
for i, c := range s {
if !unicode.IsLetter(c) && c != '_' && (i == 0 || !unicode.IsDigit(c)) {
return false
}
}
return true
}
134 changes: 134 additions & 0 deletions contrib/net/http/pattern_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2025 Datadog, Inc.

package http

import (
"net/http"
"net/http/httptest"
"testing"

"github.com/stretchr/testify/assert"
)

func TestPathParams(t *testing.T) {
for _, tt := range []struct {
name string
pattern string
url string
expected map[string]string
}{
{
name: "simple",
pattern: "/foo/{bar}",
url: "/foo/123",
expected: map[string]string{"bar": "123"},
},
{
name: "multiple",
pattern: "/foo/{bar}/{baz}",
url: "/foo/123/456",
expected: map[string]string{"bar": "123", "baz": "456"},
},
{
name: "nested",
pattern: "/foo/{bar}/baz/{qux}",
url: "/foo/123/baz/456",
expected: map[string]string{"bar": "123", "qux": "456"},
},
{
name: "empty",
pattern: "/foo/{bar}",
url: "/foo/",
expected: map[string]string{"bar": ""},
},
{
name: "http method",
pattern: "GET /foo/{bar}",
url: "/foo/123",
expected: map[string]string{"bar": "123"},
},
{
name: "host",
pattern: "example.com/foo/{bar}",
url: "http://example.com/foo/123",
expected: map[string]string{"bar": "123"},
},
{
name: "host and method",
pattern: "GET example.com/foo/{bar}",
url: "http://example.com/foo/123",
expected: map[string]string{"bar": "123"},
},
} {
t.Run(tt.name, func(t *testing.T) {
mux := NewServeMux()
mux.HandleFunc(tt.pattern, func(_ http.ResponseWriter, r *http.Request) {
_, pattern := mux.Handler(r)
params := patternValues(pattern, r)
assert.Equal(t, tt.expected, params)
})

r := httptest.NewRequest("GET", tt.url, nil)
w := httptest.NewRecorder()
mux.ServeHTTP(w, r)
})
}
}

func TestPatternNames(t *testing.T) {
tests := []struct {
pattern string
expected []string
err bool
}{
{"/foo/{bar}", []string{"bar"}, false},
{"/foo/{bar}/{baz}", []string{"bar", "baz"}, false},
{"/foo/{bar}/{bar}", nil, true},
{"/foo/{bar}...", nil, true},
{"/foo/{bar}.../baz", nil, true},
{"/foo/{bar}/{baz}...", nil, true},
{"/foo/{bar", nil, true},
{"/foo/{bar{baz}}", nil, true},
{"/foo/{bar!}", nil, true},
{"/foo/{}", nil, true},
{"{}", nil, true},
{"GET /foo/{bar}", []string{"bar"}, false},
{"POST /foo/{bar}/{baz}", []string{"bar", "baz"}, false},
{"PUT /foo/{bar}/{bar}", nil, true},
{"DELETE /foo/{bar}...", nil, true},
{"PATCH /foo/{bar}.../baz", nil, true},
{"OPTIONS /foo/{bar}/{baz}...", nil, true},
{"GET /foo/{bar", nil, true},
{"POST /foo/{bar{baz}}", nil, true},
{"PUT /foo/{bar!}", nil, true},
{"DELETE /foo/{}", nil, true},
{"OPTIONS {}", nil, true},
{"GET example.com/foo/{bar}", []string{"bar"}, false},
{"POST example.com/foo/{bar}/{baz}", []string{"bar", "baz"}, false},
{"PUT example.com/foo/{bar}/{bar}", nil, true},
{"DELETE example.com/foo/{bar}...", nil, true},
{"PATCH example.com/foo/{bar}.../baz", nil, true},
{"OPTIONS example.com/foo/{bar}/{baz}...", nil, true},
{"GET example.com/foo/{bar", nil, true},
{"POST example.com/foo/{bar{baz}}", nil, true},
{"PUT example.com/foo/{bar!}", nil, true},
{"DELETE example.com/foo/{}", nil, true},
{"OPTIONS example.com/{}", nil, true},
}

for _, tt := range tests {
t.Run(tt.pattern, func(t *testing.T) {
names, err := patternNames(tt.pattern)
if tt.err {
assert.Error(t, err)
assert.Nil(t, names)
} else {
assert.NoError(t, err)
assert.Equal(t, tt.expected, names)
}
})
}
}
Loading