Remove server addr from the context (#2722)

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

* Remove server addr from the context

This was added twice, just leave the server which also holds the
address.

Conflicts with #2719 but should be easy to fix.
Signed-off-by: Miek Gieben <miek@miek.nl>

* doesn't need server context
Signed-off-by: Miek Gieben <miek@miek.nl>

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(ctx, w, r, dns.RcodeServerFailure)
+		errorFunc(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(ctx, w, r, dns.RcodeServerFailure)
+				errorFunc(s.Addr, w, r, dns.RcodeServerFailure)
 			}
 		}()
 	}
 
 	if !s.classChaos && r.Question[0].Qclass != dns.ClassINET {
-		errorFunc(ctx, w, r, dns.RcodeRefused)
+		errorFunc(s.Addr, w, r, dns.RcodeRefused)
 		return
 	}
 
@@ -251,16 +251,11 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 		}
 
 		if h, ok := s.zones[string(b[:l])]; ok {
-
-			// Set server's address in the context so plugins can reference back to this,
-			// This will makes those metrics unique.
-			ctx = context.WithValue(ctx, plugin.ServerCtx{}, s.Addr)
-
 			if r.Question[0].Qtype != dns.TypeDS {
 				if h.FilterFunc == nil {
 					rcode, _ := h.pluginChain.ServeDNS(ctx, w, r)
 					if !plugin.ClientWrite(rcode) {
-						errorFunc(ctx, w, r, rcode)
+						errorFunc(s.Addr, w, r, rcode)
 					}
 					return
 				}
@@ -269,7 +264,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) {
-						errorFunc(ctx, w, r, rcode)
+						errorFunc(s.Addr, w, r, rcode)
 					}
 					return
 				}
@@ -291,26 +286,22 @@ 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) {
-			errorFunc(ctx, w, r, rcode)
+			errorFunc(s.Addr, w, r, rcode)
 		}
 		return
 	}
 
 	// Wildcard match, if we have found nothing try the root zone as a last resort.
 	if h, ok := s.zones["."]; ok && h.pluginChain != nil {
-
-		// See comment above.
-		ctx = context.WithValue(ctx, plugin.ServerCtx{}, s.Addr)
-
 		rcode, _ := h.pluginChain.ServeDNS(ctx, w, r)
 		if !plugin.ClientWrite(rcode) {
-			errorFunc(ctx, w, r, rcode)
+			errorFunc(s.Addr, w, r, rcode)
 		}
 		return
 	}
 
 	// Still here? Error out with REFUSED.
-	errorFunc(ctx, w, r, dns.RcodeRefused)
+	errorFunc(s.Addr, w, r, dns.RcodeRefused)
 }
 
 // OnStartupComplete lists the sites served by this server
@@ -337,7 +328,7 @@ func (s *Server) Tracer() ot.Tracer {
 }
 
 // errorFunc responds to an DNS request with an error.
