Skip to content
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

feat(libsinsp/container_engine): proper containerd support #2195

Merged
merged 14 commits into from
Jan 8, 2025

Conversation

therealbobo
Copy link
Contributor

@therealbobo therealbobo commented Dec 9, 2024

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:

Up until now the container information were populated only through the cri api. There are cases in which this is not sufficient (e.g. bottlerocket): for example the containerd runtime keeps the containers creates through the cri api separated from the ones created with the containerd api. This PR aims to support also the containers created with the vanilla containerd api.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new(userspace/libsinsp): proper containerd engine support

Copy link

github-actions bot commented Dec 9, 2024

Perf diff from master - unit tests

    11.81%     -1.13%  [.] sinsp::next
     2.66%     +0.51%  [.] sinsp_thread_manager::get_thread_ref
     0.33%     +0.40%  [.] scap_file_test::assert_num_event_type
     1.10%     -0.38%  [.] 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> >::find
     0.52%     +0.32%  [.] libsinsp::events::is_unknown_event
     8.03%     -0.30%  [.] sinsp_evt::get_type
     1.00%     +0.30%  [.] sinsp_evt::get_ts
     1.44%     +0.27%  [.] libsinsp::sinsp_suppress::process_event
     1.22%     -0.26%  [.] sinsp_parser::event_cleanup
     0.23%     +0.26%  [.] libsinsp::runc::match_one_container_id

Heap diff from master - unit tests

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

Heap diff from master - scap file

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

Benchmarks diff from master

Comparing gbench_data.json to /root/actions-runner/_work/libs/libs/build/gbench_data.json
Benchmark                                                         Time             CPU      Time Old      Time New       CPU Old       CPU New
----------------------------------------------------------------------------------------------------------------------------------------------
BM_sinsp_split_mean                                            +0.0021         +0.0021           147           147           147           147
BM_sinsp_split_median                                          +0.0052         +0.0052           146           147           146           147
BM_sinsp_split_stddev                                          +0.1594         +0.1604             1             2             1             2
BM_sinsp_split_cv                                              +0.1570         +0.1580             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  +0.0225         +0.0225            61            63            61            63
BM_sinsp_concatenate_paths_relative_path_median                +0.0163         +0.0164            61            62            61            62
BM_sinsp_concatenate_paths_relative_path_stddev                +3.6003         +3.5992             0             1             0             1
BM_sinsp_concatenate_paths_relative_path_cv                    +3.4989         +3.4978             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     -0.0032         -0.0032            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_median                   -0.0025         -0.0025            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_stddev                   -0.5145         -0.5172             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       -0.5129         -0.5156             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  -0.0217         -0.0217            66            65            66            65
BM_sinsp_concatenate_paths_absolute_path_median                -0.0508         -0.0508            67            64            67            64
BM_sinsp_concatenate_paths_absolute_path_stddev                +0.4520         +0.4512             1             2             1             2
BM_sinsp_concatenate_paths_absolute_path_cv                    +0.4842         +0.4834             0             0             0             0
BM_sinsp_split_container_image_mean                            -0.0068         -0.0068           386           384           386           384
BM_sinsp_split_container_image_median                          -0.0079         -0.0080           387           384           387           384
BM_sinsp_split_container_image_stddev                          +0.1910         +0.1918             2             2             2             2
BM_sinsp_split_container_image_cv                              +0.1992         +0.2000             0             0             0             0

Copy link

codecov bot commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 18.60465% with 175 lines in your changes missing coverage. Please review.

Project coverage is 75.06%. Comparing base (63f9cfb) to head (513b0b5).
Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
userspace/libsinsp/container_engine/containerd.cpp 8.58% 149 Missing ⚠️
userspace/libsinsp/container_engine/containerd.h 0.00% 20 Missing ⚠️
userspace/libsinsp/container.h 20.00% 4 Missing ⚠️
userspace/libsinsp/container_engine/cri.cpp 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2195      +/-   ##
==========================================
- Coverage   75.44%   75.06%   -0.39%     
==========================================
  Files         265      267       +2     
  Lines       34057    34259     +202     
  Branches     5801     5929     +128     
==========================================
+ Hits        25695    25716      +21     
- Misses       8362     8543     +181     
Flag Coverage Δ
libsinsp 75.06% <18.60%> (-0.39%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@therealbobo therealbobo force-pushed the broader-container-id-support branch from 7c3c360 to b0e0a90 Compare December 9, 2024 17:54
@therealbobo therealbobo force-pushed the broader-container-id-support branch from b0e0a90 to bc7d08d Compare December 10, 2024 12:18
@therealbobo therealbobo force-pushed the broader-container-id-support branch 5 times, most recently from 82b4b68 to 3371202 Compare December 16, 2024 16:00
@therealbobo therealbobo force-pushed the broader-container-id-support branch 2 times, most recently from 2dc0158 to 3978470 Compare December 17, 2024 13:57
@Andreagit97 Andreagit97 added this to the TBD milestone Dec 17, 2024
@therealbobo therealbobo force-pushed the broader-container-id-support branch from 3978470 to 3c6b970 Compare December 17, 2024 19:54
@therealbobo therealbobo changed the title [WIP] proper containerd support feat(libsinsp/container_engine): proper containerd support Dec 18, 2024
@therealbobo therealbobo marked this pull request as ready for review December 18, 2024 06:32
@therealbobo therealbobo force-pushed the broader-container-id-support branch from 8254b52 to 0bc4f0b Compare January 7, 2025 15:34
@therealbobo therealbobo force-pushed the broader-container-id-support branch from 0bc4f0b to 443f4d8 Compare January 7, 2025 16:00
@therealbobo therealbobo force-pushed the broader-container-id-support branch from 443f4d8 to 513b0b5 Compare January 7, 2025 16:10
Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@poiana
Copy link
Contributor

poiana commented Jan 8, 2025

LGTM label has been added.

Git tree hash: 2e69e549c7a113b3b48e070ce8f554bd85d7e9a1

@poiana
Copy link
Contributor

poiana commented Jan 8, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, 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:

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

@FedeDP
Copy link
Contributor

FedeDP commented Jan 8, 2025

/unhold

@poiana poiana merged commit f535e22 into falcosecurity:master Jan 8, 2025
46 of 49 checks passed
FedeDP added a commit to FedeDP/container_plugin that referenced this pull request Jan 8, 2025
Backport changes from falcosecurity/libs#2195.

Signed-off-by: Federico Di Pierro <[email protected]>
FedeDP added a commit to FedeDP/container_plugin that referenced this pull request Jan 17, 2025
Backport changes from falcosecurity/libs#2195.

Signed-off-by: Federico Di Pierro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants