Metrics - Influx StatsD (#664)#688
Conversation
|
Thank you very much! I gave it a quick skim and I anticipate it should be an easy merge. Please stay on the line for a day or two, until I can find the time to give it a proper read-thru. |
|
@peterbourgon, any update on the status of this? |
|
Thank you for the ping. ETA 24h. |
peterbourgon
left a comment
peterbourgon
left a comment
There was a problem hiding this comment.
I've tried to identify all of the differences between your new package and the existing Dogstatsd, do I have everything? If so, Influx, this is fucking wild — there is essentially no difference to Dogstatsd, just changes for the sake of change. Do you have a link to where this protocol is documented on the Influx site?
| var n int | ||
|
|
||
| d.counters.Reset().Walk(func(name string, lvs lv.LabelValues, values []float64) bool { | ||
| n, err = fmt.Fprintf(w, "%s%s%s:%f|c%s\n", d.prefix, name, d.tagValues(lvs), sum(values), sampling(d.rates.Get(name))) |
There was a problem hiding this comment.
Influxstatsd vs. Dogstatsd:
n, err = fmt.Fprintf(w, "%s%s%s:%f|c%s\n", d.prefix, name, d.tagValues(lvs), sum(values), sampling(d.rates.Get(name))) // Influx
n, err = fmt.Fprintf(w, "%s%s:%f|c%s%s\n", d.prefix, name, sum(values), sampling(d.rates.Get(name)), d.tagValues(lvs) // Dog| d.mtx.RLock() | ||
| for _, root := range d.gauges { | ||
| root.walk(func(name string, lvs lv.LabelValues, value float64) bool { | ||
| n, err = fmt.Fprintf(w, "%s%s%s:%f|g\n", d.prefix, name, d.tagValues(lvs), value) |
There was a problem hiding this comment.
Influxstatsd vs. Dogstatsd:
n, err = fmt.Fprintf(w, "%s%s%s:%f|g\n", d.prefix, name, d.tagValues(lvs), value) // Influx
n, err = fmt.Fprintf(w, "%s%s:%f|g%s\n", d.prefix, name, value, d.tagValues(lvs)) // Dog| d.timings.Reset().Walk(func(name string, lvs lv.LabelValues, values []float64) bool { | ||
| sampleRate := d.rates.Get(name) | ||
| for _, value := range values { | ||
| n, err = fmt.Fprintf(w, "%s%s%s:%f|ms%s\n", d.prefix, name, d.tagValues(lvs), value, sampling(sampleRate)) |
There was a problem hiding this comment.
Influxstatsd vs. Dogstatsd:
n, err = fmt.Fprintf(w, "%s%s%s:%f|ms%s\n", d.prefix, name, d.tagValues(lvs), value, sampling(sampleRate)) // Influx
n, err = fmt.Fprintf(w, "%s%s:%f|ms%s%s\n", d.prefix, name, value, sampling(sampleRate), d.tagValues(lvs)) // Dog| d.histograms.Reset().Walk(func(name string, lvs lv.LabelValues, values []float64) bool { | ||
| sampleRate := d.rates.Get(name) | ||
| for _, value := range values { | ||
| n, err = fmt.Fprintf(w, "%s%s%s:%f|h%s\n", d.prefix, name, d.tagValues(lvs), value, sampling(sampleRate)) |
There was a problem hiding this comment.
Influxstatsd vs. Dogstatsd:
n, err = fmt.Fprintf(w, "%s%s%s:%f|h%s\n", d.prefix, name, d.tagValues(lvs), value, sampling(sampleRate)) // Influx
n, err = fmt.Fprintf(w, "%s%s:%f|h%s%s\n", d.prefix, name, value, sampling(sampleRate), d.tagValues(lvs)) // Dog| pairs = append(pairs, labelValues[i]+"="+labelValues[i+1]) | ||
| } | ||
| return "," + strings.Join(pairs, ",") | ||
| } |
There was a problem hiding this comment.
Influxstatsd requires key=value pairs separated from the metric line by ,
Dogstatsd requires key:value pairs separated from the metric line by |#
|
@peterbourgon, this was basically what I was referring to with #664. The StatsD Protocols, with their different flavors, basically have minuscule differences. The Base StatsD protocol doesn't have tag support, but supports deltas; DogStatsD adds Histograms / tagging; InfluxStatsD adds basically the same thing, but as a different format, and offers support for deltas. Like I said in the GitHub Issue, it's possible to merge them all into a single package, adding conditionals where needed. What do you think? I also have ideas of how the metrics can be refactored, but this can be brought up elsewhere. Here's Influx Data referencing their StatsD flavor. |
|
Thank you for the link. When you made the initial issue I assumed there would be more significant differences. As it stands it looks like they accomplish exactly the same thing through different protocols. Which is infuriating. Your approach is sound, I'll approve it tomorrow. If we discover a third protocol with the same capabilities and slightly-different wire format, we should at that point extract the common logic to a separate package, and implement all 3 of {Dogstatsd,Influxstatsd,Xxxstatsd} as adapters to that common package. Until then, I'm happy with things as-is. |
|
Thanks for the contribution!! |
1 participant
This is my first time contributing to a large, open source project, so please let me know if I am doing anything wrong. 😊
This satisfies the issue listed in the title. This is just a simple addition that provides the ability to use the Influx StatsD protocol provided by Influx Data within Telegraf. This protocol offers tagging to the StatsD protocol, and is also delta-friendly with gauges unlike DogStatsD (although I kept the current DogStatsD implementation of Gauges at the moment so the value isn't resetting after each flush). This does not include the fix for #578, but I would love to tackle that next with the three packages (StatsD, DogStatsD, and InfluxStatsD).
If there are any discrepancies, please let me know and I'll look at them when I get the chance. Thanks!