Publisher pipeline: pass logger and metrics registry

[urso]

We should strive to not have dependencies on globals in beats. The
publisher pipeline rewrite made sure we don't work with globals
internally. Yet some globals have been introduced since, and even though
the library didn't use globals internally, initialization still did use
globals at some points.
This change removes globals for logging/metrics/telemetry, by requiring
the beat instance to pass down required instances.

[houndci-bot]

exported type Monitors should have comment or be unexported

[ruflin]

Interesting separate of telemetry and metrics here. At the moment the separation is pretty clear but I don't think it will stay like this. Perhaps we better pass a map[string]*monitoring.Registry instead?

[ph]

I prefer the proposed way instead of map[string]*monitoring.Registry, using a field reflect a clear intends, using a map complexify the access patterns instead of a nil check.

I would argue that maybe the monitoring package could expose a struct to list the possible monitors so we only have one place to change and the access definition are centralized.

Concerning having Logger inside the monitor struct Monitor, at first I was against a logger is effectively some kind of monitor.

[urso]

I agree with PH, using a map will complicate the code and make it much more error prone. Instead we should have well defined types. Right now we use globals in a few places, so I kept the struct local to the publisher pipeline for now (makes me wonder if I missed some globals in the outputs).

Wasn't sure about the logger, but then decided to put it into the struct as well. Ideas was: all functionality passed/configured is for the purpose of getting some insights into an instance of the publisher pipeline. The metrics registry share the same type, but they are used in very different ways as well.

[ph]

LGTM

[ph]

I prefer the proposed way instead of map[string]*monitoring.Registry, using a field reflect a clear intends, using a map complexify the access patterns instead of a nil check.

I would argue that maybe the monitoring package could expose a struct to list the possible monitors so we only have one place to change and the access definition are centralized.

Concerning having Logger inside the monitor struct Monitor, at first I was against a logger is effectively some kind of monitor.

[ph]

@urso
The build is falling with a valid reason:

warning: ignoring symlink /home/travis/gopath/src/github.com/elastic/beats/libbeat/build/python-env/local/include
warning: ignoring symlink /home/travis/gopath/src/github.com/elastic/beats/libbeat/build/python-env/local/lib
# github.com/elastic/beats/libbeat/publisher/pipeline/stress
publisher/pipeline/stress/run.go:61:32: cannot use nil as type pipeline.Monitors in argument to pipeline.Load
# github.com/elastic/beats/libbeat/publisher/queue/memqueue
publisher/queue/memqueue/queue_test.go:75:19: not enough arguments in call to NewBroker
	have (Settings)
	want (logger, Settings)
make[1]: *** [check] Error 2
make[1]: Leaving directory `/home/travis/gopath/src/github.com/elastic/beats/libbeat'
make: *** [check] Error 1

[urso]

The build is falling with a valid reason:

Yeah, that's why PR is still in progress. There might be some more test builds I have to fix before PR goes green.

[ruflin]

LGTM

@ph Do you want to have an other look?

@urso We should make sure that the queue types show potentially show up in monitoring and telemetry which requires a few changes in ES and KB. Do you want to open an issue in https://github.com/elastic/stack-monitoring ?

[ph]

@urso LGTM but lets add a simple changelog