Conversation
This is required to include falcosecurity/libs@362b170 Signed-off-by: Leonardo Grasso <[email protected]>
…mits Signed-off-by: Leonardo Grasso <[email protected]>
…` limits Signed-off-by: Leonardo Grasso <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
ekoops
left a comment
There was a problem hiding this comment.
I'd replace filesize with file_size everywhere, but I'm gonna approve it. Feel free to change or mantain it as is.
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ekoops, leogr The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
good point. Just to collect some more feedback about this |
| # in milliseconds. | ||
| # in milliseconds. Optionally, set `capture.default_events` to limit the | ||
| # number of captured events, and `capture.default_filesize` to limit the | ||
| # capture file size in kB. |
There was a problem hiding this comment.
Should this be "kiB"? There was a big push to use IEC units for base 2 prefixes years ago, but I'm not sure where best practices stand nowadays.
This looks great otherwise!
There was a problem hiding this comment.
Good point 🤔
I checked, and the Falco project consistently uses SI-style unit names everywhere (e.g., rollover_mb in sinsp_cycledumper, convert_memory_to_mb, and kb for RSS/PSS/VSZ in Falco metrics). There are no IEC units (kiB, MiB) used anywhere in either falco or libs.
I believe sticking with kB here is the right call for consistency with the rest of the codebase, even though technically the code multiplies by 1024.
Likely, it is worth keeping a note for a full review of Falco with respect to the units convention.
Thanks for the review! 🙏
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area engine
/area tests
What this PR does / why we need it:
This adds capture_events and
capture_filesize(kB) as new stop conditions for capture files, both as per-rule fields and as global config defaults (default_events,default_filesize). When multiple conditions are configured alongside the existing capture_duration, the first one that is met stops the capture (OR semantics).Also fixes:
Which issue(s) this PR fixes:
Fixes #3764
Special notes for your reviewer:
/milestone 0.44.0
cc @geraldcombs
Does this PR introduce a user-facing change?: