Skip to content

Conversation

mhofstetter
Copy link
Member

@mhofstetter mhofstetter commented Nov 28, 2024

Currently, the Cilium Operator only uses the metrics registry of the controller-runtime as base registry if Gateway API functionality is enabled. This has historic reasons as Gateway API functionality was the first module that used controller-runtime for its controllers.

In the meantime, other modules (Ingress Controller, Secret Sync, ...) are using the controller-runtime too - but the metrics registry of the library is still only used if Gateway API is enabled. In these cases, the metrics of the controller-runtime library (https://book.kubebuilder.io/reference/metrics-reference) aren't exposed.

Therefore, this commit changes the behaviour, that the metrics registry of the controller-runtime library is always used as the base registry.

Note: Unfortunately, the controller-runtime library configures the base registry to have Go runtime metrics included by default (https://github.com/cilium/cilium/blob/main/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/metrics/metrics.go#L91-L92). Therefore, this Go metrics collector has to be removed from the metrics registry before registering a new Go metrics collector with custom Go runtime metrics enabled. Otherwise the registration fails because some Go runtime metrics are registered twice.

Related PRs:

Currently, the Cilium Operator only uses the metrics registry
of the `controller-runtime` as base registry if Gateway API
functionality is enabled. This has historic reasons as Gateway
API functionality was the first module that used `controller-runtime`
for its controllers.

In the meantime, other modules (Ingress Controller, Secret Sync, ...)
are using the `controller-runtime` too - but the metrics registry of
the library is still only used if Gateway API is enabled. In these
cases, the metrics of the `controller-runtime` library
(https://book.kubebuilder.io/reference/metrics-reference) aren't exposed.

Therefore, this commit changes the behaviour, that the metrics registry
of the `controller-runtime` library is always used as the base registry.

Note: Unfortunately, the `controller-runtime` library configures the base
registry to have Go runtime metrics included by default
(https://github.com/cilium/cilium/blob/main/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/metrics/metrics.go#L91-L92).
Therefore, this Go metrics collector has to be removed from the metrics
registry before registering a new Go metrics collector with custom Go
runtime metrics enabled. Otherwise the registration fails because
some Go runtime metrics are registered twice.

Related PRs:
- cilium#28931 (extract `controller-runtime` integration into its own Hive Cell)
- cilium#29245 (export go runtime sched latency metrics)

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter mhofstetter added kind/cleanup This includes no functional changes. area/metrics Impacts statistics / metrics gathering, eg via Prometheus. area/operator Impacts the cilium-operator component release-note/misc This PR makes changes that have no direct user impact. labels Nov 28, 2024
@mhofstetter mhofstetter changed the title operator: use controller-runtime metric registry as base operator: always use controller-runtime metric registry as base Nov 28, 2024
@mhofstetter
Copy link
Member Author

/test

@mhofstetter mhofstetter marked this pull request as ready for review November 28, 2024 17:55
@mhofstetter mhofstetter requested a review from a team as a code owner November 28, 2024 17:55
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 29, 2024
@julianwiedmann julianwiedmann added this pull request to the merge queue Nov 29, 2024
Merged via the queue into cilium:main with commit 92097d6 Nov 29, 2024
70 checks passed
@mhofstetter mhofstetter deleted the pr/mhofstetter/metrics-rem-gwapi-condition branch November 29, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Impacts statistics / metrics gathering, eg via Prometheus. area/operator Impacts the cilium-operator component kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants