From 0030218bbac131cd9e1d42d1e0244d7f2dbceb9c Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Fri, 19 Jan 2024 09:32:37 +1100 Subject: [PATCH] Reduce cardinality of metrics emitted for requests to the Kubernetes control plane (#123) * Reduce cardinality of rollout_operator_kubernetes_api_client_request_duration_seconds metric by grouping requests to the same endpoint * Add changelog entry --- CHANGELOG.md | 2 +- pkg/instrumentation/kubernetes_api_client.go | 45 ++++++++++++++++- .../kubernetes_api_client_test.go | 49 +++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 pkg/instrumentation/kubernetes_api_client_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b5a832e07..e7f87988f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## main / unreleased -* [ENHANCEMENT] Add metrics for Kubernetes control plane calls. #118 +* [ENHANCEMENT] Add metrics for Kubernetes control plane calls. #118 #123 ## v0.11.0 diff --git a/pkg/instrumentation/kubernetes_api_client.go b/pkg/instrumentation/kubernetes_api_client.go index 49f841d36..d5263ec80 100644 --- a/pkg/instrumentation/kubernetes_api_client.go +++ b/pkg/instrumentation/kubernetes_api_client.go @@ -1,7 +1,9 @@ package instrumentation import ( + "fmt" "net/http" + "regexp" "strconv" "time" @@ -44,7 +46,48 @@ func (k *kubernetesAPIClientInstrumentation) RoundTrip(req *http.Request) (*http resp, err := k.next.RoundTrip(req) duration := time.Since(start) - instrument.ObserveWithExemplar(req.Context(), k.hist.WithLabelValues(req.URL.EscapedPath(), req.Method, strconv.Itoa(resp.StatusCode)), duration.Seconds()) + instrument.ObserveWithExemplar(req.Context(), k.hist.WithLabelValues(urlToResourceDescription(req.URL.EscapedPath()), req.Method, strconv.Itoa(resp.StatusCode)), duration.Seconds()) return resp, err } + +var ( + // Reference: https://kubernetes.io/docs/reference/using-api/api-concepts/#resource-uris + groupAndVersion = `(/api|/apis/(?P[^/]+))/(?P[^/]+)` + typeAndName = `(?P[^/]+)(/(?P[^/]+)(/(?P[^/]+))?)?` + namespacedPattern = regexp.MustCompile(`^` + groupAndVersion + `/namespaces/[^/]+/` + typeAndName + `$`) + nonNamespacedPattern = regexp.MustCompile(`^` + groupAndVersion + `/` + typeAndName + `$`) +) + +func urlToResourceDescription(path string) string { + match := namespacedPattern.FindStringSubmatch(path) + pattern := namespacedPattern + + if match == nil { + match = nonNamespacedPattern.FindStringSubmatch(path) + pattern = nonNamespacedPattern + + if match == nil { + // Path doesn't follow either expected pattern, give up. + return path + } + } + + group := match[pattern.SubexpIndex("group")] + version := match[pattern.SubexpIndex("version")] + resourceType := match[pattern.SubexpIndex("type")] + name := match[pattern.SubexpIndex("name")] + subresourceType := match[pattern.SubexpIndex("subresource")] + + if group == "" { + group = "core" + } + + if subresourceType != "" { + return fmt.Sprintf("%s/%s/%s object %s subresource", group, version, resourceType, subresourceType) + } else if name == "" { + return fmt.Sprintf("%s/%s/%s collection", group, version, resourceType) + } else { + return fmt.Sprintf("%s/%s/%s object", group, version, resourceType) + } +} diff --git a/pkg/instrumentation/kubernetes_api_client_test.go b/pkg/instrumentation/kubernetes_api_client_test.go new file mode 100644 index 000000000..999e9a1b9 --- /dev/null +++ b/pkg/instrumentation/kubernetes_api_client_test.go @@ -0,0 +1,49 @@ +package instrumentation + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestURLExtraction(t *testing.T) { + testCases := map[string]string{ + // Cluster-wide core objects + "/api/v1/nodes": "core/v1/nodes collection", + "/api/v1/nodes/the-node": "core/v1/nodes object", + "/api/v1/nodes/the-node/status": "core/v1/nodes object status subresource", + "/api/v1/namespaces": "core/v1/namespaces collection", + "/api/v1/namespaces/the-namespace": "core/v1/namespaces object", + // Namespaced core objects + "/api/v1/namespaces/the-namespace/pods": "core/v1/pods collection", + "/api/v1/namespaces/the-namespace/pods/the-pod": "core/v1/pods object", + "/api/v1/namespaces/the-namespace/pods/the-pod/status": "core/v1/pods object status subresource", + // Cluster-wide non-core objects + "/apis/admissionregistration.k8s.io/v1/validatingwebhookconfigurations": "admissionregistration.k8s.io/v1/validatingwebhookconfigurations collection", + "/apis/admissionregistration.k8s.io/v1/validatingwebhookconfigurations/the-config": "admissionregistration.k8s.io/v1/validatingwebhookconfigurations object", + "/apis/admissionregistration.k8s.io/v1/validatingwebhookconfigurations/the-config/status": "admissionregistration.k8s.io/v1/validatingwebhookconfigurations object status subresource", + // Namespaced non-core objects + "/apis/apps/v1/namespaces/the-namespace/statefulsets": "apps/v1/statefulsets collection", + "/apis/apps/v1/namespaces/the-namespace/statefulsets/the-statefulset": "apps/v1/statefulsets object", + "/apis/apps/v1/namespaces/the-namespace/statefulsets/the-statefulset/status": "apps/v1/statefulsets object status subresource", + + // Invalid paths + // These cases should never happen given we're using the official client library and it always uses the expected format, but we + // should still do something sensible if these do happen. + "": "", + "/": "/", + "/something-else": "/something-else", + "/api": "/api", + "/api/v1": "/api/v1", + "/apis": "/apis", + "/apis/apps": "/apis/apps", + "/apis/apps/v1": "/apis/apps/v1", + } + + for input, expectedDescription := range testCases { + t.Run(input, func(t *testing.T) { + actualDescription := urlToResourceDescription(input) + require.Equal(t, expectedDescription, actualDescription) + }) + } +}