[Metricbeat][Kubernetes] Refactor watchers used in Kubernetes metricsets for metadata enrichment

constanca-m commented 10 months ago

Background

This issue was first detected when trying to stop node and namespace watchers from starting (issue is this one). At first, it seemed that we only had these watchers based on features add_resource_metadata and hints, but having a better look, we were actually starting watchers at many places.

Issue

All state_* metricsets are using this function https://github.com/elastic/beats/blob/d9139c95a0b0bd74c09949a3265a0959e2d43e9c/metricbeat/helper/kubernetes/state_metricset.go#L46 to start, except from state_container and state_resourcequota.

This function adds a metadata enricher to each metricset by calling NewResourceMetadataEnricher function: https://github.com/elastic/beats/blob/d9139c95a0b0bd74c09949a3265a0959e2d43e9c/metricbeat/helper/kubernetes/state_metricset.go#L91

Also pod and node metricsets call NewResourceMetadataEnricher to enrich the events with metadata.

This enricher will create 3 watchers: https://github.com/elastic/beats/blob/d9139c95a0b0bd74c09949a3265a0959e2d43e9c/metricbeat/module/kubernetes/util/kubernetes.go#L158

One for the resource of that metricset, one for node and one for namespace.

Since we can have multiple metricsets enabled, then we will also have multiple node and namespace watchers running. One for each metricset.

To solve this, we need to find a way to share these watchers between the metricsets, similar to what we did to fetch metrics from KSM (PR) and Kubelet.

Additionally, from the way we create the resource specific watcher https://github.com/elastic/beats/blob/d9139c95a0b0bd74c09949a3265a0959e2d43e9c/metricbeat/module/kubernetes/util/kubernetes.go#L454 we can see that in case the function was called from state_node/state_namespace metricset (resource is node/namespace), we will end up creating yet another watcher for node/namespace. We need to, at least, add a condition to stop the duplicated watcher from starting.

More on this, we seem to always start the three watchers for all the resources that the NewResourceMetadataEnricher is called by. These watchers are created for metadata enrichment and for some resources the existence of one or some of these watchers is not relevant. For example node watcher is not needed in case we want to enrich deployments or statefulsets. We need to start only the relevant watchers for each resource.

Possibly this can be solved if we share the watchers between metricsets.

The better handling of watchers initialization will lead to less Kubernetes API calls and possible issues related to that in large scale clusters.

Issues

Relates to https://github.com/elastic/elastic-agent/issues/3801.

Current watchers

These are the current watchers when starting metricbeat with default configurations.

For example, like this.

``` - type: kubernetes scope: cluster node: ${NODE_NAME} unique: true templates: - config: - module: kubernetes hosts: ["kube-state-metrics:8080"] period: 10s metricsets: - state_node - state_deployment - state_daemonset - state_replicaset - state_pod - state_container - state_cronjob - state_job - state_resourcequota - state_statefulset - state_service - state_persistentvolume - state_persistentvolumeclaim - state_storageclass - state_namespace ```

For the state_* metricsets grouped as described above:

Metricset	Namespace watcher	Node watcher	Resource watcher
State cronjob	✓	✓	✓
State daemonset	✓	✓	✓
State deployment	✓	✓	✓
State job	✓	✓	✓
State namespace	✓	✓	✓
State node	✓	✓	✓
State PV	✓	✓	✓
State PVC	✓	✓	✓
State pod	✓	✓	✓
State replicaset	✓	✓	✓
State service	✓	✓	✓
State statefulset	✓	✓	✓
State storage class	✓	✓	✓
Total	14	14	-

Note: It is 14 and not 13, because the resource watcher for namespace adds 1 to the namespace watcher, and same happens for node watcher.

Note 2: state pod can also create 2 more watchers if we add resource_metadata for job and deployments (disabled by default).

For all the other metricsets:

Metricset	Namespace watcher	Node watcher	Resource watcher	Source
API Server	✕	✕	✕
Container	✓	✓	✓	NewContainerMetadataEnricher
Controller manager	✕	✕	✕
Event	✕	✕	✓	When creating metricset
Node	✓	✓	✓	NewResourceMetadataEnricher
Pod	✓	✓	✓	NewResourceMetadataEnricher
Proxy	✕	✕	✕
Scheduler	✕	✕	✕
State container	✓	✓	✓	NewContainerMetadataEnricher
State resource quota	✕	✕	✕
System	✕	✕	✕
Volume	✕	✕	✕

Note: Container and state container can also create 2 more watchers if we add resource_metadata for job and deployments (disabled by default).

Expected watchers

Watchers needed for each metricset by default (without counting add_resource_metadata.deployment/cronjob):

Metricset	Namespace watcher	Node watcher	Resource watcher	Notes
API Server	✕	✕	✕
Container	✓	✓	✓
Controller manager	✕	✕	✓
Event	✓	✕	✓
Node	✕	✓	✓	Resource watcher should be the same as node watcher.
Pod	✓	✓	✓
Proxy	✕	✕	✕
Scheduler	✕	✕	✕
State container	✓	✓	✓
State cronjob	✓	✕	✓
State daemonset	✓	✕	✓
State deployment	✓	✕	✓
State job	✓	✕	✓
State namespace	✓	✕	✓	Resource watcher should be the same as namespace watcher.
State node	✕	✓	✓	Resource watcher should be the same as node watcher.
State PV	✕	✕	✓
State PVC	✓	✕	✓
State pod	✓	✓	✓
State replicaset	✓	✕	✓
State resource quota	✓	✕	✓
State service	✓	✕	✓
State statefulset	✓	✕	✓
State storage class	✕	✕	✓
System	✕	✕	✕
Volume	✕	✕	✕

Checks

[ ] We don't start watchers based on the resource at the same time we start watchers for node/namespace, without adding a condition to avoid duplicates.
[ ] We don't have multiple watchers for the same resource. Instead we need to find a way to have them shared.
[ ] We only start watchers that are needed based on either the resource or the configuration.

PRs

[x] https://github.com/elastic/beats/pull/37332
[ ] Add enricher to state resource quota metricset: https://github.com/elastic/beats/issues/38004
[x] High level documentation of enrichment process https://github.com/elastic/beats/pull/38757
```
### Tasks
```

constanca-m commented 6 months ago

I believe we are doing the same thing for elastic agent: https://github.com/elastic/elastic-agent/blob/b7617cf47f1acd0e77022e7817e2103c347af917/internal/pkg/composable/providers/kubernetes/pod.go#L60 and creating multiple watchers there, instead of sharing. Should we create a new issue for that @MichaelKatsoulis ?

MichaelKatsoulis commented 6 months ago

I believe we are doing the same thing for elastic agent: https://github.com/elastic/elastic-agent/blob/b7617cf47f1acd0e77022e7817e2103c347af917/internal/pkg/composable/providers/kubernetes/pod.go#L60 and creating multiple watchers there, instead of sharing. Should we create a new issue for that @MichaelKatsoulis ?

This is the kubernetes provider. The provider like the add_kubernetes_metadata processor start watchers but only 3. One for pod, one for node and one for namespace. I mean that they are not per metricset.

elastic / beats