chore(metrics): Updating so that request duration uses the common metrics registry. (#5677)

docs/monitoring/metrics.md

@@ -26,6 +26,7 @@ curl https://localhost:7979/metrics
 | last_sync_timestamp_seconds | Gauge | controller | Timestamp of last successful sync with the DNS provider |
 | no_op_runs_total | Counter | controller | Number of reconcile loops ending up with no changes on the DNS provider side. |
 | verified_records | Gauge | controller | Number of DNS records that exists both in source and registry (vector). |
+| request_duration_seconds | Summaryvec | http | The HTTP request latencies in seconds. |
 | cache_apply_changes_calls | Counter | provider | Number of calls to the provider cache ApplyChanges. |
 | cache_records_calls | Counter | provider | Number of calls to the provider cache Records list. |
 | endpoints_total | Gauge | registry | Number of Endpoints in the registry |
@@ -76,7 +77,6 @@ curl https://localhost:7979/metrics
 | go_memstats_sys_bytes |
 | go_sched_gomaxprocs_threads |
 | go_threads |
-| http_request_duration_seconds |
 | process_cpu_seconds_total |
 | process_max_fds |
 | process_network_receive_bytes_total |

internal/gen/docs/metrics/main_test.go

@@ -37,7 +37,7 @@ func TestComputeMetrics(t *testing.T) {
 		t.Errorf("Expected not empty metrics registry, got %d", len(reg.Metrics))
 	}
 
-	assert.Len(t, reg.Metrics, 20)
+	assert.Len(t, reg.Metrics, 21)
 }
 
 func TestGenerateMarkdownTableRenderer(t *testing.T) {

pkg/http/http.go

@@ -21,14 +21,15 @@ package http
 import (
 	"fmt"
 	"net/http"
-	"strings"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
+
+	"sigs.k8s.io/external-dns/pkg/metrics"
 )
 
 var (
-	requestDuration = prometheus.NewSummaryVec(
+	RequestDurationMetric = metrics.NewSummaryVecWithOpts(
 		prometheus.SummaryOpts{
 			Name:        "request_duration_seconds",
 			Help:        "The HTTP request latencies in seconds.",
@@ -41,7 +42,7 @@ var (
 )
 
 func init() {
-	prometheus.MustRegister(requestDuration)
+	metrics.RegisterMetric.MustRegister(RequestDurationMetric)
 }
 
 type CustomRoundTripper struct {
@@ -61,15 +62,8 @@ func (r *CustomRoundTripper) RoundTrip(req *http.Request) (*http.Response, error
 	if resp != nil {
 		status = fmt.Sprintf("%d", resp.StatusCode)
 	}
+	RequestDurationMetric.SetWithLabels(time.Since(start).Seconds(), req.URL.Scheme, req.URL.Host, metrics.PathProcessor(req.URL.Path), req.Method, status)
 
-	labels := prometheus.Labels{
-		"scheme": req.URL.Scheme,
-		"host":   req.URL.Host,
-		"path":   pathProcessor(req.URL.Path),
-		"method": req.Method,
-		"status": status,
-	}
-	requestDuration.With(labels).Observe(time.Since(start).Seconds())
 	return resp, err
 }
 
@@ -90,8 +84,3 @@ func NewInstrumentedTransport(next http.RoundTripper) http.RoundTripper {
 
 	return &CustomRoundTripper{next: next}
 }
-
-func pathProcessor(path string) string {
-	parts := strings.Split(path, "/")
-	return parts[len(parts)-1]
-}

pkg/http/http_test.go

@@ -57,25 +57,3 @@ func TestNewInstrumentedClient(t *testing.T) {
 	_, ok = result2.Transport.(*CustomRoundTripper)
 	require.True(t, ok)
 }
-
-func TestPathProcessor(t *testing.T) {
-	tests := []struct {
-		input    string
-		expected string
-	}{
-		{"/foo/bar", "bar"},
-		{"/foo/", ""},
-		{"/", ""},
-		{"", ""},
-		{"/foo/bar/baz", "baz"},
-		{"foo/bar", "bar"},
-		{"foo", "foo"},
-		{"foo/", ""},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.input, func(t *testing.T) {
-			require.Equal(t, tt.expected, pathProcessor(tt.input))
-		})
-	}
-}

pkg/metrics/metrics.go

@@ -73,7 +73,7 @@ func NewMetricsRegister() *MetricRegistry {
 //	}
 func (m *MetricRegistry) MustRegister(cs IMetric) {
 	switch v := cs.(type) {
-	case CounterMetric, GaugeMetric, CounterVecMetric, GaugeVecMetric, GaugeFuncMetric:
+	case CounterMetric, GaugeMetric, SummaryVecMetric, CounterVecMetric, GaugeVecMetric, GaugeFuncMetric:
 		if _, exists := m.mName[cs.Get().FQDN]; exists {
 			return
 		} else {
@@ -85,6 +85,8 @@ func (m *MetricRegistry) MustRegister(cs IMetric) {
 			m.Registerer.MustRegister(metric.Counter)
 		case GaugeMetric:
 			m.Registerer.MustRegister(metric.Gauge)
+		case SummaryVecMetric:
+			m.Registerer.MustRegister(metric.SummaryVec)
 		case GaugeVecMetric:
 			m.Registerer.MustRegister(metric.Gauge)
 		case CounterVecMetric:

pkg/metrics/metrics_test.go

@@ -61,8 +61,9 @@ func TestMustRegister(t *testing.T) {
 				NewCounterWithOpts(prometheus.CounterOpts{Name: "test_counter_3"}),
 				NewCounterVecWithOpts(prometheus.CounterOpts{Name: "test_counter_vec_3"}, []string{"label"}),
 				NewGaugedVectorOpts(prometheus.GaugeOpts{Name: "test_gauge_v_3"}, []string{"label"}),
+				NewSummaryVecWithOpts(prometheus.SummaryOpts{Name: "test_summary_v_3"}, []string{"label"}),
 			},
-			expected: 4,
+			expected: 5,
 		},
 		{
 			name: "unsupported metric",

pkg/metrics/models.go

@@ -176,3 +176,42 @@ func NewGaugeFuncMetric(opts prometheus.GaugeOpts) GaugeFuncMetric {
 		GaugeFunc: prometheus.NewGaugeFunc(opts, func() float64 { return 1 }),
 	}
 }
+
+type SummaryVecMetric struct {
+	Metric
+	SummaryVec prometheus.SummaryVec
+}
+
+func (s SummaryVecMetric) Get() *Metric {
+	return &s.Metric
+}
+
+// SetWithLabels sets the value of the SummaryVec metric for the specified label values.
+// All label values are converted to lowercase before being applied.
+func (s SummaryVecMetric) SetWithLabels(value float64, lvs ...string) {
+	for i, v := range lvs {
+		lvs[i] = strings.ToLower(v)
+	}
+
+	s.SummaryVec.WithLabelValues(lvs...).Observe(value)
+}
+
+func NewSummaryVecWithOpts(opts prometheus.SummaryOpts, labelNames []string) SummaryVecMetric {
+	opts.Namespace = Namespace
+	return SummaryVecMetric{
+		Metric: Metric{
+			Type:      "summaryVec",
+			Name:      opts.Name,
+			FQDN:      fmt.Sprintf("%s_%s", opts.Subsystem, opts.Name),
+			Namespace: opts.Namespace,
+			Subsystem: opts.Subsystem,
+			Help:      opts.Help,
+		},
+		SummaryVec: *prometheus.NewSummaryVec(opts, labelNames),
+	}
+}
+
+func PathProcessor(path string) string {
+	parts := strings.Split(path, "/")
+	return parts[len(parts)-1]
+}

pkg/metrics/models_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	dto "github.com/prometheus/client_model/go"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestNewGaugeWithOpts(t *testing.T) {
@@ -128,3 +129,55 @@ func TestNewBuildInfoCollector(t *testing.T) {
 	assert.Equal(t, "external_dns_build_info", reflect.ValueOf(desc).Elem().FieldByName("fqName").String())
 	assert.Contains(t, desc.String(), "version=\"0.0.1\"")
 }
+
+func TestSummaryV_SetWithLabels(t *testing.T) {
+	opts := prometheus.SummaryOpts{
+		Name:      "test_summaryVec",
+		Namespace: "test_ns",
+		Subsystem: "test_sub",
+		Help:      "help text",
+	}
+	sv := NewSummaryVecWithOpts(opts, []string{"label1", "label2"})
+
+	sv.SetWithLabels(5.01, "Alpha", "BETA")
+
+	reg := prometheus.NewRegistry()
+	reg.MustRegister(sv.SummaryVec)
+
+	metricsFamilies, err := reg.Gather()
+	assert.NoError(t, err)
+	assert.Len(t, metricsFamilies, 1)
+
+	s, err := sv.SummaryVec.GetMetricWithLabelValues("alpha", "beta")
+	assert.NoError(t, err)
+	metricsFamilies, err = reg.Gather()
+
+	s.Observe(5.21)
+	metricsFamilies, err = reg.Gather()
+	assert.NoError(t, err)
+
+	assert.InDelta(t, 10.22, *metricsFamilies[0].Metric[0].Summary.SampleSum, 0.01)
+	assert.Len(t, metricsFamilies[0].Metric[0].Label, 2)
+}
+
+func TestPathProcessor(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected string
+	}{
+		{"/foo/bar", "bar"},
+		{"/foo/", ""},
+		{"/", ""},
+		{"", ""},
+		{"/foo/bar/baz", "baz"},
+		{"foo/bar", "bar"},
+		{"foo", "foo"},
+		{"foo/", ""},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			require.Equal(t, tt.expected, PathProcessor(tt.input))
+		})
+	}
+}