fix(metrics): Make prometheus metrics labels more type safe.

This commit is contained in:
Matt Mix 2025-08-03 19:00:23 -04:00
parent 474d1fc593
commit 90d10baea0
No known key found for this signature in database
GPG Key ID: 7A4BD5A453862F19
7 changed files with 312 additions and 30 deletions

View File

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

78
pkg/metrics/labels.go Normal file
View File

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

191
pkg/metrics/labels_test.go Normal file
View File

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

View File

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

View File

@ -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()),
}
}

View File

@ -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)

View File

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