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 3406577cc..477b943c8 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 5c80db58a..ec4969999 100644 --- a/pkg/metrics/models_test.go +++ b/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) diff --git a/provider/aws/config.go b/provider/aws/config.go index ecc53c904..7ea2d49c6 100644 --- a/provider/aws/config.go +++ b/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...) diff --git a/provider/aws/config_test.go b/provider/aws/config_test.go index 00b3b46aa..ec6ef021f 100644 --- a/provider/aws/config_test.go +++ b/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) { diff --git a/provider/aws/instrumented_config.go b/provider/aws/instrumented_config.go new file mode 100644 index 000000000..0491fa7b4 --- /dev/null +++ b/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 + }, + } +} diff --git a/provider/aws/instrumented_config_test.go b/provider/aws/instrumented_config_test.go new file mode 100644 index 000000000..397b599d9 --- /dev/null +++ b/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) +}