add flag to disable event metrics

[dangome3]

Description

Add --enable-event-metrics flag (default: true) to allow disabling event metrics while keeping health and resource metrics available.

Split InitAllMetrics into InitHealthMetrics and InitEventsMetrics. Health metrics and resource metrics are always initialized when --metrics-server is set.

Unit tests for metrics config added in 'pkg/metrics/metricwithpod_test.go' also manually tested in a no-k8s environment.

Changelog

metrics: add agent flag to disable event metrics while keeping health metrics

[netlify]

✅ Deploy Preview for tetragon ready!

Name	Link
🔨 Latest commit	b7c3966
🔍 Latest deploy log	https://app.netlify.com/projects/tetragon/deploys/6a039b561478de00085f4c78
😎 Deploy Preview	https://deploy-preview-4926--tetragon.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[kkourt]

Thanks @dangome3!

Can you please split the first patch in two patches? One for the new functionality, and one for the unit tests?

[dangome3]

Hey @kkourt ! sure! that's already done. Thanks for the suggestion

[mtardy]

Overall looks good thanks, just a few comments.

Also I know the patch is quite simple but the first one could have been also sliced into two, your commit description is a hint for this, your commit does two things:

Add --enable-event-metrics flag (default: true) to allow disabling
event metrics while keeping health and resource metrics available.

Split InitAllMetrics into InitHealthMetrics and InitEventsMetrics.
Health metrics and resource metrics are always initialized when
--metrics-server is set.

I would have split first, and then added the flag in two separate commits.

[mtardy]

question: we don't want to respect the flag from testing?

[dangome3]

This was intentional as this file contains only tests helper functions, and maybe any test could use the event metrics too

Please let me know if my assumption is wrong and if we should respect the flag here too

[mtardy]

nit: wrapping is a bit wrong here (like in the rest of that doc btw)

Suggested change

	By default, Tetragon exposes both health and events metrics. You can disable events metrics while keeping
	health metrics by adding `--enable-event-metrics=false`.
	By default, Tetragon exposes both health and events metrics. You can disable
	events metrics while keeping health metrics by adding `--enable-event-metrics=false`.

[mtardy]

I feel like you could add a note there just after 1. and 2. instead of later in the ## Enable/Disable Metrics and ### Non-Kubernetes section.

[dangome3]

Thanks @mtardy for the suggestions. I updated the PR

[kkourt]

Also I know the patch is quite simple but the first one could have been also sliced into two, your commit description is a hint for this, your commit does two things:

Add --enable-event-metrics flag (default: true) to allow disabling
event metrics while keeping health and resource metrics available.

Split InitAllMetrics into InitHealthMetrics and InitEventsMetrics.
Health metrics and resource metrics are always initialized when
--metrics-server is set.

I would have split first, and then added the flag in two separate commits.

I believe what Mahè meant here is to:

1. Split the implementation of InitAllMetrics into InitHealthMetrics and InitEventsMetrics into one commit
2. Add the flag, and use it to determine whether InitEventMetrics or not in the second commit.

In the current split, the flag is introduced in the first patch, and then used in the second patch, which I find more confusing that having both changes in a single patch (as originally).

[dangome3]

In the current split, the flag is introduced in the first patch, and then used in the second patch, which I find more confusing that having both changes in a single patch (as originally).

That's 💯 right, sorry for the confusion. That should be fixed now