trace plugin can mark traces with error tag (#4720)

Signed-off-by: Ondrej Benkovsky <ondrej.benkovsky@wandera.com>

2 files changed, 21 insertions, 1 deletions

diff --git a/plugin/trace/trace.go b/plugin/trace/trace.go
index c1b0fb4cf..4d3deb207 100644
--- a/plugin/trace/trace.go
+++ b/plugin/trace/trace.go
@@ -16,6 +16,8 @@ import (
 
 	"github.com/miekg/dns"
 	ot "github.com/opentracing/opentracing-go"
+	otext "github.com/opentracing/opentracing-go/ext"
+	otlog "github.com/opentracing/opentracing-go/log"
 	zipkinot "github.com/openzipkin-contrib/zipkin-go-opentracing"
 	"github.com/openzipkin/zipkin-go"
 	zipkinhttp "github.com/openzipkin/zipkin-go/reporter/http"
@@ -148,6 +150,10 @@ func (t *trace) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	span.SetTag(t.tagSet.Proto, req.Proto())
 	span.SetTag(t.tagSet.Remote, req.IP())
 	span.SetTag(t.tagSet.Rcode, rcode.ToString(rw.Rcode))
+	if err != nil {
+		otext.Error.Set(span, true)
+		span.LogFields(otlog.Event("error"), otlog.Error(err))
+	}
 
 	return status, err
 }
diff --git a/plugin/trace/trace_test.go b/plugin/trace/trace_test.go
index 832447138..28c2e10bd 100644
--- a/plugin/trace/trace_test.go
+++ b/plugin/trace/trace_test.go
@@ -2,6 +2,7 @@ package trace
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	"github.com/coredns/caddy"
@@ -44,6 +45,7 @@ func TestTrace(t *testing.T) {
 		rcode    int
 		question *dns.Msg
 		server   string
+		err    error
 	}{
 		{
 			name:     "NXDOMAIN",
@@ -55,6 +57,12 @@ func TestTrace(t *testing.T) {
 			rcode:    dns.RcodeSuccess,
 			question: new(dns.Msg).SetQuestion("example.net.", dns.TypeCNAME),
 		},
+		{
+			name:     "SERVFAIL",
+			rcode:    dns.RcodeServerFailure,
+			question: new(dns.Msg).SetQuestion("example.net.", dns.TypeA),
+			err:      errors.New("test error"),
+		},
 	}
 	defaultTagSet := tagByProvider["default"]
 	for _, tc := range cases {
@@ -66,6 +74,9 @@ func TestTrace(t *testing.T) {
 					m := new(dns.Msg)
 					m.SetRcode(r, tc.rcode)
 					w.WriteMsg(m)
+					if tc.err != nil {
+						return tc.rcode, tc.err
+					}
 					return tc.rcode, nil
 				}),
 				every:  1,
@@ -73,7 +84,7 @@ func TestTrace(t *testing.T) {
 				tagSet: defaultTagSet,
 			}
 			ctx := context.TODO()
-			if _, err := tr.ServeDNS(ctx, w, tc.question); err != nil {
+			if _, err := tr.ServeDNS(ctx, w, tc.question); err != nil && tc.err == nil {
 				t.Fatalf("Error during tr.ServeDNS(ctx, w, %v): %v", tc.question, err)
 			}
 
@@ -104,6 +115,9 @@ func TestTrace(t *testing.T) {
 			if rootSpan.Tag(defaultTagSet.Rcode) != rcode.ToString(tc.rcode) {
 				t.Errorf("Unexpected span tag: rootSpan.Tag(%v): want %v, got %v", defaultTagSet.Rcode, rcode.ToString(tc.rcode), rootSpan.Tag(defaultTagSet.Rcode))
 			}
+			if tc.err != nil && rootSpan.Tag("error") != true {
+				t.Errorf("Unexpected span tag: rootSpan.Tag(%v): want %v, got %v", "error", true, rootSpan.Tag("error"))
+			}
 		})
 	}
 }