Handling plugin extraction errors

[jasondellaluce]

Motivation Currently, errors are ignored during field extraction in the plugin framework. In theory, a plugin might fail extracting a field for two main reasons:

• The field is not present in the given event, for which the ss_plugin_extract_field.field_present flag is set to false
• The extract_fields exported plugin function encounters some error and returns a code different than SS_PLUGIN_SUCCESS. In the current implementation, in both cases the filtercheck returns a NULL pointer, which is interpreted as a not-available field. This is visible here 👇🏼 https://github.com/falcosecurity/libs/blob/83f460cc5ba39cd09f924b4d05a1ffd13eec07de/userspace/libsinsp/plugin.cpp#L630 https://github.com/falcosecurity/libs/blob/83f460cc5ba39cd09f924b4d05a1ffd13eec07de/userspace/libsinsp/plugin.cpp#L304

Although this is semantically correct, the two failure paths have a quite different meaning. In the second case, the plugin returns a failure code and the framework silently ignores it to maintain a non-blocking extraction flow. This is makes error handling efforts useless for plugin developers, and generally makes it harder to debug plugins at runtime.

Feature I propose to catch the error and make it visible somehow.

I agree that maintaining field extraction non-blocking might be a priority here, so maybe throwing an exception might not be a viable option. We can consider some weaker error propagation methods, or maybe logging to stderr. To the bare minimum, we might log the error if a debug mode is enabled.

Alternatives Keep things as they are, and just ignore plugin failures for extract_fields.

[jasondellaluce]

@leogr @mstemm

[leogr]

Good catch. Not sure what's the best option atm, for sure I will take a look.

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[leogr]

/remove-lifecycle stale

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[FedeDP]

/remove-lifecycle stale

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[jasondellaluce]

/remove-lifecycle stale

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[jasondellaluce]

/remove-lifecycle stale

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[jasondellaluce]

/remove-lifecycle stale

[jasondellaluce]

/milestone 0.11.0

[FedeDP]

/milestone 0.12.0

[incertum]

We have had lots of plugins refactors, is this still relevant?

[leogr]

I believe this is still relevant. @jasondellaluce to confirm.

However, I think it would be best to extend this discussion to the plugin domain and all field extraction mechanisms, including those built-in sinsp. I know Jason has some thoughts in this regard, so I'm eager to hear from him.

That being said, I don't think this is a top priority. However, it is still an improvement that is worth tackling.

Just my 2 cents

cc @Andreagit97 @FedeDP

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[leogr]

/remove-lifecycle stale

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[leogr]

/remove-lifecycle stale

[leogr]

/help

[poiana]

@leogr: This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/falcosecurity/libs/issues/125): >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[leogr]

/remove-lifecycle stale