diff --git a/pkg/http/http.go b/pkg/http/http.go index b78a763a1..78cc05380 100644 --- a/pkg/http/http.go +++ b/pkg/http/http.go @@ -29,6 +29,7 @@ import ( ) var ( + RequestDurationLabels = metrics.NewLabels([]string{"scheme", "host", "path", "method", "status"}) RequestDurationMetric = metrics.NewSummaryVecWithOpts( prometheus.SummaryOpts{ Name: "request_duration_seconds", @@ -37,7 +38,7 @@ var ( ConstLabels: prometheus.Labels{"handler": "instrumented_http"}, Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001}, }, - []string{"scheme", "host", "path", "method", "status"}, + *RequestDurationLabels, ) ) @@ -62,7 +63,16 @@ 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) + + RequestDurationLabels.WithOptions( + metrics.WithLabel("scheme", req.URL.Scheme), + metrics.WithLabel("host", req.URL.Host), + metrics.WithLabel("path", metrics.PathProcessor(req.URL.Path)), + metrics.WithLabel("method", req.Method), + metrics.WithLabel("status", status), + ) + + RequestDurationMetric.SetWithLabels(time.Since(start).Seconds(), RequestDurationLabels) return resp, err } diff --git a/pkg/metrics/labels.go b/pkg/metrics/labels.go new file mode 100644 index 000000000..d6381ed8e --- /dev/null +++ b/pkg/metrics/labels.go @@ -0,0 +1,78 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "sort" + "strings" + + "github.com/sirupsen/logrus" +) + +type Labels struct { + values map[string]string +} + +func (labels *Labels) GetKeysInOrder() []string { + keys := make([]string, 0, len(labels.values)) + for key := range labels.values { + keys = append(keys, key) + } + + sort.Strings(keys) + + return keys +} + +func (labels *Labels) GetValuesOrderedByKey() []string { + var orderedValues []string + for _, key := range labels.GetKeysInOrder() { + orderedValues = append(orderedValues, labels.values[key]) + } + + return orderedValues +} + +type LabelOption func(*Labels) + +func NewLabels(labelNames []string) *Labels { + labels := &Labels{ + values: make(map[string]string), + } + + for _, label := range labelNames { + labels.values[strings.ToLower(label)] = "" + } + + return labels +} + +func (labels *Labels) WithOptions(options ...LabelOption) { + for _, option := range options { + option(labels) + } +} + +func WithLabel(labelName string, labelValue string) LabelOption { + return func(labels *Labels) { + if _, ok := labels.values[strings.ToLower(labelName)]; !ok { + logrus.Errorf("Attempting to set a value for a label that doesn't exist! '%s' does not exist!", labelName) + } else { + labels.values[strings.ToLower(labelName)] = labelValue + } + } +} diff --git a/pkg/metrics/labels_test.go b/pkg/metrics/labels_test.go new file mode 100644 index 000000000..58e3c6c4d --- /dev/null +++ b/pkg/metrics/labels_test.go @@ -0,0 +1,191 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "sigs.k8s.io/external-dns/internal/testutils" +) + +func TestNewLabels(t *testing.T) { + tests := []struct { + name string + labelNames []string + expectedLabelNames []string + }{ + { + name: "NewLabels initializes Values with labelNames", + labelNames: []string{"label1", "label2"}, + expectedLabelNames: []string{"label1", "label2"}, + }, + { + name: "NewLabels sets labelNames as lower-case", + labelNames: []string{"LABEL1", "label2"}, + expectedLabelNames: []string{"label1", "label2"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + labels := NewLabels(tt.labelNames) + keys := labels.GetKeysInOrder() + + assert.Equal(t, tt.expectedLabelNames, keys) + }) + } +} + +func TestLabelsWithOptions(t *testing.T) { + tests := []struct { + name string + labelNames []string + options []LabelOption + expectedValuesMap map[string]string + }{ + { + name: "WithOptions sets label values", + labelNames: []string{"label1", "label2"}, + options: []LabelOption{ + WithLabel("label1", "alpha"), + WithLabel("label2", "beta"), + }, + expectedValuesMap: map[string]string{ + "label1": "alpha", + "label2": "beta", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + labels := NewLabels(tt.labelNames) + labels.WithOptions(tt.options...) + + assert.Equal(t, tt.expectedValuesMap, labels.values) + }) + } +} + +func TestLabelsWithLabel(t *testing.T) { + tests := []struct { + name string + labelNames []string + labelName string + labelValue string + expectedLabels *Labels + expectedErrorLog string + }{ + { + name: "WithLabel sets label and value", + labelNames: []string{"label1"}, + labelName: "label1", + labelValue: "alpha", + expectedLabels: &Labels{ + values: map[string]string{ + "label1": "alpha", + }}, + }, + { + name: "WithLabel sets labelName to lowercase", + labelNames: []string{"label1"}, + labelName: "LABEL1", + labelValue: "alpha", + expectedLabels: &Labels{ + values: map[string]string{ + "label1": "alpha", + }}, + }, + { + name: "WithLabel errors if label doesn't exist", + labelNames: []string{"label1"}, + labelName: "notreal", + labelValue: "", + expectedErrorLog: "Attempting to set a value for a label that doesn't exist! 'notreal' does not exist!", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hook := testutils.LogsUnderTestWithLogLevel(logrus.WarnLevel, t) + + labels := NewLabels(tt.labelNames) + labels.WithOptions(WithLabel(tt.labelName, tt.labelValue)) + + if tt.expectedLabels != nil { + assert.Equal(t, tt.expectedLabels, labels) + } + + if tt.expectedErrorLog != "" { + testutils.TestHelperLogContains(tt.expectedErrorLog, hook, t) + } + }) + } +} + +func TestLabelsGetKeysInOrder(t *testing.T) { + tests := []struct { + name string + labels *Labels + expectedKeysInOrder []string + }{ + { + "GetKeysInOrder returns keys in alphabetical order", + NewLabels([]string{"label2", "label1"}), + []string{"label1", "label2"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + orderedKeys := tt.labels.GetKeysInOrder() + + assert.Equal(t, tt.expectedKeysInOrder, orderedKeys) + }) + } +} + +func TestLabelsGetValuesOrderedByKey(t *testing.T) { + tests := []struct { + name string + labels *Labels + labelOptions []LabelOption + expectedValuesInOrder []string + }{ + { + "GetKeysInOrder returns keys in alphabetical order", + NewLabels([]string{"label1", "label2"}), + []LabelOption{ + WithLabel("label1", "alpha"), + WithLabel("label2", "beta"), + }, + []string{"alpha", "beta"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.labels.WithOptions(tt.labelOptions...) + + orderedValues := tt.labels.GetValuesOrderedByKey() + + assert.Equal(t, tt.expectedValuesInOrder, orderedValues) + }) + } +} diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index b7a0aca49..62f7746a6 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/metrics/metrics_test.go @@ -33,6 +33,12 @@ func (m *MockMetric) Get() *Metric { return &Metric{FQDN: m.FQDN} } +func getTestLabels(t *testing.T, labelNames []string) *Labels { + t.Helper() + + return NewLabels(labelNames) +} + func TestMustRegister(t *testing.T) { tests := []struct { name string @@ -61,7 +67,7 @@ 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"}), + NewSummaryVecWithOpts(prometheus.SummaryOpts{Name: "test_summary_v_3"}, *NewLabels([]string{"label"})), }, expected: 5, }, diff --git a/pkg/metrics/models.go b/pkg/metrics/models.go index 11fd47419..134e7dab1 100644 --- a/pkg/metrics/models.go +++ b/pkg/metrics/models.go @@ -186,17 +186,11 @@ 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 (s SummaryVecMetric) SetWithLabels(value float64, labels *Labels) { + s.SummaryVec.WithLabelValues(labels.GetValuesOrderedByKey()...).Observe(value) } -func NewSummaryVecWithOpts(opts prometheus.SummaryOpts, labelNames []string) SummaryVecMetric { +func NewSummaryVecWithOpts(opts prometheus.SummaryOpts, labels Labels) SummaryVecMetric { opts.Namespace = Namespace return SummaryVecMetric{ Metric: Metric{ @@ -207,7 +201,7 @@ func NewSummaryVecWithOpts(opts prometheus.SummaryOpts, labelNames []string) Sum Subsystem: opts.Subsystem, Help: opts.Help, }, - SummaryVec: *prometheus.NewSummaryVec(opts, labelNames), + SummaryVec: *prometheus.NewSummaryVec(opts, labels.GetKeysInOrder()), } } diff --git a/pkg/metrics/models_test.go b/pkg/metrics/models_test.go index dc9c7a7db..3ad0eedbc 100644 --- a/pkg/metrics/models_test.go +++ b/pkg/metrics/models_test.go @@ -137,9 +137,16 @@ func TestSummaryV_SetWithLabels(t *testing.T) { Subsystem: "test_sub", Help: "help text", } - sv := NewSummaryVecWithOpts(opts, []string{"label1", "label2"}) - sv.SetWithLabels(5.01, "Alpha", "BETA") + labels := NewLabels([]string{"label1", "label2"}) + sv := NewSummaryVecWithOpts(opts, *labels) + + labels.WithOptions( + WithLabel("label1", "alpha"), + WithLabel("label2", "beta"), + ) + + sv.SetWithLabels(5.01, labels) reg := prometheus.NewRegistry() reg.MustRegister(sv.SummaryVec) diff --git a/provider/aws/instrumented_config.go b/provider/aws/instrumented_config.go index 68134116a..0491fa7b4 100644 --- a/provider/aws/instrumented_config.go +++ b/provider/aws/instrumented_config.go @@ -23,9 +23,9 @@ import ( "github.com/aws/smithy-go/middleware" smithyhttp "github.com/aws/smithy-go/transport/http" - "github.com/prometheus/client_golang/prometheus" extdnshttp "sigs.k8s.io/external-dns/pkg/http" + "sigs.k8s.io/external-dns/pkg/metrics" ) type requestMetrics struct { @@ -61,27 +61,23 @@ var extractAWSRequestParameters = middleware.DeserializeMiddlewareFunc("extractA requestMetrics := getRequestMetric(ctx) - var host, scheme, method, path, status string + labels := metrics.NewLabels([]string{"host", "method", "path", "scheme", "status"}) if req, ok := in.Request.(*smithyhttp.Request); ok && req != nil { - host = req.URL.Host - scheme = req.URL.Scheme - method = req.Method - path = req.URL.Path + labels.WithOptions( + metrics.WithLabel("host", req.URL.Host), + metrics.WithLabel("method", req.Method), + metrics.WithLabel("path", metrics.PathProcessor(req.URL.Path)), + metrics.WithLabel("scheme", req.URL.Scheme), + ) } // Try to access HTTP response and status code if resp, ok := out.RawResponse.(*smithyhttp.Response); ok && resp != nil { - status = fmt.Sprintf("%d", resp.StatusCode) + labels.WithOptions( + metrics.WithLabel("status", fmt.Sprintf("%d", resp.StatusCode))) } - labels := prometheus.Labels{ - "scheme": scheme, - "host": host, - "path": extdnshttp.PathProcessor(path), - "method": method, - "status": status, - } - extdnshttp.RequestDuration.With(labels).Observe(time.Since(requestMetrics.StartTime).Seconds()) + extdnshttp.RequestDurationMetric.SetWithLabels(time.Since(requestMetrics.StartTime).Seconds(), labels) return out, metadata, err })