fix(userspace/falco): fixed falco_metrics::to_text implementation when running with plugins

[FedeDP]

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

This PR does 3 things:

• avoid crashing when scap_machine_info or scap_agent_info are NULL (ie: with non-linux scap platform, ie: while running plugins)
• properly use a shared_ptr for inspectors vector, so that we know they are kept alive while to_text runs and we can dereference the pointer
• only expose metrics for actually enabled sources (not loaded ones). Before, in nodriver mode with syscall source disabled, we would've tried to emit metrics for the syscall source/inspector too, but that is wrong since it is not enabled (and it would crash Falco too!)

Which issue(s) this PR fixes:

Fixes #3229

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fix(userspace/falco): fixed `falco_metrics::to_text` implementation when running with plugins

[incertum]

Right yes we knew that it was not working for plugin only since the scap regression.
Thanks @FedeDP for adding the safety checks.

[incertum]

/approve

[incertum]

@FedeDP do we also check for null in the output rule code? If not maybe we should add some checks in there too given we have the scap regression?

[incertum]

yes @FedeDP please also adjust below code 🙏 since we will continue having the scap regression for a while.

void stats_writer::collector::get_metrics_output_fields_wrapper(
		nlohmann::json& output_fields,
		const std::shared_ptr<sinsp>& inspector,
		const std::string& src, uint64_t num_evts,
		uint64_t now, double stats_snapshot_time_delta_sec)
{
	static const char* all_driver_engines[] = {
		BPF_ENGINE, KMOD_ENGINE, MODERN_BPF_ENGINE,
		SOURCE_PLUGIN_ENGINE, NODRIVER_ENGINE, GVISOR_ENGINE };
	const scap_agent_info* agent_info = inspector->get_agent_info();
	const scap_machine_info* machine_info = inspector->get_machine_info();

[FedeDP]

/hold

[FedeDP]

yes @FedeDP please also adjust below code 🙏 since we will continue having the scap regression for a while.

For some reason it was not failing on that locally :/ weird, but yep i add the extra checks!

EDIT:

falco.start_ts=0

Mmmh this is outputted by the output_rule for the metrics; indeed i can confirm that both agent_info and machine_info are not NULL here.
Perhaps we are using the wrong inspector over there? Anyway, i'll add the extra check, and leave it as-is.

EDIT2: at a quick glance, it seems like we are using the correct inspector :/

[incertum]

indeed i can confirm that both agent_info and machine_info are not NULL here.

they are not initialized when running Falco with plugin only, see #2821 :/

[incertum]

/approve

[FedeDP]

Oh well it turns out the issue was actually here: the old code also used the syscall inspector that was not even inited i guess, and thus had NULL agent and machine infos.
That's why stats_writer was not crashing.

I think leaving the checks is ok anyway, more protection for free :)

[FedeDP]

cc @incertum

[incertum]

Ahh understood, great job spotting this and agreed more checks help here and also make it clearer that these 2 structs can be NULL in some circumstances.

[FedeDP]

/milestone 0.39.0
(will probably become 0.38.1)

[FedeDP]

/milestone 0.38.1

[poiana]

@sgaist: 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.

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

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

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,LucaGuerra,incertum]

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

[FedeDP]

/unhold