-
Notifications
You must be signed in to change notification settings - Fork 66
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
Support TID #2970
Support TID #2970
Conversation
reporter/parca_reporter.go
Outdated
type labelsKey struct { | ||
pid util.PID | ||
tid util.PID | ||
comm string |
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.
does comm really need to be a part of this key, isn't pid+tid already uniquely identifying the thread?
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.
yes, you're right.
However, we should still not cache comm, since it can change (e.g. if pthread_setname_np
is called). But it doesn't need to be part of the key.
reporter/parca_reporter.go
Outdated
lb.Set("comm", comm) | ||
r.addMetadataForPID(pid, lb) | ||
lb.Set("thread_name", key.comm) | ||
lb.Set("tid", fmt.Sprint(key.tid)) |
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.
If we make these on by default as opposed to opt-in via relabeling, then we should probably also have the pid
by default.
Also, can you add documentation about these here: https://github.com/parca-dev/parca-agent?tab=readme-ov-file#metadata-labels
Bump to our fork of opentelemetry-ebpf-profiler that supports thread IDs, and add the thread ID as a label.
Previously we were just getting the
comm
of the first thread we saw a sample for and saving that as the entire process'scomm
. This PR saves that asthread_name
instead and gets the main thread's comm (i.e. process name) from procfs, saving that ascomm
instead.