Fix pkgconfig files (mostly for static linking)

[gnosek]

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:

This PR cleans up a bunch of things related to pkg-config files, so that:

• generated pkg-config files can be used to build a working binary with static/dynamic linking and bundled/system deps
• build directory paths do not leak into the generated pkg-config files

Also, it turns out we no longer have a circular dependency between scap and scap_engine_gvisor, so just make scap_engine_gvisor into a proper shared library.

Which issue(s) this PR fixes:

Fixes #2004

Special notes for your reviewer:

Is there a CI wizard in the house? We could use a similar test (build sinsp-example using pkgconfig) for static builds but I feel like I'd break more than fix if I tried that myself :)

Also, fingers crossed for all the build variants I was unable to test locally :)

Does this PR introduce a user-facing change?:

fix(build): pkgconfig files should be now generated properly even in static library builds
fix(build): scap_engine_gvisor is now a separate shared library

[github-actions]

Perf diff from master - unit tests

     8.93%     -1.87%  [.] sinsp::next
     9.14%     +1.06%  [.] sinsp_parser::reset
     4.18%     +0.96%  [.] sinsp_evt::load_params
     1.19%     +0.66%  [.] std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release
     1.27%     -0.66%  [.] sinsp_evt::get_direction
     1.99%     -0.66%  [.] libsinsp::sinsp_suppress::process_event
     2.43%     +0.65%  [.] sinsp_thread_manager::find_thread
     1.92%     -0.56%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     1.38%     +0.50%  [.] 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.71%     +0.50%  [.] libsinsp::state::stl_container_table_adapter<std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, libsinsp::state::value_table_entry_adapter<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, libsinsp::state::value_table_entry_adapter<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >::dynamic_fields_t>::stl_container_table_adapter

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

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.0329         -0.0329           147           142           147           142
BM_sinsp_split_median                                          -0.0355         -0.0355           147           142           147           142
BM_sinsp_split_stddev                                          +3.0737         +3.0737             0             2             0             2
BM_sinsp_split_cv                                              +3.2124         +3.2124             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  -0.0235         -0.0235            57            55            57            55
BM_sinsp_concatenate_paths_relative_path_median                -0.0218         -0.0218            57            55            57            55
BM_sinsp_concatenate_paths_relative_path_stddev                +1.2576         +1.2605             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_cv                    +1.3119         +1.3148             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     -0.0406         -0.0406            25            24            25            24
BM_sinsp_concatenate_paths_empty_path_median                   -0.0345         -0.0345            25            24            25            24
BM_sinsp_concatenate_paths_empty_path_stddev                   -0.8749         -0.8772             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       -0.8696         -0.8720             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  -0.0112         -0.0112            57            56            57            56
BM_sinsp_concatenate_paths_absolute_path_median                -0.0099         -0.0099            56            56            56            56
BM_sinsp_concatenate_paths_absolute_path_stddev                -0.3382         -0.3384             1             0             1             0
BM_sinsp_concatenate_paths_absolute_path_cv                    -0.3307         -0.3310             0             0             0             0
BM_sinsp_split_container_image_mean                            -0.0083         -0.0083           402           399           402           399
BM_sinsp_split_container_image_median                          -0.0093         -0.0093           403           399           403           399
BM_sinsp_split_container_image_stddev                          -0.3526         -0.3514             3             2             3             2
BM_sinsp_split_container_image_cv                              -0.3472         -0.3460             0             0             0             0

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.58%. Comparing base (aeb8793) to head (53cf2d7).
Report is 14 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2005      +/-   ##
==========================================
- Coverage   73.58%   73.58%   -0.01%     
==========================================
  Files         253      253              
  Lines       31869    31869              
  Branches     5649     5630      -19     
==========================================
- Hits        23452    23451       -1     
+ Misses       8416     8401      -15     
- Partials        1       17      +16     

Flag	Coverage Δ
libsinsp	73.58% <ø> (-0.01%)	⬇️

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.

[github-actions]

Perf diff from master - unit tests

     5.00%     +1.35%  [.] sinsp_evt::get_type
    11.45%     -0.85%  [.] sinsp_parser::reset
     0.11%     +0.55%  [.] scap_file_test_helpers::capture_search_evt_by_num
     2.12%     +0.47%  [.] 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
     1.06%     -0.42%  [.] sinsp_parser::event_cleanup
     2.01%     -0.39%  [.] std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release
     0.47%     +0.38%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>
     0.50%     -0.36%  [.] sinsp_evt::is_filtered_out
     3.34%     -0.36%  [.] sinsp_thread_manager::get_thread_ref
     0.44%     -0.36%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_fdinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_fdinfo> > >, 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> >::clear

