From 982189161bca9381694237b2eeea6a3a0475c593 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Thu, 9 Jun 2016 12:48:22 +0100 Subject: [PATCH] If we don't get a path name from the router, make one up from the url. (#1570) * If we don't get a path name from the router, make one up from the url. * Think about route naming a little more and add a comment to show it. --- common/middleware/instrument.go | 46 +++++++++++++++++++++++----- common/middleware/instrument_test.go | 30 ++++++++++++++++++ prog/app.go | 2 +- 3 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 common/middleware/instrument_test.go diff --git a/common/middleware/instrument.go b/common/middleware/instrument.go index 23d7de699..11744a10f 100644 --- a/common/middleware/instrument.go +++ b/common/middleware/instrument.go @@ -2,6 +2,7 @@ package middleware import ( "net/http" + "regexp" "strconv" "strings" "time" @@ -39,14 +40,45 @@ func (i Instrument) Wrap(next http.Handler) http.Handler { }) } +// Return a name identifier for ths request. There are three options: +// 1. The request matches a gorilla mux route, with a name. Use that. +// 2. The request matches an unamed gorilla mux router. Munge the path +// template such that templates like '/api/{org}/foo' come out as +// 'api_org_foo'. +// 3. The request doesn't match a mux route. Munge the Path in the same +// manner as (2). +// We do all this as we do not wish to emit high cardinality labels to +// prometheus. func (i Instrument) getRouteName(r *http.Request) string { var routeMatch mux.RouteMatch - if !i.RouteMatcher.Match(r, &routeMatch) { - return "unmatched_path" + if i.RouteMatcher.Match(r, &routeMatch) { + if name := routeMatch.Route.GetName(); name != "" { + return name + } + if tmpl, err := routeMatch.Route.GetPathTemplate(); err != nil { + return MakeLabelValue(tmpl) + } } - name := routeMatch.Route.GetName() - if name == "" { - return "unnamed_path" - } - return name + return MakeLabelValue(r.URL.Path) +} + +var invalidChars = regexp.MustCompile(`[^a-zA-Z0-9]+`) + +// MakeLabelValue converts a Gorilla mux path to a string suitable for use in +// a Prometheus label value. +func MakeLabelValue(path string) string { + // Convert non-alnums to underscores. + result := invalidChars.ReplaceAllString(path, "_") + + // Trim leading and trailing underscores. + result = strings.Trim(result, "_") + + // Make it all lowercase + result = strings.ToLower(result) + + // Special case. + if result == "" { + result = "root" + } + return result } diff --git a/common/middleware/instrument_test.go b/common/middleware/instrument_test.go new file mode 100644 index 000000000..4a3c5bd99 --- /dev/null +++ b/common/middleware/instrument_test.go @@ -0,0 +1,30 @@ +package middleware_test + +import ( + "testing" + + "github.com/weaveworks/scope/common/middleware" +) + +func TestMakeLabelValue(t *testing.T) { + for input, want := range map[string]string{ + "/": "root", // special case + "//": "root", // unintended consequence of special case + "a": "a", + "/foo": "foo", + "foo/": "foo", + "/foo/": "foo", + "/foo/bar": "foo_bar", + "foo/bar/": "foo_bar", + "/foo/bar/": "foo_bar", + "/foo/{orgName}/Bar": "foo_orgname_bar", + "/foo/{org_name}/Bar": "foo_org_name_bar", + "/foo/{org__name}/Bar": "foo_org_name_bar", + "/foo/{org___name}/_Bar": "foo_org_name_bar", + "/foo.bar/baz.qux/": "foo_bar_baz_qux", + } { + if have := middleware.MakeLabelValue(input); want != have { + t.Errorf("%q: want %q, have %q", input, want, have) + } + } +} diff --git a/prog/app.go b/prog/app.go index c3f9d7cc8..91e253ced 100644 --- a/prog/app.go +++ b/prog/app.go @@ -52,7 +52,7 @@ func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.RegisterPipeRoutes(router, pipeRouter) app.RegisterTopologyRoutes(router, collector) - router.PathPrefix("/").Handler(http.FileServer(FS(false))) + router.PathPrefix("/").Name("static").Handler(http.FileServer(FS(false))) instrument := middleware.Instrument{ RouteMatcher: router,