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

Add probe_dns_rcode metric #858

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add probe_dns_rcode metric #858

wants to merge 1 commit into from

Conversation

dgl
Copy link
Member

@dgl dgl commented Jan 2, 2022

This metric has the value of the response code from the DNS server; 0 is
NOERROR, per
https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-6

It can be used along with probe_success in order to diagnose why
failures occurred.

For #474.

Signed-off-by: David Leadbeater [email protected]

This metric has the value of the response code from the DNS server; 0 is
NOERROR, per
https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-6

It can be used along with probe_success in order to diagnose why
failures occurred.

Signed-off-by: David Leadbeater <[email protected]>
Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I wonder if it'd be worth it to have this as a ENUM style metric.

probe_dns_rcode{rcode="NOERROR"} 1
probe_dns_rcode{rcode="SRVFAIL"} 0
probe_dns_rcode{rcode="NXDOMAIN"} 0

@dgl
Copy link
Member Author

dgl commented Jan 2, 2022

It was mentioned in #799 -- it would be nice, but per https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-6 there's quite a few possibilities, maybe we could pick the ones we think are likely...

Say NOERROR, SERVFAIL, NXDOMAIN, REFUSED and then have an OTHER for the rest would cover it, but we do then lose what the actual rcode is (unless we use the value of OTHER to be the rcode, but that is wrong for an ENUM style metric...)

@dgl dgl requested review from mem and roidelapluie January 25, 2022 04:16
@dgl
Copy link
Member Author

dgl commented Jan 25, 2022

Adding some reviewers to get any thoughts on labels vs numbers...

@mem
Copy link
Contributor

mem commented May 9, 2022

Adding some reviewers to get any thoughts on labels vs numbers...

Sorry, I was sure I had replied to this...

I would prefer @SuperQ's proposal. That makes it easier to build a counter out of this if desired (e.g. using a recording rule).

IIRC there are ~ 40 possible response codes. As long as the returned one is always captured, I don't have a strong preference for including a few select ones all the time.

@SuperQ
Copy link
Member

SuperQ commented Dec 30, 2022

We could provide a list of default "always expodsed" rcodes as a command line flag. Something like --probe.dns.rcodes=NOERROR,SRVFAIL,NXDOMAIN". If the rcode is some other code, we could either assign it to an OTHER bucket, or since it's reasonably limited, we could directly expose the "other" code.

Opinions?

@mem
Copy link
Contributor

mem commented Jan 4, 2023

We could provide a list of default "always expodsed" rcodes as a command line flag. Something like --probe.dns.rcodes=NOERROR,SRVFAIL,NXDOMAIN". If the rcode is some other code, we could either assign it to an OTHER bucket, or since it's reasonably limited, we could directly expose the "other" code.

Opinions?

I would be OK with that (either option: exposing the "new" code, or using an "OTHER" bucket, maybe leaning slightly towards the latter).

@github-actions github-actions bot added the stale label Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants