Merge pull request #12783 from rgacogne/ddist-fix-pool-cache-metrics

dnsdist: Fix cache hit and miss metrics with DoH queries
PowerDNS · Jun 15, 2023 · aeb1cff · aeb1cff
2 parents 2da1f0d + 2c9b6e8
commit aeb1cff
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 2 deletions.
diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
@@ -1363,7 +1363,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)) {
 
         restoreFlags(dq.getHeader(), dq.ids.origFlags);
 
@@ -1379,7 +1381,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
@@ -59,6 +59,18 @@ def getMetric(self, name):
         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 @@ def testRCodeIncreaseMetrics(self):
 
         # 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 @@ def testCacheMetrics(self):
             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 @@ def testCacheMetrics(self):
             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):
         """