Perf diff from master - scap file

    16.17%     -7.33%  [.] sinsp_filter_check::tostring
     9.70%     +3.22%  [.] sinsp_filter_check::rawval_to_string
    12.96%     -2.11%  [.] sinsp_filter_check_event::extract_single
     9.69%     -2.04%  [.] sinsp_evt::get_type
     6.36%     -1.98%  [.] libsinsp::runc::match_container_id
     6.49%     -1.90%  [.] sinsp_filter_check_thread::extract_single
     3.22%     +1.61%  [.] sinsp_filter_check_container::extract_single
     3.24%     +1.59%  [.] 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.18%     -0.25%  [.] std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::emplace_back<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >
     6.43%     -0.17%  [.] sinsp_evt_formatter::tostring_withformat

Heap diff from master - unit tests

peak heap memory consumption: -5.18K
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

[geraldcombs]

LGTM. I tested a Logray build on macOS using the following CMake preset. The generated pkgconfig files look correct and after updating the Logray code to match the current libs API, everything built and ran as expected.

    {
        "name": "libs-tmp",
        "displayName": "Falco libs",
        "description": "libs for bundling with Logray, /tmp/libs-test",
        "generator": "Ninja",
        "binaryDir": "${sourceDir}/build",
        "cacheVariables": {
            "BUILD_SHARED_LIBS": "ON",
            "MINIMAL_BUILD": "ON",
            "USE_BUNDLED_DEPS": "OFF",
            "USE_BUNDLED_VALIJSON": "ON",
            "USE_BUNDLED_TINYDIR": "ON",
            "CREATE_TEST_TARGETS": "OFF",
            "CMAKE_BUILD_TYPE": "RelWithDebInfo",
            "CMAKE_INSTALL_PREFIX": "/tmp/libs-test"
        }
    },

[leogr]

Just a question after a very first look: will BUILD_SHARED_LIBS behave like before? 🤔

[gnosek]

WDYM? libscap_engine_gvisor.so becomes a separate shared library, so that's a change, but other than that, there should be no observable changes (at least, Works For Me ™️)

TBH I'm not entirely sure when the circular dependency disappeared, probably when extracting some scap_platform_util or other

[leogr]

I meant regarding BUILD_SHARED_LIBS in general, but I noticed right now that it's only related to gvisor (and I believe it's acceptable).

cc @LucaGuerra

[gnosek]

FWIW, I consider splitting out scap_engine_gvisor as a separate .so a fix

[LucaGuerra]

sounds fine!

[leogr]

Should this be PUBLIC too? 🤔

[gnosek]

Hmm we'll find out when somebody tries to include scap_gvisor's headers :)

Overall, including anything except libsinsp/sinsp.h and its dependencies is not really supported right now (IIRC you can't include libsinsp/cri.h for example) and there's a decision to make. We either:

• support that and install a bunch of extra headers (IIRC we need absl and it's a pain to extract from grpc sources)
• explicitly don't support that and (ideally) remove the headers from the installed set

I'd greatly prefer option 2 (drop most headers from the installed set) but that would require us to have a well specified API :) I guess one option is to remove them and see who complains. Another would be to decree that sinsp.h is the public API and anything it does not include is off limits (if you want to include anything else, you're not using sinsp, you're developing it).

Either way, I think it's best to have a clear way with the private(ish) headers shipped with libs and IMO figuring that out is out of scope for this PR (which AFAIK at least doesn't make things worse).

[leogr]

we'll find out when somebody tries to include scap_gvisor's headers :)
😶‍🌫️

[gnosek]

I updated the branch to handle bs_threadpool, mostly by ignoring it (it does not appear in any headers it seems). I haven't got around to using the thread pool yet, but if it turns out we do need to install the headers, we'll know eventually ;)

[FedeDP]

/milestone 0.19.0

[FedeDP]

Hey @gnosek can you rebase this PR on top of latest master (so that we test zig build? :D )

After that, this LGTM and can be merged IMHO.

[LucaGuerra]

Agreed! Let's rebase and merge this

[gnosek]

@FedeDP @LucaGuerra just rebased and pushed

[FedeDP]

Thank you Grzeg!

[FedeDP]

/approve

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, gnosek

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 [FedeDP,gnosek]

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

[poiana]

LGTM label has been added.

Git tree hash: ce095dd5ce42502b3f115984198c4b13cb82f02e