New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

statsd plugin: please be more flexible with parsing #461

Closed
its-not-a-bug-its-a-feature opened this Issue Nov 5, 2013 · 4 comments

Comments

Projects
None yet
4 participants
@its-not-a-bug-its-a-feature

With version 5.4 , I see many messages from my test setup saying that statsd messages cannot be parsed :

Nov  5 18:45:20 hostname collectd[8236]: statsd plugin: Unable to parse line: "object-server.REPLICATE.timing:0.399112701416|ms|@0.1"
Nov  5 18:45:20 hostname collectd[8236]: statsd plugin: Unable to parse line: "object-server.REPLICATE.timing:0.399112701416|ms|@0.1"
Nov  5 18:45:20 hostname collectd[8236]: statsd plugin: Unable to parse line: "object-server.REPLICATE.timing:0.375986099243|ms|@0.1"
Nov  5 18:45:20 hostnam collectd[8236]: statsd plugin: Unable to parse line: "object-server.REPLICATE.timing:0.375986099243|ms|@0.1"

I understand that the extra field ( after the type field "ms") are unnecessary for timing messages.

Within the plugin statsd.c , in statsd_parse_line() we return -1 if there is extra data for non-counters:

  /* extra is only valid for counters */
  if (extra != NULL)
    return (-1);

Other statsd parsers would allow this, would you mind please making your code more flexible - maybe just removing the two lines above ?

Thanks

@dvenza

This comment has been minimized.

Show comment
Hide comment
@dvenza

dvenza May 15, 2014

Yes, please. It is Swift that is generating these lines with extra data appended and it needs to be fixed, but in the meantime collectd is flooding the logs and loosing valuable data.

dvenza commented May 15, 2014

Yes, please. It is Swift that is generating these lines with extra data appended and it needs to be fixed, but in the meantime collectd is flooding the logs and loosing valuable data.

@johnl

This comment has been minimized.

Show comment
Hide comment
@johnl

johnl Jul 21, 2014

Contributor

This bug has been reported in swift: https://bugs.launchpad.net/swift/+bug/1248348

But having done a bit more investigation, it is collectd which is at fault.

According to the statsd docs, timing (ms) metric types should not have sample rates:

https://github.com/etsy/statsd/blob/master/docs/metric_types.md#timing

But if you look at the statsd code, it actually does use the sample rate if it's available for timing stats:

https://github.com/etsy/statsd/blob/bad366cd438cd59b2064dd03ff3d415fc7db0c4b/stats.js#L206-L225

So technically this is a bug in collectd, but an understandable one given the faulty statsd docs.

Collectd needs to accept sample rates for timing metric types and use them accordingly.

Contributor

johnl commented Jul 21, 2014

This bug has been reported in swift: https://bugs.launchpad.net/swift/+bug/1248348

But having done a bit more investigation, it is collectd which is at fault.

According to the statsd docs, timing (ms) metric types should not have sample rates:

https://github.com/etsy/statsd/blob/master/docs/metric_types.md#timing

But if you look at the statsd code, it actually does use the sample rate if it's available for timing stats:

https://github.com/etsy/statsd/blob/bad366cd438cd59b2064dd03ff3d415fc7db0c4b/stats.js#L206-L225

So technically this is a bug in collectd, but an understandable one given the faulty statsd docs.

Collectd needs to accept sample rates for timing metric types and use them accordingly.

johnl added a commit to brightbox/collectd that referenced this issue Aug 19, 2014

Statsd: support samplerate field in timing metric types
Fixes "Unable to parse line" bug, often seen receiving stats from
OpenStack Swift. Should fix GH issue #461
@johnl

This comment has been minimized.

Show comment
Hide comment
@johnl

johnl Aug 19, 2014

Contributor

as above, I've addd support for the samplerate field in timing metric types to collectd. I've built some Ubuntu Trusty packages for testing (they're the upstream Ubuntu upstream packages modified only to add this patch). The packages are available on this ppa: https://launchpad.net/~brightbox/+archive/ubuntu/collectd-statsd-testing

Contributor

johnl commented Aug 19, 2014

as above, I've addd support for the samplerate field in timing metric types to collectd. I've built some Ubuntu Trusty packages for testing (they're the upstream Ubuntu upstream packages modified only to add this patch). The packages are available on this ppa: https://launchpad.net/~brightbox/+archive/ubuntu/collectd-statsd-testing

octo added a commit that referenced this issue Aug 21, 2014

Statsd: support samplerate field in timing metric types
Fixes "Unable to parse line" bug, often seen receiving stats from
OpenStack Swift. Should fix GH issue #461
@mfournier

This comment has been minimized.

Show comment
Hide comment
@mfournier

mfournier May 26, 2015

Contributor

This was fixed in collectd 5.4.2. Closing. Thanks to everyone for reporting and tracking this down !

Contributor

mfournier commented May 26, 2015

This was fixed in collectd 5.4.2. Closing. Thanks to everyone for reporting and tracking this down !

@mfournier mfournier closed this May 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment