Beats: [Metricbeat] documenting/standardizing the return of r.Event

[fearful-symmetry]

This came up during a discussion with @ruflin and he asked me to file an issue.

So, there's some inconsistency about how we handle the bool that gets returned from reporter.Event(). This will _only_ return false if the metricset has closed.

The vast majority of metricsets ignore this, however a few check the value and log an error:

./ceph/cluster_disk/cluster_disk.go:77: if reported := reporter.Event(mb.Event{MetricSetFields: event}); !reported {
./mongodb/dbstats/dbstats.go:89:                reported := reporter.Event(mb.Event{MetricSetFields: data})
./kubernetes/state_container/state_container.go:144:            if reported := reporter.Event(mb.Event{
./kubernetes/state_replicaset/state_replicaset.go:116:          if reported := reporter.Event(mb.Event{
./kubernetes/state_deployment/state_deployment.go:117:          if reported := reporter.Event(mb.Event{
./kubernetes/state_statefulset/state_statefulset.go:115:                if reported := reporter.Event(mb.Event{

How should we standardize and document this? A blurb in the beats developer docs would be helpful, at least.

[jsoriano]

@fearful-symmetry look at this thread for a related discussion: https://github.com/elastic/beats/pull/9907#discussion_r245977781

Some ideas from there:

• This returned value should be used to stop doing anymore work in fetchers that send multiple events
• This can be completely ignored in fetchers that only send an event per fetch
• If anything is logged, it should be done at the debug level, as this is something expected to happen

[ruflin]

++ on the above. I'm even wondering if we should log something on the debug level as I'm pretty sure something else logs already on the debug level that sending was interrupted / metricset was stopped.

Now we need to document the above somewhere in the Metricbeat modules dev docs. @fearful-symmetry Want to take this on?

[fearful-symmetry]

@ruflin sure!

[fearful-symmetry]

This also sorta ties into the "best practices" I mention in #11102

[fearful-symmetry]

This should be covered by #11745

[ruflin]

@fearful-symmetry Should we close the issue?

[fearful-symmetry]

@ruflin unless you think there's additional documentation needed not covered by #11745, we should be good to close it.

I also have a lurking suspicion that there's a few metricsets not using the bool value inside a loop, but that can be another case/PR if I run across them.

[fearful-symmetry]

Okay, so I did gather a list of metricsets that call Event() in a loop but don't check the return to see if the metricset is closed. These probably aren't too urgent, but maybe worth tracking

• [ ] all of postgres
• [ ] docker/image
• [ ] all of vsphere
• [ ] most of ceph
• [ ] jolokia/jmx
• [ ] mongodb/collstats
• [ ] most of system
• [ ] consul/agent
• [ ] kafka/partition
• [ ] kvm/dommemstat
• [ ] aerospike/namespace
• [ ] prometheus/collector
• [ ] windows/perfmon
• [ ] windows/service
• [ ] couchbase/bucket
• [ ] couchbase/node
• [ ] aws (https://github.com/elastic/beats/pull/11775)

[ruflin]

@fearful-symmetry I would say let's close it but have the list you collected above in a separate issue, otherwise we keep extending the scope of this issue which was only around docs.