Unable to parse 'other' value

[bengentil]

Unbound in certain conditions increment a "other" Rtype, this makes unbound-telemetry unresponsive with the following message:

ERROR [unbound_telemetry::server] Unable to observe unbound statistics: Unable to parse 'other' value

# unbound-control stats_noreset | grep other
num.query.type.other=4

[svartalf]

@bengentil hi and thank you for finding that bug!

I'm hesistant to merge your implementation from #7, though, as I plan to use some external crate with DNS types definitions eventually (carrying my own version is just unreasonable), and since there is really no "Other" query type exist, that will just add some maintainance burden in future.

I wrote fix in #8, but due to unrelated reasons CI is broken for today, so I'll merge it tomorrow when build will be green.

If you are up to some testing, I would really appreciate it!

[bengentil]

Hi @svartalf,
Thanks for the quick reply!

I checked the fix, it works (doesn't fail on "other" type) but the downside
is that the "other" values are ignored and not included in the metrics.

I understand your point that "other" is not a real type/class and that it can, as is, prevent from using a crate implementing the RFCs but does the exporter needs to support the types/classes?
Can't we just take the string value unbound gave us?

As far as I can tell the only place where the raw value is needed is when using the memory source and it's not currently using the Rtype/Class enums, the match here could be changed to something like:

use std::convert::From;
use trust_dns_proto::rr::record_type::RecordType;

fn rr_type(value: usize) -> Option<&'static str> {
    let type_ = RecordType::from(value as u16);
    match type_ {
        RecordType::Unknown(_) => Some("other"),
        _ => Some(From::from(type_)),
    }
}

I used the trust_dns_proto crate here as an example, I don't know if it's a good fit or if there is a better crate for the job.

[svartalf]

but the downside is that the "other" values are ignored and not included in the metrics.

Not gonna lie, this is a huge argument against making them separate fields, and a ridicuoulus mistake :) Fixed that in b90f2ea

Great idea about making it a &'static str -> u64 mapping, thank you! Initially I was basing data structs on what unbound provides via the shared memory, but this sounds like a good and easily maintained change.