Fix #4395, fix out of order messages and fix forward perspective. (#4396)

Signed-off-by: Frank Riley <fhriley@gmail.com>

plugin/dnstap/handler.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/coredns/coredns/plugin"
+	"github.com/coredns/coredns/plugin/dnstap/msg"
 
 	tap "github.com/dnstap/golang-dnstap"
 	"github.com/miekg/dns"
@@ -25,6 +26,19 @@ func (h Dnstap) TapMessage(m *tap.Message) {
 	h.io.Dnstap(tap.Dnstap{Type: &t, Message: m})
 }
 
+func (h Dnstap) tapQuery(w dns.ResponseWriter, query *dns.Msg, queryTime time.Time) {
+	q := new(tap.Message)
+	msg.SetQueryTime(q, queryTime)
+	msg.SetQueryAddress(q, w.RemoteAddr())
+
+	if h.IncludeRawMessage {
+		buf, _ := query.Pack()
+		q.QueryMessage = buf
+	}
+	msg.SetType(q, tap.Message_CLIENT_QUERY)
+	h.TapMessage(q)
+}
+
 // ServeDNS logs the client query and response to dnstap and passes the dnstap Context.
 func (h Dnstap) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
 	rw := &ResponseWriter{
@@ -34,6 +48,10 @@ func (h Dnstap) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 		queryTime:      time.Now(),
 	}
 
+	// The query tap message should be sent before sending the query to the
+	// forwarder. Otherwise, the tap messages will come out out of order.
+	h.tapQuery(w, r, rw.queryTime)
+
 	return plugin.NextOrFailure(h.Name(), h.Next, ctx, rw, r)
 }
 

plugin/dnstap/writer.go

@@ -19,18 +19,6 @@ type ResponseWriter struct {
 // WriteMsg writes back the response to the client and THEN works on logging the request and response to dnstap.
 func (w *ResponseWriter) WriteMsg(resp *dns.Msg) error {
 	err := w.ResponseWriter.WriteMsg(resp)
-
-	q := new(tap.Message)
-	msg.SetQueryTime(q, w.queryTime)
-	msg.SetQueryAddress(q, w.RemoteAddr())
-
-	if w.IncludeRawMessage {
-		buf, _ := w.query.Pack()
-		q.QueryMessage = buf
-	}
-	msg.SetType(q, tap.Message_CLIENT_QUERY)
-	w.TapMessage(q)
-
 	if err != nil {
 		return err
 	}

plugin/forward/dnstap.go

@@ -34,7 +34,10 @@ func toDnstap(f *Forward, host string, state request.Request, opts options, repl
 		ta = &net.TCPAddr{IP: ip, Port: int(port)}
 	}
 
-	msg.SetQueryAddress(q, ta)
+	// Forwarder dnstap messages are from the perspective of the downstream server
+	// (upstream is the forward server)
+	msg.SetQueryAddress(q, state.W.RemoteAddr())
+	msg.SetResponseAddress(q, ta)
 
 	if f.tapPlugin.IncludeRawMessage {
 		buf, _ := state.Req.Pack()
@@ -51,7 +54,8 @@ func toDnstap(f *Forward, host string, state request.Request, opts options, repl
 			r.ResponseMessage = buf
 		}
 		msg.SetQueryTime(r, start)
-		msg.SetQueryAddress(r, ta)
+		msg.SetQueryAddress(r, state.W.RemoteAddr())
+		msg.SetResponseAddress(r, ta)
 		msg.SetResponseTime(r, time.Now())
 		msg.SetType(r, tap.Message_FORWARDER_RESPONSE)
 		f.tapPlugin.TapMessage(r)