Configurable basicstats #3580
Configurable basicstats #3580
Conversation
| statsConfig *configuredStats | ||
| } | ||
|
|
||
| type configuredStats struct { |
There was a problem hiding this comment.
Could also do this as flags, but not sure it's a win
| @@ -114,23 +127,101 @@ func (m *BasicStats) Add(in telegraf.Metric) { | |||
| } | |||
|
|
|||
| func (m *BasicStats) Push(acc telegraf.Accumulator) { | |||
There was a problem hiding this comment.
For the first pass, only using the configuration to control which stats are pushed. I'm not trying to optimize the computation of each stat as they are added.
The balance between checking the configuration per calculation seems like a micro optimization we can make later.
| } | ||
| //if count == 1 StdDev = infinite => so I won't send data | ||
| } | ||
| acc.AddFields(aggregate.name, fields, aggregate.tags) | ||
|
|
||
| if len(fields) > 0 { |
There was a problem hiding this comment.
If an empty array is specified in the configuration, then we push nothing. I plan to use this with the field specific configuration, in other words, only produce aggregations for known fields.
There was a problem hiding this comment.
I don't think you can determine an empty array from a unset array with our current toml parsing, so you should always have at least one field.
Remember you can use the regular measurement filtering options to control what fields are added in the first place.
There was a problem hiding this comment.
You are correct. I still feel like this check isn't a bad thing. It would still happen if I passed an invalid stat, for example:
[[aggregators.basicstats]]
period = "5s"
drop_original = true
stats = [ "-" ]
Which would produce this case. So in the interest of being robust, I think the check is worth it.
| parsed.stdev = true | ||
|
|
||
| default: | ||
| log.Printf("W! Unrecognized basic stat '%s', ignoring", name) |
There was a problem hiding this comment.
Sample output:
2017-12-13T23:09:30Z W! Unrecognized basic stat '[wacky]', ignoring
| return parsed | ||
| } | ||
|
|
||
| func defaultStats() *configuredStats { |
There was a problem hiding this comment.
Mostly for backwards compatibility. If not specified in the config, you get the old default of everything.
| } | ||
|
|
||
| // Test only aggregating minimum and maximum | ||
| func TestBasicStatsWithMinAndMax(t *testing.T) { |
There was a problem hiding this comment.
Test for 2 stats
danielnelson
added
the
feat
Basic implementation of configuring which stats are pushed from basicstats aggregator (#3413)
Required for all PRs: