From 2138333d277514b371d02ef01914ed94e32b2293 Mon Sep 17 00:00:00 2001 From: BenjaminBraunDev Date: Fri, 14 Mar 2025 20:16:20 +0000 Subject: [PATCH 1/4] Add nil option for metric_spec to specify metrics to not be scraped. --- pkg/epp/backend/metrics/metrics_spec.go | 3 +++ pkg/epp/backend/metrics/metrics_spec_test.go | 15 ++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/epp/backend/metrics/metrics_spec.go b/pkg/epp/backend/metrics/metrics_spec.go index ce0c075dd..f6f904a97 100644 --- a/pkg/epp/backend/metrics/metrics_spec.go +++ b/pkg/epp/backend/metrics/metrics_spec.go @@ -41,6 +41,9 @@ type MetricMapping struct { // "metric_name{label1=value1}" // "metric_name{label1=value1,label2=value2}" func stringToMetricSpec(specStr string) (*MetricSpec, error) { + if specStr == "" { + return nil, nil // Allow empty strings to represent nil MetricSpecs + } specStr = strings.TrimSpace(specStr) metricName := specStr labels := make(map[string]string) diff --git a/pkg/epp/backend/metrics/metrics_spec_test.go b/pkg/epp/backend/metrics/metrics_spec_test.go index 828042065..e62bc5ff8 100644 --- a/pkg/epp/backend/metrics/metrics_spec_test.go +++ b/pkg/epp/backend/metrics/metrics_spec_test.go @@ -32,7 +32,7 @@ func TestStringToMetricSpec(t *testing.T) { name: "empty string", input: "", want: nil, - wantErr: true, + wantErr: false, }, { name: "no labels", @@ -152,14 +152,9 @@ func TestStringToMetricSpec(t *testing.T) { t.Errorf("stringToMetricSpec() error = %v, wantErr %v", err, tt.wantErr) return } - if tt.wantErr { - if got != nil { // handles if we got a nil spec and didn't expect an error - t.Errorf("stringToMetricSpec() = %v, want %v", got, tt.want) - return - } - } else { - if got == nil { - t.Fatalf("stringToMetricSpec() = got nil but wanted %v", tt.want) + if tt.want != nil && got != nil { // compare maps directly + if tt.want.Labels == nil { + tt.want.Labels = make(map[string]string) } if !reflect.DeepEqual(got.MetricName, tt.want.MetricName) { t.Errorf("stringToMetricSpec() got MetricName = %v, want %v", got.MetricName, tt.want.MetricName) @@ -167,6 +162,8 @@ func TestStringToMetricSpec(t *testing.T) { if !reflect.DeepEqual(got.Labels, tt.want.Labels) { t.Errorf("stringToMetricSpec() got Labels = %v, want %v", got.Labels, tt.want.Labels) } + } else if tt.want != got { // handles if one is nil and the other isn't + t.Errorf("stringToMetricSpec() = %v, want %v", got, tt.want) } }) } From 9a5869b2a2e9f270f61a5775ea911e95e28409b8 Mon Sep 17 00:00:00 2001 From: BenjaminBraunDev Date: Sat, 15 Mar 2025 00:23:14 +0000 Subject: [PATCH 2/4] Add logging when a metric is not being scraped when set as an empty string. --- cmd/epp/main.go | 1 + pkg/epp/backend/metrics/metrics_spec.go | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cmd/epp/main.go b/cmd/epp/main.go index fa63f0bce..28815cc11 100644 --- a/cmd/epp/main.go +++ b/cmd/epp/main.go @@ -155,6 +155,7 @@ func run() error { // Set up mapper for metric scraping. mapping, err := backendmetrics.NewMetricMapping( + ctx, *totalQueuedRequestsMetric, *kvCacheUsagePercentageMetric, *loraInfoMetric, diff --git a/pkg/epp/backend/metrics/metrics_spec.go b/pkg/epp/backend/metrics/metrics_spec.go index f6f904a97..9bb6ae800 100644 --- a/pkg/epp/backend/metrics/metrics_spec.go +++ b/pkg/epp/backend/metrics/metrics_spec.go @@ -17,8 +17,12 @@ limitations under the License. package metrics import ( + "context" "fmt" "strings" + + "sigs.k8s.io/controller-runtime/pkg/log" + logutil "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/util/logging" ) // MetricSpec represents a single metric's specification. @@ -93,7 +97,7 @@ func stringToMetricSpec(specStr string) (*MetricSpec, error) { } // NewMetricMapping creates a MetricMapping from string values. -func NewMetricMapping(queuedStr, kvUsageStr, loraReqInfoStr string) (*MetricMapping, error) { +func NewMetricMapping(ctx context.Context, queuedStr, kvUsageStr, loraReqInfoStr string) (*MetricMapping, error) { queuedSpec, err := stringToMetricSpec(queuedStr) if err != nil { return nil, fmt.Errorf("error parsing WaitingRequests: %w", err) @@ -112,5 +116,16 @@ func NewMetricMapping(queuedStr, kvUsageStr, loraReqInfoStr string) (*MetricMapp LoraRequestInfo: loraReqInfoSpec, } + logger := log.FromContext(ctx) + if mapping.TotalQueuedRequests == nil { + logger.V(logutil.TRACE).Info("Not scraping metric: TotalQueuedRequests") + } + if mapping.KVCacheUtilization == nil { + logger.V(logutil.TRACE).Info("Not scraping metric: KVCacheUtilization") + } + if mapping.LoraRequestInfo == nil { + logger.V(logutil.TRACE).Info("Not scraping metric: LoraRequestInfo") + } + return mapping, nil } From 4d8d3f0e685a3fa077f28fccff697720bd9e8dde Mon Sep 17 00:00:00 2001 From: BenjaminBraunDev Date: Mon, 17 Mar 2025 18:57:47 +0000 Subject: [PATCH 3/4] Move unscraped metric setup logging to main. --- cmd/epp/main.go | 15 ++++++++++++++- pkg/epp/backend/metrics/metrics_spec.go | 17 +---------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/cmd/epp/main.go b/cmd/epp/main.go index 28815cc11..39baf18b1 100644 --- a/cmd/epp/main.go +++ b/cmd/epp/main.go @@ -155,7 +155,6 @@ func run() error { // Set up mapper for metric scraping. mapping, err := backendmetrics.NewMetricMapping( - ctx, *totalQueuedRequestsMetric, *kvCacheUsagePercentageMetric, *loraInfoMetric, @@ -164,6 +163,7 @@ func run() error { setupLog.Error(err, "Failed to create metric mapping from flags.") return err } + verifyMetricMapping(*mapping, setupLog) pmf := backendmetrics.NewPodMetricsFactory(&backendmetrics.PodMetricsClientImpl{MetricMapping: mapping}, *refreshMetricsInterval) // Setup runner. @@ -305,3 +305,16 @@ func validateFlags() error { return nil } + +func verifyMetricMapping(mapping backendmetrics.MetricMapping, logger logr.Logger) { + if mapping.TotalQueuedRequests == nil { + logger.Info("Not scraping metric: TotalQueuedRequests") + } + if mapping.KVCacheUtilization == nil { + logger.Info("Not scraping metric: KVCacheUtilization") + } + if mapping.LoraRequestInfo == nil { + logger.Info("Not scraping metric: LoraRequestInfo") + } + +} diff --git a/pkg/epp/backend/metrics/metrics_spec.go b/pkg/epp/backend/metrics/metrics_spec.go index 9bb6ae800..f6f904a97 100644 --- a/pkg/epp/backend/metrics/metrics_spec.go +++ b/pkg/epp/backend/metrics/metrics_spec.go @@ -17,12 +17,8 @@ limitations under the License. package metrics import ( - "context" "fmt" "strings" - - "sigs.k8s.io/controller-runtime/pkg/log" - logutil "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/util/logging" ) // MetricSpec represents a single metric's specification. @@ -97,7 +93,7 @@ func stringToMetricSpec(specStr string) (*MetricSpec, error) { } // NewMetricMapping creates a MetricMapping from string values. -func NewMetricMapping(ctx context.Context, queuedStr, kvUsageStr, loraReqInfoStr string) (*MetricMapping, error) { +func NewMetricMapping(queuedStr, kvUsageStr, loraReqInfoStr string) (*MetricMapping, error) { queuedSpec, err := stringToMetricSpec(queuedStr) if err != nil { return nil, fmt.Errorf("error parsing WaitingRequests: %w", err) @@ -116,16 +112,5 @@ func NewMetricMapping(ctx context.Context, queuedStr, kvUsageStr, loraReqInfoStr LoraRequestInfo: loraReqInfoSpec, } - logger := log.FromContext(ctx) - if mapping.TotalQueuedRequests == nil { - logger.V(logutil.TRACE).Info("Not scraping metric: TotalQueuedRequests") - } - if mapping.KVCacheUtilization == nil { - logger.V(logutil.TRACE).Info("Not scraping metric: KVCacheUtilization") - } - if mapping.LoraRequestInfo == nil { - logger.V(logutil.TRACE).Info("Not scraping metric: LoraRequestInfo") - } - return mapping, nil } From cb396dbfef3e38c72c35931eaac5f39096fcf9f3 Mon Sep 17 00:00:00 2001 From: BenjaminBraunDev Date: Mon, 17 Mar 2025 19:14:25 +0000 Subject: [PATCH 4/4] Update Dockerfile go version from 1.23 to 1.24 --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 312700bc3..8fb00dfbf 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ # Dockerfile has specific requirement to put this ARG at the beginning: # https://docs.docker.com/engine/reference/builder/#understand-how-arg-and-from-interact -ARG BUILDER_IMAGE=golang:1.23 +ARG BUILDER_IMAGE=golang:1.24 ARG BASE_IMAGE=gcr.io/distroless/static:nonroot ## Multistage build