Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Support for introducing custom tags to gRPC/HTTP metrics #746

Closed
semistrict opened this issue May 9, 2018 · 10 comments
Closed

Support for introducing custom tags to gRPC/HTTP metrics #746

semistrict opened this issue May 9, 2018 · 10 comments

Comments

@semistrict
Copy link
Contributor

Consider the example of a server that serves results either from database or a cache. One important dimension you might want to capture as part of all your HTTP/gRPC metrics is whether a particular request was served from cache or not.

Currently it isn't possible to augment the set of tags applied to the stats.Record call made by ocgrpc or ochttp. In OpenCensus Java I believe it is possible to accomplish this by changing the thread-local tags in the thread processing of the request but in Go, Context is immutable so a similar approach is not possible.

@semistrict semistrict added the tags label May 9, 2018
@zeako
Copy link

zeako commented May 9, 2018

I don't understand how the context immutability is an issue, you can pass along a context with your own set of keys (for example you can make your own handler which wraps ochttp.Handler and passes along a context to it),
still though it isn't possible (yet?) to pass mutators for to the relevant handler.

@semistrict
Copy link
Contributor Author

Consider the caching example above. You only know the correct value of the "was_cached" tag after the request has finished processing. How do you pass this up from inside your HandlerFunc to the ochttp.Handler that wraps it?

@semistrict
Copy link
Contributor Author

/cc @bogdandrutu

@groob
Copy link
Contributor

groob commented May 16, 2018

could the oc handler take a custom finalizer callback as an option?

@semistrict
Copy link
Contributor Author

semistrict commented May 16, 2018

@groob what would the arguments to such a callback be? Say you determine that a particular request was a cache hit, how do you get that information into the callback? The only option I can think of would be to have something mutable in the context, which you modify in your request handler and then pass back to the context.

@semistrict
Copy link
Contributor Author

Another use of this would be for HTTP responses that are logically errors but that return a 200 OK code for compatibility reasons. It would be interesting for the user to be able to add an "error" tag which could be used to break out errors, otherwise they would have no way to tell error responses from OK responses.

@groob
Copy link
Contributor

groob commented May 18, 2018

I like the functions that Go-Kit provides that enable you to instrument the HTTP handler.

// RequestFunc may take information from an HTTP request and put it into a
// request context. In Servers, RequestFuncs are executed prior to invoking the
// endpoint. In Clients, RequestFuncs are executed after creating the request
// but prior to invoking the HTTP client.
type RequestFunc func(context.Context, *http.Request) context.Context


// ServerFinalizerFunc can be used to perform work at the end of an HTTP
// request, after the response has been written to the client. The principal
// intended use is for request logging. In addition to the response code
// provided in the function signature, additional response parameters are
// provided in the context under keys with the ContextKeyResponse prefix.
type ServerFinalizerFunc func(ctx context.Context, code int, r *http.Request)

so the request func can be use to Populate a context which is then used in the finalizer.

@semistrict
Copy link
Contributor Author

So you would install something internally mutable (like a pointer to a struct) into the context in RequestFunc, mutate it in your handler/endpoint and then read it back in ServerFinalizerFunc?

@rakyll
Copy link
Contributor

rakyll commented Jul 13, 2018

"was_cached" is a dimension you want to collect at the handler that does caching or not, not at the client.

@rakyll
Copy link
Contributor

rakyll commented Jul 13, 2018

This is again a cross-language proposal and needs to be discussed at the specs repo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants