Skip to content
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

Added sanization of fieldName #1118

Closed
wants to merge 4 commits into from
Closed

Added sanization of fieldName #1118

wants to merge 4 commits into from

Conversation

G-regL
Copy link
Contributor

@G-regL G-regL commented Apr 28, 2016

When sending Telegraf data to Graphite, many of the metrics generated by the win_perf_counters input plugin aren't properly sanitized, and get ignored by Carbon.

For example, for the following config, only Processes and Threads succeed:

[[outputs.graphite]]
  template = "host.measurement.tags.field"

[[inputs.win_perf_counters]]
  [[inputs.win_perf_counters.object]]
    ObjectName = "System"
    Counters = ["Context Switches/sec", "System Calls/sec", "Threads", "System Up Time", "Processes", "Processor Queue Length"]
    Instances = ["------"]
    Measurement = "win_system"

If run in debug mode, telegraf produces this output:

> win_system,host=ServerA,objectname=System Context\ Switches/sec=9189.787 1461855980003819600
> win_system,host=ServerA,objectname=System System\ Calls/sec=41990.85 1461855980003819600
> win_system,host=ServerA,objectname=System Threads=2260 1461855980013820100
> win_system,host=ServerA,objectname=System System\ Up\ Time=1480454.9 1461855980013820100
> win_system,host=ServerA,objectname=System Processes=158 1461855980013820100
> win_system,host=ServerA,objectname=System Processor\ Queue\ Length=0 1461855980023820600

The metrics are received by carbon, but it throws these 4 errors:

28/04/2016 11:06:20 :: invalid line (ServerA.win_system.System.Context Switches/sec 9189.787 1461855980) received from client 127.0.0.1:57807, ignoring
28/04/2016 11:06:20 :: invalid line (ServerA.win_system.System.System Calls/sec 41990.85 1461855980) received from client 127.0.0.1:57807, ignoring
28/04/2016 11:06:20 :: invalid line (ServerA.win_system.System.System Up Time 1.4804549e+06 1461855980) received from client 127.0.0.1:57807, ignoring
28/04/2016 11:06:20 :: invalid line (ServerA.win_system.System.Processor Queue Length 0 1461855980) received from client 127.0.0.1:57807, ignoring

I see that the Graphite output serializer sanitizes the bucket name, but it doesn't sanitize the field itself, and win_perf_counters doesn't do any kind of sanitizing when gathering metrics.

@sparrc
Copy link
Contributor

sparrc commented Apr 28, 2016

we might want to start sanitizing win perf counters on collection. Especially so we can replace /sec with per_sec, which would be better than replacing / with a -

Replace '/[sS]ec' for '_persec' and spaces with underscores.
@G-regL
Copy link
Contributor Author

G-regL commented Apr 28, 2016

That makes sense. Updated the PR to reflect that.

@sparrc
Copy link
Contributor

sparrc commented Apr 28, 2016

thanks @G-regL, could you update the CHANGELOG with a release not about this? This is a breaking change.

And on a related note-- do we need to sanitize % characters too? Or does graphite support those?

@G-regL
Copy link
Contributor Author

G-regL commented Apr 28, 2016

Sure, I'll update the CHANGELOG.

Graphite supports %, based on my testing. Wouild you prefer I remove that sanitization?

@sparrc
Copy link
Contributor

sparrc commented Apr 28, 2016

This is good the way it is, I'll merge when the tests pass, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants