-
Notifications
You must be signed in to change notification settings - Fork 4.7k
server: Set a pprof label on new stream goroutines #9082
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
84dda06
f2029fc
3b7e587
c3f70d9
7aaf9c9
5329657
819e7a7
e1d919e
e32f01e
cbcdf44
e176a2b
05172ce
921d8cb
e238722
3854612
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
arjan-bal marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ import ( | |
| "net/http" | ||
| "reflect" | ||
| "runtime" | ||
| "runtime/pprof" | ||
| "strings" | ||
| "sync" | ||
| "sync/atomic" | ||
|
|
@@ -1785,6 +1786,12 @@ func (s *Server) handleMalformedMethodName(stream *transport.ServerStream, ti *t | |
| func (s *Server) handleStream(t transport.ServerTransport, stream *transport.ServerStream) { | ||
| ctx := stream.Context() | ||
| ctx = contextWithServer(ctx, s) | ||
| if envconfig.LabelServerStreamGoroutines { | ||
| // This method always runs in its own goroutine, so we can set a | ||
| // goroutine label without needing to restore a previous context. | ||
| ctx = pprof.WithLabels(ctx, pprof.Labels("grpc.server.method", stream.Method())) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use the key "grpc.method" to be consistent with the OpenTelemetry plugins? gRPC is moving away from OpenCensus in favour of OpenTelemetry. See https://grpc.io/docs/guides/opentelemetry-metrics/#server-instruments and https://grpc.io/docs/guides/opentelemetry-metrics/#labelsattributes
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. Done. |
||
| pprof.SetGoroutineLabels(ctx) | ||
| } | ||
| var ti *traceInfo | ||
| if EnableTracing { | ||
| tr := newTrace("grpc.Recv."+methodFamily(stream.Method()), stream.Method()) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking ahead to when we might have more labels, maintaining one environment variable per label seems tedious. Could we instead use a single variable whose value is a comma-separated list of key-value pairs? For example:
GRPC_GO_PPROF_LABELS_ENABLED="grpc.method=false".This would be similar to the parsing of the GODEBUG environment variable
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea
... and Done.