[Metricbeat] Force metricsets to implement a Close()?

[fearful-symmetry]

Summarizing some discussions I had with @kaiyan-sheng and @ruflin

Right now metricbeat provides a Closer interface that metricsets can implement. However, not all metricsets need a Close() obviously. However, I'm beginning to wonder if some things slipped under the radar. I can't find where the docker/container metricset closes it's client handler, for example:

beats/metricbeat/module/docker/container/container.go

Line 54 in 9fb274a

client, err := docker.NewDockerClient(base.HostData().URI, config)

We have something like 155 metricsets, and it's easy for this kinda thing to slip unnoticed. I wonder if we should make Close() part of the ReporterV2 interface, and metricsets that don't need it can simply no op it. This may result in some extra code, but it could aid in auditing and consistency across the metricsets. Thoughts? Opinions?

[ruflin]

Initially we had a Close() method required but realised for a lot of metricsets it was not needed why we tried to lower the bar of entry and made it opt in. I definitively see the point of making it required to rise awareness but not sure if it's going to solve the problem. My suggestion is to keep it as is for now but make sure we revisit our own metricset to check if all close connections as expected.

[exekias]

It seems #11981 will go in, in some cases it should be enough with using it instead of waiting for a Close call?