-func errorFunc(ctx context.Context, w dns.ResponseWriter, r *dns.Msg, rc int) {
+func errorFunc(server string, w dns.ResponseWriter, r *dns.Msg, rc int) {
 	state := request.Request{W: w, Req: r}
 
 	answer := new(dns.Msg)
@@ -345,7 +336,7 @@ func errorFunc(ctx context.Context, w dns.ResponseWriter, r *dns.Msg, rc int) {
 
 	state.SizeAndDo(answer)
 
-	vars.Report(ctx, state, vars.Dropped, rcode.ToString(rc), answer.Len(), time.Now())
+	vars.Report(server, state, vars.Dropped, rcode.ToString(rc), answer.Len(), time.Now())
 
 	w.WriteMsg(answer)
 }
@@ -356,7 +347,7 @@ const (
 	maxreentries = 10
 )
 
-// Key is the context key for the current server
+// Key is the context key for the current server added to the context.
 type Key struct{}
 
 // EnableChaos is a map with plugin names for which we should open CH class queries as we block these by default.

plugin/metrics/context.go

@@ -3,7 +3,7 @@ package metrics
 import (
 	"context"
 
-	"github.com/coredns/coredns/plugin/metrics/vars"
+	"github.com/coredns/coredns/core/dnsserver"
 )
 
 // WithServer returns the current server handling the request. It returns the
@@ -15,4 +15,10 @@ import (
 // Basic usage with a metric:
 //
 // <metric>.WithLabelValues(metrics.WithServer(ctx), labels..).Add(1)
-func WithServer(ctx context.Context) string { return vars.WithServer(ctx) }
+func WithServer(ctx context.Context) string {
+	srv := ctx.Value(dnsserver.Key{})
+	if srv == nil {
+		return ""
+	}
+	return srv.(*dnsserver.Server).Addr
+}

plugin/metrics/handler.go

@@ -26,7 +26,7 @@ func (m *Metrics) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
 	rw := dnstest.NewRecorder(w)
 	status, err := plugin.NextOrFailure(m.Name(), m.Next, ctx, rw, r)
 
-	vars.Report(ctx, state, zone, rcode.ToString(rw.Rcode), rw.Len, rw.Start)
+	vars.Report(WithServer(ctx), state, zone, rcode.ToString(rw.Rcode), rw.Len, rw.Start)
 
 	return status, err
 }

plugin/metrics/vars/report.go

 package vars
 
 import (
-	"context"
 	"time"
 
-	"github.com/coredns/coredns/plugin"
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
 )
 
 // Report reports the metrics data associated with request.
-func Report(ctx context.Context, req request.Request, zone, rcode string, size int, start time.Time) {
+func Report(server string, req request.Request, zone, rcode string, size int, start time.Time) {
 	// Proto and Family.
 	net := req.Proto()
 	fam := "1"
@@ -19,8 +17,6 @@ func Report(ctx context.Context, req request.Request, zone, rcode string, size i
 		fam = "2"
 	}
 
-	server := WithServer(ctx)
-
 	typ := req.QType()
 	RequestCount.WithLabelValues(server, zone, net, fam).Inc()
 	RequestDuration.WithLabelValues(server, zone).Observe(time.Since(start).Seconds())
@@ -41,15 +37,6 @@ func Report(ctx context.Context, req request.Request, zone, rcode string, size i
 	ResponseRcode.WithLabelValues(server, zone, rcode).Inc()
 }
 
-// WithServer returns the current server handling the request.
-func WithServer(ctx context.Context) string {
-	srv := ctx.Value(plugin.ServerCtx{})
-	if srv == nil {
-		return ""
-	}
-	return srv.(string)
-}
-
 var monitorType = map[uint16]struct{}{
 	dns.TypeAAAA:   struct{}{},
 	dns.TypeA:      struct{}{},

plugin/plugin.go

@@ -106,6 +106,3 @@ var TimeBuckets = prometheus.ExponentialBuckets(0.00025, 2, 16) // from 0.25ms t
 
 // ErrOnce is returned when a plugin doesn't support multiple setups per server.
 var ErrOnce = errors.New("this plugin can only be used once per Server Block")
-
-// ServerCtx is the context key to pass server address context to the plugins handling the request.
-type ServerCtx struct{}

plugin/trace/trace_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/coredns/coredns/plugin"
 	"github.com/coredns/coredns/plugin/pkg/dnstest"
 	"github.com/coredns/coredns/plugin/pkg/rcode"
 	"github.com/coredns/coredns/plugin/test"
@@ -69,7 +68,7 @@ func TestTrace(t *testing.T) {
 				every:  1,
 				tracer: m,
 			}
-			ctx := context.WithValue(context.TODO(), plugin.ServerCtx{}, server)
+			ctx := context.TODO()
 			if _, err := tr.ServeDNS(ctx, w, tc.question); err != nil {
 				t.Fatalf("Error during tr.ServeDNS(ctx, w, %v): %v", tc.question, err)
 			}