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

Haproxy input missing lots of fields #2318

Closed
phemmer opened this issue Jan 25, 2017 · 4 comments
Closed

Haproxy input missing lots of fields #2318

phemmer opened this issue Jan 25, 2017 · 4 comments
Milestone

Comments

@phemmer
Copy link
Contributor

phemmer commented Jan 25, 2017

Directions

Bug report

Relevant telegraf.conf:

[[inputs.haproxy]]
  servers = ["/var/run/haproxy.sock"]

System info:

[Include Telegraf version, operating system name, and other relevant details]
telegraf current master
CentOS/7
Haproxy 1.7.2

Steps to reproduce:

  1. telegraf -test

Expected behavior:

See all the metrics as exported by haproxy present

Actual behavior:

Many of the metrics missing. In fact more metrics are missing than are present. Specifically the missing ones are:

  • status
  • weight
  • chkfail
  • chkdown
  • lastchg
  • qlimit
  • pid
  • iid
  • sid
  • tracked
  • type
  • rate_lim
  • check_duration
  • hrsp_other
  • hanafail
  • comp_in
  • comp_out
  • comp_byp
  • comp_rsp
  • lastsess
  • last_chk
  • last_agt
  • agent_status
  • agent_code
  • agent_duration
  • check_desc
  • agent_desc
  • check_rise
  • check_fall
  • check_health
  • agent_rise
  • agent_fall
  • agent_health
  • addr
  • cookie
  • mode
  • algo
  • conn_rate
  • conn_rate_max
  • conn_tot
  • intercepted
  • dcon
  • dses

Additionally some of the haproxy metrics are renamed ("act" -> "active_servers") while most others are not. The inconsistency makes the metrics hard to work with. I think it would be better to leave the field names as what haproxy calls them.

Additional info:

The plugin is also using a hard-coded list of column positions. If the columns in the CSV output ever changed in any way (additions, removals, or ordering), the plugin would break. It would be much safer, and future proof (and have prevented issues like the one this ticket is opened for) if the CSV headers were parsed & used.

@sparrc
Copy link
Contributor

sparrc commented Jan 25, 2017

I agree that it should change, but I'd rather not break current users of the plugin

there was a contributor who made a couple attempts already but it needs to not break backwards compatibility, since this is a widely used plugin: #2193

@phemmer
Copy link
Contributor Author

phemmer commented Jan 25, 2017

I can change the behavior to a whitelist type thing, where all fields are passed through unchanged with a few exceptions. Do we want a flag to be able to turn this off? I personally would rather it not muck with the column names, but a flag to enable breaking compatibility feels a little weird.

@phemmer
Copy link
Contributor Author

phemmer commented Jan 25, 2017

Should we start keeping a list of things to change for telegraf 2.0? A list of all the things that would clean up telegraf, but would cause breakage for existing configs or output data?

@sparrc
Copy link
Contributor

sparrc commented Jan 25, 2017

yes, that is probably a good idea, I've created an issue to track potential breaking changes here: #2320

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

No branches or pull requests

2 participants