cleanup(userspace/libsinsp): drop user and group infos embedded in threadinfo #2165
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
|
/milestone 0.20.0 |
FedeDP
force-pushed
the
cleanup/drop_user_group_threadinfo
branch
from
3a9f7b0 to
727fce2
Compare
…readinfo. Only store `uid`, `gid` and `loginuid` info. Signed-off-by: Federico Di Pierro <[email protected]>
FedeDP
force-pushed
the
cleanup/drop_user_group_threadinfo
branch
from
727fce2 to
43754ca
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2165 +/- ##
==========================================
+ Coverage 74.77% 75.06% +0.28%
==========================================
Files 254 255 +1
Lines 33505 33521 +16
Branches 5748 5719 -29
==========================================
+ Hits 25053 25162 +109
+ Misses 8452 8359 -93
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
FedeDP
changed the title
…avior. Signed-off-by: Federico Di Pierro <[email protected]>
Perf diff from master - unit testsHeap diff from master - unit testsHeap diff from master - scap fileBenchmarks diff from master |
FedeDP
changed the title
FedeDP
commented
Nov 20, 2024
| new_child->set_user(new_child->m_user.uid()); | ||
| new_child->set_loginuser(new_child->m_loginuser.uid()); | ||
| new_child->set_group(new_child->m_group.gid()); | ||
| new_child->set_group(new_child->m_gid); |
There was a problem hiding this comment.
Always group first, since it is used by set_user as gid to be passed to user_group_manager.add_user : https://github.com/falcosecurity/libs/pull/2165/files#diff-f1f5adc2cc836ddaf885f86ded61c940d5c957781ea3ef5cef7f4c6f407054f7R539
FedeDP
commented
Nov 20, 2024
userspace/libsinsp/threadinfo.cpp
Outdated
Comment on lines
517
to
532
| const sinsp_threadinfo::cgroups_t& sinsp_threadinfo::cgroups() const { | ||
| return m_cgroups; | ||
| } | ||
|
|
||
| std::string sinsp_threadinfo::get_comm() const { | ||
| return m_comm; | ||
| } | ||
|
|
||
| std::string sinsp_threadinfo::get_exe() const { | ||
| return m_exe; | ||
| } | ||
|
|
||
| std::string sinsp_threadinfo::get_exepath() const { | ||
| return m_exepath; | ||
| } | ||
|
|
Andreagit97
reviewed
Nov 27, 2024
|
Thanks @Andreagit97 ! Pushed the changes addressing your review comments! |
|
ops you missed a replace of |
|
yup just saw that ahah |
Signed-off-by: Federico Di Pierro <[email protected]> Co-authored-by: Andrea Terzolo <[email protected]>
FedeDP
force-pushed
the
cleanup/drop_user_group_threadinfo
branch
from
45cd5af to
0791a6d
Compare
|
Fixed! Thanks! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, FedeDP The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jasondellaluce
approved these changes
Nov 29, 2024
This was referenced Dec 2, 2024
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.
What type of PR is this?
/kind cleanup
Any specific area of the project related to this PR?
/area libsinsp
Does this PR require a change in the driver versions?
What this PR does / why we need it:
Only store
uid,gidandloginuidinfo.Storing full info was done to workaround scenarios like:
Basically, we are now respecting what unix expects us to do, since user 1000 will now be mapped to "bar".
Moreover, since threadinfo is used many times, not embedding those bytes saves us lots of memory.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: