dnsdist: Add a per-server metric for non-compliant responses

diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc
index cc55ee43646ec52f501967b0baea315a7c0e7fb6..c46d766927042d9611f191b29dffec4e45066a40 100644 (file)
--- a/pdns/dnsdist-tcp.cc
+++ b/pdns/dnsdist-tcp.cc
@@ -522,7 +522,7 @@ void IncomingTCPConnectionState::handleResponse(const struct timeval& now, TCPRe
   try {
     auto& ids = response.d_idstate;
     unsigned int qnameWireLength;
-    if (!response.d_connection || !responseContentMatches(response.d_buffer, ids.qname, ids.qtype, ids.qclass, response.d_connection->getRemote(), qnameWireLength)) {
+    if (!response.d_connection || !responseContentMatches(response.d_buffer, ids.qname, ids.qtype, ids.qclass, response.d_connection->getDS(), qnameWireLength)) {
       state->terminateClientConnection();
       return;
     }

diff --git a/pdns/dnsdist-web.cc b/pdns/dnsdist-web.cc
index 0af3822a37e6d64e7fe610387af25fa9d6d5d375..5779c267d2f0f52415d61b324b3dce059ec646fb 100644 (file)
--- a/pdns/dnsdist-web.cc
+++ b/pdns/dnsdist-web.cc
@@ -510,6 +510,8 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp)
   output << "# TYPE " << statesbase << "queries "                     << "counter"                                                           << "\n";
   output << "# HELP " << statesbase << "responses "                   << "Amount of responses received from this server"                     << "\n";
   output << "# TYPE " << statesbase << "responses "                   << "counter"                                                           << "\n";
+  output << "# HELP " << statesbase << "noncompliantresponses "       << "Amount of non-compliant responses received from this server"       << "\n";
+  output << "# TYPE " << statesbase << "noncompliantresponses "       << "counter"                                                           << "\n";
   output << "# HELP " << statesbase << "drops "                       << "Amount of queries not answered by server"                          << "\n";
   output << "# TYPE " << statesbase << "drops "                       << "counter"                                                           << "\n";
   output << "# HELP " << statesbase << "latency "                     << "Server's latency when answering questions in milliseconds"         << "\n";
@@ -569,6 +571,7 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp)
     output << statesbase << "status"                       << label << " " << (state->isUp() ? "1" : "0")        << "\n";
     output << statesbase << "queries"                      << label << " " << state->queries.load()              << "\n";
     output << statesbase << "responses"                    << label << " " << state->responses.load()            << "\n";
+    output << statesbase << "noncompliantresponses"        << label << " " << state->nonCompliantResponses.load()<< "\n";
     output << statesbase << "drops"                        << label << " " << state->reuseds.load()              << "\n";
     if (state->isUp()) {
       output << statesbase << "latency"                    << label << " " << state->latencyUsec/1000.0          << "\n";
@@ -989,6 +992,7 @@ static void addServerToJSON(Json::array& servers, int id, const std::shared_ptr<
     {"latency", (double)(a->latencyUsec/1000.0)},
     {"queries", (double)a->queries},
     {"responses", (double)a->responses},
+    {"nonCompliantResponses", (double)a->nonCompliantResponses},
     {"sendErrors", (double)a->sendErrors},
     {"tcpDiedSendingQuery", (double)a->tcpDiedSendingQuery},
     {"tcpDiedReadingResponse", (double)a->tcpDiedReadingResponse},

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index ca7d471c2df249ee4aa263da29a70388dfe25305..e79f31fb0246ed57ffea2316fb611d281b824f12 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -267,7 +267,7 @@ void doLatencyStats(double udiff)
   doAvg(g_stats.latencyAvg1000000, udiff, 1000000);
 }
 
-bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const ComboAddress& remote, unsigned int& qnameWireLength)
+bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const std::shared_ptr<DownstreamState>& remote, unsigned int& qnameWireLength)
 {
   if (response.size() < sizeof(dnsheader)) {
     return false;
@@ -276,6 +276,9 @@ bool responseContentMatches(const PacketBuffer& response, const DNSName& qname,
   const struct dnsheader* dh = reinterpret_cast<const struct dnsheader*>(response.data());
   if (dh->qr == 0) {
     ++g_stats.nonCompliantResponses;
+    if (remote) {
+      ++remote->nonCompliantResponses;
+    }
     return false;
   }
 
@@ -285,6 +288,9 @@ bool responseContentMatches(const PacketBuffer& response, const DNSName& qname,
     }
     else {
       ++g_stats.nonCompliantResponses;
+      if (remote) {
+        ++remote->nonCompliantResponses;
+      }
       return false;
     }
   }
@@ -295,10 +301,13 @@ bool responseContentMatches(const PacketBuffer& response, const DNSName& qname,
     rqname = DNSName(reinterpret_cast<const char*>(response.data()), response.size(), sizeof(dnsheader), false, &rqtype, &rqclass, &qnameWireLength);
   }
   catch (const std::exception& e) {
-    if(response.size() > 0 && static_cast<size_t>(response.size()) > sizeof(dnsheader)) {
-      infolog("Backend %s sent us a response with id %d that did not parse: %s", remote.toStringWithPort(), ntohs(dh->id), e.what());
+    if (remote && response.size() > 0 && static_cast<size_t>(response.size()) > sizeof(dnsheader)) {
+      infolog("Backend %s sent us a response with id %d that did not parse: %s", remote->d_config.remote.toStringWithPort(), ntohs(dh->id), e.what());
     }
     ++g_stats.nonCompliantResponses;
+    if (remote) {
+      ++remote->nonCompliantResponses;
+    }
     return false;
   }
 
@@ -647,7 +656,7 @@ void responderThread(std::shared_ptr<DownstreamState> dss)
         int origFD = ids->origFD;
 
         unsigned int qnameWireLength = 0;
-        if (fd != ids->backendFD || !responseContentMatches(response, ids->qname, ids->qtype, ids->qclass, dss->d_config.remote, qnameWireLength)) {
+        if (fd != ids->backendFD || !responseContentMatches(response, ids->qname, ids->qtype, ids->qclass, dss, qnameWireLength)) {
           continue;
         }
 

diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh
index f0b178e551550d8b710adc22534955a794f31533..f671ed49b33949ed58306857509d7f9f06112d89 100644 (file)
--- a/pdns/dnsdist.hh
+++ b/pdns/dnsdist.hh
@@ -768,6 +768,7 @@ struct DownstreamState: public std::enable_shared_from_this<DownstreamState>
   stat_t reuseds{0};
   stat_t queries{0};
   stat_t responses{0};
+  stat_t nonCompliantResponses{0};
   struct {
     stat_t sendErrors{0};
     stat_t reuseds{0};
@@ -1101,7 +1102,7 @@ void setLuaSideEffect();   // set to report a side effect, cancelling all _no_ s
 bool getLuaNoSideEffect(); // set if there were only explicit declarations of _no_ side effect
 void resetLuaSideEffect(); // reset to indeterminate state
 
-bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const ComboAddress& remote, unsigned int& qnameWireLength);
+bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const std::shared_ptr<DownstreamState>& remote, unsigned int& qnameWireLength);
 bool processResponse(PacketBuffer& response, LocalStateHolder<vector<DNSDistResponseRuleAction> >& localRespRuleActions, DNSResponse& dr, bool muted, bool receivedOverUDP);
 bool processRulesResult(const DNSAction::Action& action, DNSQuestion& dq, std::string& ruleresult, bool& drop);
 

diff --git a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc
index 6b73ef9fb16394b4a36fc1118d61a2394b4150c3..ca8ed42d94457844189ed8ea00f20549aed183d2 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdisttcp_cc.cc
+++ b/pdns/dnsdistdist/test-dnsdisttcp_cc.cc
@@ -73,14 +73,8 @@ ProcessQueryResult processQuery(DNSQuestion& dq, ClientState& cs, LocalHolders&
   return ProcessQueryResult::Drop;
 }
 
-static std::function<bool(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const ComboAddress& remote, unsigned int& qnameWireLength)> s_responseContentMatches;
-
-bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const ComboAddress& remote, unsigned int& qnameWireLength)
+bool responseContentMatches(const PacketBuffer& response, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const std::shared_ptr<DownstreamState>& remote, unsigned int& qnameWireLength)
 {
-  if (s_responseContentMatches) {
-    return s_responseContentMatches(response, qname, qtype, qclass, remote, qnameWireLength);
-  }
-
   return true;
 }
 

diff --git a/regression-tests.dnsdist/test_API.py b/regression-tests.dnsdist/test_API.py
index 7aa0699362a5cdd3d9e693d745ab3f58ffd97a3c..784cdeaa144d12da5944976636edbb350c2742d6 100644 (file)
--- a/regression-tests.dnsdist/test_API.py
+++ b/regression-tests.dnsdist/test_API.py
@@ -129,14 +129,14 @@ class TestAPIBasics(APITestsBase):
         for server in content['servers']:
             for key in ['id', 'latency', 'name', 'weight', 'outstanding', 'qpsLimit',
                         'reuseds', 'state', 'address', 'pools', 'qps', 'queries', 'order', 'sendErrors',
-                        'dropRate', 'responses', 'tcpDiedSendingQuery', 'tcpDiedReadingResponse',
+                        'dropRate', 'responses', 'nonCompliantResponses', 'tcpDiedSendingQuery', 'tcpDiedReadingResponse',
                         'tcpGaveUp', 'tcpReadTimeouts', 'tcpWriteTimeouts', 'tcpCurrentConnections',
                         'tcpNewConnections', 'tcpReusedConnections', 'tlsResumptions', 'tcpAvgQueriesPerConnection',
                         'tcpAvgConnectionDuration', 'tcpLatency', 'protocol']:
                 self.assertIn(key, server)
 
             for key in ['id', 'latency', 'weight', 'outstanding', 'qpsLimit', 'reuseds',
-                        'qps', 'queries', 'order', 'tcpLatency']:
+                        'qps', 'queries', 'order', 'tcpLatency', 'responses', 'nonCompliantResponses']:
                 self.assertTrue(server[key] >= 0)
 
             self.assertTrue(server['state'] in ['up', 'down', 'UP', 'DOWN'])
@@ -186,14 +186,14 @@ class TestAPIBasics(APITestsBase):
         for server in content['servers']:
             for key in ['id', 'latency', 'name', 'weight', 'outstanding', 'qpsLimit',
                         'reuseds', 'state', 'address', 'pools', 'qps', 'queries', 'order', 'sendErrors',
-                        'dropRate', 'responses', 'tcpDiedSendingQuery', 'tcpDiedReadingResponse',
+                        'dropRate', 'responses', 'nonCompliantResponses', 'tcpDiedSendingQuery', 'tcpDiedReadingResponse',
                         'tcpGaveUp', 'tcpReadTimeouts', 'tcpWriteTimeouts', 'tcpCurrentConnections',
                         'tcpNewConnections', 'tcpReusedConnections', 'tcpAvgQueriesPerConnection',
                         'tcpAvgConnectionDuration', 'tcpLatency', 'protocol']:
                 self.assertIn(key, server)
 
             for key in ['id', 'latency', 'weight', 'outstanding', 'qpsLimit', 'reuseds',
-                        'qps', 'queries', 'order', 'tcpLatency']:
+                        'qps', 'queries', 'order', 'tcpLatency', 'responses', 'nonCompliantResponses']:
                 self.assertTrue(server[key] >= 0)
 
             self.assertTrue(server['state'] in ['up', 'down', 'UP', 'DOWN'])