From 90a85e3f9475356dcd601e6d47b949a67437e3cb Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 16 Oct 2026 15:49:36 -0700 Subject: [PATCH] bdns: don't strip port number in metrics It's no longer the case that we run Unbound listening on many ports (now we use DoH), and the port number is required to accurately identify an instance. --- bdns/dns.go | 18 ++++-------------- bdns/dns_test.go | 10 +++++----- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/bdns/dns.go b/bdns/dns.go index ae7c0aadee1..9df626225c5 100644 --- a/bdns/dns.go +++ b/bdns/dns.go @@ -201,16 +201,6 @@ func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) ( tries = i + 1 chosenServer := servers[i%len(servers)] - // Strip off the IP address part of the server address because - // we talk to the same server on multiple ports, and don't want - // to blow up the cardinality. - // Note: validateServerAddress() has already checked net.SplitHostPort() - // and ensures that chosenServer can't be a bare port, e.g. ":1337" - chosenServerIP, _, err = net.SplitHostPort(chosenServer) - if err != nil { - return nil, chosenServer, err - } - // Do a bare assignment (not :=) to populate the `resp` used by the defer above. var rtt time.Duration resp, rtt, err = c.exchanger.ExchangeContext(ctx, req, chosenServer) @@ -223,7 +213,7 @@ func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) ( c.queryTime.With(prometheus.Labels{ "qtype": qtypeStr, "result": result, - "resolver": chosenServerIP, + "resolver": chosenServer, }).Observe(rtt.Seconds()) if err != nil { @@ -240,21 +230,21 @@ func (c *impl) exchangeOne(ctx context.Context, hostname string, qtype uint16) ( c.timeoutCounter.With(prometheus.Labels{ "qtype": qtypeStr, "result": "out of retries", - "resolver": chosenServerIP, + "resolver": chosenServer, "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), }).Inc() } else if errors.Is(err, context.DeadlineExceeded) { c.timeoutCounter.With(prometheus.Labels{ "qtype": qtypeStr, "result": "deadline exceeded", - "resolver": chosenServerIP, + "resolver": chosenServer, "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), }).Inc() } else if errors.Is(err, context.Canceled) { c.timeoutCounter.With(prometheus.Labels{ "qtype": qtypeStr, "result": "canceled", - "resolver": chosenServerIP, + "resolver": chosenServer, "isTLD": fmt.Sprintf("%t", !strings.Contains(hostname, ".")), }).Inc() } diff --git a/bdns/dns_test.go b/bdns/dns_test.go index 0210b0efef4..6c0d2c8d382 100644 --- a/bdns/dns_test.go +++ b/bdns/dns_test.go @@ -780,7 +780,7 @@ func TestRetry(t *testing.T) { t, dr.timeoutCounter, prometheus.Labels{ "qtype": "TXT", "result": "out of retries", - "resolver": "127.0.0.1", + "resolver": "127.0.0.1:4053", "isTLD": "false", }, tc.metricsAllRetries) } @@ -810,7 +810,7 @@ func TestRetryMetrics(t *testing.T) { t, dr.timeoutCounter, prometheus.Labels{ "qtype": "TXT", "result": "canceled", - "resolver": "127.0.0.1", + "resolver": "127.0.0.1:4053", }, 1) // Same as above, except rather than cancelling the context ourselves, we @@ -829,7 +829,7 @@ func TestRetryMetrics(t *testing.T) { t, dr.timeoutCounter, prometheus.Labels{ "qtype": "TXT", "result": "deadline exceeded", - "resolver": "127.0.0.1", + "resolver": "127.0.0.1:4053", }, 1) } @@ -959,13 +959,13 @@ func TestDOHMetric(t *testing.T) { resolver.exchanger = &dohAlwaysRetryExchanger{err: &url.Error{Op: "read", Err: testTimeoutError(true)}} // Starting out, we should count 0 "out of retries" errors. - test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1", "isTLD": "false"}, 0) + test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1:4053", "isTLD": "false"}, 0) // Trigger the error. _, _, _ = resolver.exchangeOne(context.Background(), "example.com", 0) // Now, we should count 1 "out of retries" errors. - test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1", "isTLD": "false"}, 1) + test.AssertMetricWithLabelsEquals(t, resolver.timeoutCounter, prometheus.Labels{"qtype": "None", "type": "out of retries", "resolver": "127.0.0.1:4053", "isTLD": "false"}, 1) } // truncatedExchanger returns a truncated (TC bit set) response with the given