fix(userspace/falco): properly format numeric values in metrics

[jasondellaluce]

What type of PR is this?

/kind bug

/kind cleanup

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

Following up #2333, this makes sure that numeric values are formatted as numbers in the JSON outputs of the new metrics system.

Which issue(s) this PR fixes:

Special notes for your reviewer:

cc @incertum

Does this PR introduce a user-facing change?:

fix(userspace/falco): properly format numeric values in metrics

[jasondellaluce]

/milestone 0.35.0

[leogr]

I don't like too much using nlohmann::json, but I can't think of a better solution right now.

So, LGTM.

[poiana]

LGTM label has been added.

Git tree hash: 7f7f4dbad72727cc9848553c1efb314c0d66ecf7

[jasondellaluce]

I don't like too much using nlohmann::json, but I can't think of a better solution right now.

Same 😸

[poiana]

LGTM label has been added.

Git tree hash: c5d429cfd86661daa76b3ec35128b49ceaa512d2

[FedeDP]

@jasondellaluce
please consider fixing #2333 (comment) here :)

[FedeDP]

LGTM; left a comment about strncat issue.
/hold

[incertum]

LGTM; left a comment about strncat issue. /hold

@FedeDP please note in libs we do it the way it currently is for example kmod_path = strncat(cwd, KMOD_DEFAULT_PATH, FILENAME_MAX - strlen(cwd)) hence we would need a commit in libs to uniformly change it as well.

[incertum]

Thanks @jasondellaluce for uniformly changing this. Wouldn't we need a ! as we change the data types for all Falco rules not just the internal stats / metrics related rules?

LGTM!

[jasondellaluce]

Wouldn't we need a ! as we change the data types for all Falco rules not just the internal stats / metrics related rules?

I don't think so, this changes the types only for the new metrics internal alerts, which is something we haven't released yet.

[incertum]

Wouldn't we need a ! as we change the data types for all Falco rules not just the internal stats / metrics related rules?

I don't think so, this changes the types only for the new metrics internal alerts, which is something we haven't released yet.

Wait so handle_msg is just used for the internal rules and not for the regular Falco rules? In that case just out of curiosity could you point me to the place where regular Falco rules are formatted?

[FedeDP]

/approve

[poiana]

LGTM label has been added.

Git tree hash: 4a70a7d640c90185e5124c8d5f833a7a6c23272e

[leogr]

Wouldn't we need a ! as we change the data types for all Falco rules not just the internal stats / metrics related rules?

I don't think so, this changes the types only for the new metrics internal alerts, which is something we haven't released yet.

Wait so handle_msg is just used for the internal rules and not for the regular Falco rules? In that case just out of curiosity could you point me to the place where regular Falco rules are formatted?

handle_event() 👇
https://github.com/falcosecurity/falco/blob/master/userspace/falco/falco_outputs.cpp#L120

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, jasondellaluce, leogr

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,jasondellaluce,leogr]

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

[incertum]

Wouldn't we need a ! as we change the data types for all Falco rules not just the internal stats / metrics related rules?

I don't think so, this changes the types only for the new metrics internal alerts, which is something we haven't released yet.

Wait so handle_msg is just used for the internal rules and not for the regular Falco rules? In that case just out of curiosity could you point me to the place where regular Falco rules are formatted?

handle_event() 👇

https://github.com/falcosecurity/falco/blob/master/userspace/falco/falco_outputs.cpp#L120

Interesting, seems like there is an opportunity to consolidate more in the future.

[FedeDP]

/unhold