Chrony input improvements

[phemmer]

Feature Request

Proposal:

The chrony input should

• provide metrics on all sources, not just the currently active sync source
• stratum should not be a tag. A sync source's stratum can change. This should not result in a new series being created. It also prevents being able to query based on stratum (e.g. select ... where stratum > 5)
• leap_status should not be a tag. The leap status can change, and should not result in a new series being created. Already covered in chrony leap_status should be a field? #2072 though.
• The sync source's host name should be provided, not just its IP address.

[Gelob]

It would also be nice to be able to track clients if that option is turned on within chrony.

[reimda]

If we move tags to fields and provide metrics on all sources we should add a setting and let users opt in to the new behavior.

[reimda]

If we don't want to add a setting to opt in, we could make these changes during the next major release. #9478

[srebhan]

@phemmer the stratum and leap_status can be converted to fields using the converter processor. The IP can be converted to hostname using the reverse_dns processor.

Would that solve this issue?

[telegraf-tiger]

Hello! I am closing this issue due to inactivity. I hope you were able to resolve your problem, if not please try posting this question in our Community Slack or Community Forums or provide additional details in this issue and reqeust that it be re-opened. Thank you!

[dsvk]

@srebhan unfortunately the suggestions above don't really solve this:

provide metrics on all sources, not just the currently active sync source

Not addressed - while this is IMHO the most important improvement proposed here: enabling accuracy comparisons, detection of diverged/drifting sources; uptime, latency and source state change tracking etc. This requires extending the plugin to utilize new command outputs - sources, sourcestats and selectdata could suffice.

stratum should not be a tag
leap_status should not be a tag

I suppose the converter processor could solve this but it's still incorrect default behavior that users shouldn't have to manually workaround. I agree with the comments above that this should be a non-default config option for 1.x and become the default in 2.x, in which case it probably needs to stay on the radar as an open issue.

The sync source's host name should be provided, not just its IP address.

PTR lookups won't help when the server's IP address doesn't have a PTR record, nor when that record points to a different FQDN (e. g. servers running on dynamic/residential connections or hosting with no control over PTR; or when PTR needs to point to a different record - e. g. email FCrDNS). Based on a quick test I saw that even some stratum 1s from various national metrology institutes with dedicated NTP IPs lack PTRs - and most users are supposed to sync against stratum 2 or lower pool servers where PTR will be a lot more hit and miss.

This could actually be improved easily - chronyc could be executed with -nN to not just disable the reverse DNS lookups (-n) but to list the servers using their original FQDNs (-N) if they were configured as such. One won't natively get the forward-lookup IP address info added on top for any servers configured as FQDNs but having each server identified in the metrics output with the exact same record (be it FQDN or IP) as it has been configured in the chrony config seems like the most intuitive and UX-friendly approach.

[srebhan]

@phemmer, @Gelob or @dsvk I'm trying to tackle this issue (after soo long... sorry!) and reworked the plugin to get rid of the chronyc dependency and improve the testing capabilities. So far there is no feature enhancement but I need to make sure there is no regression with the change.

Can anyone please verify that the binary in PR #14629 (available as-soon-as CI finished the tests) works as before and produces sensible metrics!?!? Please let me know your findings!