collector: add tasks API collection

[devoxel]

This commit adds simple aggregation of Elasticsearch Tasks API.
There are 4 new metrics; though 3 are just bookkeeping.

elasticsearch_task_stats_action_total is a gague reporting the total
number of tasks running for a given action. Because there are no stats
endpoints available for this, this change introduces an aggregation step
to group the number of tasks by action name.

This metric is useful for ensuring long running actions of a specific
kind stay within a specific limit. Of particular use to me is
the action: 'indices:data/write/delete/byquery'.

In my usecase, our ES access patterns mean we have a predefined limit
of these actions running on the cluster.

This change also adds two new CLI flags to manage the collection of tasks API:

--es.tasks (to enable task collection)
--es.tasks.actions (to filter tasks by action param)

Issue #525 proposed addition of collection of these tasks.

[devoxel]

@sysadmind hey - would it be possible to review this?

[sysadmind]

We are moving towards the Collector interface for collectors. If you look at cluster_info and cluster_settings, you will see examples of the newer style. If that was the only change, I would say we could fix it up after merging, but with the other review remarks, I think it's worthwhile to do at the same time. Using the Collector interface means --es.tasks is no longer necessary. It will be --collector.tasks automatically. The --es.tasks.actions flag will still be necessary, but I don't think that the "es." prefix is necessary.

Overall, I think this is close and the logic seems to be correct.

[sysadmind]

I'm not sure that there is a benefit to having this type. We could just pass the map instead.

[devoxel]

I think having the struct makes the code a bit more self documenting; it makes it clear we're doing aggregation and by what. If you'd prefer I can make that change though

[devoxel]

Thanks for the prompt & useful review @sysadmind :)

The --es.tasks.actions flag will still be necessary

I ended up moving the flag into tasks.go directly and using a global; because the collector factory cannot pass params during construction.

The downside of directly gathering flags in the init() are:

• A package level global mutable variable is never nice; some other function could accidentally change the behaviour of tasks

• If the package is imported it's flags are infectious. However, registerCollector does this anyway; so I don't think this issue is worth discussion.

I considered some alternatives:

• Extending Collector interface: this ends up with us either exposing flag implementation details or only supporting string value options

• A struct tag solution with reflect.TypeOf: this seems nice on paper but implementation is way too complex for such a small gain

If you have a better implementation please suggest.

[devoxel]

Hey - @sysadmind - any update here? I have addressed your most recent set of comments

[sysadmind]

Everything looks good except the metric name. Thanks for your patience.

[devoxel]

Thanks for your patience.

It's understandable - maintaining OSS is a thankless & endless task and I appreciate the reviews.

[sysadmind]

LGTM