new(driver): add 2 new scap stats #1303

Andreagit97 · 2023-08-21T11:00:38Z

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area API-version

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap

/area libpman

/area libsinsp

Does this PR require a change in the driver versions?

/version driver-API-version-major

What this PR does / why we need it:

This PR adds 2 new kernel stats:

n_drops_buffer_close_exit: CLOSE exit event drops.
n_drops_buffer_proc_exit: SCHED_PROC_EXIT event drops.

They could be very useful when debugging possible mem-leak or high memory usage since the PROC_EXIT event is strictly related to the number of threads in our table while the CLOSE_X is related to the number of file descriptors.

For the same debugging purpose I added some logs when the capture is closed to print the dimension of the thread table and the total number of fd still opened.

Which issue(s) this PR fixes:

Special notes for your reviewer:

This PR requires an API major bump since the stat struct sent by the drivers is changed

Does this PR introduce a user-facing change?:

new(driver): add 2 new scap stats

- n_drops_buffer_close_exit: CLOSE exit event drops. - n_drops_buffer_proc_exit: SCHED_PROC_EXIT event drops. Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>

Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>

Andreagit97 · 2023-08-21T11:34:46Z

userspace/libsinsp/sinsp.cpp

+		"\nn_evts:%" PRIu64
+		"\nn_drops:%" PRIu64
+		"\nn_drops_buffer:%" PRIu64
+		"\nn_drops_buffer_clone_fork_enter:%" PRIu64
+		"\nn_drops_buffer_clone_fork_exit:%" PRIu64
+		"\nn_drops_buffer_execve_enter:%" PRIu64
+		"\nn_drops_buffer_execve_exit:%" PRIu64


I've slightly changed the way in which we print the log. Now we don't have anymore a single line but we put a \n after each stat. This is an output example

08-21 11:32:31.914754 n_evts:211937 n_drops:0 n_drops_buffer:0 n_drops_buffer_clone_fork_enter:0 n_drops_buffer_clone_fork_exit:0 n_drops_buffer_execve_enter:0 n_drops_buffer_execve_exit:0 n_drops_buffer_connect_enter:0 n_drops_buffer_connect_exit:0 n_drops_buffer_open_enter:0 n_drops_buffer_open_exit:0 n_drops_buffer_dir_file_enter:0 n_drops_buffer_dir_file_exit:0 n_drops_buffer_other_interest_enter:0 n_drops_buffer_other_interest_exit:0 n_drops_buffer_close_exit:0 n_drops_buffer_proc_exit:0 n_drops_scratch_map:0 n_drops_pf:0 n_drops_bug:0 08-21 11:32:31.914981 total threads in the table:1522, total fds in all threads:5319

So as you can see the output is quite small but if we should face issues in the future we could always revert it on one line, WDYT?

It's used for debugging aka no problems at all would say

incertum

@Andreagit97 great idea, love it and you know I love any useful stats always 😄

On Falco side the new metrics will pick up these new stats automatically, but the old drop internal messages / stats would need an update PR as well.

Just left a small ask for some unrelated cleanup to consider while you are here. Can then approve tomorrow!

incertum · 2023-08-23T06:22:22Z

userspace/libsinsp/sinsp.cpp

+		"\nn_evts:%" PRIu64
+		"\nn_drops:%" PRIu64
+		"\nn_drops_buffer:%" PRIu64
+		"\nn_drops_buffer_clone_fork_enter:%" PRIu64
+		"\nn_drops_buffer_clone_fork_exit:%" PRIu64
+		"\nn_drops_buffer_execve_enter:%" PRIu64
+		"\nn_drops_buffer_execve_exit:%" PRIu64


It's used for debugging aka no problems at all would say

incertum · 2023-08-23T06:27:06Z

driver/bpf/fillers.h

@@ -190,6 +190,12 @@ FILLER_RAW(terminate_filler)
 					++state->n_drops_buffer_other_interest_enter;
 				}
 				break;
+			case PPME_PROCEXIT_E:


@Andreagit97 while we are here, I think it was during the loginuid PR where I saw we could cleanup and remove some of the old PPME_SYSCALL_CLONE_* events. Could we sneak this cleanup in?

Also do you think the categories n_drops_buffer_dir_file* and n_drops_buffer_other_interest* could benefit from some updates while here?

Done :)

Also do you think the categories n_drops_buffer_dir_file* and n_drops_buffer_other_interest* could benefit from some updates while here?

During my experiments I've never used other stats so at the moment I would say no 🤔

