Feature Request: Handler callbacks

[rauanmayemir]

Currently, transcoder handlers are opaque and there is no way to pry into what's happening until we get into grpc handler.

One use case I have in mind is opentelemetry. I start the tracer before transcoder with no knowledge about my routes so my traces explorer will be full of unique transactions like:

GET /package/version/entity/foo1?param=bar
GET /package/version/entity/foo2?param=baz

If I had access to route matches in transcoder, I could have set http.route attribute or anything else from the http rule.

I've thought about not starting the tracer at all before reaching grpc handler, but it's not always possible. Some things are meant to happen with a non-transcoded request, like logging or authz (not ideal, but that usually depends on url paths like /api/admin/entity/*). And some of those things benefit from the knowledge of transcoder metadata (like detecting route tags from http rules).

[jhump]

This has been something we've discussed a lot. The main challenge was the potential for drastically increasing the surface area of the API and the extra surface area for bugs with how user-provided code would be interacting with the transcoder.

A very early version of callbacks existed in older versions of the repo, but we got rid of it before open-sourcing in an API overhaul that attempted to make the API more clear and also much smaller/simpler:

vanguard-go/vanguard.go

Lines 133 to 152 in 5517025

	// HooksCallback, if non-nil, is called at the beginning of an operation. If the
	// callback is interested in more details about the operation, it can return Hooks,
	// whose methods will be called as the operation progresses. If it returns an error,
	// the operation is immediately failed with the returned error, and the server
	// handler will never be invoked.
	//
	// For invalid requests (where Operation.IsValid returns false), if non-nil Hooks
	// are returned, they are ignored; no other hooks will be invoked. For valid
	// operations, the Hooks.OnClientRequestHeaders and Hooks.OnOperationEnd hooks
	// will always be invoked, even when a hook aborts the operation with an error.
	//
	// This callback can be overridden on a per-service level. But the function defined
	// in this field will always be used for operations that are invalid when the RPC
	// method has not been determined.
	//
	// If the operation is invalid because the method could not be determined and both
	// the HooksCallback and the UnknownHandler are defined, the HooksCallback will be
	// invoked first. The UnknownHandler will only be invoked if the callback returns
	// a nil error.
	HooksCallback func(context.Context, Operation) (Hooks, error)

There was also a draft PR to introduce the idea of "interceptors", which would be much more powerful than just callbacks, but also required rather significant changes to the transcoder internals: #86. (Overhaul the internals isn't necessarily a bad thing; the benchmark results are all good. But we mainly didn't want to add API surface area like this, so we didn't move forward with it.)

One thing that both an interceptor and the original callbacks could do is to abort operations with errors, with the idea being that we'd want to be able to plug validation into this. So this library could then be used to build a reverse proxy/API gateway, and it could do semantic validation of the messages before forwarding them to backends.

Your request sounds like it's more for observability, so wouldn't require that kind of support so could likely be added with very small new API. But we need to think about it and the other possible use cases to come up with the best/most coherent user-facing API for this stuff.

[rauanmayemir]

I agree, I started learning vanguard’s architecture and having callbacks that cover all critical parts won’t be trivial and only complicate things. I believe this could be addressed by implementing some sort of event listener instead having interceptor pipeline. Listener could allow covering all stages with a smaller api surface. (I’ve even seen people calling such things ‘tracer’ in golang ecosystem)

Re interceptors, since we don’t have that now I solved it in a very simple way, I have both http middlewares that fire before transcoder and I also have grpc interceptors that fire on the grpc handler side. So e.g cors, authz happens at http stage and request validation with protovalidate happens in grpc interceptor.

It works great so far, but there’s an extra hassle to make sure we’re responding with correct output (e.g when we need to return 401), so client protocol level interceptors could help with that.

[emcfarlane]

Stream style interceptors would provide lots of flexibility to users allowing interceptors that change the control flow, and making the decoder and encoder side pluggable. The requests trip through vanguard decoder -> stream -> encoder is focused on net/http requests. This works well for the proxy model that vanguard-go was initial designed for, but when wrapping gRPC services we are always required to re-encode the messages. This is one of the most expensive tasks. Ideally we could implement invoking RPC handler implementation #97 as one of these stream interceptors, removing the re-encoding step entirely. This would be more efficient and avoid any issues with gRPC's ServeHTTP method. Observability could be added ontop of a stream, effectively wrapping a stream like how a net/http handler is wrapped. Or we might need to explore a event callback solution like in connect-go connectrpc/connect-go#665 to better provider metrics like bytes on the wire, etc.