Don't double report metrics on error (coredns#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: coredns#2717 Signed-off-by: Miek Gieben <miek@miek.nl> * Compile again Signed-off-by: Miek Gieben <miek@miek.nl>
horahoradev · Mar 25, 2019 · 93f6350 · 93f6350
1 parent 9a8c301
commit 93f6350
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 8 deletions.
diff --git 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
@@ -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()