proposal: net/http: add PatternContextKey package variable to track routing

[treuherz]

I propose adding a PatternContextKey field to net/http, which would be set on a request context by ServeMux. Proposed doc-visible API as follows

var (
	// PatternContextKey is a context key. It can be used in HTTP handlers with Context.Value to access
	// the pattern used by the request router to route a request to a handler. The associated value will be
	// of type string, and can be set by ServeMux or another router where applicable.
	PatternContextKey = &contextKey{"pattern"}
)

When processing a Request, ServeMux should inject the pattern provided to Handle or HandleFunc into a request-context value against this key.

This functionality would extremely helpful for the observability or testability of routing behaviour. It is very helpful to have some sort of key to identify the route an incoming request came via, and currently there is no good way to determine this dynamically from the handler.

It is not always desirable for this key to be the request URL, or part of it. The host may be variable, the handler may be under a prefix, or the URL path may contain high-cardinality variables. To derive a useful key from the URL, dealing with the first two issues requires a handler to know a lot about the routers above it, and the third makes URLs almost unusable. The second issue is that the route may be ambiguous. A request may reach a handler via two different routing paths, perhaps because different middleware was defined on different paths or methods, but the requests are eventually handled by the same code. More likely is that the handler doing the logging or metric-recording is middleware itself, and is called in multiple places with varying requests. These ambiguity and cardinality issues already make this difficult in the existing ServeMux and it is only worse in third-party routers which support path variables. I expect this problem to become more difficult in code using the standard library router if #61410 is accepted.

This can be worked around already, but the methods aren't very satisfactory. A value can be provided statically and either given straight to the handler when instantiating it, or given to a middleware which injects it into the context where it can be pulled out by other code. Both of these require specifying a value for the tag, which either means stuttering the routing pattern or making up a new meaningful tag for every route, both of which feel unpleasantly boilerplate-y and neither of which play well with the ecosystem of handlers and middlewares. Adding a standard way to provide this information would improve how far a user can get with the standard library, and hopefully provide something for the ecosystem to cohere around.

I don't think the net/http package should provide guarantees about the structure of the data in this context value, but I think ServeMux should. This leaves the door open for other routers to add relevant routing info here when they have it, and means they can use this context key without needing to specify their routing patterns in the same format as ServeMux.

The part of this I am undecided on is how nested ServeMuxes should handle this value. In increasing order of both usefulness and complexity:

• Last wins, each router sets it on the context without checking first.
• Simple concatenation, this string is appended to by different router. Leaves the awkward problem of what character to use for the concatenation.
• Smart combination, where a previous value is combined with the new one in order to increase its specificity. This might be impossible to do correctly in all cases for paths as the router doesn't necessarily know if the path has had prefixes stripped already, so can't always combine paths correctly.

I realise not being able to solve this satisfactorily could be a dealbreaker for this, I'm hoping discussion will reveal solutions I haven't seen.

There is prior art for similar functionality in gorilla/mux and chi, both of which support path variables and routing based on methods. I have used these before to provide keys for request-recording metrics, but cannot provide public links.

I previously suggested this in #61410 (comment) as adding a Pattern string field to a Request. I thought better of this as it seemed odd to add a new field to the Request struct specifically to be set server-side by a ServeMux.

[earthboundkid]

The part of this I am undecided on is how nested ServeMuxes should handle this value. In increasing order of both usefulness and complexity

It could always be a slice of string with nested routers just appending on the end.

[treuherz]

@carlmjohnson that might well be simplest, leaving resolving it to whatever needs to use it. The downside is it makes the most common case more complicated.

[ianlancetaylor]

CC @neild @jba

[treuherz]

It’s occurred to me that “Pattern” might be too prescriptive, especially if this is intended to be used by other routers as well. “RoutingTag” or something may be more generic, but I’m not sure it’s worth it at the expense of deviating from std’s terminology

It’s probably also worth noting that chi and gorilla/mux, who I cited as prior art, don’t afaict do anything special with nested routers and just overwrite the value each time. It doesn’t seem unreasonable to say that if you’re doing something as complex as nested routers you can deal with handling routing tags yourself, in order to keep the most common usage simple.

[alnr]

I support this proposal as an extension to #61410; this functionality would be very helpful for 90% of use cases for logging and tracing. Note that beside gorilla/mux and chi, the super popular httprouter by Julien Schmidt also supports this functionality on master branch: julienschmidt/httprouter#344.

IMO, the fact that it's implemented in all major routers soundly demonstrates its usefulness.

Edit: to add an example where this function would be used: https://github.com/open-telemetry/opentelemetry-go-contrib/blob/fdfa6e3abf03caa6a1d3267913e01526d97eab8a/instrumentation/net/http/otelhttp/config.go#L188

[jba]

In pkgsite, we do this with a wrapper around a handler that allows for arbitrary mappings between the pattern and the route key. We actually use that freedom. For example, we include a query parameter in the tag.

It seems to me that that approach is simple enough, more general, and doesn't suffer from the nested-mux problem. But maybe I'm missing something.

[treuherz]

I see how building a wrapper like that solves the problem, but I do still think leaving that functionality to be implemented and re-implemented locally misses a good opportunity to unify the approaches of different ecosystem packages that need something like this.

[jba]

Yes, but if the pkgsite code is representative, then the proposal here falls short.

[treuherz]

Ah, I see your point. I think adding some sort of Tagger callback to the router would feel too invasive, so having that versatility might well require wrapping the route registration as pkgsite does.

I don't know if I can make the call single-handedly as to whether just providing the routing pattern is useful enough in enough cases to justify my original proposal.

[zekth]

@jba

wrapper around a handler

The issue with this approach it makes every service requiring to implement a router wrapper to be able to tag/append in the context to have access to the routing pattern. For example to have this kind of feature datadog wraps the whole ServeMux: https://github.com/DataDog/dd-trace-go/blob/a69e46c1ba973f5adb1dd8a35b325da02e718b9b/contrib/net/http/http.go#L46

When working on custom shared tracing/logging/audit libraries this PatternContext would be a huge plus as it won't require custom wrappers for lib consumers to integrate with their current apis.

Like @alnr said

IMO, the fact that it's implemented in all major routers soundly demonstrates its usefulness.

[rsc]

Will close this as a duplicate of #66405.

[rsc]

This proposal is a duplicate of a previously discussed proposal, as noted above,
and there is no significant new information to justify reopening the discussion.
The issue has therefore been declined as a duplicate.
— rsc for the proposal review group