plugin/dnstap: some cleanup (#1172)

Some cleanup in proxy and dnstap:
* just use time pkg directly and side step the indirection for Epoch
* Use Set in SetQueryEpoch to be more Go like. (Looked like a reader)
* Don't maintain two sets of time, we already track start, so use that.
* Use time.Time and convert when needed
* dedent the toDnstap function and put in a separate file

6 files changed, 64 insertions, 61 deletions

diff --git a/plugin/dnstap/handler.go b/plugin/dnstap/handler.go
index 63838492c..798da6014 100644
--- a/plugin/dnstap/handler.go
+++ b/plugin/dnstap/handler.go
@@ -79,7 +79,7 @@ func (h Dnstap) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	newCtx := context.WithValue(ctx, DnstapSendOption, &sendOption)
 
 	rw := &taprw.ResponseWriter{ResponseWriter: w, Tapper: &h, Query: r, Send: &sendOption}
-	rw.QueryEpoch()
+	rw.SetQueryEpoch()
 
 	code, err := plugin.NextOrFailure(h.Name(), h.Next, tapContext{newCtx, h}, rw, r)
 	if err != nil {
diff --git a/plugin/dnstap/msg/msg.go b/plugin/dnstap/msg/msg.go
index 07930929e..95442301e 100644
--- a/plugin/dnstap/msg/msg.go
+++ b/plugin/dnstap/msg/msg.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"net"
 	"strconv"
-	"time"
 
 	tap "github.com/dnstap/golang-dnstap"
 	"github.com/miekg/dns"
@@ -103,16 +102,6 @@ func (d *Data) Pack(m *dns.Msg) error {
 	return nil
 }
 
-// Epoch returns the epoch time in seconds.
-func Epoch() uint64 {
-	return uint64(time.Now().Unix())
-}
-
-// Epoch sets the dnstap message epoch.
-func (d *Data) Epoch() {
-	d.TimeSec = Epoch()
-}
-
 // ToClientResponse transforms Data into a client response message.
 func (d *Data) ToClientResponse() *tap.Message {
 	t := tap.Message_CLIENT_RESPONSE
diff --git a/plugin/dnstap/taprw/writer.go b/plugin/dnstap/taprw/writer.go
index 14a899443..b0b505ce5 100644
--- a/plugin/dnstap/taprw/writer.go
+++ b/plugin/dnstap/taprw/writer.go
@@ -4,6 +4,7 @@ package taprw
 
 import (
 	"fmt"
+	"time"
 
 	"github.com/coredns/coredns/plugin/dnstap/msg"
 
@@ -41,9 +42,9 @@ func (w ResponseWriter) DnstapError() error {
 	return w.err
 }
 
-// QueryEpoch sets the query epoch as reported by dnstap.
-func (w *ResponseWriter) QueryEpoch() {
-	w.queryEpoch = msg.Epoch()
+// SetQueryEpoch sets the query epoch as reported by dnstap.
+func (w *ResponseWriter) SetQueryEpoch() {
+	w.queryEpoch = uint64(time.Now().Unix())
 }
 
 // WriteMsg writes back the response to the client and THEN works on logging the request
@@ -51,7 +52,7 @@ func (w *ResponseWriter) QueryEpoch() {
 // Dnstap errors are to be checked by DnstapError.
 func (w *ResponseWriter) WriteMsg(resp *dns.Msg) (writeErr error) {
 	writeErr = w.ResponseWriter.WriteMsg(resp)
-	writeEpoch := msg.Epoch()
+	writeEpoch := uint64(time.Now().Unix())
 
 	b := w.TapBuilder()
 	b.TimeSec = w.queryEpoch
diff --git a/plugin/proxy/dnstap.go b/plugin/proxy/dnstap.go
new file mode 100644
index 000000000..430b43847
--- /dev/null
+++ b/plugin/proxy/dnstap.go
@@ -0,0 +1,54 @@
+package proxy
+
+import (
+	"time"
+
+	"github.com/coredns/coredns/plugin/dnstap"
+	"github.com/coredns/coredns/request"
+
+	tap "github.com/dnstap/golang-dnstap"
+	"github.com/miekg/dns"
+	"golang.org/x/net/context"
+)
+
+func toDnstap(ctx context.Context, host string, ex Exchanger, state request.Request, reply *dns.Msg, start time.Time) error {
+	tapper := dnstap.TapperFromContext(ctx)
+	if tapper == nil {
+		return nil
+	}
+
+	// Query
+	b := tapper.TapBuilder()
+	b.TimeSec = uint64(start.Unix())
+	if err := b.HostPort(host); err != nil {
+		return err
+	}
+
+	t := ex.Transport()
+	if t == "" {
+		t = state.Proto()
+	}
+	if t == "tcp" {
+		b.SocketProto = tap.SocketProtocol_TCP
+	} else {
+		b.SocketProto = tap.SocketProtocol_UDP
+	}
+
+	if err := b.Msg(state.Req); err != nil {
+		return err
+	}
+
+	if err := tapper.TapMessage(b.ToOutsideQuery(tap.Message_FORWARDER_QUERY)); err != nil {
+		return err
+	}
+
+	// Response
+	if reply != nil {
+		b.TimeSec = uint64(time.Now().Unix())
+		if err := b.Msg(reply); err != nil {
+			return err
+		}
+		return tapper.TapMessage(b.ToOutsideResponse(tap.Message_FORWARDER_RESPONSE))
+	}
+	return nil
+}
diff --git a/plugin/proxy/dnstap_test.go b/plugin/proxy/dnstap_test.go
index 05169a1ca..6ed0c61ed 100644
--- a/plugin/proxy/dnstap_test.go
+++ b/plugin/proxy/dnstap_test.go
@@ -2,6 +2,7 @@ package proxy
 
 import (
 	"testing"
+	"time"
 
 	"github.com/coredns/coredns/plugin/dnstap/msg"
 	"github.com/coredns/coredns/plugin/dnstap/test"
@@ -18,7 +19,7 @@ func testCase(t *testing.T, ex Exchanger, q, r *dns.Msg, datq, datr *msg.Data) {
 	tapr := datr.ToOutsideResponse(tap.Message_FORWARDER_RESPONSE)
 	ctx := test.Context{}
 	err := toDnstap(&ctx, "10.240.0.1:40212", ex,
-		request.Request{W: &mwtest.ResponseWriter{}, Req: q}, r, 0, 0)
+		request.Request{W: &mwtest.ResponseWriter{}, Req: q}, r, time.Now())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -50,7 +51,7 @@ func TestDnstap(t *testing.T) {
 }
 
 func TestNoDnstap(t *testing.T) {
-	err := toDnstap(context.TODO(), "", nil, request.Request{}, nil, 0, 0)
+	err := toDnstap(context.TODO(), "", nil, request.Request{}, nil, time.Now())
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/plugin/proxy/proxy.go b/plugin/proxy/proxy.go
index 7b2abd89e..476eddaff 100644
--- a/plugin/proxy/proxy.go
+++ b/plugin/proxy/proxy.go
@@ -9,12 +9,9 @@ import (
 	"time"
 
 	"github.com/coredns/coredns/plugin"
-	"github.com/coredns/coredns/plugin/dnstap"
-	"github.com/coredns/coredns/plugin/dnstap/msg"
 	"github.com/coredns/coredns/plugin/pkg/healthcheck"
 	"github.com/coredns/coredns/request"
 
-	tap "github.com/dnstap/golang-dnstap"
 	"github.com/miekg/dns"
 	ot "github.com/opentracing/opentracing-go"
 	"golang.org/x/net/context"
@@ -89,20 +86,18 @@ func (p Proxy) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
 			}
 
 			atomic.AddInt64(&host.Conns, 1)
-			queryEpoch := msg.Epoch()
 
 			RequestCount.WithLabelValues(state.Proto(), upstream.Exchanger().Protocol(), familyToString(state.Family()), host.Name).Add(1)
 
 			reply, backendErr = upstream.Exchanger().Exchange(ctx, host.Name, state)
 
-			respEpoch := msg.Epoch()
 			atomic.AddInt64(&host.Conns, -1)
 
 			if child != nil {
 				child.Finish()
 			}
 
-			taperr := toDnstap(ctx, host.Name, upstream.Exchanger(), state, reply, queryEpoch, respEpoch)
+			taperr := toDnstap(ctx, host.Name, upstream.Exchanger(), state, reply, start)
 
 			if backendErr == nil {
 				w.WriteMsg(reply)
@@ -172,43 +167,6 @@ func (p Proxy) match(state request.Request) (u Upstream) {
 // Name implements the Handler interface.
 func (p Proxy) Name() string { return "proxy" }
 
-func toDnstap(ctx context.Context, host string, ex Exchanger, state request.Request, reply *dns.Msg, queryEpoch, respEpoch uint64) (err error) {
-	if tapper := dnstap.TapperFromContext(ctx); tapper != nil {
-		// Query
-		b := tapper.TapBuilder()
-		b.TimeSec = queryEpoch
-		if err = b.HostPort(host); err != nil {
-			return
-		}
-		t := ex.Transport()
-		if t == "" {
-			t = state.Proto()
-		}
-		if t == "tcp" {
-			b.SocketProto = tap.SocketProtocol_TCP
-		} else {
-			b.SocketProto = tap.SocketProtocol_UDP
-		}
-		if err = b.Msg(state.Req); err != nil {
-			return
-		}
-		err = tapper.TapMessage(b.ToOutsideQuery(tap.Message_FORWARDER_QUERY))
-		if err != nil {
-			return
-		}
-
-		// Response
-		if reply != nil {
-			b.TimeSec = respEpoch
-			if err = b.Msg(reply); err != nil {
-				return
-			}
-			err = tapper.TapMessage(b.ToOutsideResponse(tap.Message_FORWARDER_RESPONSE))
-		}
-	}
-	return
-}
-
 const (
 	defaultFailTimeout = 2 * time.Second
 	defaultTimeout     = 5 * time.Second