From 93f635023a7515087bb92d542f059d29a4d7c711 Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Mon, 25 Mar 2019 19:04:03 +0000
Subject: [PATCH] Don't double report metrics on error (#2719)

* Don't double report metrics on error

When there is an error use a different function to report the metrics,
in case the plugin chain handled the request the metrics are already
reported.

Fixes: #2717

Signed-off-by: Miek Gieben <miek@miek.nl>

* Compile again

Signed-off-by: Miek Gieben <miek@miek.nl>
---
 core/dnsserver/server.go      | 22 +++++++++++++++-------
 plugin/metrics/vars/report.go |  4 +++-
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go
index 1026ae49b..e656cb17e 100644
--- a/core/dnsserver/server.go
+++ b/core/dnsserver/server.go
@@ -205,7 +205,7 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	// The default dns.Mux checks the question section size, but we have our
 	// own mux here. Check if we have a question section. If not drop them here.
 	if r == nil || len(r.Question) == 0 {
-		errorFunc(s.Addr, w, r, dns.RcodeServerFailure)
+		errorAndMetricsFunc(s.Addr, w, r, dns.RcodeServerFailure)
 		return
 	}
 
@@ -215,13 +215,13 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 			// need to make sure that we stay alive up here
 			if rec := recover(); rec != nil {
 				vars.Panic.Inc()
-				errorFunc(s.Addr, w, r, dns.RcodeServerFailure)
+				errorAndMetricsFunc(s.Addr, w, r, dns.RcodeServerFailure)
 			}
 		}()
 	}
 
 	if !s.classChaos && r.Question[0].Qclass != dns.ClassINET {
-		errorFunc(s.Addr, w, r, dns.RcodeRefused)
+		errorAndMetricsFunc(s.Addr, w, r, dns.RcodeRefused)
 		return
 	}
 
@@ -301,7 +301,7 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	}
 
 	// Still here? Error out with REFUSED.
-	errorFunc(s.Addr, w, r, dns.RcodeRefused)
+	errorAndMetricsFunc(s.Addr, w, r, dns.RcodeRefused)
 }
 
 // OnStartupComplete lists the sites served by this server
@@ -333,7 +333,16 @@ func errorFunc(server string, w dns.ResponseWriter, r *dns.Msg, rc int) {
 
 	answer := new(dns.Msg)
 	answer.SetRcode(r, rc)
+	state.SizeAndDo(answer)
 
+	w.WriteMsg(answer)
+}
+
+func errorAndMetricsFunc(server string, w dns.ResponseWriter, r *dns.Msg, rc int) {
+	state := request.Request{W: w, Req: r}
+
+	answer := new(dns.Msg)
+	answer.SetRcode(r, rc)
 	state.SizeAndDo(answer)
 
 	vars.Report(server, state, vars.Dropped, rcode.ToString(rc), answer.Len(), time.Now())
@@ -342,9 +351,8 @@ func errorFunc(server string, w dns.ResponseWriter, r *dns.Msg, rc int) {
 }
 
 const (
-	tcp          = 0
-	udp          = 1
-	maxreentries = 10
+	tcp = 0
+	udp = 1
 )
 
 // Key is the context key for the current server added to the context.
diff --git a/plugin/metrics/vars/report.go b/plugin/metrics/vars/report.go
index fe6a5dccd..343d98a22 100644
--- a/plugin/metrics/vars/report.go
+++ b/plugin/metrics/vars/report.go
@@ -8,7 +8,9 @@ import (
 	"github.com/miekg/dns"
 )
 
-// Report reports the metrics data associated with request.
+// Report reports the metrics data associated with request. This function is exported because it is also
+// called from core/dnsserver to report requests hitting the server that should not be handled and are thus
+// not sent down the plugin chain.
 func Report(server string, req request.Request, zone, rcode string, size int, start time.Time) {
 	// Proto and Family.
 	net := req.Proto()