Annotate CPU and goroutine profiles #4204
Conversation
neilalexander
force-pushed
the
neil/pproflabels
branch
from
37227b4
to
9aa3a6c
Compare
|
Lots of tests failures that should be adressed. |
|
Yep, PR is still marked as draft as I haven't looked at those failures yet, but I've gone back to the consumer defaults for now. |
neilalexander
force-pushed
the
neil/pproflabels
branch
from
9aa3a6c
to
b5ed7d9
Compare
server/opts.go
Outdated
| @@ -986,6 +987,8 @@ func (o *Options) processConfigFileLine(k string, v interface{}, errors *[]error | |||
| o.PortsFileDir = v.(string) | |||
| case "prof_port": | |||
| o.ProfPort = int(v.(int64)) | |||
| case "prof_tags": | |||
There was a problem hiding this comment.
I don't think this is needed, if you turn on profiling we will add in the tags.
There was a problem hiding this comment.
Are you sure it's OK to always tag? It will mean for the first time that the profiles contain potentially sensitive information (account/stream/consumer names) with no way to toggle, whereas before they did not contain anything that really uniquely identifies a specific environment.
There was a problem hiding this comment.
I can see an argument for both. If someone was sharing with us let's say would want all the information so that we could tie together any NATS cli triage, e.g. nats stream info with any profiling they may share.
neilalexander
force-pushed
the
neil/pproflabels
branch
from
b5ed7d9
to
387a118
Compare
neilalexander
changed the title
prof_tags for annotating CPU and goroutine profiles
|
This was modified to just include the tags in all cases and removed the configuration option. |
server/jetstream_cluster.go
Outdated
| js.srv.startGoRoutine(js.monitorCluster) | ||
| js.srv.startGoRoutine( | ||
| js.monitorCluster, | ||
| map[string]string{ |
There was a problem hiding this comment.
Hmm yea this seems worse. Maybe back to strings but concat the key and value?
js.srv.startGoRoutine( js.monitorCluster, "type: metaleader", ..)There was a problem hiding this comment.
Only thing with this approach is that it might end up ugly for other reasons, i.e. fmt.Sprintf littered everywhere, but I'll happily go with that if that is what you prefer:
js.srv.startGoRoutine( js.monitorCluster, "type:metaleader", fmt.Sprintf("stream:%s", streamName), ...)
Signed-off-by: Neil Twigg <neil@nats.io>
neilalexander
force-pushed
the
neil/pproflabels
branch
from
387a118
to
d2615b7
Compare
This allows the CPU and goroutine profiles to be annotated with information that allows us to break down load based on accounts, streams and consumers. We could probably add more labels elsewhere for other purposes too. It makes it easier to spot whether there are certain assets that are responsible for heavy CPU usage, i.e. snapshotting certain stream states.
Signed-off-by: Neil Twigg neil@nats.io