Merge 90d10baea0 into 9fc01b272b

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
 }

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
+		}
+	}
+}

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)
+		})
+	}
+}

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,
 		},

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)
+func (s SummaryVecMetric) SetWithLabels(value float64, labels *Labels) {
+	s.SummaryVec.WithLabelValues(labels.GetValuesOrderedByKey()...).Observe(value)
 }
 
-	s.SummaryVec.WithLabelValues(lvs...).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()),
 	}
 }
 

pkg/metrics/models_test.go

@@ -191,9 +191,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)

provider/aws/config.go

@@ -19,17 +19,15 @@ package aws
 import (
 	"context"
 	"fmt"
-	"net/http"
 
 	awsv2 "github.com/aws/aws-sdk-go-v2/aws"
+
 	"github.com/aws/aws-sdk-go-v2/aws/retry"
 	"github.com/aws/aws-sdk-go-v2/config"
 	stscredsv2 "github.com/aws/aws-sdk-go-v2/credentials/stscreds"
 	"github.com/aws/aws-sdk-go-v2/service/sts"
 	"github.com/sirupsen/logrus"
 
-	extdnshttp "sigs.k8s.io/external-dns/pkg/http"
-
 	"sigs.k8s.io/external-dns/pkg/apis/externaldns"
 )
 
@@ -84,8 +82,8 @@ func newV2Config(awsConfig AWSSessionConfig) (awsv2.Config, error) {
 		config.WithRetryer(func() awsv2.Retryer {
 			return retry.AddWithMaxAttempts(retry.NewStandard(), awsConfig.APIRetries)
 		}),
-		config.WithHTTPClient(extdnshttp.NewInstrumentedClient(&http.Client{})),
 		config.WithSharedConfigProfile(awsConfig.Profile),
+		config.WithAPIOptions(GetInstrumentationMiddlewares()),
 	}
 
 	cfg, err := config.LoadDefaultConfig(context.Background(), defaultOpts...)

provider/aws/config_test.go

@@ -62,6 +62,19 @@ func Test_newV2Config(t *testing.T) {
 		assert.Equal(t, "AKIAIOSFODNN7EXAMPLE", creds.AccessKeyID)
 		assert.Equal(t, "topsecret", creds.SecretAccessKey)
 	})
