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

Add notion of generic formatters/formatter factories #77

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

mstemm
Copy link
Contributor

@mstemm mstemm commented Aug 25, 2021

Add the notion of "generic" event formatters and formatter factories
that can create a formatter for a given format string:

  • gen_filter.h defines two new classes: gen_event_formatter provides
    the interface to format events:
    • set_format(): set the output format and format string
    • tostring(): resolve the format with info from a gen_event,
      populating a resolved string.
    • tostring_withformat(): like tostring() but with a one-off output
      format.
    • get_field_values(): return all templated field names and values
      from the configured format string.
    • get_output_format(): get the current output format.
  • gen_event_formatter_factory performs a similar role as the existing
    sinsp_evt_formatter_cache, in that it maintains a cache of previously
    used format strings/formatters, to avoid the overhead of creating
    formatters. It simply returns a formatter, though, rather than
    duplicating the format methods like sinsp_evt_formatter_cache does.

This can be used in programs like falco to format general purpose
events without having a direct connection to an
inspector/filterchecks/etc.

  • The eventformatter changes simply add gen_event_formatter as a
    parent class and implements the interfaces. To aid in backwards
    compatibility with other parts of libsinsp, this only adds new
    methods as needed to conform to the gen_event_formatter
    interface. In some cases, the new methods just call existing methods
    that did the same thing.

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 build

/area driver-kmod

/area driver-ebpf

/area libscap

/area libsinsp

/area tests

/area proposals

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?:

new(libsinsp): add notion of generic event formatters/formatter factories, similar to how filters have generic filters/factories

@poiana poiana requested review from gnosek and leogr August 25, 2021 22:24
mstemm added a commit to falcosecurity/falco that referenced this pull request Aug 26, 2021
Modify falco_formats to only be responsible for resolving a rule's
output string or coming up with a map of field name->field values from
a given output string.

It relies on the changes in
falcosecurity/libs#77 to use generic
formatters for a given source.

Remove lua bindings to create a formatter/free a formatter. Those were
unused as of the changes in
#1451, so finally remove
them now.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
Copy link
Contributor

@gnosek gnosek left a comment

Choose a reason for hiding this comment

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

Can't say I like the duplicated APIs introduced but there probably is a reason for this

userspace/libsinsp/eventformatter.cpp Outdated Show resolved Hide resolved
userspace/libsinsp/eventformatter.cpp Outdated Show resolved Hide resolved
@mstemm
Copy link
Contributor Author

mstemm commented Sep 10, 2021

/test build-libs

@mstemm
Copy link
Contributor Author

mstemm commented Sep 10, 2021

/test build-libs-with-chisels

leogr
leogr previously approved these changes Sep 17, 2021
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

The duplicated API is not ideal, otherwise SGTM.
But I can't think of a way around that, so approved.
/approve

@poiana
Copy link
Contributor

poiana commented Sep 17, 2021

LGTM label has been added.

Git tree hash: e197fd830089ad2fa969a2fb4657bbf6b2326d2c

gnosek
gnosek previously approved these changes Oct 1, 2021
@poiana
Copy link
Contributor

poiana commented Oct 1, 2021

LGTM label has been added.

Git tree hash: 95ea41f503d52f247f1be655f357d469e0cee0e1

gnosek
gnosek previously approved these changes Oct 1, 2021
@poiana poiana added the lgtm label Oct 1, 2021
@poiana
Copy link
Contributor

poiana commented Oct 1, 2021

LGTM label has been added.

Git tree hash: b9c0994277c309231bae5d30f2b9df166dfa7c01

leogr
leogr previously approved these changes Oct 4, 2021
Copy link
Member

@leogr leogr 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 Oct 4, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leogr, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Add the notion of "generic" event formatters and formatter factories
that can create a formatter for a given format string:

- gen_filter.h defines two new classes: gen_event_formatter provides
  the interface to format events:
    - set_format(): set the output format and format string
    - tostring(): resolve the format with info from a gen_event,
      populating a resolved string.
    - tostring_withformat(): like tostring() but with a one-off output
      format.
    - get_field_values(): return all templated field names and values
      from the configured format string.
    - get_output_format(): get the current output format.
- gen_event_formatter_factory performs a similar role as the existing
  sinsp_evt_formatter_cache, in that it maintains a cache of previously
  used format strings/formatters, to avoid the overhead of creating
  formatters. It simply returns a formatter, though, rather than
  duplicating the format methods like sinsp_evt_formatter_cache does.

This can be used in programs like falco to format general purpose
events without having a direct connection to an
inspector/filterchecks/etc.

- The eventformatter changes simply add gen_event_formatter as a
  parent class and implements the interfaces. To aid in backwards
  compatibility with other parts of libsinsp, this only adds new
  methods as needed to conform to the gen_event_formatter
  interface. In some cases, the new methods just call existing methods
  that did the same thing.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
