plugin/forward Add rcode and rtype to request_duration_seconds metric (#4391)

* plugin/forward Add rcode and rtype to request_duration_seconds metric

Signed-off-by: Maxime Ginters <maxime.ginters@shopify.com>

* Control the cardinality of query type

Signed-off-by: Maxime Ginters <maxime.ginters@shopify.com>

5 files changed, 47 insertions, 36 deletions

diff --git a/plugin/forward/README.md b/plugin/forward/README.md
index 52625249c..64e561106 100644
--- a/plugin/forward/README.md
+++ b/plugin/forward/README.md
@@ -107,7 +107,7 @@ If monitoring is enabled (via the *prometheus* plugin) then the following metric
 
 * `coredns_forward_requests_total{to}` - query count per upstream.
 * `coredns_forward_responses_total{to}` - Counter of responses received per upstream.
-* `coredns_forward_request_duration_seconds{to}` - duration per upstream interaction.
+* `coredns_forward_request_duration_seconds{to, rcode, type}` - duration per upstream, RCODE, type
 * `coredns_forward_responses_total{to, rcode}` - count of RCODEs per upstream.
 * `coredns_forward_healthcheck_failures_total{to}` - number of failed health checks per upstream.
 * `coredns_forward_healthcheck_broken_total{}` - counter of when all upstreams are unhealthy,
diff --git a/plugin/forward/connect.go b/plugin/forward/connect.go
index c181c98cb..7e34837e1 100644
--- a/plugin/forward/connect.go
+++ b/plugin/forward/connect.go
@@ -11,6 +11,7 @@ import (
 	"sync/atomic"
 	"time"
 
+	"github.com/coredns/coredns/plugin/pkg/dnsutil"
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
@@ -129,9 +130,11 @@ func (p *Proxy) Connect(ctx context.Context, state request.Request, opts options
 		rc = strconv.Itoa(ret.Rcode)
 	}
 
+	qtype := dnsutil.QTypeMonitorLabel(state.QType())
+
 	RequestCount.WithLabelValues(p.addr).Add(1)
 	RcodeCount.WithLabelValues(rc, p.addr).Add(1)
-	RequestDuration.WithLabelValues(p.addr).Observe(time.Since(start).Seconds())
+	RequestDuration.WithLabelValues(p.addr, rc, qtype).Observe(time.Since(start).Seconds())
 
 	return ret, nil
 }
diff --git a/plugin/forward/metrics.go b/plugin/forward/metrics.go
index 98f2805c1..51685a599 100644
--- a/plugin/forward/metrics.go
+++ b/plugin/forward/metrics.go
@@ -27,7 +27,7 @@ var (
 		Name:      "request_duration_seconds",
 		Buckets:   plugin.TimeBuckets,
 		Help:      "Histogram of the time each request took.",
-	}, []string{"to"})
+	}, []string{"to", "rcode", "type"})
 	HealthcheckFailureCount = promauto.NewCounterVec(prometheus.CounterOpts{
 		Namespace: plugin.Namespace,
 		Subsystem: "forward",
diff --git a/plugin/metrics/vars/report.go b/plugin/metrics/vars/report.go
index 96b29d9fb..9bad43582 100644
--- a/plugin/metrics/vars/report.go
+++ b/plugin/metrics/vars/report.go
@@ -3,9 +3,8 @@ package vars
 import (
 	"time"
 
+	"github.com/coredns/coredns/plugin/pkg/dnsutil"
 	"github.com/coredns/coredns/request"
-
-	"github.com/miekg/dns"
 )
 
 // Report reports the metrics data associated with request. This function is exported because it is also
@@ -19,45 +18,17 @@ func Report(server string, req request.Request, zone, rcode string, size int, st
 		fam = "2"
 	}
 
-	typ := req.QType()
+	qtype := dnsutil.QTypeMonitorLabel(req.QType())
 
 	if req.Do() {
 		RequestDo.WithLabelValues(server, zone).Inc()
 	}
 
-	if _, known := monitorType[typ]; known {
-		RequestCount.WithLabelValues(server, zone, net, fam, dns.Type(typ).String()).Inc()
-		RequestDuration.WithLabelValues(server, zone, dns.Type(typ).String()).Observe(time.Since(start).Seconds())
-	} else {
-		RequestCount.WithLabelValues(server, zone, net, fam, other).Inc()
-		RequestDuration.WithLabelValues(server, zone, other).Observe(time.Since(start).Seconds())
-	}
+	RequestCount.WithLabelValues(server, zone, net, fam, qtype).Inc()
+	RequestDuration.WithLabelValues(server, zone, qtype).Observe(time.Since(start).Seconds())
 
 	ResponseSize.WithLabelValues(server, zone, net).Observe(float64(size))
 	RequestSize.WithLabelValues(server, zone, net).Observe(float64(req.Len()))
 
 	ResponseRcode.WithLabelValues(server, zone, rcode).Inc()
 }
-
-var monitorType = map[uint16]struct{}{
-	dns.TypeAAAA:   {},
-	dns.TypeA:      {},
-	dns.TypeCNAME:  {},
-	dns.TypeDNSKEY: {},
-	dns.TypeDS:     {},
-	dns.TypeMX:     {},
-	dns.TypeNSEC3:  {},
-	dns.TypeNSEC:   {},
-	dns.TypeNS:     {},
-	dns.TypePTR:    {},
-	dns.TypeRRSIG:  {},
-	dns.TypeSOA:    {},
-	dns.TypeSRV:    {},
-	dns.TypeTXT:    {},
-	// Meta Qtypes
-	dns.TypeIXFR: {},
-	dns.TypeAXFR: {},
-	dns.TypeANY:  {},
-}
-
-const other = "other"
diff --git a/plugin/pkg/dnsutil/monitor.go b/plugin/pkg/dnsutil/monitor.go
new file mode 100644
index 000000000..b86f8e43a
--- /dev/null
+++ b/plugin/pkg/dnsutil/monitor.go
@@ -0,0 +1,37 @@
+package dnsutil
+
+import (
+	"github.com/miekg/dns"
+)
+
+var monitorType = map[uint16]struct{}{
+	dns.TypeAAAA:   {},
+	dns.TypeA:      {},
+	dns.TypeCNAME:  {},
+	dns.TypeDNSKEY: {},
+	dns.TypeDS:     {},
+	dns.TypeMX:     {},
+	dns.TypeNSEC3:  {},
+	dns.TypeNSEC:   {},
+	dns.TypeNS:     {},
+	dns.TypePTR:    {},
+	dns.TypeRRSIG:  {},
+	dns.TypeSOA:    {},
+	dns.TypeSRV:    {},
+	dns.TypeTXT:    {},
+	// Meta Qtypes
+	dns.TypeIXFR: {},
+	dns.TypeAXFR: {},
+	dns.TypeANY:  {},
+}
+
+const other = "other"
+
+// QTypeMonitorLabel returns dns type label based on a list of monitored types.
+// Will return "other" for unmonitored ones.
+func QTypeMonitorLabel(qtype uint16) string {
+	if _, known := monitorType[qtype]; known {
+		return dns.Type(qtype).String()
+	}
+	return other
+}