From 45624a0c0a93833bc136f20f316f78dd16462e82 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Mon, 25 Mar 2019 03:36:46 +0000 Subject: [PATCH] plugin/log: remove ErrorFunc (#2716) The server handles this case no need to also do it in the log plugin. Means DefaultErrorFunc can be private to the dnsserver and is now renamed to just errorFunc Fixes: #2715 Signed-off-by: Miek Gieben --- core/dnsserver/server.go | 20 ++++++++++---------- plugin/log/log.go | 23 ++--------------------- plugin/log/log_test.go | 4 ++-- plugin/log/setup.go | 2 +- 4 files changed, 15 insertions(+), 34 deletions(-) diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go index 2fc1529d1..933fe8a4f 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 { - DefaultErrorFunc(ctx, w, r, dns.RcodeServerFailure) + errorFunc(ctx, 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() - DefaultErrorFunc(ctx, w, r, dns.RcodeServerFailure) + errorFunc(ctx, w, r, dns.RcodeServerFailure) } }() } if !s.classChaos && r.Question[0].Qclass != dns.ClassINET { - DefaultErrorFunc(ctx, w, r, dns.RcodeRefused) + errorFunc(ctx, w, r, dns.RcodeRefused) return } @@ -260,7 +260,7 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) if h.FilterFunc == nil { rcode, _ := h.pluginChain.ServeDNS(ctx, w, r) if !plugin.ClientWrite(rcode) { - DefaultErrorFunc(ctx, w, r, rcode) + errorFunc(ctx, w, r, rcode) } return } @@ -269,7 +269,7 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) if h.FilterFunc(q) { rcode, _ := h.pluginChain.ServeDNS(ctx, w, r) if !plugin.ClientWrite(rcode) { - DefaultErrorFunc(ctx, w, r, rcode) + errorFunc(ctx, w, r, rcode) } return } @@ -291,7 +291,7 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) // DS request, and we found a zone, use the handler for the query. rcode, _ := dshandler.pluginChain.ServeDNS(ctx, w, r) if !plugin.ClientWrite(rcode) { - DefaultErrorFunc(ctx, w, r, rcode) + errorFunc(ctx, w, r, rcode) } return } @@ -304,13 +304,13 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) rcode, _ := h.pluginChain.ServeDNS(ctx, w, r) if !plugin.ClientWrite(rcode) { - DefaultErrorFunc(ctx, w, r, rcode) + errorFunc(ctx, w, r, rcode) } return } // Still here? Error out with REFUSED. - DefaultErrorFunc(ctx, w, r, dns.RcodeRefused) + errorFunc(ctx, w, r, dns.RcodeRefused) } // OnStartupComplete lists the sites served by this server @@ -336,8 +336,8 @@ func (s *Server) Tracer() ot.Tracer { return s.trace.Tracer() } -// DefaultErrorFunc responds to an DNS request with an error. -func DefaultErrorFunc(ctx context.Context, w dns.ResponseWriter, r *dns.Msg, rc int) { +// errorFunc responds to an DNS request with an error. +func errorFunc(ctx context.Context, w dns.ResponseWriter, r *dns.Msg, rc int) { state := request.Request{W: w, Req: r} answer := new(dns.Msg) diff --git a/plugin/log/log.go b/plugin/log/log.go index 2489e03d1..49581dfc4 100644 --- a/plugin/log/log.go +++ b/plugin/log/log.go @@ -6,10 +6,8 @@ import ( "time" "github.com/coredns/coredns/plugin" - "github.com/coredns/coredns/plugin/metrics/vars" "github.com/coredns/coredns/plugin/pkg/dnstest" clog "github.com/coredns/coredns/plugin/pkg/log" - "github.com/coredns/coredns/plugin/pkg/rcode" "github.com/coredns/coredns/plugin/pkg/replacer" "github.com/coredns/coredns/plugin/pkg/response" "github.com/coredns/coredns/request" @@ -19,9 +17,8 @@ import ( // Logger is a basic request logging plugin. type Logger struct { - Next plugin.Handler - Rules []Rule - ErrorFunc func(context.Context, dns.ResponseWriter, *dns.Msg, int) // failover error handler + Next plugin.Handler + Rules []Rule repl replacer.Replacer } @@ -37,22 +34,6 @@ func (l Logger) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) rrw := dnstest.NewRecorder(w) rc, err := plugin.NextOrFailure(l.Name(), l.Next, ctx, rrw, r) - if rc > 0 { - // There was an error up the chain, but no response has been written yet. - // The error must be handled here so the log entry will record the response size. - if l.ErrorFunc != nil { - l.ErrorFunc(ctx, rrw, r, rc) - } else { - answer := new(dns.Msg) - answer.SetRcode(r, rc) - - vars.Report(ctx, state, vars.Dropped, rcode.ToString(rc), answer.Len(), time.Now()) - - w.WriteMsg(answer) - } - rc = 0 - } - tpe, _ := response.Typify(rrw.Msg, time.Now().UTC()) class := response.Classify(tpe) // If we don't set up a class in config, the default "all" will be added diff --git a/plugin/log/log_test.go b/plugin/log/log_test.go index 97e2b8c71..e7f29fff1 100644 --- a/plugin/log/log_test.go +++ b/plugin/log/log_test.go @@ -41,8 +41,8 @@ func TestLoggedStatus(t *testing.T) { rec := dnstest.NewRecorder(&test.ResponseWriter{}) rcode, _ := logger.ServeDNS(ctx, rec, r) - if rcode != 0 { - t.Errorf("Expected rcode to be 0 - was: %d", rcode) + if rcode != 2 { + t.Errorf("Expected rcode to be 2 - was: %d", rcode) } logged := f.String() diff --git a/plugin/log/setup.go b/plugin/log/setup.go index 81a2004f2..b9ecb1f72 100644 --- a/plugin/log/setup.go +++ b/plugin/log/setup.go @@ -26,7 +26,7 @@ func setup(c *caddy.Controller) error { } dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler { - return Logger{Next: next, Rules: rules, ErrorFunc: dnsserver.DefaultErrorFunc, repl: replacer.New()} + return Logger{Next: next, Rules: rules, repl: replacer.New()} }) return nil