From bdbf477207164f9f7973509d4b50653b2af42e14 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 10 Jun 2015 12:52:28 -0400 Subject: [PATCH] REST Client metrics capture grows unbounded The URLs were unique by namespace and query parameter value, which means an infinite number of metrics counters could be created. --- pkg/client/request.go | 21 +++++++++++++++++---- pkg/client/request_test.go | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/pkg/client/request.go b/pkg/client/request.go index a5467dac6f4..9fda4e6b0b9 100644 --- a/pkg/client/request.go +++ b/pkg/client/request.go @@ -530,12 +530,25 @@ func (r *Request) URL() *url.URL { return finalURL } -// finalURLTemplate is similar to URL(), but if the request contains name of an object -// (e.g. GET for a specific Pod) it will be substited with "". -func (r *Request) finalURLTemplate() string { +// finalURLTemplate is similar to URL(), but will make all specific parameter values equal +// - instead of name or namespace, "{name}" and "{namespace}" will be used, and all query +// parameters will be reset. This creates a copy of the request so as not to change the +// underyling object. This means some useful request info (like the types of field +// selectors in use) will be lost. +// TODO: preserve field selector keys +func (r Request) finalURLTemplate() string { if len(r.resourceName) != 0 { - r.resourceName = "" + r.resourceName = "{name}" } + if r.namespaceSet && len(r.namespace) != 0 { + r.namespace = "{namespace}" + } + newParams := url.Values{} + v := []string{"{value}"} + for k := range r.params { + newParams[k] = v + } + r.params = newParams return r.URL().String() } diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go index 4b476ec8091..34e6305e5c7 100644 --- a/pkg/client/request_test.go +++ b/pkg/client/request_test.go @@ -215,6 +215,24 @@ func TestResultIntoWithErrReturnsErr(t *testing.T) { } } +func TestURLTemplate(t *testing.T) { + uri, _ := url.Parse("http://localhost") + r := NewRequest(nil, "POST", uri, "test", nil, false, false) + r.Prefix("pre1").Resource("r1").Namespace("ns").Name("nm").Param("p0", "v0") + full := r.URL() + if full.String() != "http://localhost/pre1/namespaces/ns/r1/nm?p0=v0" { + t.Errorf("unexpected initial URL: %s", full) + } + actual := r.finalURLTemplate() + expected := "http://localhost/pre1/namespaces/%7Bnamespace%7D/r1/%7Bname%7D?p0=%7Bvalue%7D" + if actual != expected { + t.Errorf("unexpected URL template: %s %s", actual, expected) + } + if r.URL().String() != full.String() { + t.Errorf("creating URL template changed request: %s -> %s", full.String(), r.URL().String()) + } +} + func TestTransformResponse(t *testing.T) { invalid := []byte("aaaaa") uri, _ := url.Parse("http://localhost")