From 734a01d603ead7e78914230b19a71b4583c0d06d Mon Sep 17 00:00:00 2001
From: Tom Wilkie <tom.wilkie@gmail.com>
Date: Wed, 6 Apr 2016 15:59:02 +0100
Subject: [PATCH] Review feedback, lint & fix tests.

---
 app/api_report_test.go          |  4 ++--
 app/api_topologies_test.go      |  2 +-
 app/router.go                   |  9 +--------
 common/middleware/instrument.go | 13 ++++++-------
 prog/app.go                     | 12 ++++++------
 5 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/app/api_report_test.go b/app/api_report_test.go
index 98c32e2d4..6d6933db9 100644
--- a/app/api_report_test.go
+++ b/app/api_report_test.go
@@ -12,8 +12,8 @@ import (
 )
 
 func topologyServer() *httptest.Server {
-	router := mux.NewRouter()
-	app.RegisterTopologyRoutes(StaticReport{}, router)
+	router := mux.NewRouter().SkipClean(true)
+	app.RegisterTopologyRoutes(router, StaticReport{})
 	return httptest.NewServer(router)
 }
 
diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go
index 346d2f9e1..371ad6774 100644
--- a/app/api_topologies_test.go
+++ b/app/api_topologies_test.go
@@ -54,7 +54,7 @@ func TestAPITopologyAddsKubernetes(t *testing.T) {
 	router := mux.NewRouter()
 	c := app.NewCollector(1 * time.Minute)
 	app.RegisterReportPostHandler(c, router)
-	app.RegisterTopologyRoutes(c, router)
+	app.RegisterTopologyRoutes(router, c)
 	ts := httptest.NewServer(router)
 	defer ts.Close()
 
diff --git a/app/router.go b/app/router.go
index 986b61ce0..0f9f19780 100644
--- a/app/router.go
+++ b/app/router.go
@@ -80,14 +80,7 @@ func gzipHandler(h http.HandlerFunc) http.HandlerFunc {
 	return handlers.GZIPHandlerFunc(h, nil)
 }
 
-// TopologyHandler registers the various topology routes with a http mux.
-//
-// The returned http.Handler has to be passed directly to http.ListenAndServe,
-// and cannot be nested inside another gorrilla.mux.
-//
-// Routes which should be matched before the topology routes should be added
-// to a router and passed in preRoutes.  Routes to be matches after topology
-// routes should be added to a router and passed to postRoutes.
+// RegisterTopologyRoutes registers the various topology routes with a http mux.
 func RegisterTopologyRoutes(router *mux.Router, r Reporter) {
 	get := router.Methods("GET").Subrouter()
 	get.HandleFunc("/api",
diff --git a/common/middleware/instrument.go b/common/middleware/instrument.go
index 83fe533a3..018aed52a 100644
--- a/common/middleware/instrument.go
+++ b/common/middleware/instrument.go
@@ -9,16 +9,15 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
+// Instrument is a Middleware which records timings for every HTTP request
 type Instrument struct {
-	RouteMatcher RouteMatcher
-	Duration     *prometheus.SummaryVec
-}
-
-// RouteMatcher is implemented by mux.Router.
-type RouteMatcher interface {
-	Match(*http.Request, *mux.RouteMatch) bool
+	RouteMatcher interface {
+		Match(*http.Request, *mux.RouteMatch) bool
+	}
+	Duration *prometheus.SummaryVec
 }
 
+// Wrap implements middleware.Interface
 func (i Instrument) Wrap(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		begin := time.Now()
diff --git a/prog/app.go b/prog/app.go
index 64d3e1f4b..31acf3f64 100644
--- a/prog/app.go
+++ b/prog/app.go
@@ -138,11 +138,11 @@ func pipeRouterFactory(userIDer multitenant.UserIDer, pipeRouterURL, consulInf s
 // Main runs the app
 func appMain() {
 	var (
-		window      = flag.Duration("window", 15*time.Second, "window")
-		listen      = flag.String("http.address", ":"+strconv.Itoa(xfer.AppPort), "webserver listen address")
-		logLevel    = flag.String("log.level", "info", "logging threshold level: debug|info|warn|error|fatal|panic")
-		logPrefix   = flag.String("log.prefix", "<app>", "prefix for each log line")
-		logRequests = flag.Bool("log.requests", false, "Log individual HTTP requests")
+		window    = flag.Duration("window", 15*time.Second, "window")
+		listen    = flag.String("http.address", ":"+strconv.Itoa(xfer.AppPort), "webserver listen address")
+		logLevel  = flag.String("log.level", "info", "logging threshold level: debug|info|warn|error|fatal|panic")
+		logPrefix = flag.String("log.prefix", "<app>", "prefix for each log line")
+		logHTTP   = flag.Bool("log.http", false, "Log individual HTTP requests")
 
 		weaveAddr      = flag.String("weave.addr", app.DefaultWeaveURL, "Address on which to contact WeaveDNS")
 		weaveHostname  = flag.String("weave.hostname", app.DefaultHostname, "Hostname to advertise in WeaveDNS")
@@ -219,7 +219,7 @@ func appMain() {
 	}
 
 	handler := router(collector, controlRouter, pipeRouter)
-	if *logRequests {
+	if *logHTTP {
 		handler = middleware.Logging.Wrap(handler)
 	}
 	go func() {