@mstemm mstemm dismissed stale reviews from leogr and gnosek via 805e466 October 4, 2021 15:15
@poiana poiana removed the lgtm label Oct 4, 2021
@poiana poiana added the lgtm label Oct 4, 2021
@poiana
Copy link
Contributor

poiana commented Oct 4, 2021

LGTM label has been added.

Git tree hash: 5b20a78ca3875e164fcb5faeed84e1163579dbdf

@poiana poiana merged commit a5ea367 into master Oct 4, 2021
@poiana poiana deleted the general-purpose-formatters branch October 4, 2021 15:43
mstemm added a commit to leogr/falco that referenced this pull request Oct 4, 2021
Pinned to a commit that contains PRs
falcosecurity/libs#74,
falcosecurity/libs#75,
falcosecurity/libs#76,
falcosecurity/libs#77

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
mstemm added a commit to falcosecurity/falco that referenced this pull request Oct 4, 2021
Modify falco_formats to only be responsible for resolving a rule's
output string or coming up with a map of field name->field values from
a given output string.

It relies on the changes in
falcosecurity/libs#77 to use generic
formatters for a given source.

Remove lua bindings to create a formatter/free a formatter. Those were
unused as of the changes in
#1451, so finally remove
them now.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
mstemm added a commit to falcosecurity/falco that referenced this pull request Oct 4, 2021
Modify falco_formats to only be responsible for resolving a rule's
output string or coming up with a map of field name->field values from
a given output string.

It relies on the changes in
falcosecurity/libs#77 to use generic
formatters for a given source.

Remove lua bindings to create a formatter/free a formatter. Those were
unused as of the changes in
#1451, so finally remove
them now.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
mstemm added a commit to falcosecurity/falco that referenced this pull request Oct 5, 2021
Modify falco_formats to only be responsible for resolving a rule's
output string or coming up with a map of field name->field values from
a given output string.

It relies on the changes in
falcosecurity/libs#77 to use generic
formatters for a given source.

Remove lua bindings to create a formatter/free a formatter. Those were
unused as of the changes in
#1451, so finally remove
them now.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
mstemm added a commit to falcosecurity/falco that referenced this pull request Oct 5, 2021
Modify falco_formats to only be responsible for resolving a rule's
output string or coming up with a map of field name->field values from
a given output string.

It relies on the changes in
falcosecurity/libs#77 to use generic
formatters for a given source.

Remove lua bindings to create a formatter/free a formatter. Those were
unused as of the changes in
#1451, so finally remove
them now.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
mstemm added a commit to falcosecurity/falco that referenced this pull request Oct 5, 2021
Modify falco_formats to only be responsible for resolving a rule's
output string or coming up with a map of field name->field values from
a given output string.

It relies on the changes in
falcosecurity/libs#77 to use generic
formatters for a given source.

Remove lua bindings to create a formatter/free a formatter. Those were
unused as of the changes in
#1451, so finally remove
them now.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
poiana pushed a commit to falcosecurity/falco that referenced this pull request Oct 12, 2021
Modify falco_formats to only be responsible for resolving a rule's
output string or coming up with a map of field name->field values from
a given output string.

It relies on the changes in
falcosecurity/libs#77 to use generic
formatters for a given source.

Remove lua bindings to create a formatter/free a formatter. Those were
unused as of the changes in
#1451, so finally remove
them now.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
mstemm added a commit that referenced this pull request Jan 26, 2022
In #74 we pushed down some code from falco that determines the sinsp
event types that are applicable for a given filter. The set of event
types starts with "all events", and as the filter condition is parsed,
including logical operators like "and", "or", "not", etc, the set of
event types is changed, honoring the logical operators.

For example, for a condition proc.name=nginx, the filter is applicable
for all event types, as the condition does not include any evt.type
field. For a more complicated condition like evt.type=openat and
proc.name=nginx, the first field restricts the event types to openat,
which is logical anded against all event types from proc.name,
resulting in only the event type openat.

With the introduction of plugins, there's also a need to segregate
plugin-related filterchecks from non-plugin-related filterchecks by
event source, but that's handled at a higher level, using a notion of
filter factories and formatter factories (#77).

The bug is that plugin filtercheck fields like ct.name, json.value
were mistakenly being restricted to only the plugin event
PPME_PLUGINEVENT_E. This was being mistakenly inverted when conditions
had a "not" operator, with the result being that they did not run on
any event types at all.

The fix is to treat plugin fields as working with all event types,
just like almost all other fields like proc.name, etc. are. This
allows the logical operators to combine event type sets properly.

This, along with other small changes in falco + plugins, fixes
falcosecurity/plugins#56.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
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.

None yet

4 participants