Support improved tracing integration by exposing router / path earlier

[jalaziz]

🚀 Feature

I should note that I am trying to integrate the Datadog APM, but I believe this applies to OpenTracing/OpenTelemetry as well.

In trying to integration the Datadog APM, I realized that the the annotated context containing the path pattern is available only after HTTP handlers are called. At a high level, I understand why this is the case, but it complicates adding context to telemetry.

For example, when integrating the Datadog APM, the most straightforward way is to use the WrapHandler function:

mux := runtime.NewServeMux()
handler = httptrace.WrapHandler(mux, "service_name", "")

The second parameters here is meant to be the resource name. Datadog also offers a WithResourceNamer to dynamically generate the resource name (typically the HTTP method and path pattern). Unfortunately, WithResourceNamer works at the handler level and therefore it cannot be used to access the path pattern, just the full URL.

When looking at how Datadog integrates with other HTTP libraries, including the standard ServerMux, there is usually some capability to lookup the handler and route based on the request.

Unfortunately, grpc-gateway doesn't expose the route matching logic in a way that can be accessed outside of ServeHTTP, making it very difficult to know the parameterized route at the "wrapping" layer.

Ultimately, it would be nice if the route matching logic could be extracted from ServeHTTP in a way that it was both fast and re-usable to be able to lookup the route given a request.

Workarounds

Yes, I could also use the gRPC client/server integration as the docs suggest. In fact, I also do this. However, telemetry at the HTTP layer is very useful.

As a workaround, I am able to use runtime.WithMetadata as recommended here to inject the resource name as a span tag, but it feels a bit hacky and disjointed:

runtime.WithMetadata(func(ctx context.Context, req *http.Request) metadata.MD {
	span, ok := tracer.SpanFromContext(ctx)
	if ok {
		if path, ok := runtime.HTTPPathPattern(ctx); ok {
			span.SetTag(ext.ResourceName, req.Method+" "+path)
		} 
	}
	return metadata.MD{}
}),

[johanbrandhorst]

Thanks for your issue! What are you proposing exactly? I don't have enough context to come up with a solution here. Do you want new API from the runtime package? New methods on the ServeMux? I'm happy to discuss pros and cons and review and PRs that may stem from the discussion :).

[jalaziz]

@johanbrandhorst Apologies for not being more clear. To be honest, it's a bit ambiguous because looking at the source code, it's not clear what the best path forward is.

What is clear is that whatever it is, it should allow looking up the path pattern (and maybe the gRPC method) from a given HTTP request parameter.

Looking through the source code, I think the method would need be on ServerMux. At a very high level, I was thinking it may be possible to refactor the routing logic from ServeHTTP into a separate method that can be called independently. What I haven't looked into, though, is how easily that could be optimized to avoid paying the matching price multiple times for the same request.

[johanbrandhorst]

Thanks for the added detail. Is runtime.HTTPPathPattern not sufficient? I'm not sure about mapping that to a grpc method, you might have to do that in a grpc interceptor instead.

[jalaziz]

@johanbrandhorst runtime.HTTPPathPattern only works after annotateContext is called which happens within the handler itself. So it's only useful at the gRPC layer or by passing a func to WithMetadata.

What I'm looking for is the same information, but at the ServerMux/http.Handler level.

[johanbrandhorst]

I see. I don't even know how we'd do that tbh. The information around paths lives in the generated files, and they're not used until the handler is called. Perhaps this metadata workaround is the best we can do without significant re-architecting? Let me know if you come up with some solution, I'm still happy to consider it.

[nastik-kum]

Hi there! I’m curious what will happen with this request. Should we store the pattern in the http.Request itself?

By default, http.ServerMux sets the Pattern field after it finds a match.
From then on, the http.Request keeps that pattern so other code can use it.

The otelhttp package also relies on the Pattern field.

I suggest that runtime.ServeMux follow the same behaviour as http.ServerMux.
Although this information is already available in the context, matching the default Go mux would make life easier for developers.

This may be done in handleHandler function. Or maybe you know the better place for it. I'll be happy to help with implementation!

[johanbrandhorst]

I'm confused, what exactly is it you are proposing? It sounds like the pattern can be accessed already with the runtime.HTTPPattern function. I wouldn't want to overwrite the Pattern field set by net/http with our internal Pattern type.

[nastik-kum]

Hi @johanbrandhorst! The proposal is to set Pattern in our mux, just like it's done by the http.ServerMux. Note: the field is string, so we'd need to convert our pattern before storing it in the field.

The thing is that it is only set by the mux once the route match is done. The http.ServeMux does it: link to sources As soon as we are using the custom mux, it's on us to populate it with the routing info.

The information in the context also have this information, true. However, it's a custom behavior and this information is only avaliable in the handlers and inner middlewares. I'm proposing to improve the runtime.ServerMux to make use of this field. In that way other libraries may be not aware of the mux implementation and just rely on the field presence.

I could prepare the PR and we'll discuss the proposal there. Probably it would be easier to understand then. Do you think it makes sense?

[johanbrandhorst]

Hi @nastik-kum. Yes, please prepare a PR and we can take it from there, thanks!