Add Splunk MultiMetric support #6640
Conversation
| } | ||
|
|
||
| func NewSerializer(splunkmetric_hec_routing bool) (*serializer, error) { | ||
| // NewSerializer Setup our new serializer | ||
| func NewSerializer(splunkmetric_hec_routing bool, splunkmetric_multimetric bool) (*serializer, error) { |
There was a problem hiding this comment.
We should probably switch to passing in enum types, similar to what was done in the influx serializer with UintSupport. This is just much more readable at the call point.
There was a problem hiding this comment.
I don't understand what your ask here is. Happy to make changes with a little more direction.
There was a problem hiding this comment.
This isn't super important, so we can do it later. What I've found is that having multiple boolean parameters to a function is somewhat hard to read. Compare these calls of the functions:
splunkmetric.NewSerializer(true, false)
splunkmetric.NewSerializer(HecRouting, NoMultiMetric)
Another advantage of this is that it is harder to mix up the argument order without getting a compile error.
Here is an example of defining a boolean type in the influx serializer:
telegraf/plugins/serializers/influx/influx.go
Lines 17 to 22 in 7ff6ec1
Again though, this isn't urgent, we can add it in a future change.
| metricJson, err = json.Marshal(dataGroup.Fields) | ||
| } | ||
|
|
||
| metricGroup = append(metricGroup, metricJson...) |
There was a problem hiding this comment.
Check the error before appending.
| } | ||
| } | ||
| commonTags.Time = float64(metric.Time().UnixNano()) / float64(1000000000) | ||
| switch s.SplunkmetricMultiMetric { |
There was a problem hiding this comment.
I suggest having two functions, one for multi metric and one for single. The idea being to keep these tasks as separate as possible and reduce the function length so that it can be more easily understood.
danielnelson
added
the
feat
Required for all PRs:
Adds support for Splunk 8.0's multi metric format.