fix(libsinsp): add function to set compiler filter and filter string #1976

therealbobo · 2024-07-26T14:03:24Z

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:

When setting the compiled filter through set_filter, we lose the filter string information and never populate the m_filterstring. This adds a method to set them both.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

github-actions · 2024-07-26T15:52:51Z

Perf diff from master - unit tests

     8.09%     -1.30%  [.] sinsp::next
     1.32%     -0.90%  [.] scap_next
     9.11%     +0.90%  [.] sinsp_parser::reset
     4.82%     -0.84%  [.] sinsp_parser::process_event
     2.87%     +0.81%  [.] sinsp_thread_manager::find_thread
     0.91%     +0.65%  [.] sinsp::fetch_next_event
     0.75%     +0.64%  [.] std::vector<sinsp_evt_param, std::allocator<sinsp_evt_param> >::emplace_back<sinsp_evt*, unsigned int&, char const*, unsigned long&>
     2.58%     +0.59%  [.] sinsp_thread_manager::get_thread_ref
     0.98%     +0.54%  [.] libsinsp::sinsp_suppress::process_event
     1.03%     -0.48%  [.] sinsp_parser::parse_context_switch

Perf diff from master - scap file

    20.26%     -5.70%  [.] sinsp_evt_formatter::tostring_withformat
     3.56%     +5.17%  [.] sinsp_split[abi:cxx11]
     4.05%     +2.92%  [.] sinsp::next
     8.08%     -2.07%  [.] sinsp_threadinfo::~sinsp_threadinfo
     8.09%     -2.04%  [.] sinsp_filter_check::get_transformed_field_info
     4.04%     +1.95%  [.] sinsp_filter_check::extract
     3.81%     +1.91%  [.] sinsp_filter_check::tostring
     4.06%     +1.65%  [.] sinsp_parser::reset
     4.03%     +1.57%  [.] libsinsp::runc::match_container_id
     8.10%     +1.50%  [.] sinsp_filter_check::rawval_to_string

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: -30B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

codecov · 2024-07-26T16:00:38Z

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 50.99%. Comparing base (1feeb37) to head (8bc17ef).
Report is 3 commits behind head on master.

Files	Patch %	Lines
userspace/libsinsp/sinsp.cpp	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1976      +/-   ##
==========================================
- Coverage   50.99%   50.99%   -0.01%     
==========================================
  Files         310      310              
  Lines       39607    39608       +1     
  Branches    17533    17242     -291     
==========================================
- Hits        20199    20198       -1     
+ Misses      14336    14320      -16     
- Partials     5072     5090      +18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

FedeDP · 2024-07-31T08:37:19Z

/milestone 0.18.0

userspace/libsinsp/sinsp.cpp

Signed-off-by: Roberto Scolaro <roberto.scolaro21@gmail.com>

github-actions · 2024-08-05T17:00:29Z

Perf diff from master - unit tests

     9.08%     +2.09%  [.] sinsp_parser::reset
     1.94%     -0.95%  [.] sinsp_evt::get_direction
     3.60%     +0.83%  [.] gzfile_read
     1.27%     -0.63%  [.] sinsp_evt::get_param
     2.05%     -0.52%  [.] libsinsp::sinsp_suppress::process_event
     7.14%     +0.42%  [.] sinsp::next
     4.99%     -0.41%  [.] sinsp_parser::process_event
     2.49%     -0.39%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     0.62%     -0.37%  [.] sinsp_container_manager::resolve_container
     0.44%     -0.36%  [.] scap_cgroup_resolve_v2

Perf diff from master - scap file

    29.07%    -17.32%  [.] sinsp_filter_check::extract_nocache
     9.66%     +6.73%  [.] sinsp_evt_formatter::tostring_withformat
     3.22%     +5.22%  [.] sinsp_filter_check::rawval_to_string
     6.87%     -2.69%  [.] sinsp::fetch_next_event
     6.43%     -2.40%  [.] sinsp_filter_check_thread::extract_single
     6.07%     -2.28%  [.] sinsp_split[abi:cxx11]
     9.66%     -0.84%  [.] sinsp_filter_check_event::extract_single
     6.54%     -0.55%  [.] next
     6.40%     -0.53%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     6.43%     -0.49%  [.] sinsp_filter_check::tostring

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

jasondellaluce · 2024-08-06T09:46:33Z

userspace/libsinsp/sinsp.cpp

@@ -1661,7 +1661,7 @@ void sinsp::start_dropping_mode(uint32_t sampling_ratio)
 }
 #endif // _WIN32

-void sinsp::set_filter(std::unique_ptr<sinsp_filter> filter)
+void sinsp::set_filter(std::unique_ptr<sinsp_filter> filter, const std::optional<std::string>& filterstring)


Suggested change

void sinsp::set_filter(std::unique_ptr<sinsp_filter> filter, const std::optional<std::string>& filterstring)

void sinsp::set_filter(std::unique_ptr<sinsp_filter> filter, const std::string& filterstring = "")

I think the previous suggestion was referring to something like this -- otherwise it'd become a breaking change for whoever uses this method.

Signed-off-by: Roberto Scolaro <roberto.scolaro21@gmail.com>

github-actions · 2024-08-06T10:42:02Z

Perf diff from master - unit tests

     1.95%     -1.25%  [.] sinsp_evt::get_direction
     5.01%     -1.21%  [.] sinsp_parser::process_event
     9.13%     +1.20%  [.] sinsp_parser::reset
     0.61%     +0.82%  [.] sinsp_parser::parse_context_switch
     0.89%     +0.77%  [.] libsinsp::events::is_unknown_event
     2.51%     -0.58%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     3.62%     +0.56%  [.] gzfile_read
     1.00%     -0.47%  [.] scap_event_has_large_payload
     0.33%     +0.46%  [.] sinsp_fdtable::find
     3.61%     +0.46%  [.] sinsp_evt::load_params

Perf diff from master - scap file

    28.28%    -16.16%  [.] sinsp_filter_check::extract_nocache
     6.26%     +6.25%  [.] sinsp::next
     9.39%     +5.41%  [.] sinsp_evt_formatter::tostring_withformat
     6.25%     -2.75%  [.] sinsp_evt::load_params
     5.90%     -2.65%  [.] sinsp_split[abi:cxx11]
     6.23%     -1.29%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     6.26%     -1.24%  [.] sinsp_filter_check::tostring
     3.13%     +0.92%  [.] sinsp_filter_check::rawval_to_string
     3.25%     -0.68%  [.] gzfile_read
     3.13%     -0.64%  [.] sinsp_parser::process_event

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

poiana · 2024-08-06T16:51:53Z

LGTM label has been added.

Git tree hash: 7b9e038461122e9cf2988a7a017491405c64d445

Andreagit97

/approve

poiana · 2024-08-07T08:15:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, therealbobo

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:

~~OWNERS~~ [Andreagit97]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

poiana added release-note-none kind/feature New feature or request dco-signoff: yes labels Jul 26, 2024

poiana requested review from FedeDP and hbrueckner July 26, 2024 14:03

poiana added the size/S label Jul 26, 2024

therealbobo force-pushed the add-set-filter-string branch from 21a39c2 to 23185fe Compare July 26, 2024 14:03

therealbobo changed the title ~~cix(libsinsp): add function to set compiler filter and filter string~~ fix(libsinsp): add function to set compiler filter and filter string Jul 26, 2024

therealbobo force-pushed the add-set-filter-string branch from 23185fe to 668213e Compare July 26, 2024 15:48

poiana added this to the 0.18.0 milestone Jul 31, 2024

FedeDP reviewed Jul 31, 2024

View reviewed changes

userspace/libsinsp/sinsp.cpp Outdated Show resolved Hide resolved

therealbobo added 2 commits August 5, 2024 16:50

fix(libsinsp): add function to set compiler filter and filter string

ddce334

Signed-off-by: Roberto Scolaro <roberto.scolaro21@gmail.com>

refactor(libsinsp): set filterstring optionally

f5c20e4

Signed-off-by: Roberto Scolaro <roberto.scolaro21@gmail.com>

therealbobo force-pushed the add-set-filter-string branch from 668213e to f5c20e4 Compare August 5, 2024 16:50

poiana added size/XS and removed size/S labels Aug 5, 2024

jasondellaluce reviewed Aug 6, 2024

View reviewed changes

fix(libsinsp): address review comments

8bc17ef

Signed-off-by: Roberto Scolaro <roberto.scolaro21@gmail.com>

jasondellaluce approved these changes Aug 6, 2024

View reviewed changes

poiana assigned jasondellaluce Aug 6, 2024

poiana added the lgtm label Aug 6, 2024

Andreagit97 approved these changes Aug 7, 2024

View reviewed changes

poiana assigned Andreagit97 Aug 7, 2024

poiana added the approved label Aug 7, 2024

poiana merged commit ae812ee into falcosecurity:master Aug 7, 2024
45 of 46 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(libsinsp): add function to set compiler filter and filter string #1976

fix(libsinsp): add function to set compiler filter and filter string #1976

therealbobo commented Jul 26, 2024

github-actions bot commented Jul 26, 2024

codecov bot commented Jul 26, 2024 •

edited

Loading

FedeDP commented Jul 31, 2024

github-actions bot commented Aug 5, 2024

jasondellaluce Aug 6, 2024

github-actions bot commented Aug 6, 2024

poiana commented Aug 6, 2024

Andreagit97 left a comment

poiana commented Aug 7, 2024

	void sinsp::set_filter(std::unique_ptr<sinsp_filter> filter, const std::optional<std::string>& filterstring)
	void sinsp::set_filter(std::unique_ptr<sinsp_filter> filter, const std::string& filterstring = "")

fix(libsinsp): add function to set compiler filter and filter string #1976

fix(libsinsp): add function to set compiler filter and filter string #1976

Conversation

therealbobo commented Jul 26, 2024

github-actions bot commented Jul 26, 2024

Perf diff from master - unit tests

Perf diff from master - scap file

Heap diff from master - unit tests

Heap diff from master - scap file

codecov bot commented Jul 26, 2024 • edited Loading

Codecov Report

FedeDP commented Jul 31, 2024

github-actions bot commented Aug 5, 2024

Perf diff from master - unit tests

Perf diff from master - scap file

Heap diff from master - unit tests

Heap diff from master - scap file

jasondellaluce Aug 6, 2024

Choose a reason for hiding this comment

github-actions bot commented Aug 6, 2024

Perf diff from master - unit tests

Perf diff from master - scap file

Heap diff from master - unit tests

Heap diff from master - scap file

poiana commented Aug 6, 2024

Andreagit97 left a comment

Choose a reason for hiding this comment

poiana commented Aug 7, 2024

codecov bot commented Jul 26, 2024 •

edited

Loading