new(metrics): enable plugins metrics

[mrgian]

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

/kind release

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area build

/area engine

/area tests

/area proposals

/area CI

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(metrics): enable plugins metrics

[incertum]

/approve

Bummer we could not include this for Falco 0.38.0.

[poiana]

LGTM label has been added.

Git tree hash: 7727207ee99834204b7925d641c637b454b07501

[incertum]

/milestone 0.39.0

[FedeDP]

@incertum don't we need to implement this for falco_metrics too? (ie: prometheus)?
/hold

@mrgian

[FedeDP]

/milestone 0.38.1

[incertum]

@incertum don't we need to implement this for falco_metrics too? (ie: prometheus)? /hold

@mrgian

Good call @FedeDP -- overlooked that, yes @mrgian could you add the changes to the webserver as well?

[mrgian]

@incertum don't we need to implement this for falco_metrics too? (ie: prometheus)? /hold

@mrgian

Good call @FedeDP -- overlooked that, yes @mrgian could you add the changes to the webserver as well?

It shouldn't be needed since libs_metrics_collector is already initialized with the METRICS_V2_PLUGINS flag.
https://github.com/falcosecurity/falco/blob/master/userspace/falco/falco_metrics.cpp#L64

But seems like plugin-provided metrics cause a segfault when a request is made to the webserver.
The issue may lie in prometheus_metrics_converter::convert_metric_to_text_prometheus in the libs, however I'm still searching for the root cause of this.

@FedeDP @incertum

[mrgian]

False alarm on the segfault! I was using an old plugin as test 🤦

As for enabling plugin metrics in the webserver, we don't need to do that since since libs_metrics_collector is already initialized with the METRICS_V2_PLUGINS flag.

We can think of changing the namespace for plugin-provided metrics to something like plugin or falco_plugin in:
https://github.com/falcosecurity/falco/blob/master/userspace/falco/falco_metrics.cpp#L227

WDYT?

[FedeDP]

Left a small question: everything else sgtm!
Thanks both Melissa and Gian for this!

[leogr]

Left a small question: everything else sgtm! Thanks both Melissa and Gian for this!

Otherwise, SGTM too!

[FedeDP]

/approve

[poiana]

LGTM label has been added.

Git tree hash: 93fb28e758f3a0465fa6340cbfc76924ad9a2e85

[incertum]

/approve

[incertum]

@FedeDP CI is still not cooperating.

[FedeDP]

Oh of course, we need to bump libs to master too!
I will open the bump PR so that @mrgian can then rebase this one and we are good to go!

[FedeDP]

@mrgian can you rebase on master to fix the CI?

[mrgian]

@FedeDP Rebased :)

[incertum]

/approve

[poiana]

LGTM label has been added.

Git tree hash: 685f492ca5843d053268c84ba05ed50235318695

[FedeDP]

/approve

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

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

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

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

[incertum]

Btw @mrgian we need to update the website now ... https://falco.org/docs/metrics/falco-metrics/#plugins-metrics
CC @LucaGuerra it's gonna be interesting to explain that the plugins metrics are custom (no default ones) vs we don't truly support the other metrics for plugins atm 🙃 because of some regressions ...

[mrgian]

@incertum I'm currently working on the website to document the latest plugin API features.
I'll update the page about Falco metrics too :)

[FedeDP]

/unhold