+
+	t.Run("should not error when AWS_CA_BUNDLE set", func(t *testing.T) {
+		// setup
+		os.Setenv("AWS_CA_BUNDLE", "../../internal/testresources/ca.pem")
+		defer os.Unsetenv("AWS_CA_BUNDLE")
+
+		// when
+		_, err := newV2Config(AWSSessionConfig{})
+		require.NoError(t, err)
+
+		// then
+		assert.NoError(t, err)
+	})
 }
 
 func prepareCredentialsFile(t *testing.T) (*os.File, error) {

provider/aws/instrumented_config.go

@@ -0,0 +1,99 @@
+/*
+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 aws
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/aws/smithy-go/middleware"
+	smithyhttp "github.com/aws/smithy-go/transport/http"
+
+	extdnshttp "sigs.k8s.io/external-dns/pkg/http"
+	"sigs.k8s.io/external-dns/pkg/metrics"
+)
+
+type requestMetrics struct {
+	StartTime time.Time
+}
+
+type requestMetricsKey struct{}
+
+func getRequestMetric(ctx context.Context) requestMetrics {
+	requestMetrics, _ := middleware.GetStackValue(ctx, requestMetricsKey{}).(requestMetrics)
+	return requestMetrics
+}
+
+func setRequestMetric(ctx context.Context, requestMetrics requestMetrics) context.Context {
+	return middleware.WithStackValue(ctx, requestMetricsKey{}, requestMetrics)
+}
+
+var initializeTimedOperationMiddleware = middleware.InitializeMiddlewareFunc("timedOperation", func(
+	ctx context.Context, in middleware.InitializeInput, next middleware.InitializeHandler,
+) (middleware.InitializeOutput, middleware.Metadata, error) {
+	requestMetrics := requestMetrics{}
+	requestMetrics.StartTime = time.Now()
+	ctx = setRequestMetric(ctx, requestMetrics)
+
+	return next.HandleInitialize(ctx, in)
+})
+
+var extractAWSRequestParameters = middleware.DeserializeMiddlewareFunc("extractAWSRequestParameters", func(
+	ctx context.Context, in middleware.DeserializeInput, next middleware.DeserializeHandler,
+) (middleware.DeserializeOutput, middleware.Metadata, error) {
+	// Call the next middleware first to get the response
+	out, metadata, err := next.HandleDeserialize(ctx, in)
+
+	requestMetrics := getRequestMetric(ctx)
+
+	labels := metrics.NewLabels([]string{"host", "method", "path", "scheme", "status"})
+	if req, ok := in.Request.(*smithyhttp.Request); ok && req != nil {
+		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 {
+		labels.WithOptions(
+			metrics.WithLabel("status", fmt.Sprintf("%d", resp.StatusCode)))
+	}
+
+	extdnshttp.RequestDurationMetric.SetWithLabels(time.Since(requestMetrics.StartTime).Seconds(), labels)
+
+	return out, metadata, err
+})
+
+func GetInstrumentationMiddlewares() []func(*middleware.Stack) error {
+	return []func(s *middleware.Stack) error{
+		func(s *middleware.Stack) error {
+			if err := s.Initialize.Add(initializeTimedOperationMiddleware, middleware.Before); err != nil {
+				return fmt.Errorf("error adding timedOperationMiddleware: %w", err)
+			}
+
+			if err := s.Deserialize.Add(extractAWSRequestParameters, middleware.After); err != nil {
+				return fmt.Errorf("error adding extractAWSRequestParameters: %w", err)
+			}
+
+			return nil
+		},
+	}
+}

provider/aws/instrumented_config_test.go

@@ -0,0 +1,102 @@
+/*
+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 aws
+
+import (
+	"context"
+	"net/http"
+	"net/url"
+	"testing"
+	"time"
+
+	"github.com/aws/smithy-go/middleware"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	smithyhttp "github.com/aws/smithy-go/transport/http"
+)
+
+func Test_GetInstrumentationMiddlewares(t *testing.T) {
+	t.Run("adds expected middlewares", func(t *testing.T) {
+		stack := middleware.NewStack("test-stack", nil)
+
+		for _, mw := range GetInstrumentationMiddlewares() {
+			err := mw(stack)
+			require.NoError(t, err)
+		}
+
+		// Check Initialize stage
+		timedOperationMiddleware, found := stack.Initialize.Get("timedOperation")
+		assert.True(t, found, "timedOperation middleware should be present in Initialize stage")
+		assert.NotNil(t, timedOperationMiddleware)
+
+		// Check Deserialize stage
+		extractAWSRequestParametersMiddleware, found := stack.Deserialize.Get("extractAWSRequestParameters")
+		assert.True(t, found, "extractAWSRequestParameters middleware should be present in Deserialize stage")
+		assert.NotNil(t, extractAWSRequestParametersMiddleware)
+	})
+}
+
+type MockInitializeHandler struct {
+	CapturedContext context.Context
+}
+
+func (mock *MockInitializeHandler) HandleInitialize(ctx context.Context, in middleware.InitializeInput) (middleware.InitializeOutput, middleware.Metadata, error) {
+	mock.CapturedContext = ctx
+
+	return middleware.InitializeOutput{}, middleware.Metadata{}, nil
+}
+
+func Test_InitializedTimedOperationMiddleware(t *testing.T) {
+	testContext := context.Background()
+	mockInitializeHandler := &MockInitializeHandler{}
+
+	_, _, err := initializeTimedOperationMiddleware.HandleInitialize(testContext, middleware.InitializeInput{}, mockInitializeHandler)
+	require.NoError(t, err)
+
+	requestMetrics := middleware.GetStackValue(mockInitializeHandler.CapturedContext, requestMetricsKey{}).(requestMetrics)
+	assert.NotNil(t, requestMetrics.StartTime)
+}
+
+type MockDeserializeHandler struct {
+}
+
+func (mock *MockDeserializeHandler) HandleDeserialize(ctx context.Context, in middleware.DeserializeInput) (middleware.DeserializeOutput, middleware.Metadata, error) {
+	return middleware.DeserializeOutput{}, middleware.Metadata{}, nil
+}
+
+func Test_ExtractAWSRequestParameters(t *testing.T) {
+	testContext := context.Background()
+	middleware.WithStackValue(testContext, requestMetricsKey{}, requestMetrics{StartTime: time.Now()})
+
+	mockDeserializeHandler := &MockDeserializeHandler{}
+
+	deserializeInput := middleware.DeserializeInput{
+		Request: &smithyhttp.Request{
+			Request: &http.Request{
+				Method: http.MethodGet,
+				URL: &url.URL{
+					Host:   "example.com",
+					Scheme: "HTTPS",
+					Path:   "/testPath",
+				},
+			},
+		},
+	}
+	_, _, err := extractAWSRequestParameters.HandleDeserialize(testContext, deserializeInput, mockDeserializeHandler)
+	require.NoError(t, err)
+}