dnsdist: Fix cache hit and miss metrics with DoH queries

Since we do two lookups for DoH queries forwarded over UDP (first
TCP then UDP), we need to be careful to only record a cache miss
in our last attempt.

(cherry picked from commit bc4d98b7cb2ecad488560d1dbef156708a1166af)

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index 7642b3a2d139d530a2cc64c34f28e87961e18b4c..9f2ab57dcd702f46529262584ba99f24f0d1e2a7 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -1347,7 +1347,9 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dq, LocalHolders& holders
         forwardedOverUDP = false;
       }
 
-      if (dq.ids.packetCache->get(dq, dq.getHeader()->id, &dq.ids.cacheKey, dq.ids.subnet, dq.ids.dnssecOK, forwardedOverUDP, allowExpired, false, true, true)) {
+      /* we do not record a miss for queries received over DoH and forwarded over TCP
+         yet, as we will do a second-lookup */
+      if (dq.ids.packetCache->get(dq, dq.getHeader()->id, &dq.ids.cacheKey, dq.ids.subnet, dq.ids.dnssecOK, forwardedOverUDP, allowExpired, false, true, (dq.ids.protocol != dnsdist::Protocol::DoH || forwardedOverUDP) ? true : false)) {
 
         restoreFlags(dq.getHeader(), dq.ids.origFlags);
 
@@ -1363,7 +1365,7 @@ ProcessQueryResult processQueryAfterRules(DNSQuestion& dq, LocalHolders& holders
       }
       else if (dq.ids.protocol == dnsdist::Protocol::DoH && !forwardedOverUDP) {
         /* do a second-lookup for UDP responses, but we do not want TC=1 answers */
-        if (dq.ids.packetCache->get(dq, dq.getHeader()->id, &dq.ids.cacheKeyUDP, dq.ids.subnet, dq.ids.dnssecOK, true, allowExpired, false, false, false)) {
+        if (dq.ids.packetCache->get(dq, dq.getHeader()->id, &dq.ids.cacheKeyUDP, dq.ids.subnet, dq.ids.dnssecOK, true, allowExpired, false, false, true)) {
           if (!prepareOutgoingResponse(holders, *dq.ids.cs, dq, true)) {
             return ProcessQueryResult::Drop;
           }

diff --git a/regression-tests.dnsdist/test_Metrics.py b/regression-tests.dnsdist/test_Metrics.py
index d907b1135463a25b1c12d15d8ac0e75e17b2899e..5a70d0d8cefda59b5d9ea6987cd3bb782323d862 100644 (file)
--- a/regression-tests.dnsdist/test_Metrics.py
+++ b/regression-tests.dnsdist/test_Metrics.py
@@ -59,6 +59,18 @@ class TestRuleMetrics(DNSDistTest):
         self.assertIn(name, stats)
         return int(stats[name])
 
+    def getPoolMetric(self, poolName, metricName):
+        headers = {'x-api-key': self._webServerAPIKey}
+        url = 'http://127.0.0.1:' + str(self._webServerPort) + '/api/v1/servers/localhost/pool?name=' + poolName
+        r = requests.get(url, headers=headers, timeout=self._webTimeout)
+        self.assertTrue(r)
+        self.assertEqual(r.status_code, 200)
+        self.assertTrue(r.json())
+        content = r.json()
+        stats = content['stats']
+        self.assertIn(metricName, stats)
+        return int(stats[metricName])
+
     def testRCodeIncreaseMetrics(self):
         """
         Metrics: Check that metrics are correctly updated for RCodeAction
@@ -93,6 +105,7 @@ class TestRuleMetrics(DNSDistTest):
 
         # self-generated responses should not increase this metric
         self.assertEqual(self.getMetric('servfail-responses'), servfailBackendResponses)
+
     def testCacheMetrics(self):
         """
         Metrics: Check that metrics are correctly updated for cache misses and hits
@@ -114,6 +127,8 @@ class TestRuleMetrics(DNSDistTest):
             responsesBefore = self.getMetric('responses')
             cacheHitsBefore = self.getMetric('cache-hits')
             cacheMissesBefore = self.getMetric('cache-misses')
+            poolCacheHitsBefore = self.getPoolMetric('cache', 'cacheHits')
+            poolCacheMissesBefore = self.getPoolMetric('cache', 'cacheMisses')
 
             sender = getattr(self, method)
             # first time, cache miss
@@ -128,6 +143,8 @@ class TestRuleMetrics(DNSDistTest):
             self.assertEqual(self.getMetric('responses'), responsesBefore + 2)
             self.assertEqual(self.getMetric('cache-hits'), cacheHitsBefore + 1)
             self.assertEqual(self.getMetric('cache-misses'), cacheMissesBefore + 1)
+            self.assertEqual(self.getPoolMetric('cache', 'cacheHits'), poolCacheHitsBefore + 1)
+            self.assertEqual(self.getPoolMetric('cache', 'cacheMisses'), poolCacheMissesBefore + 1)
 
     def testServFailMetrics(self):
         """