Fix Metricbeat k8s metadata sometimes not being present at startup

[swiatekm]

Proposed commit message

Fix Metricbeat k8s metadata sometimes not being present at startup

In the metricbeat k8s module, each metricset has an enricher, which is used to add k8s metadata to events. Enrichers use watchers to watch k8s resource changes, and these watchers can be shared between enrichers. This is implemented by each enricher maintaining a writethrough cache of metadata - when a K8s event indicating a change to a resource is received, the metadata cache is updated.

The problem with this is that a watcher is only started once, so enrichers which are created after it was started, have to catch up on the resource change events. This is a great opportunity for race conditions, and is in general unnecessarily complicated.

Instead, we turn the writethrough cache into a readthrough cache. The change event handlers now only invalidate the cache, and the enricher computes the metadata when it is first requested. This completely sidesteps the catch up problem, as we only compute the metadata lazily.

I've also refactored the code a bit:

• Resource event handlers are now added to a watcher when it is created, as opposed to when an enricher is created. They're always the same, so there's no reason to do it multiple times.
• Removed TestBuildMetadataEnricher_EventHandler_PastObjects, as it doesn't seem to check anything not covered by other tests.
• Rearranged the createWatcher function, so actual watcher creation (as opposed to skipping because it's already there) happens on the main path.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Install elastic-agent in K8s using the standalone manifests and track the metadata.

Related issues

• Closes K8s metadata for metricbeat Kubernetes module missing at startup #41213

[mergify]

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @swiatekm? 🙏.
For such, you'll need to label your PR with:

• The upcoming major version of the Elastic Stack
• The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit

[mergify]

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

[elasticmachine]

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

[MichaelKatsoulis]

I took a look and the code changes look ok to me. The part of the enricher initialization code was the most tricky one when we implemented the shared watchers logic. Maybe we overcomplicated it.

I am not sure still about the problem it tries to solve, as we had tested these scenarios a lot.
But this new approach looks simpler.

[pkoutsovasilis]

nit: not required since the Clone is now handled by the e.getMetadata(...)

[swiatekm]

This is just renaming the variable for clarity. Maps are reference types in Go, so this really just copies a pointer.

[pkoutsovasilis]

I know that maps are built-in references ty 🙂 I have to admit that this is the first time I witness a new variable assignment in the same scope for clarity reasons; however, I have seen comments added or renaming the variable at the creation time, for clarity reasons

[swiatekm]

The real meaning of this series of statements is "ecsMeta is meta without the kubernetes key". It was clearer when we were actually cloning meta, but we don't need to do that anymore. I can change it to only use meta if you think this is confusing.

[swiatekm]

@gizas are you ok with me merging this? I know you wanted to try reproducing #41213 first.

[gizas]

Sure @swiatekm , I did not have time to reproduce tbh. But as long I understood the visualisation and why the issue was raised I am ok