Drop XML statistics v2 support #171
Conversation
|
Just checking what versions distros ship with. Seems like Redhat-based (RHEL7) had 9.11 and Debian Stretch (oldoldstable) had 9.10, but 9.11 in backports. So I think this is OK. |
According to debian-devel-announce, Stretch is scheduled to be archived this month, around April 23. |
|
@SuperQ I'm also fine with leaving the "auto" / default as XML (v3) for at least another release. The "auto" concept made sense in the past when we had to support archaic BIND versions that only provided XML v2. Then there were a couple of BIND versions that supported both XML v2 and v3, so it made sense to try using v3, with a fallback to v2. But by dropping XML v2 support, we are effectively setting the minimum supported BIND version to 9.10, which has always supported both XML (v3) and JSON. For that reason, I'm not sure if we even need to support XML at all in future. |
|
Yea, json as a default is fine, we just need to document it. |
|
Want to rebase this? |
XML v2 stats was deprecated in BIND v9.10.0, which was itself EOL'd in July 2018. Closes: prometheus-community#170 Signed-off-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
Also add .xml extensions to fixtures to make them more editor-friendly. Signed-off-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
Signed-off-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
This also drops support for "auto" determination of stats version, which is too tightly coupled to the XML stats channel to easily support JSON too. It is essentially redundant anyway, since the JSON stats channel is supported and always present since BIND v9.10.0. We also no longer unmarshal the memory or socketmgr nodes of the XML stats, since we don't currently expose any metrics for these, and they have been responsible for unmarshal errors in the past. Once we actually _do_ expose metrics for them, they should be included in tests. Signed-off-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
dswarbrick
force-pushed
the
drop-xml-v2
branch
from
76db3e3 to
720b399
Compare
Done. |
* [CHANGE] Drop XML statistics v2 support #171 * [CHANGE] Deprecate collection of task stats by default #200 * [CHANGE] Update logging library #212 * [ENHANCEMENT] Add metric `rpz_rewrites` #208 * [BUGFIX] Make log level configurable via command-line flag #182 Signed-off-by: SuperQ <superq@gmail.com>
* [CHANGE] Drop XML statistics v2 support #171 * [CHANGE] Deprecate collection of task stats by default #200 * [CHANGE] Update logging library #212 * [ENHANCEMENT] Add metric `rpz_rewrites` #208 * [BUGFIX] Make log level configurable via command-line flag #182 Signed-off-by: SuperQ <superq@gmail.com>
Drop support for XML statistics v2, which has been deprecated since BIND v9.10.0 (which itself was EOL'ed in July 2018).
Also some code reorganisation to keep some of the XML-specific code out of the core of the exporter.
@SuperQ I still want to make another pass at decoupling the remaining XML code from the core. It is embedded fairly deeply in the guts of the exporter, which has made adding the JSON support not as clean as I'd like. The key difference between unmarshalling JSON stats vs. XML stats is that JSON uses
map[string]uint64for counters, whereas XML uses slices of counter structs. Since this exporter originally only supported XML, it was fairly tightly coupled to the counter slices, and has meant that the JSON stats has to manipulate data into that format. We could implement some generic structs that were neither JSON nor XML specific, and have both parsers transform data into those generic structs. However that's going to create more allocations than necessary. I feel that some of the core code that produces metrics from structs needs to be split out into the appropriate parser packages.Also... one has to wonder whether we need to support XML at all, since JSON stats have been available since BIND v9.10.
Closes: #170