-
Notifications
You must be signed in to change notification settings - Fork 563
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
grpc/http: support custom KeyValues when reporting metrics #195
Comments
Thanks for opening the issue. We're excited to include improvements as we try to make OpenTelemetry awesome for users. Can you provide a simplified code example that would describe what you are looking for? I'm not 100% sure I follow the ask here. |
Essentially I want to be able to provide per request labels. I'll use OpenCensus code snippets since that's what I have, but it should provide enough info. I could add any tags to the context and they would then be available to any views that were measured with that context.
The problem with ochttp for example is that it by default lives outside the lifespan of my context, so I can't provide any mutators in the context to it. I could invert the router, but typically I don't know the keys until I actually get into my route and pull things from the db, etc.
The problem is how to hoist my context back up to where the stats are being recorded. I've toyed with some various concepts around using interfaces on the response writer.
That might be usable above this line
It's not a beautiful solution, but I think it illustrates my point. This lets me slice all the ochttp metrics with my own tags/keys without having to calculate and record the stats myself. |
@derekperkins thanks for the context, this seems like a reasonable ask. I'm interested to hear what @Aneurysm9 would propose as a solution for this. He is currently rewriting and migrating this instrumentation to the contrib repo (#190) and will have the most up to date understanding of the problem space I feel. |
Any thoughts? I saw a related issue in the main ot-go repo. |
My initial thought is perhaps to have the handler inject something into the request context that can be used to add labels: // in instrumentation
labeler := &Labeler{}
ctx = injectLabeler(ctx, labeler)
h.handler.ServeHTTP(w, r.WithContext(ctx))
setAfterServeAttributes(span, bw.read, rww.written, rww.statusCode, bw.err, rww.err)
// Add request metrics
labels := append(labeler.labels, semconv.HTTPServerMetricAttributesFromHTTPRequest(h.operation, r))
... // in handler
labeler := otelhttp.LabelerFromContext(r.Context())
labeler.Add(label.String("key", "value")) I'll try to prototype something along these lines and see how it feels. |
@Aneurysm9 thanks for the response, that looks like a much nicer api than the one I proposed. It's simple to use, minimal overhead, looks great! |
We've been running OpenCensus for ~2 years and are excited to move to OpenTelemetry. The standard API has served us well, but our biggest issue with some of the wrapper packages like ocgrpc and ochttp are that they don't support custom KeyValues (TagKeys) when recording metrics. I can register my own views that include them, but because of the way that they wrap my server, I'm not passing a context that contains said values. I need this on a per request basis, to report specific error types, latency per kv, etc.
Is that in scope for OpenTelemetry? Without it, I'm forced to copy and fork the implementation.
The text was updated successfully, but these errors were encountered: