Cleanup list fields #153
Cleanup list fields #153
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mstemm 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 |
mstemm
force-pushed
the
cleanup-list-fields
branch
from
de6e2de to
9a085e2
Compare
mstemm
force-pushed
the
cleanup-list-fields
branch
from
9a085e2 to
345aa4e
Compare
|
Sorry I didn't know markdown output was actually used. I added it back. PTAL again! |
|
Tested this change and terminal+markdown formatting seems to be right. Just a little thing: I noticed that |
Previously, as plugins were loaded, each plugin would create two filtercheck objects: - A sinsp_filter_check_plugininfo object that defined the fields evt.pluginname/evt.plugininfo, with an implementation that only returned values for events coming from that specific plugin. - A sinsp_filter_check_plugin object that defined the fields exported by the plugin. This made interpreting the output of sysdig -l/falco --list confusing, as you would get multiple sections in the output for the almost-duplicate filterchecks. Fix this by moving evt.plugininfo/evt.pluginname into sinsp_filter_check_gen_event and looking up the plugin object on the fly. This avoids the almost-duplicate filtercheck objects, which makes it easier to print a clean list of fields, as these fields are only defined in one filtercheck object. Also, *only* create a sinsp_filter_check_gen_event object for source plugins. There isn't any need to create one for extractor plugins, which don't work directly with event meta-data like event number, timestamp, etc. Signed-off-by: Mark Stemm <[email protected]>
Add additional info to filter_field_info: - shortdesc: a small (3-5 word) description of the field class, suitable for printing on the same line as the field class without line wrapping. - data_type: the data type the filtercheck field works with (e.g. CHARBUF, UINT64, IPADDR, etc). - tags: a set of free-form tags for the field. Examples include "FILTER ONLY", IDX_REQUIRED, etc. This will make it easier to print rich information about each field. Also fill in shortdesc for selected filtercheck objects to help provide context when printing fields. Signed-off-by: Mark Stemm <[email protected]>
When plugins are loaded for programs that don't use event types, you might end up with duplicate sinsp_filter_check_gen_event objects being added to the g_filterlist filter_check_list. This isn't harmful, but it results in duplicate entries when listing the full set of filter fields. Handle this by checking to see if there's already a filter_check object with the same name (e.g. "evt") and shortdesc (e.g. "All event types"). If there's a duplicate, simply delete the to-be-added object. Signed-off-by: Mark Stemm <[email protected]>
Add a convienence method sinsp_filter_factory::check_infos_to_fieldclass_infos() that converts from a vector<filter_check_info> (e.g. filterchecks specifically for sinsp events) to a list<filter_fieldclass_info> (e.g. description of fields for all event types). This makes it easier to have a single method to print info about filtercheck fields. Signed-off-by: Mark Stemm <[email protected]>
Add methods to filter_fieldclass_info to return a pretty-printed representation of the field class, as a string. The output looks like the following: ------------------------------- Field Class: evt (All event types) Description: These fields can be used for all event types Event Sources: aws_cloudtrail syscall evt.num (Type: UINT64) event number. ... Internally, it uses stringstreams to tokenize long strings like the description, etc, and wrap them as needed. It also cleans up the output so words aren't broken across lines. It has options to print verbose information (adding Types for each field), and if a set of event sources was provided, those are printed as well. This will allow the --list outputs of falco, as well as other programs that use the libs, to have a single representation of fields. Signed-off-by: Mark Stemm <[email protected]>
list_fields() is used by programs that use the falcosecurity libs but are not falco, with its strong notion of event types. Replace the old version, which used printf and character-by-character iteration over line strings, with using sinsp_filter_factory::check_infos_to_fieldclass_infos to get a generic representation of field classes, and filter_fieldclass_info::as_string to format the field class info as a string. markdown output is now in a dedicated (static) function list_fields_markdown(). This allows for a consistent display of field info across programs. Signed-off-by: Mark Stemm <[email protected]>
mstemm
force-pushed
the
cleanup-list-fields
branch
from
345aa4e to
db5b6cf
Compare
This ensures that they will be displayed next to each other when printing all fields. Signed-off-by: Mark Stemm <[email protected]>
|
Sorry I didn't know markdown output was actually used. I added it back. PTAL again!
There isn't any explicit sorting of fields at the moment, the order is the order in which the filterchecks are added to the global sinsp_filter_check_list: https://github.com/falcosecurity/libs/blob/master/userspace/libsinsp/filter_check_list.cpp#L106. But I did move sinsp_filter_check_gen_event to the top so it will be next to evt (Syscall events only). |
There was a problem hiding this comment.
@mstemm yes, I did the same when I wanted to reorder the doc page. LGTM.
|
@LucaGuerra: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/retest |
|
/check-dco |
|
LGTM label has been added. Git tree hash: 3beda0f0710961840fac24f3aacb20266d390b9e
|
|
@FedeDP: changing LGTM is restricted to collaborators In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
What type of PR is this?
/kind cleanup
Any specific area of the project related to this PR?
/area libsinsp
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: