feature(proctree): Introduce a process tree #3364
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rafaeldtinoco
commented
Aug 5, 2023
rafaeldtinoco
commented
Aug 5, 2023
rafaeldtinoco
commented
Aug 6, 2023
rafaeldtinoco
commented
Aug 6, 2023
rafaeldtinoco
commented
Aug 6, 2023
AlonZivony
reviewed
Aug 6, 2023
There was a problem hiding this comment.
I haven't finished going over all of the PR, but I have the following general comments:
- This PR is a merge of 3 different concepts - control plane signals, user-mode process tree and procfs initialization. Each of them is independent, and I think would be better to be his unique PR (though maybe the control plane and process tree division now will be hard, so its your choice).
- From what I have seen (and I might be wrong) you only have threads in the tree. I think it will be hard to get interesting information about threads and processes this way. I will read it more deeply when I will review the rest of the code.
- I like the idea of exporting the responsibility of parsing the events from the tree. It looks much cleaner.
- I am a bit afraid that we use 3rd party for uesr-mode hashing but our for the eBPF hashing. We should think how to avoid the 3rd party changes creating inconsistency in the code.
- The whole control plane events should have much more designing, as the current design has a lot of issues that arose here.
- I don't think we can merge it till all the memory is capped, but I guess you didn't plan to as well :).
Nice work overall
AlonZivony
reviewed
Aug 10, 2023
yanivagman
previously requested changes
Aug 17, 2023
AlonZivony
reviewed
Aug 22, 2023
rafaeldtinoco
force-pushed
the
newproctreefull
branch
2 times, most recently
from
7756208
to
d25d8c6
Compare
rafaeldtinoco
force-pushed
the
newproctreefull
branch
from
d25d8c6
to
a7cf5c4
Compare
geyslan
reviewed
Aug 25, 2023
rafaeldtinoco
force-pushed
the
newproctreefull
branch
2 times, most recently
from
4abf20d
to
2a3d852
Compare
rafaeldtinoco
force-pushed
the
newproctreefull
branch
from
2a3d852
to
ea4f697
Compare
- Also: avoid duplicate entries in changelog
- improve proctree boottime calculation - use same function for tracee singleton starttime calculation
It is hard to debug the process tree. This commit removes the debug messages used during process tree development. The reason why there is a commit for this is because one might want to revert this commit in order to debug a new issue found until process tree is considered stable.
... at least until it is fully implemented, tested and stable.
- add debug message when same timestamp gets 2 diff values
... other than caching only.
rafaeldtinoco
force-pushed
the
newproctreefull
branch
from
3c79d54
to
5321b26
Compare
This was referenced Sep 18, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This set of commits introduce the Process Tree feature to tracee.
1cd7123 debug(proctree): remove debug information
25c7c0e chore(time): centralize timing functions
30abbef chore(changelog): change values if given timestamp exists
16cd971 debug(proctree): split evictions from procs and threads
9482357 chore(proctree): move anon procfs functions to real ones
b89b97b chore(proctree): turn async procfs channel writes non blocking
b37197c feature(proctree): create config setting for proctree cache size
d7cf7e8 feature(proctree): enable async procfs updates to proctree
e3ae4a4 feature(proctree): add changelog support to fileinfo
ab5e285 feature(proctree): add name to processes and threads
45e337d debug(proctree): display proctree cache status in debug mode
bc5b89e chore(proctree): do not update parent and leader when they exist
4297538 chore(proctree) adjust the place for process tree initialization
f08de61 feature(proctree): read single PID from procfs
ce564bb chore(controlplane): comment how to debug proctree
4928826 chore(controlplane): rename containers file and eventID var
8f06bbe feature(proctree): start the process tree data structure
8bef843 feature(changelog): introduce the changelog package
e06e284 feature(utils/proc): add stat and status file parsing
80df989 chore(controlplane): rename local variable to ctrl
d7cb57c feature(controller): create processes lifecycle functions
f5e6b5b chore(parse): tell argument type on argument parsing errors
9aca78c chore(ebpf): move functions with inline asm
55a782e feature(controlplane): create signal events to be used by controlplane
0e2de67 feature(events): task unique identifier murmur hashing
126c929 chore(types): update to latest containing entityID
Missing features:
All those being worked at: #3468