Top k processor plugin

[mirath]

This processor objective is to provide the possibility to pick the top k metrics from a set of metrics. It aims to solve #3192

The pluging uses:

Selectors to pick the metrics that will be filtered
A group by clause to group the metrics based on metric names and tags
A list of fields
An aggregator function to agreggate each field of the grouped metrics
Once the aggregations for each group are calculated, the plugin orders the groups by each aggregation and returns all the metrics of each group in the top k spots

Required for all PRs:

• Signed CLA.
• Associated README.md updated.
• Has appropriate unit tests.

[mirath]

This PR is not ready yet. I'm creating a PR allow feedback from the mantainers now that the plugin is more or less stable in design and structure.Some things that are missing are: unit test, more aggregation functions, more extensive testing

[danielnelson]

I've mostly just looked at the README, don't forget we are using Go naming conventions: uglyCamelCase vs the_one_true_way.

[danielnelson]

I needed to check the code on this, but it looks that the normal measurement filtering options work for processors, although perhaps in a somewhat non-intuitive way.

If a metric is filtered out, the processor is bypassed but the metric continues to the outputs[1]. This means that you could use these for selecting the metrics here instead of custom params. I'm not sure if this is something that should be changed or if the documentation just needs improved.

[1] https://github.com/influxdata/telegraf/blob/master/internal/models/running_processor.go#L39

[mirath]

I think the documentation could be improved. Looking around the only documentation regarding processors that I could find was this. I think I saw the measurements filtering docs before, but I didn't connect them to the processors for some reason, that was my bad.

However, I think that the main docs in docs.influxdata.com should be updated. I just looked around and there isn't a section dedicated to processor configuration, at least not one that is easy to spot. At the very least a link to the measurement filtering should be added IMO.

I'll simply the code so it doesn't use these custom options, since now I now that they are just clutter.

[danielnelson]

Here is the change I made to the docs, 777b84d, I hope this will connect the measurement filtering to the plugins more directly.

[mirath]

I like the changes to the docs

[mirath]

One thing that would be nice are examples of the use of the metrics filtering options groups under the metrics filtering options section. There are enough examples of their use along CONFIGURATION.md, but since they are spread out consider centralizing them if you are interested on making the docs extra friendly.

[danielnelson]

Will it work if we always group by series key? The series key is measurment,tagkey=tagvalue,tagkey2=tagvalue2 and you can use metric.HashID() to get an opaque series identifier.

[mirath]

I don't quite follow you here. It will work if you always group by metric name and all the tags. Is that what you want to know?

From what I understand metric.HashID() returns some hash over the series name and the tags. That is useful, but for this use case, it would prevent me from grouping over only tags, or only over a subset of tags.

[danielnelson]

Can you give an example of when grouping over a subset of tags is useful?

[mirath]

You could have metrics of CPU from several datacenters, but want to group only by process name, so as to get a picture of the most intensive processes across all data-centers.

A more concrete example, the procstat module puts the process_name in a tag, but you can optinally put the PID in a tag. If you put the PID in a tag, grouping across all tags will agreggate invocations of processes, which might not be what you need as it fails to agreggate across process restarts.

[mirath]

In a more general note, I think grouping across a subset of tags gives the plugin much more expressive power that has an excellent potential of addressing a lot of potential needs from Telegraf users.

[danielnelson]

👍

[mirath]

Regarding UglyCamelCase vs the_one_true_way (though I'm really netutral on that subject), I based my style from the procstat module. They use the_one_true_way for variables and some functions names, but UglyCamelCase for interface names and interface members.

So I'm not sure if you mentioned that just as a general tip, or if you spotted some things that I should change to the_one_true_way?

[danielnelson]

I'll work on improving the processor documentation today. On style, the code needs to be camelCase and the created metrics should be snake_case.

[mirath]

I've made a mess out of this branch, many rebases later I don't know what is going on, so I'm just going to create a new branch and create a PR for that