-
Notifications
You must be signed in to change notification settings - Fork 219
Conversation
Interesting, would you mind rebasing this against the current master? |
Allow excluding metrics for server slots depending on their status. Using this you can avoid reporting metrics for unused server slots when using dynamic scaling in haproxy. E.g. you can use --haproxy.server-exclude-states='DOWN,MAINT,MAINT (resolution)' Signed-off-by: Magnus Hyllander <[email protected]>
Rebased on master |
quay.io is having problems today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! I think we can keep the original interface of the Exporter and NewExporter() and do the filtering before, as we already have similar functionality. This will also avoid the changes to the test file as the interface stays the same.
A test for the filter function would be nice as well.
return 1 | ||
case "DOWN", "DOWN 1/2", "NOLB", "MAINT": | ||
default: //case "DOWN", "DOWN 1/2", "NOLB", "MAINT", "MAINT(via)", "MAINT(resolution)": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already return 0 / down as a fallback. What's the motivation for this change instead of listing known down status explicitly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure what to do with this function, since I haven't found a complete list of possible server statuses. I considered extending the list with the statuses I knew about, but decided there was no point, especially since (as you say) the fallback was 0 / down anyway. I guess I could have done it in a nicer way, letting it fall through to the fallback instead.
} | ||
|
||
for field, metric := range serverMetrics { | ||
if _, ok := selected[field]; ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup! 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your contribution! If you have time, adding a test would be great to ensure this requested feature survives changes over time. I'll resolve the merge conflicts and merge for now to not block this even longer.
* [CHANGE] Switch logging to go-kit #171 * [CHANGE] Fix metric types #182 * [CHANGE] Fix unit of time metric #183 * [FEATURE] Add filtering on server status #160 * [ENHANCEMENT] Add compression and server selection metrics #154 * [ENHANCEMENT] Add client/server abort metrics #167 * [ENHANCEMENT] Add version info metric (when using UNIX sockets) #180 Note: This release fixes the metric types of counters and renames the following metrics: * `haproxy_exporter_csv_parse_failures` -> `haproxy_exporter_csv_parse_failures_total` * `haproxy_exporter_total_scrapes` -> `haproxy_exporter_scrapes_total` * `haproxy_server_check_duration_milliseconds` -> `haproxy_server_check_duration_seconds` Signed-off-by: Tobias Schmidt <[email protected]>
* [CHANGE] Switch logging to go-kit #171 * [CHANGE] Fix metric types #182 * [CHANGE] Fix unit of time metric #183 * [FEATURE] Add filtering on server status #160 * [ENHANCEMENT] Add compression and server selection metrics #154 * [ENHANCEMENT] Add client/server abort metrics #167 * [ENHANCEMENT] Add version info metric (when using UNIX sockets) #180 Note: This release fixes the metric types of counters and renames the following metrics: * `haproxy_exporter_csv_parse_failures` -> `haproxy_exporter_csv_parse_failures_total` * `haproxy_exporter_total_scrapes` -> `haproxy_exporter_scrapes_total` * `haproxy_server_check_duration_milliseconds` -> `haproxy_server_check_duration_seconds` Signed-off-by: Tobias Schmidt <[email protected]> Co-authored-by: Tobias Schmidt <[email protected]>
Allow excluding metrics for server slots depending on their status. Using
this you can avoid reporting metrics for unused server slots when using
dynamic scaling in haproxy.
E.g. you can use
--haproxy.server-exclude-states='DOWN,MAINT,MAINT (resolution)'