From 51d29cd4c844daa6fee0c1dd6634342f3d801330 Mon Sep 17 00:00:00 2001 From: Omer Sasson Date: Thu, 4 Dec 2025 23:57:30 -0500 Subject: [PATCH 1/2] adding user metrics --- pkg/sloop/server/internal/config/config.go | 3 + pkg/sloop/server/server.go | 23 +- .../server/server_metrics/server_metrics.go | 301 +++++++++ .../server_metrics/server_metrics_test.go | 579 ++++++++++++++++++ pkg/sloop/webserver/middleware.go | 21 +- pkg/sloop/webserver/webserver.go | 25 +- 6 files changed, 928 insertions(+), 24 deletions(-) create mode 100644 pkg/sloop/server/server_metrics/server_metrics.go create mode 100644 pkg/sloop/server/server_metrics/server_metrics_test.go diff --git a/pkg/sloop/server/internal/config/config.go b/pkg/sloop/server/internal/config/config.go index 2933299f..56045fb1 100644 --- a/pkg/sloop/server/internal/config/config.go +++ b/pkg/sloop/server/internal/config/config.go @@ -76,6 +76,7 @@ type SloopConfig struct { BadgerVLogTruncate bool `json:"badgerVLogTruncate"` EnableDeleteKeys bool `json:"enableDeleteKeys"` EnableGranularMetrics bool `json:"enableGranularMetrics"` + EnableUserMetrics bool `json:"enableUserMetrics"` PrivilegedAccess bool `json:"PrivilegedAccess"` BadgerDetailLogEnabled bool `json:"badgerDetailLogEnabled"` } @@ -124,6 +125,7 @@ func registerFlags(fs *flag.FlagSet, config *SloopConfig) { fs.BoolVar(&config.BadgerSyncWrites, "badger-sync-writes", config.BadgerSyncWrites, "Sync Writes ensures writes are synced to disk if set to true") fs.BoolVar(&config.EnableDeleteKeys, "enable-delete-keys", config.EnableDeleteKeys, "Use delete prefixes instead of dropPrefix for GC") fs.BoolVar(&config.EnableGranularMetrics, "enable-granular-metrics", config.EnableGranularMetrics, "default:True , allows metrics of event kind to be granular with reason, type and name") + fs.BoolVar(&config.EnableUserMetrics, "enable-user-metrics", config.EnableUserMetrics, "Enable collection of user metrics from request headers") fs.BoolVar(&config.PrivilegedAccess, "allow-privileged-access", config.PrivilegedAccess, "default:True , allows sloop to access kube root path") fs.BoolVar(&config.BadgerVLogFileIOMapping, "badger-vlog-fileIO-mapping", config.BadgerVLogFileIOMapping, "Indicates which file loading mode should be used for the value log data, in memory constrained environments the value is recommended to be true") fs.BoolVar(&config.BadgerVLogTruncate, "badger-vlog-truncate", config.BadgerVLogTruncate, "Truncate value log if badger db offset is different from badger db size") @@ -176,6 +178,7 @@ func getDefaultConfig() *SloopConfig { BadgerVLogTruncate: true, EnableDeleteKeys: false, EnableGranularMetrics: false, + EnableUserMetrics: false, PrivilegedAccess: true, BadgerDetailLogEnabled: false, ExclusionRules: map[string][]any{}, diff --git a/pkg/sloop/server/server.go b/pkg/sloop/server/server.go index 3b6eb9d9..d9401977 100644 --- a/pkg/sloop/server/server.go +++ b/pkg/sloop/server/server.go @@ -147,17 +147,18 @@ func RealMain() error { } webConfig := webserver.WebConfig{ - BindAddress: conf.BindAddress, - Port: conf.Port, - WebFilesPath: conf.WebFilesPath, - ConfigYaml: conf.ToYaml(), - MaxLookback: conf.MaxLookback, - DefaultNamespace: conf.DefaultNamespace, - DefaultLookback: conf.DefaultLookback, - DefaultResources: conf.DefaultKind, - ResourceLinks: conf.ResourceLinks, - LeftBarLinks: conf.LeftBarLinks, - CurrentContext: displayContext, + BindAddress: conf.BindAddress, + Port: conf.Port, + WebFilesPath: conf.WebFilesPath, + ConfigYaml: conf.ToYaml(), + MaxLookback: conf.MaxLookback, + DefaultNamespace: conf.DefaultNamespace, + DefaultLookback: conf.DefaultLookback, + DefaultResources: conf.DefaultKind, + ResourceLinks: conf.ResourceLinks, + LeftBarLinks: conf.LeftBarLinks, + CurrentContext: displayContext, + EnableUserMetrics: conf.EnableUserMetrics, } err = webserver.Run(webConfig, tables) if err != nil { diff --git a/pkg/sloop/server/server_metrics/server_metrics.go b/pkg/sloop/server/server_metrics/server_metrics.go new file mode 100644 index 00000000..b50e01c8 --- /dev/null +++ b/pkg/sloop/server/server_metrics/server_metrics.go @@ -0,0 +1,301 @@ +/* + * Copyright (c) 2019, salesforce.com, inc. + * All rights reserved. + * SPDX-License-Identifier: BSD-3-Clause + * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +package server_metrics + +import ( + "net/http" + "regexp" + "strings" + + "github.com/golang/glog" + "github.com/prometheus/client_golang/prometheus" +) + +const ( + // Header names + HeaderXUsername = "X-Username" + HeaderXUsernameLower = "x-username" + HeaderUserAgent = "User-Agent" + HeaderXForwardedClientCert = "X-Forwarded-Client-Cert" + + // Metric names + MetricUserRequests = "user_requests" + MetricServiceRequests = "service_requests" + + // label keys + KeyUsername = "username" + KeyUserAgent = "user_agent" + KeyURIService = "uri_service" + KeyURIServiceInstance = "uri_service_instance" + + // URI field names + FieldService = "service=" + FieldServiceInstance = "service_instance=" +) + +var ( + userRequestCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: MetricUserRequests, + Help: "Counter of dashboard requests broken out by user information from headers.", + }, + []string{KeyUsername, KeyUserAgent, KeyURIService, KeyURIServiceInstance}, + ) + + serviceRequestCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: MetricServiceRequests, + Help: "Counter of dashboard requests broken out by service information from headers.", + }, + []string{KeyUserAgent, KeyURIService, KeyURIServiceInstance}, + ) +) + +// Initialize user metrics in prometheus +func init() { + prometheus.MustRegister(userRequestCounter) + prometheus.MustRegister(serviceRequestCounter) +} + +// EnsureUserMetricsRegistered ensures user metrics are registered +// This can be called after configuration is loaded to ensure proper initialization +func EnsureUserMetricsRegistered() { + // Metrics are already registered in init(), but this provides a hook + // for any future initialization needs +} + +// ============================================================================ +// Header Information Extraction Functions +// ============================================================================ + +// These functions handle extracting user information from HTTP request headers + +// getHeaderWithFallback tries multiple header names and returns the first non-empty value or empty string if no value is found +func getHeaderWithFallback(headers *http.Header, headerNames []string) string { + for _, headerName := range headerNames { + if value := headers.Get(headerName); value != "" { + return value + } + } + return "" +} + +// extractRequesterInfo extracts user/service information from request headers and returns a map with requester information +func extractRequesterInfo(headers *http.Header) map[string]string { + requesterInfo := map[string]string{} + + // Extract username from headers if present otherwise extract service information from URI + userName := getHeaderWithFallback(headers, []string{HeaderXUsernameLower, HeaderXUsername}) + if userName != "" { + requesterInfo[KeyUsername] = userName + } else { + // Extract XFCC information + requesterInfo[KeyURIService], requesterInfo[KeyURIServiceInstance] = extractXFCCInfo(headers) + } + + requesterInfo[KeyUserAgent] = getHeaderWithFallback(headers, []string{HeaderUserAgent}) + + return requesterInfo +} + +// extractXFCCInfo extracts service information from X-Forwarded-Client-Cert header +func extractXFCCInfo(headers *http.Header) (service, serviceInstance string) { + // Initialize with default values + service = "" + serviceInstance = "" + + clientCert := headers.Get(HeaderXForwardedClientCert) + if clientCert == "" { + glog.V(4).Infof("X-Forwarded-Client-Cert header not found") + return service, serviceInstance + } + + glog.V(4).Infof("X-Forwarded-Client-Cert header found: %s", clientCert) + + // Parse XFCC header - look for URI field with service information + // The XFCC header format is: By=...;Hash=...;Subject=...;URI=spiffe://.../service=value,service_instance=value + if strings.Contains(clientCert, "URI=") { + // Extract the URI portion + uriStart := strings.Index(clientCert, "URI=") + if uriStart != -1 { + uriPart := clientCert[uriStart+4:] // Skip "URI=" + + // Find the end of the URI (next semicolon or end of string) + if semicolonIdx := strings.Index(uriPart, ";"); semicolonIdx != -1 { + uriPart = uriPart[:semicolonIdx] + } + + service, serviceInstance = extractServiceFromURI(uriPart) + if service != "" { + glog.V(4).Infof("Parsed XFCC info - service: '%s', service_instance: '%s'", service, serviceInstance) + return service, serviceInstance + } + } + } + + // If parsing didn't work, return empty values + glog.V(4).Infof("No service information found in XFCC header") + return service, serviceInstance +} + +// extractServiceFromURI extracts service and service instance from SPIFFE URI +func extractServiceFromURI(uri string) (service, serviceInstance string) { + // Extract service=value from URI + service = extractField(uri, FieldService) + if service != "" { + // Extract service_instance=value from URI + serviceInstance = extractField(uri, FieldServiceInstance) + return service, serviceInstance + } + + // If no service found, return empty strings + return "", "" +} + +// extractField extracts a field value from a string using regex +func extractField(text, field string) string { + // Create regex pattern to match field=value, stopping at next / or = or end of string + pattern := regexp.MustCompile(field + `([^/=]+)`) + matches := pattern.FindStringSubmatch(text) + if len(matches) > 1 { + return strings.TrimSpace(matches[1]) + } + return "" +} + +// mergeMaps merges two maps, with values from additionalTags taking precedence +func mergeMaps(base, additional map[string]string) map[string]string { + result := make(map[string]string) + + // Copy base map + for k, v := range base { + result[k] = v + } + + // Override with additional tags + for k, v := range additional { + result[k] = v + } + + return result +} + +// publishHeaderMetrics publishes header metrics to the metrics server +func PublishHeaderMetrics(headers *http.Header, additionalTags map[string]string, enableUserMetrics bool) { + if !enableUserMetrics { + return + } + + userInfo := extractRequesterInfo(headers) + + // merge the userInfo and additionalTags maps into a new map + labels := mergeMaps(userInfo, additionalTags) + + // if username is present it is a user request + if labels[KeyUsername] != "" { + userRequestCounter.WithLabelValues( + labels[KeyUsername], + labels[KeyUserAgent], + labels[KeyURIService], + labels[KeyURIServiceInstance], + ).Inc() + return + } + + // if uri_service is present it is a service request + if labels[KeyURIService] != "" { + serviceRequestCounter.WithLabelValues( + labels[KeyUserAgent], + labels[KeyURIService], + labels[KeyURIServiceInstance], + ).Inc() + return + } + + glog.Warningf("Failed to determine the request type") +} + +// ============================================================================ +// Legacy compatibility functions +// ============================================================================ + +// These functions maintain compatibility with the existing filter.go code + +// UserInfo contains user information extracted from request headers +type UserInfo struct { + Email string + Username string + UserAgent string + Service string + ServiceInstance string +} + +// requestMeta contains metadata about the request for metrics +type requestMeta struct { + userInfo UserInfo +} + +// extractUserInfo extracts user information from request headers and returns a UserInfo struct +// This is a compatibility wrapper for the existing filter.go code +func extractUserInfo(request *http.Request) UserInfo { + userInfo := UserInfo{} + + // Extract standard headers + userInfo.Username = getHeaderWithFallback(&request.Header, []string{HeaderXUsernameLower, HeaderXUsername}) + userInfo.UserAgent = getHeaderWithFallback(&request.Header, []string{HeaderUserAgent}) + + // Extract XFCC information + userInfo.Service, userInfo.ServiceInstance = extractXFCCInfo(&request.Header) + + return userInfo +} + +// publishUserMetrics publishes user-specific metrics if enabled +// This is a compatibility wrapper for the existing filter.go code +func publishUserMetrics(requestMeta requestMeta, resourceRequest *string, enableUserMetrics bool) { + if !enableUserMetrics { + return + } + + // Build additional tags from resource request + additionalTags := make(map[string]string) + if resourceRequest != nil { + additionalTags["resource"] = *resourceRequest + } + + // Create a temporary request to extract headers + // Since we already have the userInfo, we'll construct the labels directly + labels := make(map[string]string) + labels[KeyUserAgent] = requestMeta.userInfo.UserAgent + labels[KeyURIService] = requestMeta.userInfo.Service + labels[KeyURIServiceInstance] = requestMeta.userInfo.ServiceInstance + + // if username is present it is a user request + if requestMeta.userInfo.Username != "" { + labels[KeyUsername] = requestMeta.userInfo.Username + userRequestCounter.WithLabelValues( + labels[KeyUsername], + labels[KeyUserAgent], + labels[KeyURIService], + labels[KeyURIServiceInstance], + ).Inc() + return + } + + // if uri_service is present it is a service request + if labels[KeyURIService] != "" { + serviceRequestCounter.WithLabelValues( + labels[KeyUserAgent], + labels[KeyURIService], + labels[KeyURIServiceInstance], + ).Inc() + return + } + + glog.Warningf("Failed to determine the request type") +} diff --git a/pkg/sloop/server/server_metrics/server_metrics_test.go b/pkg/sloop/server/server_metrics/server_metrics_test.go new file mode 100644 index 00000000..0ef616b7 --- /dev/null +++ b/pkg/sloop/server/server_metrics/server_metrics_test.go @@ -0,0 +1,579 @@ +/* + * Copyright (c) 2019, salesforce.com, inc. + * All rights reserved. + * SPDX-License-Identifier: BSD-3-Clause + * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +package server_metrics + +import ( + "net/http" + "testing" + + "github.com/prometheus/client_golang/prometheus/testutil" +) + +func TestGetHeaderWithFallback(t *testing.T) { + tests := []struct { + name string + headers map[string]string + headerNames []string + want string + }{ + { + name: "no headers", + headers: map[string]string{}, + headerNames: []string{"x-email", "X-Email"}, + want: "", + }, + { + name: "first header present", + headers: map[string]string{"x-email": "test@example.com"}, + headerNames: []string{"x-email", "X-Email"}, + want: "test@example.com", + }, + { + name: "second header present", + headers: map[string]string{"X-Email": "uppercase@example.com"}, + headerNames: []string{"x-email", "X-Email"}, + want: "uppercase@example.com", + }, + { + name: "empty header value", + headers: map[string]string{"x-email": ""}, + headerNames: []string{"x-email", "X-Email"}, + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + headers := &http.Header{} + + for k, v := range tt.headers { + headers.Set(k, v) + } + + got := getHeaderWithFallback(headers, tt.headerNames) + if got != tt.want { + t.Errorf("getHeaderWithFallback() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestExtractRequesterInfo(t *testing.T) { + tests := []struct { + name string + headers map[string]string + want map[string]string + }{ + { + name: "username present", + headers: map[string]string{HeaderXUsername: "testuser", HeaderUserAgent: "test-agent"}, + want: map[string]string{ + KeyUsername: "testuser", + KeyUserAgent: "test-agent", + }, + }, + { + name: "no username, has XFCC with service and instance", + headers: map[string]string{ + HeaderXForwardedClientCert: "By=spiffe://example.com/service=myservice/service_instance=instance1;Hash=abc123;URI=spiffe://example.com/service=myservice/service_instance=instance1", + HeaderUserAgent: "test-agent", + }, + want: map[string]string{ + KeyURIService: "myservice", + KeyURIServiceInstance: "instance1", + KeyUserAgent: "test-agent", + }, + }, + { + name: "no username, has XFCC with only service", + headers: map[string]string{ + HeaderXForwardedClientCert: "By=spiffe://example.com/service=myservice;Hash=abc123;URI=spiffe://example.com/service=myservice", + HeaderUserAgent: "test-agent", + }, + want: map[string]string{ + KeyURIService: "myservice", + KeyURIServiceInstance: "", + KeyUserAgent: "test-agent", + }, + }, + { + name: "no username, has XFCC without URI", + headers: map[string]string{ + HeaderXForwardedClientCert: "By=spiffe://example.com;Hash=abc123", + HeaderUserAgent: "test-agent", + }, + want: map[string]string{ + KeyURIService: "", + KeyURIServiceInstance: "", + KeyUserAgent: "test-agent", + }, + }, + { + name: "no username, no XFCC", + headers: map[string]string{HeaderUserAgent: "test-agent"}, + want: map[string]string{ + KeyURIService: "", + KeyURIServiceInstance: "", + KeyUserAgent: "test-agent", + }, + }, + { + name: "case insensitive username", + headers: map[string]string{HeaderXUsernameLower: "testuser", HeaderUserAgent: "test-agent"}, + want: map[string]string{ + KeyUsername: "testuser", + KeyUserAgent: "test-agent", + }, + }, + { + name: "XFCC with spaces in service names", + headers: map[string]string{ + HeaderXForwardedClientCert: "By=spiffe://example.com/service= my service /service_instance= my instance ;Hash=abc123;URI=spiffe://example.com/service= my service /service_instance= my instance ", + HeaderUserAgent: "test-agent", + }, + want: map[string]string{ + KeyURIService: "my service", + KeyURIServiceInstance: "my instance", + KeyUserAgent: "test-agent", + }, + }, + { + name: "XFCC with special characters", + headers: map[string]string{ + HeaderXForwardedClientCert: "By=spiffe://example.com/service=my-service_v1/service_instance=instance-1.0;Hash=abc123;URI=spiffe://example.com/service=my-service_v1/service_instance=instance-1.0", + HeaderUserAgent: "test-agent", + }, + want: map[string]string{ + KeyURIService: "my-service_v1", + KeyURIServiceInstance: "instance-1.0", + KeyUserAgent: "test-agent", + }, + }, + { + name: "XFCC with comma-separated values (regex stops at = or /)", + headers: map[string]string{ + HeaderXForwardedClientCert: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123;Subject=\"\";URI=spiffe://cluster.local/ns/default/sa/default/service=my-service,service_instance=my-instance", + HeaderUserAgent: "test-agent", + }, + want: map[string]string{ + KeyURIService: "my-service,service_instance", + KeyURIServiceInstance: "my-instance", // extractServiceFromURI extracts this separately + KeyUserAgent: "test-agent", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + headers := &http.Header{} + + for k, v := range tt.headers { + headers.Set(k, v) + } + + got := extractRequesterInfo(headers) + + // Compare maps + if len(got) != len(tt.want) { + t.Errorf("extractRequesterInfo() map length = %d, want %d. Got: %v, Want: %v", len(got), len(tt.want), got, tt.want) + } + + for k, v := range tt.want { + if got[k] != v { + t.Errorf("extractRequesterInfo() [%s] = %v, want %v. Full map: %v", k, got[k], v, got) + } + } + + // Check that username and service are mutually exclusive + if got[KeyUsername] != "" && got[KeyURIService] != "" { + t.Errorf("extractRequesterInfo() should not have both username and uri_service set") + } + }) + } +} + +func TestExtractXFCCInfo(t *testing.T) { + tests := []struct { + name string + xfccHeader string + wantService string + wantServiceInst string + }{ + { + name: "no XFCC header", + xfccHeader: "", + wantService: "", + wantServiceInst: "", + }, + { + name: "XFCC with service and instance", + xfccHeader: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123;Subject=\"\";URI=spiffe://cluster.local/ns/default/sa/default/service=my-service/service_instance=my-instance", + wantService: "my-service", + wantServiceInst: "my-instance", + }, + { + name: "XFCC with only service", + xfccHeader: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123;Subject=\"\";URI=spiffe://cluster.local/ns/default/sa/default/service=my-service", + wantService: "my-service", + wantServiceInst: "", + }, + { + name: "XFCC without URI", + xfccHeader: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123", + wantService: "", + wantServiceInst: "", + }, + { + name: "malformed XFCC header", + xfccHeader: "malformed-header-without-proper-format", + wantService: "", + wantServiceInst: "", + }, + { + name: "XFCC with comma-separated values in URI", + xfccHeader: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123;Subject=\"\";URI=spiffe://cluster.local/ns/default/sa/default/service=my-service,service_instance=my-instance", + wantService: "my-service,service_instance", + wantServiceInst: "my-instance", // extractServiceFromURI extracts this separately + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + headers := &http.Header{} + if tt.xfccHeader != "" { + headers.Set(HeaderXForwardedClientCert, tt.xfccHeader) + } + + gotService, gotServiceInst := extractXFCCInfo(headers) + if gotService != tt.wantService { + t.Errorf("extractXFCCInfo() service = %v, want %v", gotService, tt.wantService) + } + if gotServiceInst != tt.wantServiceInst { + t.Errorf("extractXFCCInfo() serviceInstance = %v, want %v", gotServiceInst, tt.wantServiceInst) + } + }) + } +} + +func TestExtractField(t *testing.T) { + tests := []struct { + name string + text string + field string + want string + }{ + { + name: "extract service from comma-separated", + text: "service=my-service,service_instance=my-instance,other=value", + field: "service=", + want: "my-service,service_instance", // Stops at next =, not comma + }, + { + name: "extract service_instance from comma-separated", + text: "service=my-service,service_instance=my-instance,other=value", + field: "service_instance=", + want: "my-instance,other", // Stops at next =, not comma + }, + { + name: "extract service with slash separator", + text: "service=my-service/service_instance=my-instance", + field: "service=", + want: "my-service", // Stops at / + }, + { + name: "extract service_instance with slash separator", + text: "service=my-service/service_instance=my-instance", + field: "service_instance=", + want: "my-instance", + }, + { + name: "non-existent field", + text: "service=my-service,service_instance=my-instance", + field: "non_existent=", + want: "", + }, + { + name: "field with spaces", + text: "service= my service /service_instance= my instance ", + field: "service=", + want: "my service", // Stops at / + }, + { + name: "field with special characters", + text: "service=my-service_v1/service_instance=instance-1.0", + field: "service=", + want: "my-service_v1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractField(tt.text, tt.field) + if got != tt.want { + t.Errorf("extractField(%q, %q) = %v, want %v", tt.text, tt.field, got, tt.want) + } + }) + } +} + +func TestExtractServiceFromURI(t *testing.T) { + tests := []struct { + name string + uri string + wantService string + wantServiceInst string + }{ + { + name: "both service and service_instance with slash", + uri: "spiffe://cluster.local/ns/default/sa/default/service=my-service/service_instance=my-instance", + wantService: "my-service", + wantServiceInst: "my-instance", + }, + { + name: "only service with slash", + uri: "spiffe://cluster.local/ns/default/sa/default/service=my-service", + wantService: "my-service", + wantServiceInst: "", + }, + { + name: "no service", + uri: "spiffe://cluster.local/ns/default/sa/default", + wantService: "", + wantServiceInst: "", + }, + { + name: "both service and service_instance with comma", + uri: "spiffe://cluster.local/ns/default/sa/default/service=my-service,service_instance=my-instance", + wantService: "my-service,service_instance", + wantServiceInst: "my-instance", // extractField extracts this separately + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotService, gotServiceInst := extractServiceFromURI(tt.uri) + if gotService != tt.wantService { + t.Errorf("extractServiceFromURI() service = %v, want %v", gotService, tt.wantService) + } + if gotServiceInst != tt.wantServiceInst { + t.Errorf("extractServiceFromURI() serviceInstance = %v, want %v", gotServiceInst, tt.wantServiceInst) + } + }) + } +} + +func TestMergeMaps(t *testing.T) { + tests := []struct { + name string + base map[string]string + additional map[string]string + want map[string]string + }{ + { + name: "merge with override", + base: map[string]string{"key1": "value1", "key2": "value2"}, + additional: map[string]string{"key2": "newvalue2", "key3": "value3"}, + want: map[string]string{"key1": "value1", "key2": "newvalue2", "key3": "value3"}, + }, + { + name: "empty additional", + base: map[string]string{"key1": "value1"}, + additional: map[string]string{}, + want: map[string]string{"key1": "value1"}, + }, + { + name: "empty base", + base: map[string]string{}, + additional: map[string]string{"key1": "value1"}, + want: map[string]string{"key1": "value1"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := mergeMaps(tt.base, tt.additional) + if len(got) != len(tt.want) { + t.Errorf("mergeMaps() map length = %d, want %d", len(got), len(tt.want)) + } + for k, v := range tt.want { + if got[k] != v { + t.Errorf("mergeMaps() [%s] = %v, want %v", k, got[k], v) + } + } + }) + } +} + +func TestPublishHeaderMetrics(t *testing.T) { + tests := []struct { + name string + enableMetrics bool + headers map[string]string + additionalTags map[string]string + expectedMetric string + expectedCount int + expectedLabels []string + }{ + { + name: "username present with metrics enabled", + enableMetrics: true, + headers: map[string]string{ + HeaderXUsername: "testuser", + HeaderUserAgent: "test-agent", + }, + additionalTags: map[string]string{}, + expectedMetric: MetricUserRequests, + expectedCount: 1, + expectedLabels: []string{"testuser", "test-agent", "", ""}, + }, + { + name: "no username, no service with metrics enabled", + enableMetrics: true, + headers: map[string]string{ + HeaderUserAgent: "test-agent", + }, + additionalTags: map[string]string{}, + expectedMetric: "", + expectedCount: 0, + expectedLabels: nil, + }, + { + name: "metrics disabled", + enableMetrics: false, + headers: map[string]string{ + HeaderXUsername: "testuser", + HeaderUserAgent: "test-agent", + }, + additionalTags: map[string]string{}, + expectedMetric: "", + expectedCount: 0, + expectedLabels: nil, + }, + { + name: "XFCC header with metrics enabled", + enableMetrics: true, + headers: map[string]string{ + HeaderXForwardedClientCert: "By=spiffe://example.com/service=myservice/service_instance=instance1;Hash=abc123;URI=spiffe://example.com/service=myservice/service_instance=instance1", + HeaderUserAgent: "test-agent", + }, + additionalTags: map[string]string{}, + expectedMetric: MetricServiceRequests, + expectedCount: 1, + expectedLabels: []string{"test-agent", "myservice", "instance1"}, + }, + { + name: "XFCC with only service", + enableMetrics: true, + headers: map[string]string{ + HeaderXForwardedClientCert: "By=spiffe://example.com/service=myservice;Hash=abc123;URI=spiffe://example.com/service=myservice", + HeaderUserAgent: "test-agent", + }, + additionalTags: map[string]string{}, + expectedMetric: MetricServiceRequests, + expectedCount: 1, + expectedLabels: []string{"test-agent", "myservice", ""}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Reset metrics before test + userRequestCounter.Reset() + serviceRequestCounter.Reset() + + // Set up headers + headers := &http.Header{} + for k, v := range tt.headers { + headers.Set(k, v) + } + + // Call the function + PublishHeaderMetrics(headers, tt.additionalTags, tt.enableMetrics) + + // Check results + if tt.expectedMetric == MetricUserRequests { + count := testutil.ToFloat64(userRequestCounter.WithLabelValues(tt.expectedLabels...)) + if int(count) != tt.expectedCount { + t.Errorf("PublishHeaderMetrics() userRequestCounter count = %d, want %d", int(count), tt.expectedCount) + } + } else if tt.expectedMetric == MetricServiceRequests { + count := testutil.ToFloat64(serviceRequestCounter.WithLabelValues(tt.expectedLabels...)) + if int(count) != tt.expectedCount { + t.Errorf("PublishHeaderMetrics() serviceRequestCounter count = %d, want %d", int(count), tt.expectedCount) + } + } else { + // No metric should be published + userCount := testutil.CollectAndCount(userRequestCounter) + serviceCount := testutil.CollectAndCount(serviceRequestCounter) + if userCount+serviceCount != 0 { + t.Errorf("PublishHeaderMetrics() should not publish metrics, but got userCount=%d, serviceCount=%d", userCount, serviceCount) + } + } + }) + } +} + +// Legacy compatibility tests +func TestExtractUserInfo(t *testing.T) { + req, _ := http.NewRequest("GET", "/api/v1/pods", nil) + + // Test with no headers + userInfo := extractUserInfo(req) + if userInfo.Email != "" { + t.Errorf("Expected email to be empty, got %s", userInfo.Email) + } + if userInfo.Username != "" { + t.Errorf("Expected username to be empty, got %s", userInfo.Username) + } + if userInfo.UserAgent != "" { + t.Errorf("Expected user agent to be empty, got %s", userInfo.UserAgent) + } + + // Test with headers + req.Header.Set("x-username", "testuser") + req.Header.Set("User-Agent", "Mozilla/5.0") + + userInfo = extractUserInfo(req) + if userInfo.Username != "testuser" { + t.Errorf("Expected username to be 'testuser', got %s", userInfo.Username) + } + if userInfo.UserAgent != "Mozilla/5.0" { + t.Errorf("Expected user agent to be 'Mozilla/5.0', got %s", userInfo.UserAgent) + } +} + +func TestExtractUserInfoCaseSensitivity(t *testing.T) { + req, _ := http.NewRequest("GET", "/api/v1/pods", nil) + + // Test case sensitivity - HTTP headers are case-insensitive, so setting both + // will result in the last one set being returned + req.Header.Set("x-username", "lowercase") + req.Header.Set("X-Username", "uppercase") + + userInfo := extractUserInfo(req) + // HTTP headers are case-insensitive, so the last one set wins + if userInfo.Username != "uppercase" { + t.Errorf("Expected username to be 'uppercase', got %s", userInfo.Username) + } +} + +func TestExtractUserInfoEmptyHeaders(t *testing.T) { + req, _ := http.NewRequest("GET", "/api/v1/pods", nil) + + // Test with empty header values + req.Header.Set("x-username", "") + req.Header.Set("User-Agent", "") + + userInfo := extractUserInfo(req) + if userInfo.Username != "" { + t.Errorf("Expected username to be empty for empty header, got %s", userInfo.Username) + } + if userInfo.UserAgent != "" { + t.Errorf("Expected user agent to be empty for empty header, got %s", userInfo.UserAgent) + } +} diff --git a/pkg/sloop/webserver/middleware.go b/pkg/sloop/webserver/middleware.go index 910fa348..99b65a31 100644 --- a/pkg/sloop/webserver/middleware.go +++ b/pkg/sloop/webserver/middleware.go @@ -17,6 +17,8 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/client_golang/prometheus/promhttp" + + "github.com/salesforce/sloop/pkg/sloop/server/server_metrics" ) const requestIDKey string = "reqId" @@ -71,6 +73,20 @@ func glogMiddleware(next http.Handler) http.Handler { }) } +// userMetricsMiddleware collects user metrics from request headers +func userMetricsMiddleware(handlerName string, next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Call the next handler first + next.ServeHTTP(w, r) + + // Collect metrics after the request is processed + additionalTags := map[string]string{ + "handler": handlerName, + } + server_metrics.PublishHeaderMetrics(&r.Header, additionalTags, enableUserMetrics) + }) +} + func middlewareChain(handlerName string, next http.Handler) http.HandlerFunc { return promhttp.InstrumentHandlerCounter( metricWebServerRequestCount.MustCurryWith(prometheus.Labels{"handler": handlerName}), @@ -78,7 +94,7 @@ func middlewareChain(handlerName string, next http.Handler) http.HandlerFunc { metricWebServerRequestDuration.MustCurryWith(prometheus.Labels{"handler": handlerName}), traceMiddleware( glogMiddleware( - next, + userMetricsMiddleware(handlerName, next), ), ), ), @@ -87,5 +103,6 @@ func middlewareChain(handlerName string, next http.Handler) http.HandlerFunc { func metricCountMiddleware(handlerName string, next http.Handler) http.HandlerFunc { return promhttp.InstrumentHandlerCounter( - metricWebServerRequestCount.MustCurryWith(prometheus.Labels{"handler": handlerName}), next) + metricWebServerRequestCount.MustCurryWith(prometheus.Labels{"handler": handlerName}), + userMetricsMiddleware(handlerName, next)) } diff --git a/pkg/sloop/webserver/webserver.go b/pkg/sloop/webserver/webserver.go index c39d8e6f..cbabd742 100644 --- a/pkg/sloop/webserver/webserver.go +++ b/pkg/sloop/webserver/webserver.go @@ -51,22 +51,24 @@ const ( ) type WebConfig struct { - BindAddress string - Port int - WebFilesPath string - DefaultNamespace string - DefaultLookback string - DefaultResources string - MaxLookback time.Duration - ConfigYaml string - ResourceLinks []ResourceLinkTemplate - LeftBarLinks []LinkTemplate - CurrentContext string + BindAddress string + Port int + WebFilesPath string + DefaultNamespace string + DefaultLookback string + DefaultResources string + MaxLookback time.Duration + ConfigYaml string + ResourceLinks []ResourceLinkTemplate + LeftBarLinks []LinkTemplate + CurrentContext string + EnableUserMetrics bool } // This is not going to change and we don't want to pass it to every function // so use a static for now var webFilesPath string +var enableUserMetrics bool func logWebError(err error, note string, r *http.Request, w http.ResponseWriter) { message := fmt.Sprintf("Error rendering url: %q. Note: %v. Error: %v", r.URL, note, err) @@ -221,6 +223,7 @@ func registerRoutes(mux *http.ServeMux, config WebConfig, tables typed.Tables) { func Run(config WebConfig, tables typed.Tables) error { webFilesPath = config.WebFilesPath + enableUserMetrics = config.EnableUserMetrics mux := http.NewServeMux() registerRoutes(mux, config, tables) From f888fe0b745097c0d63ef28339a6d77088a7ada2 Mon Sep 17 00:00:00 2001 From: Omer Sasson Date: Mon, 8 Dec 2025 19:20:48 -0500 Subject: [PATCH 2/2] update to only send user metrics --- pkg/sloop/server/internal/config/config.go | 8 +- pkg/sloop/server/server.go | 6 + .../server/server_metrics/server_metrics.go | 334 ++++------ .../server_metrics/server_metrics_test.go | 573 +++++------------- pkg/sloop/webserver/middleware.go | 21 +- 5 files changed, 279 insertions(+), 663 deletions(-) diff --git a/pkg/sloop/server/internal/config/config.go b/pkg/sloop/server/internal/config/config.go index 56045fb1..3dc1eda8 100644 --- a/pkg/sloop/server/internal/config/config.go +++ b/pkg/sloop/server/internal/config/config.go @@ -20,6 +20,7 @@ import ( "github.com/golang/glog" "github.com/pkg/errors" + "github.com/salesforce/sloop/pkg/sloop/server/server_metrics" "github.com/salesforce/sloop/pkg/sloop/webserver" ) @@ -29,9 +30,10 @@ type SloopConfig struct { // These fields can only come from command line ConfigFile string // These fields can only come from file because they use complex types - LeftBarLinks []webserver.LinkTemplate `json:"leftBarLinks"` - ResourceLinks []webserver.ResourceLinkTemplate `json:"resourceLinks"` - ExclusionRules map[string][]any `json:"exclusionRules"` + LeftBarLinks []webserver.LinkTemplate `json:"leftBarLinks"` + ResourceLinks []webserver.ResourceLinkTemplate `json:"resourceLinks"` + ExclusionRules map[string][]any `json:"exclusionRules"` + UserMetricsHeaders []server_metrics.UserMetricsConfig `json:"userMetricsHeaders"` // Normal fields that can come from file or cmd line DisableKubeWatcher bool `json:"disableKubeWatch"` KubeWatchResyncInterval time.Duration `json:"kubeWatchResyncInterval"` diff --git a/pkg/sloop/server/server.go b/pkg/sloop/server/server.go index d9401977..a33a0949 100644 --- a/pkg/sloop/server/server.go +++ b/pkg/sloop/server/server.go @@ -18,6 +18,7 @@ import ( "github.com/salesforce/sloop/pkg/sloop/ingress" "github.com/salesforce/sloop/pkg/sloop/server/internal/config" + "github.com/salesforce/sloop/pkg/sloop/server/server_metrics" "github.com/salesforce/sloop/pkg/sloop/store/typed" "github.com/salesforce/sloop/pkg/sloop/store/untyped" @@ -146,6 +147,11 @@ func RealMain() error { displayContext = conf.DisplayContext } + // Initialize user metrics if enabled + if conf.EnableUserMetrics { + server_metrics.InitUserMetrics(conf.UserMetricsHeaders) + } + webConfig := webserver.WebConfig{ BindAddress: conf.BindAddress, Port: conf.Port, diff --git a/pkg/sloop/server/server_metrics/server_metrics.go b/pkg/sloop/server/server_metrics/server_metrics.go index b50e01c8..1926031b 100644 --- a/pkg/sloop/server/server_metrics/server_metrics.go +++ b/pkg/sloop/server/server_metrics/server_metrics.go @@ -9,73 +9,119 @@ package server_metrics import ( "net/http" - "regexp" - "strings" + "sort" + "sync" "github.com/golang/glog" "github.com/prometheus/client_golang/prometheus" ) const ( - // Header names - HeaderXUsername = "X-Username" - HeaderXUsernameLower = "x-username" - HeaderUserAgent = "User-Agent" - HeaderXForwardedClientCert = "X-Forwarded-Client-Cert" - // Metric names - MetricUserRequests = "user_requests" - MetricServiceRequests = "service_requests" - - // label keys - KeyUsername = "username" - KeyUserAgent = "user_agent" - KeyURIService = "uri_service" - KeyURIServiceInstance = "uri_service_instance" - - // URI field names - FieldService = "service=" - FieldServiceInstance = "service_instance=" + MetricUserRequests = "user_requests" + + // Built-in request context label keys (always included) + LabelHandler = "handler" + LabelCluster = "cluster" + LabelQuery = "query" + LabelNamespace = "namespace" + LabelKind = "kind" + LabelName = "name" + LabelLookback = "lookback" ) +// UserMetricsConfig defines a single header extraction rule for user metrics +type UserMetricsConfig struct { + // Label is the Prometheus metric label name (e.g., "username", "email", "team") + Label string `json:"label"` + // Headers is a list of HTTP header names to check, in order of preference + // The first non-empty header value found will be used + Headers []string `json:"headers"` +} + var ( - userRequestCounter = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: MetricUserRequests, - Help: "Counter of dashboard requests broken out by user information from headers.", - }, - []string{KeyUsername, KeyUserAgent, KeyURIService, KeyURIServiceInstance}, - ) + userRequestCounter *prometheus.CounterVec + configuredLabels []string + configuredHeaderRules []UserMetricsConfig + initOnce sync.Once - serviceRequestCounter = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Name: MetricServiceRequests, - Help: "Counter of dashboard requests broken out by service information from headers.", - }, - []string{KeyUserAgent, KeyURIService, KeyURIServiceInstance}, - ) + // Built-in labels that are always included for request context + builtInLabels = []string{LabelHandler, LabelCluster, LabelQuery, LabelNamespace, LabelKind, LabelName, LabelLookback} ) -// Initialize user metrics in prometheus -func init() { - prometheus.MustRegister(userRequestCounter) - prometheus.MustRegister(serviceRequestCounter) +// GetDefaultUserMetricsConfig returns the default configuration for user metrics (username and user_agent) +func GetDefaultUserMetricsConfig() []UserMetricsConfig { + return []UserMetricsConfig{ + { + Label: "username", + Headers: []string{"X-Username", "x-username"}, + }, + { + Label: "user_agent", + Headers: []string{"User-Agent"}, + }, + } } -// EnsureUserMetricsRegistered ensures user metrics are registered -// This can be called after configuration is loaded to ensure proper initialization -func EnsureUserMetricsRegistered() { - // Metrics are already registered in init(), but this provides a hook - // for any future initialization needs +// InitUserMetrics initializes the user metrics with the given configuration +// This should be called once during application startup before serving requests +// If config is nil or empty, default configuration will be used +func InitUserMetrics(config []UserMetricsConfig) { + initOnce.Do(func() { + // Use default config if none provided + if len(config) == 0 { + config = GetDefaultUserMetricsConfig() + glog.Infof("No user metrics configuration provided, using defaults") + } + + // Store the configuration + configuredHeaderRules = config + + // Start with built-in labels for request context + labelSet := make(map[string]bool) + for _, label := range builtInLabels { + labelSet[label] = true + } + + // Add header-based labels from configuration + for _, rule := range config { + if rule.Label != "" { + labelSet[rule.Label] = true + } + } + + // Sort labels for consistent ordering + configuredLabels = make([]string, 0, len(labelSet)) + for label := range labelSet { + configuredLabels = append(configuredLabels, label) + } + sort.Strings(configuredLabels) + + // Create the counter with configured labels + userRequestCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: MetricUserRequests, + Help: "Counter of requests broken out by user and request information.", + }, + configuredLabels, + ) + + // Register with Prometheus + prometheus.MustRegister(userRequestCounter) + + glog.Infof("User metrics initialized with labels: %v", configuredLabels) + glog.Infof(" Built-in request labels: %v", builtInLabels) + for _, rule := range config { + glog.Infof(" Header label '%s': headers=%v", rule.Label, rule.Headers) + } + }) } // ============================================================================ // Header Information Extraction Functions // ============================================================================ -// These functions handle extracting user information from HTTP request headers - -// getHeaderWithFallback tries multiple header names and returns the first non-empty value or empty string if no value is found +// getHeaderWithFallback tries multiple header names and returns the first non-empty value func getHeaderWithFallback(headers *http.Header, headerNames []string) string { for _, headerName := range headerNames { if value := headers.Get(headerName); value != "" { @@ -85,99 +131,27 @@ func getHeaderWithFallback(headers *http.Header, headerNames []string) string { return "" } -// extractRequesterInfo extracts user/service information from request headers and returns a map with requester information +// extractRequesterInfo extracts user information from request headers based on configuration func extractRequesterInfo(headers *http.Header) map[string]string { - requesterInfo := map[string]string{} - - // Extract username from headers if present otherwise extract service information from URI - userName := getHeaderWithFallback(headers, []string{HeaderXUsernameLower, HeaderXUsername}) - if userName != "" { - requesterInfo[KeyUsername] = userName - } else { - // Extract XFCC information - requesterInfo[KeyURIService], requesterInfo[KeyURIServiceInstance] = extractXFCCInfo(headers) - } - - requesterInfo[KeyUserAgent] = getHeaderWithFallback(headers, []string{HeaderUserAgent}) + requesterInfo := make(map[string]string) - return requesterInfo -} - -// extractXFCCInfo extracts service information from X-Forwarded-Client-Cert header -func extractXFCCInfo(headers *http.Header) (service, serviceInstance string) { - // Initialize with default values - service = "" - serviceInstance = "" - - clientCert := headers.Get(HeaderXForwardedClientCert) - if clientCert == "" { - glog.V(4).Infof("X-Forwarded-Client-Cert header not found") - return service, serviceInstance - } - - glog.V(4).Infof("X-Forwarded-Client-Cert header found: %s", clientCert) - - // Parse XFCC header - look for URI field with service information - // The XFCC header format is: By=...;Hash=...;Subject=...;URI=spiffe://.../service=value,service_instance=value - if strings.Contains(clientCert, "URI=") { - // Extract the URI portion - uriStart := strings.Index(clientCert, "URI=") - if uriStart != -1 { - uriPart := clientCert[uriStart+4:] // Skip "URI=" - - // Find the end of the URI (next semicolon or end of string) - if semicolonIdx := strings.Index(uriPart, ";"); semicolonIdx != -1 { - uriPart = uriPart[:semicolonIdx] - } - - service, serviceInstance = extractServiceFromURI(uriPart) - if service != "" { - glog.V(4).Infof("Parsed XFCC info - service: '%s', service_instance: '%s'", service, serviceInstance) - return service, serviceInstance - } + for _, rule := range configuredHeaderRules { + if rule.Label != "" && len(rule.Headers) > 0 { + requesterInfo[rule.Label] = getHeaderWithFallback(headers, rule.Headers) } } - // If parsing didn't work, return empty values - glog.V(4).Infof("No service information found in XFCC header") - return service, serviceInstance -} - -// extractServiceFromURI extracts service and service instance from SPIFFE URI -func extractServiceFromURI(uri string) (service, serviceInstance string) { - // Extract service=value from URI - service = extractField(uri, FieldService) - if service != "" { - // Extract service_instance=value from URI - serviceInstance = extractField(uri, FieldServiceInstance) - return service, serviceInstance - } - - // If no service found, return empty strings - return "", "" -} - -// extractField extracts a field value from a string using regex -func extractField(text, field string) string { - // Create regex pattern to match field=value, stopping at next / or = or end of string - pattern := regexp.MustCompile(field + `([^/=]+)`) - matches := pattern.FindStringSubmatch(text) - if len(matches) > 1 { - return strings.TrimSpace(matches[1]) - } - return "" + return requesterInfo } // mergeMaps merges two maps, with values from additionalTags taking precedence func mergeMaps(base, additional map[string]string) map[string]string { result := make(map[string]string) - // Copy base map for k, v := range base { result[k] = v } - // Override with additional tags for k, v := range additional { result[k] = v } @@ -185,117 +159,23 @@ func mergeMaps(base, additional map[string]string) map[string]string { return result } -// publishHeaderMetrics publishes header metrics to the metrics server +// PublishHeaderMetrics publishes header metrics to the metrics server func PublishHeaderMetrics(headers *http.Header, additionalTags map[string]string, enableUserMetrics bool) { if !enableUserMetrics { return } userInfo := extractRequesterInfo(headers) + allLabels := mergeMaps(userInfo, additionalTags) - // merge the userInfo and additionalTags maps into a new map - labels := mergeMaps(userInfo, additionalTags) - - // if username is present it is a user request - if labels[KeyUsername] != "" { - userRequestCounter.WithLabelValues( - labels[KeyUsername], - labels[KeyUserAgent], - labels[KeyURIService], - labels[KeyURIServiceInstance], - ).Inc() - return - } - - // if uri_service is present it is a service request - if labels[KeyURIService] != "" { - serviceRequestCounter.WithLabelValues( - labels[KeyUserAgent], - labels[KeyURIService], - labels[KeyURIServiceInstance], - ).Inc() - return - } - - glog.Warningf("Failed to determine the request type") -} - -// ============================================================================ -// Legacy compatibility functions -// ============================================================================ - -// These functions maintain compatibility with the existing filter.go code - -// UserInfo contains user information extracted from request headers -type UserInfo struct { - Email string - Username string - UserAgent string - Service string - ServiceInstance string -} - -// requestMeta contains metadata about the request for metrics -type requestMeta struct { - userInfo UserInfo -} - -// extractUserInfo extracts user information from request headers and returns a UserInfo struct -// This is a compatibility wrapper for the existing filter.go code -func extractUserInfo(request *http.Request) UserInfo { - userInfo := UserInfo{} - - // Extract standard headers - userInfo.Username = getHeaderWithFallback(&request.Header, []string{HeaderXUsernameLower, HeaderXUsername}) - userInfo.UserAgent = getHeaderWithFallback(&request.Header, []string{HeaderUserAgent}) - - // Extract XFCC information - userInfo.Service, userInfo.ServiceInstance = extractXFCCInfo(&request.Header) - - return userInfo -} - -// publishUserMetrics publishes user-specific metrics if enabled -// This is a compatibility wrapper for the existing filter.go code -func publishUserMetrics(requestMeta requestMeta, resourceRequest *string, enableUserMetrics bool) { - if !enableUserMetrics { - return - } - - // Build additional tags from resource request - additionalTags := make(map[string]string) - if resourceRequest != nil { - additionalTags["resource"] = *resourceRequest + // Build label values in the correct order + labelValues := make([]string, len(configuredLabels)) + for i, label := range configuredLabels { + labelValues[i] = allLabels[label] } - // Create a temporary request to extract headers - // Since we already have the userInfo, we'll construct the labels directly - labels := make(map[string]string) - labels[KeyUserAgent] = requestMeta.userInfo.UserAgent - labels[KeyURIService] = requestMeta.userInfo.Service - labels[KeyURIServiceInstance] = requestMeta.userInfo.ServiceInstance - - // if username is present it is a user request - if requestMeta.userInfo.Username != "" { - labels[KeyUsername] = requestMeta.userInfo.Username - userRequestCounter.WithLabelValues( - labels[KeyUsername], - labels[KeyUserAgent], - labels[KeyURIService], - labels[KeyURIServiceInstance], - ).Inc() - return - } - - // if uri_service is present it is a service request - if labels[KeyURIService] != "" { - serviceRequestCounter.WithLabelValues( - labels[KeyUserAgent], - labels[KeyURIService], - labels[KeyURIServiceInstance], - ).Inc() - return - } + userRequestCounter.WithLabelValues(labelValues...).Inc() - glog.Warningf("Failed to determine the request type") + // Log user request at V(2) verbosity level for debugging + glog.V(2).Infof("User request made with labels: %v", allLabels) } diff --git a/pkg/sloop/server/server_metrics/server_metrics_test.go b/pkg/sloop/server/server_metrics/server_metrics_test.go index 0ef616b7..994c347e 100644 --- a/pkg/sloop/server/server_metrics/server_metrics_test.go +++ b/pkg/sloop/server/server_metrics/server_metrics_test.go @@ -9,11 +9,31 @@ package server_metrics import ( "net/http" + "sync" "testing" - "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/prometheus/client_golang/prometheus" ) +// testOnce ensures metrics are only initialized once across all tests +var testOnce sync.Once + +// initTestMetrics initializes metrics once for all tests using the default config +func initTestMetrics() { + testOnce.Do(func() { + // Reset in case previous test run left state + if userRequestCounter != nil { + prometheus.Unregister(userRequestCounter) + } + initOnce = sync.Once{} + configuredLabels = nil + configuredHeaderRules = nil + userRequestCounter = nil + + InitUserMetrics(GetDefaultUserMetricsConfig()) + }) +} + func TestGetHeaderWithFallback(t *testing.T) { tests := []struct { name string @@ -63,307 +83,76 @@ func TestGetHeaderWithFallback(t *testing.T) { } } -func TestExtractRequesterInfo(t *testing.T) { - tests := []struct { - name string - headers map[string]string - want map[string]string - }{ - { - name: "username present", - headers: map[string]string{HeaderXUsername: "testuser", HeaderUserAgent: "test-agent"}, - want: map[string]string{ - KeyUsername: "testuser", - KeyUserAgent: "test-agent", - }, - }, - { - name: "no username, has XFCC with service and instance", - headers: map[string]string{ - HeaderXForwardedClientCert: "By=spiffe://example.com/service=myservice/service_instance=instance1;Hash=abc123;URI=spiffe://example.com/service=myservice/service_instance=instance1", - HeaderUserAgent: "test-agent", - }, - want: map[string]string{ - KeyURIService: "myservice", - KeyURIServiceInstance: "instance1", - KeyUserAgent: "test-agent", - }, - }, - { - name: "no username, has XFCC with only service", - headers: map[string]string{ - HeaderXForwardedClientCert: "By=spiffe://example.com/service=myservice;Hash=abc123;URI=spiffe://example.com/service=myservice", - HeaderUserAgent: "test-agent", - }, - want: map[string]string{ - KeyURIService: "myservice", - KeyURIServiceInstance: "", - KeyUserAgent: "test-agent", - }, - }, - { - name: "no username, has XFCC without URI", - headers: map[string]string{ - HeaderXForwardedClientCert: "By=spiffe://example.com;Hash=abc123", - HeaderUserAgent: "test-agent", - }, - want: map[string]string{ - KeyURIService: "", - KeyURIServiceInstance: "", - KeyUserAgent: "test-agent", - }, - }, - { - name: "no username, no XFCC", - headers: map[string]string{HeaderUserAgent: "test-agent"}, - want: map[string]string{ - KeyURIService: "", - KeyURIServiceInstance: "", - KeyUserAgent: "test-agent", - }, - }, - { - name: "case insensitive username", - headers: map[string]string{HeaderXUsernameLower: "testuser", HeaderUserAgent: "test-agent"}, - want: map[string]string{ - KeyUsername: "testuser", - KeyUserAgent: "test-agent", - }, - }, - { - name: "XFCC with spaces in service names", - headers: map[string]string{ - HeaderXForwardedClientCert: "By=spiffe://example.com/service= my service /service_instance= my instance ;Hash=abc123;URI=spiffe://example.com/service= my service /service_instance= my instance ", - HeaderUserAgent: "test-agent", - }, - want: map[string]string{ - KeyURIService: "my service", - KeyURIServiceInstance: "my instance", - KeyUserAgent: "test-agent", - }, - }, - { - name: "XFCC with special characters", - headers: map[string]string{ - HeaderXForwardedClientCert: "By=spiffe://example.com/service=my-service_v1/service_instance=instance-1.0;Hash=abc123;URI=spiffe://example.com/service=my-service_v1/service_instance=instance-1.0", - HeaderUserAgent: "test-agent", - }, - want: map[string]string{ - KeyURIService: "my-service_v1", - KeyURIServiceInstance: "instance-1.0", - KeyUserAgent: "test-agent", - }, - }, - { - name: "XFCC with comma-separated values (regex stops at = or /)", - headers: map[string]string{ - HeaderXForwardedClientCert: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123;Subject=\"\";URI=spiffe://cluster.local/ns/default/sa/default/service=my-service,service_instance=my-instance", - HeaderUserAgent: "test-agent", - }, - want: map[string]string{ - KeyURIService: "my-service,service_instance", - KeyURIServiceInstance: "my-instance", // extractServiceFromURI extracts this separately - KeyUserAgent: "test-agent", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - headers := &http.Header{} +func TestGetDefaultUserMetricsConfig(t *testing.T) { + config := GetDefaultUserMetricsConfig() - for k, v := range tt.headers { - headers.Set(k, v) - } - - got := extractRequesterInfo(headers) - - // Compare maps - if len(got) != len(tt.want) { - t.Errorf("extractRequesterInfo() map length = %d, want %d. Got: %v, Want: %v", len(got), len(tt.want), got, tt.want) - } - - for k, v := range tt.want { - if got[k] != v { - t.Errorf("extractRequesterInfo() [%s] = %v, want %v. Full map: %v", k, got[k], v, got) - } - } + if len(config) != 2 { + t.Errorf("GetDefaultUserMetricsConfig() returned %d configs, want 2", len(config)) + } - // Check that username and service are mutually exclusive - if got[KeyUsername] != "" && got[KeyURIService] != "" { - t.Errorf("extractRequesterInfo() should not have both username and uri_service set") + // Check username config + usernameFound := false + for _, c := range config { + if c.Label == "username" { + usernameFound = true + if len(c.Headers) < 1 { + t.Errorf("username config should have at least one header") } - }) + } } -} - -func TestExtractXFCCInfo(t *testing.T) { - tests := []struct { - name string - xfccHeader string - wantService string - wantServiceInst string - }{ - { - name: "no XFCC header", - xfccHeader: "", - wantService: "", - wantServiceInst: "", - }, - { - name: "XFCC with service and instance", - xfccHeader: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123;Subject=\"\";URI=spiffe://cluster.local/ns/default/sa/default/service=my-service/service_instance=my-instance", - wantService: "my-service", - wantServiceInst: "my-instance", - }, - { - name: "XFCC with only service", - xfccHeader: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123;Subject=\"\";URI=spiffe://cluster.local/ns/default/sa/default/service=my-service", - wantService: "my-service", - wantServiceInst: "", - }, - { - name: "XFCC without URI", - xfccHeader: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123", - wantService: "", - wantServiceInst: "", - }, - { - name: "malformed XFCC header", - xfccHeader: "malformed-header-without-proper-format", - wantService: "", - wantServiceInst: "", - }, - { - name: "XFCC with comma-separated values in URI", - xfccHeader: "By=spiffe://cluster.local/ns/default/sa/default;Hash=abc123;Subject=\"\";URI=spiffe://cluster.local/ns/default/sa/default/service=my-service,service_instance=my-instance", - wantService: "my-service,service_instance", - wantServiceInst: "my-instance", // extractServiceFromURI extracts this separately - }, + if !usernameFound { + t.Errorf("GetDefaultUserMetricsConfig() should include username label") } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - headers := &http.Header{} - if tt.xfccHeader != "" { - headers.Set(HeaderXForwardedClientCert, tt.xfccHeader) - } - - gotService, gotServiceInst := extractXFCCInfo(headers) - if gotService != tt.wantService { - t.Errorf("extractXFCCInfo() service = %v, want %v", gotService, tt.wantService) - } - if gotServiceInst != tt.wantServiceInst { - t.Errorf("extractXFCCInfo() serviceInstance = %v, want %v", gotServiceInst, tt.wantServiceInst) - } - }) + // Check user_agent config + userAgentFound := false + for _, c := range config { + if c.Label == "user_agent" { + userAgentFound = true + } + } + if !userAgentFound { + t.Errorf("GetDefaultUserMetricsConfig() should include user_agent label") } } -func TestExtractField(t *testing.T) { - tests := []struct { - name string - text string - field string - want string - }{ - { - name: "extract service from comma-separated", - text: "service=my-service,service_instance=my-instance,other=value", - field: "service=", - want: "my-service,service_instance", // Stops at next =, not comma - }, - { - name: "extract service_instance from comma-separated", - text: "service=my-service,service_instance=my-instance,other=value", - field: "service_instance=", - want: "my-instance,other", // Stops at next =, not comma - }, - { - name: "extract service with slash separator", - text: "service=my-service/service_instance=my-instance", - field: "service=", - want: "my-service", // Stops at / - }, - { - name: "extract service_instance with slash separator", - text: "service=my-service/service_instance=my-instance", - field: "service_instance=", - want: "my-instance", - }, - { - name: "non-existent field", - text: "service=my-service,service_instance=my-instance", - field: "non_existent=", - want: "", - }, - { - name: "field with spaces", - text: "service= my service /service_instance= my instance ", - field: "service=", - want: "my service", // Stops at / - }, - { - name: "field with special characters", - text: "service=my-service_v1/service_instance=instance-1.0", - field: "service=", - want: "my-service_v1", - }, - } +func TestInitUserMetricsIncludesBuiltInLabels(t *testing.T) { + initTestMetrics() - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := extractField(tt.text, tt.field) - if got != tt.want { - t.Errorf("extractField(%q, %q) = %v, want %v", tt.text, tt.field, got, tt.want) - } - }) + // Check that built-in labels are included + labelMap := make(map[string]bool) + for _, label := range configuredLabels { + labelMap[label] = true } -} -func TestExtractServiceFromURI(t *testing.T) { - tests := []struct { - name string - uri string - wantService string - wantServiceInst string - }{ - { - name: "both service and service_instance with slash", - uri: "spiffe://cluster.local/ns/default/sa/default/service=my-service/service_instance=my-instance", - wantService: "my-service", - wantServiceInst: "my-instance", - }, - { - name: "only service with slash", - uri: "spiffe://cluster.local/ns/default/sa/default/service=my-service", - wantService: "my-service", - wantServiceInst: "", - }, - { - name: "no service", - uri: "spiffe://cluster.local/ns/default/sa/default", - wantService: "", - wantServiceInst: "", - }, - { - name: "both service and service_instance with comma", - uri: "spiffe://cluster.local/ns/default/sa/default/service=my-service,service_instance=my-instance", - wantService: "my-service,service_instance", - wantServiceInst: "my-instance", // extractField extracts this separately - }, + if !labelMap[LabelHandler] { + t.Errorf("configuredLabels should include '%s'", LabelHandler) + } + if !labelMap[LabelCluster] { + t.Errorf("configuredLabels should include '%s'", LabelCluster) + } + if !labelMap[LabelQuery] { + t.Errorf("configuredLabels should include '%s'", LabelQuery) + } + if !labelMap[LabelNamespace] { + t.Errorf("configuredLabels should include '%s'", LabelNamespace) + } + if !labelMap[LabelKind] { + t.Errorf("configuredLabels should include '%s'", LabelKind) + } + if !labelMap[LabelName] { + t.Errorf("configuredLabels should include '%s'", LabelName) + } + if !labelMap[LabelLookback] { + t.Errorf("configuredLabels should include '%s'", LabelLookback) } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotService, gotServiceInst := extractServiceFromURI(tt.uri) - if gotService != tt.wantService { - t.Errorf("extractServiceFromURI() service = %v, want %v", gotService, tt.wantService) - } - if gotServiceInst != tt.wantServiceInst { - t.Errorf("extractServiceFromURI() serviceInstance = %v, want %v", gotServiceInst, tt.wantServiceInst) - } - }) + // Check that header-based labels are also included + if !labelMap["username"] { + t.Errorf("configuredLabels should include 'username'") + } + if !labelMap["user_agent"] { + t.Errorf("configuredLabels should include 'user_agent'") } } @@ -409,171 +198,93 @@ func TestMergeMaps(t *testing.T) { } } -func TestPublishHeaderMetrics(t *testing.T) { - tests := []struct { - name string - enableMetrics bool - headers map[string]string - additionalTags map[string]string - expectedMetric string - expectedCount int - expectedLabels []string - }{ - { - name: "username present with metrics enabled", - enableMetrics: true, - headers: map[string]string{ - HeaderXUsername: "testuser", - HeaderUserAgent: "test-agent", - }, - additionalTags: map[string]string{}, - expectedMetric: MetricUserRequests, - expectedCount: 1, - expectedLabels: []string{"testuser", "test-agent", "", ""}, - }, - { - name: "no username, no service with metrics enabled", - enableMetrics: true, - headers: map[string]string{ - HeaderUserAgent: "test-agent", - }, - additionalTags: map[string]string{}, - expectedMetric: "", - expectedCount: 0, - expectedLabels: nil, - }, - { - name: "metrics disabled", - enableMetrics: false, - headers: map[string]string{ - HeaderXUsername: "testuser", - HeaderUserAgent: "test-agent", - }, - additionalTags: map[string]string{}, - expectedMetric: "", - expectedCount: 0, - expectedLabels: nil, - }, - { - name: "XFCC header with metrics enabled", - enableMetrics: true, - headers: map[string]string{ - HeaderXForwardedClientCert: "By=spiffe://example.com/service=myservice/service_instance=instance1;Hash=abc123;URI=spiffe://example.com/service=myservice/service_instance=instance1", - HeaderUserAgent: "test-agent", - }, - additionalTags: map[string]string{}, - expectedMetric: MetricServiceRequests, - expectedCount: 1, - expectedLabels: []string{"test-agent", "myservice", "instance1"}, - }, - { - name: "XFCC with only service", - enableMetrics: true, - headers: map[string]string{ - HeaderXForwardedClientCert: "By=spiffe://example.com/service=myservice;Hash=abc123;URI=spiffe://example.com/service=myservice", - HeaderUserAgent: "test-agent", - }, - additionalTags: map[string]string{}, - expectedMetric: MetricServiceRequests, - expectedCount: 1, - expectedLabels: []string{"test-agent", "myservice", ""}, - }, +func TestUserMetricsConfigJSON(t *testing.T) { + // Test that the config structure can be represented correctly + config := UserMetricsConfig{ + Label: "email", + Headers: []string{"X-Email", "X-User-Email", "x-email"}, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Reset metrics before test - userRequestCounter.Reset() - serviceRequestCounter.Reset() - - // Set up headers - headers := &http.Header{} - for k, v := range tt.headers { - headers.Set(k, v) - } - - // Call the function - PublishHeaderMetrics(headers, tt.additionalTags, tt.enableMetrics) - - // Check results - if tt.expectedMetric == MetricUserRequests { - count := testutil.ToFloat64(userRequestCounter.WithLabelValues(tt.expectedLabels...)) - if int(count) != tt.expectedCount { - t.Errorf("PublishHeaderMetrics() userRequestCounter count = %d, want %d", int(count), tt.expectedCount) - } - } else if tt.expectedMetric == MetricServiceRequests { - count := testutil.ToFloat64(serviceRequestCounter.WithLabelValues(tt.expectedLabels...)) - if int(count) != tt.expectedCount { - t.Errorf("PublishHeaderMetrics() serviceRequestCounter count = %d, want %d", int(count), tt.expectedCount) - } - } else { - // No metric should be published - userCount := testutil.CollectAndCount(userRequestCounter) - serviceCount := testutil.CollectAndCount(serviceRequestCounter) - if userCount+serviceCount != 0 { - t.Errorf("PublishHeaderMetrics() should not publish metrics, but got userCount=%d, serviceCount=%d", userCount, serviceCount) - } - } - }) + if config.Label != "email" { + t.Errorf("Expected label 'email', got '%s'", config.Label) + } + if len(config.Headers) != 3 { + t.Errorf("Expected 3 headers, got %d", len(config.Headers)) } } -// Legacy compatibility tests -func TestExtractUserInfo(t *testing.T) { - req, _ := http.NewRequest("GET", "/api/v1/pods", nil) +func TestExtractRequesterInfo(t *testing.T) { + initTestMetrics() // Test with no headers - userInfo := extractUserInfo(req) - if userInfo.Email != "" { - t.Errorf("Expected email to be empty, got %s", userInfo.Email) - } - if userInfo.Username != "" { - t.Errorf("Expected username to be empty, got %s", userInfo.Username) + headers := &http.Header{} + info := extractRequesterInfo(headers) + if info["username"] != "" { + t.Errorf("Expected username to be empty, got %s", info["username"]) } - if userInfo.UserAgent != "" { - t.Errorf("Expected user agent to be empty, got %s", userInfo.UserAgent) + if info["user_agent"] != "" { + t.Errorf("Expected user_agent to be empty, got %s", info["user_agent"]) } // Test with headers - req.Header.Set("x-username", "testuser") - req.Header.Set("User-Agent", "Mozilla/5.0") + headers.Set("X-Username", "testuser") + headers.Set("User-Agent", "Mozilla/5.0") - userInfo = extractUserInfo(req) - if userInfo.Username != "testuser" { - t.Errorf("Expected username to be 'testuser', got %s", userInfo.Username) + info = extractRequesterInfo(headers) + if info["username"] != "testuser" { + t.Errorf("Expected username to be 'testuser', got %s", info["username"]) } - if userInfo.UserAgent != "Mozilla/5.0" { - t.Errorf("Expected user agent to be 'Mozilla/5.0', got %s", userInfo.UserAgent) + if info["user_agent"] != "Mozilla/5.0" { + t.Errorf("Expected user_agent to be 'Mozilla/5.0', got %s", info["user_agent"]) } } -func TestExtractUserInfoCaseSensitivity(t *testing.T) { - req, _ := http.NewRequest("GET", "/api/v1/pods", nil) +func TestExtractRequesterInfoEmptyHeaders(t *testing.T) { + initTestMetrics() - // Test case sensitivity - HTTP headers are case-insensitive, so setting both - // will result in the last one set being returned - req.Header.Set("x-username", "lowercase") - req.Header.Set("X-Username", "uppercase") + headers := &http.Header{} + headers.Set("X-Username", "") + headers.Set("User-Agent", "") - userInfo := extractUserInfo(req) - // HTTP headers are case-insensitive, so the last one set wins - if userInfo.Username != "uppercase" { - t.Errorf("Expected username to be 'uppercase', got %s", userInfo.Username) + info := extractRequesterInfo(headers) + if info["username"] != "" { + t.Errorf("Expected username to be empty for empty header, got %s", info["username"]) + } + if info["user_agent"] != "" { + t.Errorf("Expected user_agent to be empty for empty header, got %s", info["user_agent"]) } } -func TestExtractUserInfoEmptyHeaders(t *testing.T) { - req, _ := http.NewRequest("GET", "/api/v1/pods", nil) +func TestPublishHeaderMetrics(t *testing.T) { + initTestMetrics() + + // Test with metrics disabled - should not panic + headers := &http.Header{} + headers.Set("X-Username", "testuser") + additionalTags := map[string]string{ + LabelHandler: "test", + LabelCluster: "test-cluster", + LabelQuery: "EventHeatMap", + LabelNamespace: "default", + LabelKind: "Pod", + LabelName: "", + LabelLookback: "1h", + } - // Test with empty header values - req.Header.Set("x-username", "") - req.Header.Set("User-Agent", "") + // Should not panic when disabled + PublishHeaderMetrics(headers, additionalTags, false) - userInfo := extractUserInfo(req) - if userInfo.Username != "" { - t.Errorf("Expected username to be empty for empty header, got %s", userInfo.Username) - } - if userInfo.UserAgent != "" { - t.Errorf("Expected user agent to be empty for empty header, got %s", userInfo.UserAgent) + // Should not panic when enabled + PublishHeaderMetrics(headers, additionalTags, true) +} + +func TestLabelCount(t *testing.T) { + initTestMetrics() + + // Default config has 2 header labels (username, user_agent) + 7 built-in labels = 9 total + expectedCount := 2 + len(builtInLabels) + if len(configuredLabels) != expectedCount { + t.Errorf("Expected %d configured labels with default config, got %d: %v", + expectedCount, len(configuredLabels), configuredLabels) } } diff --git a/pkg/sloop/webserver/middleware.go b/pkg/sloop/webserver/middleware.go index 99b65a31..1fb7e546 100644 --- a/pkg/sloop/webserver/middleware.go +++ b/pkg/sloop/webserver/middleware.go @@ -11,6 +11,7 @@ import ( "context" "fmt" "net/http" + "strings" "time" "github.com/golang/glog" @@ -79,10 +80,26 @@ func userMetricsMiddleware(handlerName string, next http.Handler) http.Handler { // Call the next handler first next.ServeHTTP(w, r) - // Collect metrics after the request is processed + // Extract cluster/context from path (first segment after leading slash) + cluster := "" + pathParts := strings.Split(strings.TrimPrefix(r.URL.Path, "/"), "/") + if len(pathParts) > 0 && pathParts[0] != "" { + cluster = pathParts[0] + } + + // Extract query parameters - always include all built-in labels (empty string if not present) + // Prometheus requires all labels to be provided for each metric observation + queryParams := r.URL.Query() additionalTags := map[string]string{ - "handler": handlerName, + server_metrics.LabelHandler: handlerName, + server_metrics.LabelCluster: cluster, + server_metrics.LabelQuery: queryParams.Get("query"), + server_metrics.LabelNamespace: queryParams.Get("namespace"), + server_metrics.LabelKind: queryParams.Get("kind"), + server_metrics.LabelName: queryParams.Get("namematch"), + server_metrics.LabelLookback: queryParams.Get("lookback"), } + server_metrics.PublishHeaderMetrics(&r.Header, additionalTags, enableUserMetrics) }) }