Easier way to dnstap? (#1496)

* Easier way to dnstap?

* Remove unnecessary function parameter from Tapper

* golint

* golint 2

* Proxy dnstap tests

* README.md & doc

* net.IP

* Proxy test was incorrect

* Small changes

* Update README.md

* Was not reporting dnstap errors + test

* Wasn't working at all, now it's ok

* Thanks Travis

plugin/dnstap/taprw/writer.go

@@ -21,67 +21,59 @@ type SendOption struct {
 
 // Tapper is what ResponseWriter needs to log to dnstap.
 type Tapper interface {
-	TapMessage(m *tap.Message) error
-	TapBuilder() msg.Builder
+	TapMessage(*tap.Message)
+	Pack() bool
 }
 
 // ResponseWriter captures the client response and logs the query to dnstap.
 // Single request use.
 // SendOption configures Dnstap to selectively send Dnstap messages. Default is send all.
 type ResponseWriter struct {
-	queryEpoch uint64
+	QueryEpoch time.Time
 	Query      *dns.Msg
 	dns.ResponseWriter
 	Tapper
-	err  error
 	Send *SendOption
+
+	dnstapErr error
 }
 
 // DnstapError check if a dnstap error occurred during Write and returns it.
-func (w ResponseWriter) DnstapError() error {
-	return w.err
-}
-
-// SetQueryEpoch sets the query epoch as reported by dnstap.
-func (w *ResponseWriter) SetQueryEpoch() {
-	w.queryEpoch = uint64(time.Now().Unix())
+func (w *ResponseWriter) DnstapError() error {
+	return w.dnstapErr
 }
 
 // WriteMsg writes back the response to the client and THEN works on logging the request
 // and response to dnstap.
-// Dnstap errors are to be checked by DnstapError.
 func (w *ResponseWriter) WriteMsg(resp *dns.Msg) (writeErr error) {
 	writeErr = w.ResponseWriter.WriteMsg(resp)
-	writeEpoch := uint64(time.Now().Unix())
+	writeEpoch := time.Now()
 
-	b := w.TapBuilder()
-	b.TimeSec = w.queryEpoch
+	b := msg.New().Time(w.QueryEpoch).Addr(w.RemoteAddr())
 
 	if w.Send == nil || w.Send.Cq {
-		if err := func() (err error) {
-			err = b.AddrMsg(w.ResponseWriter.RemoteAddr(), w.Query)
-			if err != nil {
-				return
-			}
-			return w.TapMessage(b.ToClientQuery())
-		}(); err != nil {
-			w.err = fmt.Errorf("client query: %s", err)
-			// don't forget to call DnstapError later
+		if w.Pack() {
+			b.Msg(w.Query)
+		}
+		if m, err := b.ToClientQuery(); err != nil {
+			w.dnstapErr = fmt.Errorf("client query: %s", err)
+		} else {
+			w.TapMessage(m)
 		}
 	}
 
 	if w.Send == nil || w.Send.Cr {
 		if writeErr == nil {
-			if err := func() (err error) {
-				b.TimeSec = writeEpoch
-				if err = b.Msg(resp); err != nil {
-					return
-				}
-				return w.TapMessage(b.ToClientResponse())
-			}(); err != nil {
-				w.err = fmt.Errorf("client response: %s", err)
+			if w.Pack() {
+				b.Msg(resp)
+			}
+			if m, err := b.Time(writeEpoch).ToClientResponse(); err != nil {
+				w.dnstapErr = fmt.Errorf("client response: %s", err)
+			} else {
+				w.TapMessage(m)
 			}
 		}
 	}
-	return
+
+	return writeErr
 }

plugin/dnstap/taprw/writer_test.go

@@ -1,41 +1,14 @@
 package taprw
 
 import (
-	"errors"
 	"testing"
 
-	"github.com/coredns/coredns/plugin/dnstap/msg"
 	"github.com/coredns/coredns/plugin/dnstap/test"
 	mwtest "github.com/coredns/coredns/plugin/test"
 
-	tap "github.com/dnstap/golang-dnstap"
 	"github.com/miekg/dns"
 )
 
-type TapFailer struct {
-}
-
-func (TapFailer) TapMessage(*tap.Message) error {
-	return errors.New("failed")
-}
-func (TapFailer) TapBuilder() msg.Builder {
-	return msg.Builder{Full: true}
-}
-
-func TestDnstapError(t *testing.T) {
-	rw := ResponseWriter{
-		Query:          new(dns.Msg),
-		ResponseWriter: &mwtest.ResponseWriter{},
-		Tapper:         TapFailer{},
-	}
-	if err := rw.WriteMsg(new(dns.Msg)); err != nil {
-		t.Errorf("dnstap error during Write: %s", err)
-	}
-	if rw.DnstapError() == nil {
-		t.Fatal("no dnstap error")
-	}
-}
-
 func testingMsg() (m *dns.Msg) {
 	m = new(dns.Msg)
 	m.SetQuestion("example.com.", dns.TypeA)
@@ -69,12 +42,18 @@ func TestClientQueryResponse(t *testing.T) {
 		t.Fatalf("%d msg trapped", l)
 		return
 	}
-	want := d.ToClientQuery()
+	want, err := d.ToClientQuery()
+	if err != nil {
+		t.Fatal("Testing data must build", err)
+	}
 	have := trapper.Trap[0]
 	if !test.MsgEqual(want, have) {
 		t.Fatalf("query: want: %v\nhave: %v", want, have)
 	}
-	want = d.ToClientResponse()
+	want, err = d.ToClientResponse()
+	if err != nil {
+		t.Fatal("Testing data must build", err)
+	}
 	have = trapper.Trap[1]
 	if !test.MsgEqual(want, have) {
 		t.Fatalf("response: want: %v\nhave: %v", want, have)