Repository: prometheus/common
Pull request #217 (2020-01-10 22:54:29)

route.go: allow to chain instrumentations
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>

roidelapluie: The purpose is to clean https://github.com/prometheus/prometheus/blob/64194f7d45cbb7fa2f41c8f219706308f65d511a/web/web.go#L1121-L1129
roidelapluie: We check the existing instrumentation (r.ins) not the new one.

Le sam. 11 janv. 2020 à 10:10, Brian Brazil <notifications@github.com> a
écrit :

> *@brian-brazil* commented on this pull request.
> ------------------------------
>
> In route/route.go
> <https://github.com/prometheus/common/pull/217#discussion_r365510001>:
>
> > @@ -53,6 +53,12 @@ func New() *Router {
>
>  // WithInstrumentation returns a router with instrumentation support.
>  func (r *Router) WithInstrumentation(instrh func(handlerName string, handler http.HandlerFunc) http.HandlerFunc) *Router {
> +	if r.instrh != nil {
>
> That doesn't answer my question. Where are we calling this with nil, and
> why?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/prometheus/common/pull/217?email_source=notifications&email_token=AACHHJWDLNUXXFI34F74P53Q5GEIHA5CNFSM4KFOJ772YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCRNRGFQ#discussion_r365510001>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AACHHJWKEWRAZTGECB6LTETQ5GEIHANCNFSM4KFOJ77Q>
> .
>


Patch:
diff --git a/route/route.go b/route/route.go
index 1bd0a1e..cc05516 100644
--- a/route/route.go
+++ b/route/route.go
@@ -53,6 +53,12 @@ func New() *Router {
 
 // WithInstrumentation returns a router with instrumentation support.
 func (r *Router) WithInstrumentation(instrh func(handlerName string, handler http.HandlerFunc) http.HandlerFunc) *Router {
+	if r.instrh != nil {
+		newInstrh := instrh
+		instrh = func(handlerName string, handler http.HandlerFunc) http.HandlerFunc {
+			return newInstrh(handlerName, r.instrh(handlerName, handler))
+		}
+	}
 	return &Router{rtr: r.rtr, prefix: r.prefix, instrh: instrh}
 }
 


Test patch:
diff --git a/route/route_test.go b/route/route_test.go
index 4d90203..0c22a66 100644
--- a/route/route_test.go
+++ b/route/route_test.go
@@ -141,3 +141,52 @@ func TestInstrumentation(t *testing.T) {
 		}
 	}
 }
+
+func TestInstrumentations(t *testing.T) {
+	got := make([]string, 0)
+	cases := []struct {
+		router *Router
+		want   []string
+	}{
+		{
+			router: New(),
+			want:   []string{},
+		}, {
+			router: New().
+				WithInstrumentation(
+					func(handlerName string, handler http.HandlerFunc) http.HandlerFunc {
+						got = append(got, "1"+handlerName)
+						return handler
+					}).
+				WithInstrumentation(
+					func(handlerName string, handler http.HandlerFunc) http.HandlerFunc {
+						got = append(got, "2"+handlerName)
+						return handler
+					}).
+				WithInstrumentation(
+					func(handlerName string, handler http.HandlerFunc) http.HandlerFunc {
+						got = append(got, "3"+handlerName)
+						return handler
+					}),
+			want: []string{"1/foo", "2/foo", "3/foo"},
+		},
+	}
+
+	for _, c := range cases {
+		c.router.Get("/foo", func(w http.ResponseWriter, r *http.Request) {})
+
+		r, err := http.NewRequest("GET", "http://localhost:9090/foo", nil)
+		if err != nil {
+			t.Fatalf("Error building test request: %s", err)
+		}
+		c.router.ServeHTTP(nil, r)
+		if len(c.want) != len(got) {
+			t.Fatalf("Unexpected value: want %q, got %q", c.want, got)
+		}
+		for i, v := range c.want {
+			if v != got[i] {
+				t.Fatalf("Unexpected value: want %q, got %q", c.want, got)
+			}
+		}
+	}
+}