Add statsd module #13109
Add statsd module #13109
Conversation
roncohen
added
Team:Integrations
| for mk, mv := range v { | ||
| metric[mk] = mv | ||
| } | ||
| fields[k] = metric |
There was a problem hiding this comment.
For reference, this is how mv will look: https://github.com/rcrowley/go-metrics/blob/9beb055b7962d16947a14e1cd718098a2431e20e/registry.go#L127
I think it should not cause issues, as keys don't collide. I'm wondering if mapping will break for the ones containing dots, as the resulting field name will be: statsd.my_histogram.99.9% (notice the final dot)
There was a problem hiding this comment.
good point. Should i just replace with dots with underlines in the value or what's the standard way?
There was a problem hiding this comment.
SGTM, that's what we normally do
There was a problem hiding this comment.
is '%' a problem? replace with 'pct' ?
There was a problem hiding this comment.
It won't be a problem for Elasticsearch, I wonder if some parts of Kibana could be affected. Adding pct may be misleading too, as someone could think this value is a percentage.
How about something like p95, p99, p99_9
There was a problem hiding this comment.
i went for 95p etc for now as it was simple to do. What do you think?
There was a problem hiding this comment.
That sounds ok, I see the resulting metrics are like statsd.metric4_95p. We normally would do statsd.metric4.95p, for instance, for AWS Cloudwatch metrics, where the statistic is after the dot. That said... I don't have an strong opinion here
There was a problem hiding this comment.
updated 👍 thanks for the input
|
unrelated failure |
| Statsd module | ||
| release: beta | ||
| fields: | ||
| - name: statsd.* |
There was a problem hiding this comment.
What about using statsd.metrics.* for the metrics? In case we want to add additional statsd metadata as the tags at some moment.
There was a problem hiding this comment.
Oh, I see we are adding the tags to the labels field.
There was a problem hiding this comment.
I've updated the mappings. Would be great to get some input
|
I've updated the mapping to use float instead of double. This is from the assumption that it will save a bit of disk space and I don't expect the precision loss to matter. |
|
mapping isn't working at the moment, will fix that tomorrow |
|
|
||
| events[idx] = &mb.Event{ | ||
| MetricSetFields: sanitizedMetrics, | ||
| RootFields: common.MapStr{"labels": mapstrTags}, |
There was a problem hiding this comment.
I always have the concern of using labels ECS field for different kind of labels. But let's keep it, we can always filter by service.type if we want different querying depending on the service.
|
great comments, thank @jsoriano ! |
gauges can be inc/dec/set while counters are just inc and get reset after each report Follow up on #13109
This adds a statsd endpoint.
Does not support sample rate yet.
Closes #10426