userspace/libscap/examples/01-open/README.md

leogr · 2023-08-23T10:47:05Z

/milestone 0.13.0

Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com> Co-authored-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

FedeDP · 2023-08-23T16:22:29Z

driver/bpf/fillers.h

Nice cleanup :D

FedeDP

/approve

poiana · 2023-08-23T16:22:39Z

LGTM label has been added.

Git tree hash: 5dcbda3dbbfb5e3ea5bf2471a97686564854c766

incertum

/approve

poiana · 2023-08-23T17:16:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, incertum

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,FedeDP,incertum]

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

Andreagit97 added 3 commits August 21, 2023 12:10

new(driver): add 2 new scap stats

920785e

- n_drops_buffer_close_exit: CLOSE exit event drops. - n_drops_buffer_proc_exit: SCHED_PROC_EXIT event drops. Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>

new(sinsp): introduce a common print stats function

fb43003

Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>

new(sinsp): add some debug logs when the capture is closed

ed8276a

Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>

poiana added release-note dco-signoff: yes kind/feature New feature or request area/API-version area/driver-kmod area/driver-bpf area/driver-modern-bpf area/libscap-engine-bpf area/libscap-engine-kmod area/libscap-engine-modern-bpf area/libscap area/libpman area/libsinsp labels Aug 21, 2023

Andreagit97 added this to the 0.13.0 milestone Aug 21, 2023

poiana added the size/L label Aug 21, 2023

Andreagit97 modified the milestones: 0.13.0, driver-backlog Aug 21, 2023

poiana added the approved label Aug 21, 2023

poiana requested review from FedeDP and hbrueckner August 21, 2023 11:01

Andreagit97 modified the milestones: driver-backlog, next-driver Aug 21, 2023

Andreagit97 force-pushed the add_sinsp_stats branch from 5461298 to ed8276a Compare August 21, 2023 11:01

Andreagit97 commented Aug 21, 2023

View reviewed changes

incertum reviewed Aug 23, 2023

View reviewed changes

userspace/libscap/examples/01-open/README.md Outdated Show resolved Hide resolved

poiana modified the milestones: next-driver, 0.13.0 Aug 23, 2023

Andreagit97 and others added 2 commits August 23, 2023 16:10

cleanup(driver): remove old event versions

11c0b2c

Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com> Co-authored-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

docs: update scap-open README

d22d8b7

Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com> Co-authored-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

poiana added size/XL and removed size/L labels Aug 23, 2023

FedeDP reviewed Aug 23, 2023

View reviewed changes

driver/bpf/fillers.h

Copy link

Contributor

FedeDP Aug 23, 2023

Choose a reason for hiding this comment

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

Nice cleanup :D

FedeDP approved these changes Aug 23, 2023

View reviewed changes

poiana assigned FedeDP Aug 23, 2023

poiana added the lgtm label Aug 23, 2023

incertum approved these changes Aug 23, 2023

View reviewed changes

poiana assigned incertum Aug 23, 2023

poiana merged commit 3805779 into falcosecurity:master Aug 23, 2023
33 checks passed

Andreagit97 mentioned this pull request Sep 4, 2023

update: introduce new stats updated to the latest libs version falcosecurity/falco#2766

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new(driver): add 2 new scap stats #1303

new(driver): add 2 new scap stats #1303

Andreagit97 commented Aug 21, 2023

Andreagit97 Aug 21, 2023

incertum Aug 23, 2023

incertum left a comment

incertum Aug 23, 2023

incertum Aug 23, 2023

Andreagit97 Aug 23, 2023

leogr commented Aug 23, 2023

FedeDP Aug 23, 2023

FedeDP left a comment

poiana commented Aug 23, 2023

incertum left a comment

poiana commented Aug 23, 2023

new(driver): add 2 new scap stats #1303

new(driver): add 2 new scap stats #1303

Conversation

Andreagit97 commented Aug 21, 2023

Andreagit97 Aug 21, 2023

Choose a reason for hiding this comment

incertum Aug 23, 2023

Choose a reason for hiding this comment

incertum left a comment

Choose a reason for hiding this comment

incertum Aug 23, 2023

Choose a reason for hiding this comment

incertum Aug 23, 2023

Choose a reason for hiding this comment

Andreagit97 Aug 23, 2023

Choose a reason for hiding this comment

leogr commented Aug 23, 2023

FedeDP Aug 23, 2023

Choose a reason for hiding this comment

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Aug 23, 2023

incertum left a comment

Choose a reason for hiding this comment

poiana commented Aug 23, 2023