Add --usdt-file-activation to activate USDT semaphores by filename #1317
Conversation
danobi
force-pushed
the
usdt_file_activation
branch
from
3345733
to
ad37790
Compare
|
cc @dalehamel |
danobi
force-pushed
the
usdt_file_activation
branch
from
ad37790
to
0af5e7a
Compare
|
Hmm clang format is failing but I don't think I touched those files. Gonna ignore it and hope it goes away. Otherwise I'll put up another PR later to format over it. |
|
Thanks for this will take a look soon @danobi |
| @@ -192,6 +195,7 @@ int main(int argc, char *argv[]) | |||
| option long_options[] = { | |||
| option{ "help", no_argument, nullptr, 'h' }, | |||
| option{ "version", no_argument, nullptr, 'V' }, | |||
| option{ "usdt-file-activation", no_argument, nullptr, '$' }, | |||
There was a problem hiding this comment.
$ seems like a weird flag, or it is a special thing in getopt long?
There was a problem hiding this comment.
It's only used for the switch/case. No short option for this flag.
danobi
force-pushed
the
usdt_file_activation
branch
2 times, most recently
from
8b868d9
to
c4408a8
Compare
|
I added another runtime tests for multi-process too |
|
Any objections? |
| // with execute permission. If found, we will try to attach to each | ||
| // process we find. | ||
| glob_t globbuf; | ||
| if (::glob("/proc/[0-9]*/maps", GLOB_NOSORT, nullptr, &globbuf)) |
There was a problem hiding this comment.
I wonder how this will work for containers?
If we have different process namespaces, they will have their own view of /proc, so there are probably edge cases with a different process / mount namespace that we need to think through.
Provided bpftrace is in the same process / mount namespace you want to target though, this should work. If bpftrace runs from the root pid namespace i think this will be fine, but we may want to document this quick
There was a problem hiding this comment.
So if bpftrace runs in root host I believe all processes running in containers can be activated. If bpftrace runs inside a pid/mount namespace then only the processes visible inside that container will be activated. This seems like fairly desirable and expected behavior.
Let me confirm that this is the behavior.
|
makes sense, i added a note about mount namespaces but that can probably be solved later (and there are already lots of issues with USDT and separate mount/process namespaces, so people are probably used to quirks here already) |
danobi
force-pushed
the
usdt_file_activation
branch
from
c4408a8
to
6dd24e8
Compare
|
I added another runtime test for running inside pid + mount namespaces. I also added a note in the documentation about how doing funny things w/ private mount namespaces and bind mounts can bread usdt file activation. Lots of quirks indeed. |
danobi
force-pushed
the
usdt_file_activation
branch
from
6dd24e8
to
c75cc7e
Compare
|
Removed the container test. Worked on all my machines but fails in CI. No idea. |
|
There are actions available to allow ssh in to GitHub actions, maybe that can provide some insight into the failure? It might also be because actions runs in docker maybe? Can’t remember if it runs with host pid, I would have assumed so. (Checking from phone sorry) |
|
I'll put up another PR with the additional test and debug there |
While debugging a high memory consumption issue in bpftrace, I noticed that a USDT::Context object can take ~10M per instance [0]. Along with the new --usdt-file-activation feature in bpftrace ( bpftrace/bpftrace#1317 ), bpftrace can potentially hold onto many dozens of USDT:Context instances, causing memory issues. While reducing the amount of memory USDT::Context uses is one option, we can potentially side step it by allowing the usdt semaphore count to be set independently. Before, the only way to increment the count (by 1) is to call bcc_usdt_enable*(). bcc_usdt_enable*() has checks that limit it to a single increment per context. The only way to decrement the count is by calling bcc_usdt_close() which naturally only allows for one decrement. With independent semaphore helpers, we can avoid holding onto a USDT::Context instance for the lifetime of the tracing session. We can simply: 1. create a USDT::Context 2. increment the semaphore count for the probe we care about 3. destroy the USDT::Context 4. repeat 1-3 for all probes we want to attach to 5. do our tracing 6. create a USDT::Context for the probe we care about 7. decrement the semaphore count 8. destroy the USDT::Context 9. repeat 6-8 for all the probes we're attached to This approach also has the benefit of 1 USDT::Context instance being alive at a time which can help keep memory high watermark low. [0]: Through gdb single stepping and /proc/pid/status. Exact process is not described here b/c memory usage probably varies based on tracee binary.
While debugging a high memory consumption issue in bpftrace, I noticed that a USDT::Context object can take ~10M per instance [0]. Along with the new --usdt-file-activation feature in bpftrace ( bpftrace/bpftrace#1317 ), bpftrace can potentially hold onto many dozens of USDT:Context instances, causing memory issues. While reducing the amount of memory USDT::Context uses is one option, we can potentially side step it by allowing the usdt semaphore count to be set independently. Before, the only way to increment the count (by 1) is to call bcc_usdt_enable*(). bcc_usdt_enable*() has checks that limit it to a single increment per context. The only way to decrement the count is by calling bcc_usdt_close() which naturally only allows for one decrement. With independent semaphore helpers, we can avoid holding onto a USDT::Context instance for the lifetime of the tracing session. We can simply: 1. create a USDT::Context 2. increment the semaphore count for the probe we care about 3. destroy the USDT::Context 4. repeat 1-3 for all probes we want to attach to 5. do our tracing 6. create a USDT::Context for the probe we care about 7. decrement the semaphore count 8. destroy the USDT::Context 9. repeat 6-8 for all the probes we're attached to This approach also has the benefit of 1 USDT::Context instance being alive at a time which can help keep memory high watermark low. [0]: Through gdb single stepping and /proc/pid/status. Exact process is not described here b/c memory usage probably varies based on tracee binary.
While debugging a high memory consumption issue in bpftrace, I noticed that a USDT::Context object can take ~10M per instance [0]. Along with the new --usdt-file-activation feature in bpftrace ( bpftrace/bpftrace#1317 ), bpftrace can potentially hold onto many dozens of USDT:Context instances, causing memory issues. While reducing the amount of memory USDT::Context uses is one option, we can potentially side step it by allowing the usdt semaphore count to be set independently. Before, the only way to increment the count (by 1) is to call bcc_usdt_enable*(). bcc_usdt_enable*() has checks that limit it to a single increment per context. The only way to decrement the count is by calling bcc_usdt_close() which naturally only allows for one decrement. With independent semaphore helpers, we can avoid holding onto a USDT::Context instance for the lifetime of the tracing session. We can simply: 1. create a USDT::Context 2. increment the semaphore count for the probe we care about 3. destroy the USDT::Context 4. repeat 1-3 for all probes we want to attach to 5. do our tracing 6. create a USDT::Context for the probe we care about 7. decrement the semaphore count 8. destroy the USDT::Context 9. repeat 6-8 for all the probes we're attached to This approach also has the benefit of 1 USDT::Context instance being alive at a time which can help keep memory high watermark low. [0]: Through gdb single stepping and /proc/pid/status. Exact process is not described here b/c memory usage probably varies based on tracee binary.
This PR implements USDT semaphore probe activation. Previously, you had
to pass in
-p $PIDor-c $CMDto activate usdt semaphores on a process.This has the limitation of single binaries. It's also difficult to make work in a
distributed tracing environment where you don't know the PIDs beforehand are
attaching to running processes.
Checklist
docs/reference_guide.mdCHANGELOG.md