From be11738ee06865a9a854a413078cbeb8184c5f33 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <ys@uber.com>
Date: Mon, 19 Mar 2018 14:54:01 -0400
Subject: [PATCH 1/7] Allow overriding base path for UI/API routes

Signed-off-by: Yuri Shkuro <ys@uber.com>
---
 cmd/query/app/flags.go           | 12 ++++-----
 cmd/query/app/handler.go         | 11 ++++----
 cmd/query/app/handler_options.go | 11 ++++++--
 cmd/query/app/static_handler.go  | 46 ++++++++++++++++++++++++++++----
 cmd/query/main.go                |  7 +++--
 cmd/standalone/main.go           |  4 ++-
 jaeger-ui                        |  2 +-
 7 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go
index 824842a33f6..9cdfb757be7 100644
--- a/cmd/query/app/flags.go
+++ b/cmd/query/app/flags.go
@@ -22,7 +22,7 @@ import (
 
 const (
 	queryPort                = "query.port"
-	queryPrefix              = "query.prefix"
+	queryBasePath            = "query.base-path"
 	queryStaticFiles         = "query.static-files"
 	queryUIConfig            = "query.ui-config"
 	queryHealthCheckHTTPPort = "query.health-check-http-port"
@@ -32,8 +32,8 @@ const (
 type QueryOptions struct {
 	// Port is the port that the query service listens in on
 	Port int
-	// Prefix is the prefix of the query service api
-	Prefix string
+	// BasePath is the prefix for all UI and API HTTP routes
+	BasePath string
 	// StaticAssets is the path for the static assets for the UI (https://github.com/uber/jaeger-ui)
 	StaticAssets string
 	// UIConfig is the path to a configuration file for the UI
@@ -45,8 +45,8 @@ type QueryOptions struct {
 // AddFlags adds flags for QueryOptions
 func AddFlags(flagSet *flag.FlagSet) {
 	flagSet.Int(queryPort, 16686, "The port for the query service")
-	flagSet.String(queryPrefix, "api", "The prefix for the url of the query service")
-	flagSet.String(queryStaticFiles, "jaeger-ui-build/build/", "The path for the static assets for the UI")
+	flagSet.String(queryBasePath, "/", "The base path for all HTTP routes, e.g. /jaeger; useful when running behind a reverse proxy")
+	flagSet.String(queryStaticFiles, "jaeger-ui-build/build/", "The directory path for the static assets for the UI")
 	flagSet.String(queryUIConfig, "", "The path to the UI configuration file in JSON format")
 	flagSet.Int(queryHealthCheckHTTPPort, 16687, "The http port for the health check service")
 }
@@ -54,7 +54,7 @@ func AddFlags(flagSet *flag.FlagSet) {
 // InitFromViper initializes QueryOptions with properties from viper
 func (qOpts *QueryOptions) InitFromViper(v *viper.Viper) *QueryOptions {
 	qOpts.Port = v.GetInt(queryPort)
-	qOpts.Prefix = v.GetString(queryPrefix)
+	qOpts.BasePath = v.GetString(queryBasePath)
 	qOpts.StaticAssets = v.GetString(queryStaticFiles)
 	qOpts.UIConfig = v.GetString(queryUIConfig)
 	qOpts.HealthCheckHTTPPort = v.GetInt(queryHealthCheckHTTPPort)
diff --git a/cmd/query/app/handler.go b/cmd/query/app/handler.go
index 2224266c389..b499bac1a2b 100644
--- a/cmd/query/app/handler.go
+++ b/cmd/query/app/handler.go
@@ -43,7 +43,7 @@ const (
 
 	defaultDependencyLookbackDuration = time.Hour * 24
 	defaultTraceQueryLookbackDuration = time.Hour * 24 * 2
-	defaultHTTPPrefix                 = "api"
+	defaultAPIPrefix                  = "api"
 )
 
 var (
@@ -78,7 +78,8 @@ type APIHandler struct {
 	adjuster          adjuster.Adjuster
 	logger            *zap.Logger
 	queryParser       queryParser
-	httpPrefix        string
+	basePath          string
+	apiPrefix         string
 	tracer            opentracing.Tracer
 }
 
@@ -96,8 +97,8 @@ func NewAPIHandler(spanReader spanstore.Reader, dependencyReader dependencystore
 	for _, option := range options {
 		option(aH)
 	}
-	if aH.httpPrefix == "" {
-		aH.httpPrefix = defaultHTTPPrefix
+	if aH.apiPrefix == "" {
+		aH.apiPrefix = defaultAPIPrefix
 	}
 	if aH.adjuster == nil {
 		aH.adjuster = adjuster.Sequence(StandardAdjusters...)
@@ -141,7 +142,7 @@ func (aH *APIHandler) handleFunc(
 }
 
 func (aH *APIHandler) route(route string, args ...interface{}) string {
-	args = append([]interface{}{aH.httpPrefix}, args...)
+	args = append([]interface{}{aH.apiPrefix}, args...)
 	return fmt.Sprintf("/%s"+route, args...)
 }
 
diff --git a/cmd/query/app/handler_options.go b/cmd/query/app/handler_options.go
index b494688d594..49cec6feec0 100644
--- a/cmd/query/app/handler_options.go
+++ b/cmd/query/app/handler_options.go
@@ -47,10 +47,17 @@ func (handlerOptions) Adjusters(adjusters ...adjuster.Adjuster) HandlerOption {
 	}
 }
 
-// Prefix creates a HandlerOption that initializes prefix HTTP prefix of the API
+// BasePath creates a HandlerOption that initializes the base path for all HTTP routes
+func (handlerOptions) BasePath(prefix string) HandlerOption {
+	return func(apiHandler *APIHandler) {
+		apiHandler.basePath = prefix
+	}
+}
+
+// Prefix creates a HandlerOption that initializes the HTTP prefix for API routes
 func (handlerOptions) Prefix(prefix string) HandlerOption {
 	return func(apiHandler *APIHandler) {
-		apiHandler.httpPrefix = prefix
+		apiHandler.apiPrefix = prefix
 	}
 }
 
diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go
index 5924a306177..0d30197536a 100644
--- a/cmd/query/app/static_handler.go
+++ b/cmd/query/app/static_handler.go
@@ -31,11 +31,17 @@ import (
 var (
 	staticRootFiles = []string{"favicon.ico"}
 	configPattern   = regexp.MustCompile("JAEGER_CONFIG *= *DEFAULT_CONFIG;")
+	basePathPattern = regexp.MustCompile(`<base href="/"`)
+	basePathReplace = `<base href="%s/"`
+	errBadBasePath  = errors.New("Base path must start with / but not end with /; valid path: /jaeger/ui")
 )
 
 // RegisterStaticHandler adds handler for static assets to the router.
 func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOptions) {
-	staticHandler, err := NewStaticAssetsHandler(qOpts.StaticAssets, qOpts.UIConfig)
+	staticHandler, err := NewStaticAssetsHandler(qOpts.StaticAssets, StaticAssetsHandlerOptions{
+		BasePath:     qOpts.BasePath,
+		UIConfigPath: qOpts.UIConfig,
+	})
 	if err != nil {
 		logger.Panic("Could not create static assets handler", zap.Error(err))
 	}
@@ -48,12 +54,19 @@ func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOption
 
 // StaticAssetsHandler handles static assets
 type StaticAssetsHandler struct {
+	options          StaticAssetsHandlerOptions
 	staticAssetsRoot string
 	indexHTML        []byte
 }
 
+// StaticAssetsHandlerOptions defines options for NewStaticAssetsHandler
+type StaticAssetsHandlerOptions struct {
+	BasePath     string
+	UIConfigPath string
+}
+
 // NewStaticAssetsHandler returns a StaticAssetsHandler
-func NewStaticAssetsHandler(staticAssetsRoot string, uiConfig string) (*StaticAssetsHandler, error) {
+func NewStaticAssetsHandler(staticAssetsRoot string, options StaticAssetsHandlerOptions) (*StaticAssetsHandler, error) {
 	if staticAssetsRoot == "" {
 		return nil, nil
 	}
@@ -65,7 +78,7 @@ func NewStaticAssetsHandler(staticAssetsRoot string, uiConfig string) (*StaticAs
 		return nil, errors.Wrap(err, "Cannot read UI static assets")
 	}
 	configString := "JAEGER_CONFIG = DEFAULT_CONFIG"
-	if config, err := loadUIConfig(uiConfig); err != nil {
+	if config, err := loadUIConfig(options.UIConfigPath); err != nil {
 		return nil, err
 	} else if config != nil {
 		// TODO if we want to support other config formats like YAML, we need to normalize `config` to be
@@ -74,9 +87,17 @@ func NewStaticAssetsHandler(staticAssetsRoot string, uiConfig string) (*StaticAs
 		bytes, _ := json.Marshal(config)
 		configString = fmt.Sprintf("JAEGER_CONFIG = %v", string(bytes))
 	}
+	indexBytes = configPattern.ReplaceAll(indexBytes, []byte(configString+";"))
+	if options.BasePath != "/" {
+		if !strings.HasPrefix(options.BasePath, "/") || strings.HasSuffix(options.BasePath, "/") {
+			return nil, errBadBasePath
+		}
+		indexBytes = basePathPattern.ReplaceAll(indexBytes, []byte(fmt.Sprintf(basePathReplace, options.BasePath)))
+	}
 	return &StaticAssetsHandler{
+		options:          options,
 		staticAssetsRoot: staticAssetsRoot,
-		indexHTML:        configPattern.ReplaceAll(indexBytes, []byte(configString+";")),
+		indexHTML:        indexBytes,
 	}, nil
 }
 
@@ -108,7 +129,8 @@ func loadUIConfig(uiConfig string) (map[string]interface{}, error) {
 
 // RegisterRoutes registers routes for this handler on the given router
 func (sH *StaticAssetsHandler) RegisterRoutes(router *mux.Router) {
-	router.PathPrefix("/static").Handler(http.FileServer(http.Dir(sH.staticAssetsRoot)))
+	// router.PathPrefix("/static").Handler(http.FileServer(http.Dir(sH.staticAssetsRoot)))
+	router.PathPrefix("/static").Handler(sH.fileHandler())
 	for _, file := range staticRootFiles {
 		router.Path("/" + file).HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			http.ServeFile(w, r, sH.staticAssetsRoot+file)
@@ -117,6 +139,20 @@ func (sH *StaticAssetsHandler) RegisterRoutes(router *mux.Router) {
 	router.NotFoundHandler = http.HandlerFunc(sH.notFound)
 }
 
+func (sH *StaticAssetsHandler) fileHandler() http.Handler {
+	fs := http.FileServer(http.Dir(sH.staticAssetsRoot))
+	base := sH.options.BasePath
+	if base == "/" {
+		return fs
+	}
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if strings.HasPrefix(r.URL.Path, base) {
+			r.URL.Path = r.URL.Path[len(base):]
+		}
+		fs.ServeHTTP(w, r)
+	})
+}
+
 func (sH *StaticAssetsHandler) notFound(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	w.Write(sH.indexHTML)
diff --git a/cmd/query/main.go b/cmd/query/main.go
index 770381109d0..b31662c0581 100644
--- a/cmd/query/main.go
+++ b/cmd/query/main.go
@@ -109,7 +109,6 @@ func main() {
 			}
 
 			apiHandlerOptions := []app.HandlerOption{
-				app.HandlerOptions.Prefix(queryOpts.Prefix),
 				app.HandlerOptions.Logger(logger),
 				app.HandlerOptions.Tracer(tracer),
 			}
@@ -119,6 +118,9 @@ func main() {
 				dependencyReader,
 				apiHandlerOptions...)
 			r := mux.NewRouter()
+			if queryOpts.BasePath != "/" {
+				r = r.PathPrefix(queryOpts.BasePath).Subrouter()
+			}
 			apiHandler.RegisterRoutes(r)
 			app.RegisterStaticHandler(r, logger, queryOpts)
 
@@ -128,7 +130,8 @@ func main() {
 			}
 
 			portStr := ":" + strconv.Itoa(queryOpts.Port)
-			compressHandler := handlers.CompressHandler(r)
+			loggingHandler := handlers.LoggingHandler(os.Stdout, r)
+			compressHandler := handlers.CompressHandler(loggingHandler)
 			recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true)
 
 			go func() {
diff --git a/cmd/standalone/main.go b/cmd/standalone/main.go
index eb547685c81..9ea14d4be08 100644
--- a/cmd/standalone/main.go
+++ b/cmd/standalone/main.go
@@ -266,11 +266,13 @@ func startQuery(
 	apiHandler := queryApp.NewAPIHandler(
 		spanReader,
 		depReader,
-		queryApp.HandlerOptions.Prefix(qOpts.Prefix),
 		queryApp.HandlerOptions.Logger(logger),
 		queryApp.HandlerOptions.Tracer(tracer))
 
 	r := mux.NewRouter()
+	if qOpts.BasePath != "/" {
+		r = r.PathPrefix(qOpts.BasePath).Subrouter()
+	}
 	apiHandler.RegisterRoutes(r)
 	queryApp.RegisterStaticHandler(r, logger, qOpts)
 
diff --git a/jaeger-ui b/jaeger-ui
index 8c50c6cb11a..5520c89d9b8 160000
--- a/jaeger-ui
+++ b/jaeger-ui
@@ -1 +1 @@
-Subproject commit 8c50c6cb11a78a78580ab955470f749246c812a3
+Subproject commit 5520c89d9b84f9e86e8413f578158e2780bb8807

From 62ac673cf48115931736e57b596dbfeb044a1b84 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <ys@uber.com>
Date: Mon, 19 Mar 2018 14:56:24 -0400
Subject: [PATCH 2/7] Allow overriding base path for UI/API routes

Signed-off-by: Yuri Shkuro <ys@uber.com>
---
 cmd/query/app/flags_test.go          |  4 ++--
 cmd/query/app/handler_test.go        |  2 +-
 cmd/query/app/static_handler_test.go | 10 +++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go
index 555d3920290..dcdb843c23f 100644
--- a/cmd/query/app/flags_test.go
+++ b/cmd/query/app/flags_test.go
@@ -27,12 +27,12 @@ func TestQueryBuilderFlags(t *testing.T) {
 	command.ParseFlags([]string{
 		"--query.static-files=/dev/null",
 		"--query.ui-config=some.json",
-		"--query.prefix=api",
+		"--query.base-path=/jaeger",
 		"--query.port=80",
 	})
 	qOpts := new(QueryOptions).InitFromViper(v)
 	assert.Equal(t, "/dev/null", qOpts.StaticAssets)
 	assert.Equal(t, "some.json", qOpts.UIConfig)
-	assert.Equal(t, "api", qOpts.Prefix)
+	assert.Equal(t, "/jaeger", qOpts.BasePath)
 	assert.Equal(t, 80, qOpts.Port)
 }
diff --git a/cmd/query/app/handler_test.go b/cmd/query/app/handler_test.go
index 7fb833c9fa4..ae520527c2f 100644
--- a/cmd/query/app/handler_test.go
+++ b/cmd/query/app/handler_test.go
@@ -85,7 +85,7 @@ func initializeTestServerWithHandler(options ...HandlerOption) (*httptest.Server
 		append(
 			[]HandlerOption{
 				HandlerOptions.Logger(zap.NewNop()),
-				HandlerOptions.Prefix(defaultHTTPPrefix),
+				HandlerOptions.Prefix(defaultAPIPrefix),
 				HandlerOptions.QueryLookbackDuration(defaultTraceQueryLookbackDuration),
 			},
 			options...,
diff --git a/cmd/query/app/static_handler_test.go b/cmd/query/app/static_handler_test.go
index 315bb4962be..fc900799d1c 100644
--- a/cmd/query/app/static_handler_test.go
+++ b/cmd/query/app/static_handler_test.go
@@ -32,7 +32,7 @@ import (
 
 func TestStaticAssetsHandler(t *testing.T) {
 	r := mux.NewRouter()
-	handler, err := NewStaticAssetsHandler("fixture", "")
+	handler, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{})
 	require.NoError(t, err)
 	handler.RegisterRoutes(r)
 	server := httptest.NewServer(r)
@@ -49,13 +49,13 @@ func TestStaticAssetsHandler(t *testing.T) {
 }
 
 func TestDefaultStaticAssetsRoot(t *testing.T) {
-	handler, err := NewStaticAssetsHandler("", "")
+	handler, err := NewStaticAssetsHandler("", StaticAssetsHandlerOptions{})
 	assert.Nil(t, handler)
 	assert.Nil(t, err)
 }
 
 func TestNotExistingUiConfig(t *testing.T) {
-	handler, err := NewStaticAssetsHandler("/foo/bar", "")
+	handler, err := NewStaticAssetsHandler("/foo/bar", StaticAssetsHandlerOptions{})
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "Cannot read UI static assets")
 	assert.Nil(t, handler)
@@ -103,10 +103,10 @@ func TestRegisterStaticHandler(t *testing.T) {
 }
 
 func TestNewStaticAssetsHandlerWithConfig(t *testing.T) {
-	_, err := NewStaticAssetsHandler("fixture", "fixture/invalid-config")
+	_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/invalid-config"})
 	assert.Error(t, err)
 
-	handler, err := NewStaticAssetsHandler("fixture", "fixture/ui-config.json")
+	handler, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/ui-config.json"})
 	require.NoError(t, err)
 	require.NotNil(t, handler)
 	html := string(handler.indexHTML)

From 01d09b6ece2916a19ac7eb271f642b6d55479003 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <ys@uber.com>
Date: Mon, 19 Mar 2018 18:59:38 -0400
Subject: [PATCH 3/7] Add tests

Signed-off-by: Yuri Shkuro <ys@uber.com>
---
 cmd/query/app/fixture/index.html       |   1 +
 cmd/query/app/fixture/static/asset.txt |   1 +
 cmd/query/app/handler_test.go          |   2 +
 cmd/query/app/static_handler.go        |   7 +-
 cmd/query/app/static_handler_test.go   | 104 +++++++++++++++++++------
 jaeger-ui                              |   2 +-
 6 files changed, 89 insertions(+), 28 deletions(-)
 create mode 100644 cmd/query/app/fixture/static/asset.txt

diff --git a/cmd/query/app/fixture/index.html b/cmd/query/app/fixture/index.html
index 159ece5a20d..bc5b89f5a43 100644
--- a/cmd/query/app/fixture/index.html
+++ b/cmd/query/app/fixture/index.html
@@ -1,6 +1,7 @@
 <!DOCTYPE html>
 <html lang="en">
     <meta charset="UTF-8">
+    <base href="/" data-inject-target="BASE_URL"/>
     <title>Test Page</title>
     <!-- JAEGER_CONFIG=DEFAULT_CONFIG; -->
 </html>
diff --git a/cmd/query/app/fixture/static/asset.txt b/cmd/query/app/fixture/static/asset.txt
new file mode 100644
index 00000000000..3e20b5edaef
--- /dev/null
+++ b/cmd/query/app/fixture/static/asset.txt
@@ -0,0 +1 @@
+some asset
diff --git a/cmd/query/app/handler_test.go b/cmd/query/app/handler_test.go
index ae520527c2f..2d88f21e43c 100644
--- a/cmd/query/app/handler_test.go
+++ b/cmd/query/app/handler_test.go
@@ -85,7 +85,9 @@ func initializeTestServerWithHandler(options ...HandlerOption) (*httptest.Server
 		append(
 			[]HandlerOption{
 				HandlerOptions.Logger(zap.NewNop()),
+				// add options for test coverage
 				HandlerOptions.Prefix(defaultAPIPrefix),
+				HandlerOptions.BasePath("/"),
 				HandlerOptions.QueryLookbackDuration(defaultTraceQueryLookbackDuration),
 			},
 			options...,
diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go
index 0d30197536a..6569ff9ab46 100644
--- a/cmd/query/app/static_handler.go
+++ b/cmd/query/app/static_handler.go
@@ -33,7 +33,7 @@ var (
 	configPattern   = regexp.MustCompile("JAEGER_CONFIG *= *DEFAULT_CONFIG;")
 	basePathPattern = regexp.MustCompile(`<base href="/"`)
 	basePathReplace = `<base href="%s/"`
-	errBadBasePath  = errors.New("Base path must start with / but not end with /; valid path: /jaeger/ui")
+	errBadBasePath  = "Invalid base path '%s'. Must start with / but not end with /, e.g. /jaeger/ui."
 )
 
 // RegisterStaticHandler adds handler for static assets to the router.
@@ -88,9 +88,12 @@ func NewStaticAssetsHandler(staticAssetsRoot string, options StaticAssetsHandler
 		configString = fmt.Sprintf("JAEGER_CONFIG = %v", string(bytes))
 	}
 	indexBytes = configPattern.ReplaceAll(indexBytes, []byte(configString+";"))
+	if options.BasePath == "" {
+		options.BasePath = "/"
+	}
 	if options.BasePath != "/" {
 		if !strings.HasPrefix(options.BasePath, "/") || strings.HasSuffix(options.BasePath, "/") {
-			return nil, errBadBasePath
+			return nil, fmt.Errorf(errBadBasePath, options.BasePath)
 		}
 		indexBytes = basePathPattern.ReplaceAll(indexBytes, []byte(fmt.Sprintf(basePathReplace, options.BasePath)))
 	}
diff --git a/cmd/query/app/static_handler_test.go b/cmd/query/app/static_handler_test.go
index fc900799d1c..3ee5ea1de1e 100644
--- a/cmd/query/app/static_handler_test.go
+++ b/cmd/query/app/static_handler_test.go
@@ -19,7 +19,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
 	"time"
 
@@ -77,40 +76,95 @@ func TestRegisterStaticHandlerNotCreated(t *testing.T) {
 }
 
 func TestRegisterStaticHandler(t *testing.T) {
-	logger, buf := testutils.NewLogger()
-	r := mux.NewRouter()
-	RegisterStaticHandler(r, logger, &QueryOptions{StaticAssets: "fixture/"})
-	assert.Empty(t, buf.String())
+	testCases := []struct {
+		subroute         bool
+		baseURL          string
+		basePath         string
+		expectedBasePath string
+	}{
+		{baseURL: "/", basePath: "", expectedBasePath: `<base href="/"`},
+		{baseURL: "/", basePath: "/", expectedBasePath: `<base href="/"`},
+		{baseURL: "/jaeger/", basePath: "/jaeger", expectedBasePath: `<base href="/jaeger/"`, subroute: true},
+	}
+	for _, testCase := range testCases {
+		t.Run("", func(t *testing.T) {
+			logger, buf := testutils.NewLogger()
+			r := mux.NewRouter()
+			if testCase.subroute {
+				r = r.PathPrefix(testCase.basePath).Subrouter()
+			}
+			RegisterStaticHandler(r, logger, &QueryOptions{
+				StaticAssets: "fixture/",
+				BasePath:     testCase.basePath,
+				UIConfig:     "fixture/ui-config.json",
+			})
+			assert.Empty(t, buf.String())
+
+			server := httptest.NewServer(r)
+			defer server.Close()
+			httpClient = &http.Client{
+				Timeout: 2 * time.Second,
+			}
 
-	server := httptest.NewServer(r)
-	defer server.Close()
-	expectedRespString := "Test Favicon\n"
+			httpGet := func(path string) string {
+				resp, err := httpClient.Get(fmt.Sprintf("%s%s%s", server.URL, testCase.baseURL, path))
+				require.NoError(t, err)
+				defer resp.Body.Close()
 
-	httpClient = &http.Client{
-		Timeout: 2 * time.Second,
-	}
+				respByteArray, _ := ioutil.ReadAll(resp.Body)
+				respString := string(respByteArray)
 
-	resp, err := httpClient.Get(fmt.Sprintf("%s/favicon.ico", server.URL))
-	require.NoError(t, err)
-	defer resp.Body.Close()
+				assert.NoError(t, err)
+				assert.Equal(t, http.StatusOK, resp.StatusCode)
+				return respString
+			}
 
-	respByteArray, _ := ioutil.ReadAll(resp.Body)
-	respString := string(respByteArray)
+			respString := httpGet("favicon.ico")
+			assert.Contains(t, respString, "Test Favicon") // this text is present in fixtures/favicon.ico
 
-	assert.NoError(t, err)
-	assert.Equal(t, http.StatusOK, resp.StatusCode)
-	assert.Equal(t, expectedRespString, respString)
+			html := httpGet("") // get index.html
+			assert.Contains(t, html, `JAEGER_CONFIG = {"x":"y"};`, "actual: %v", html)
+			assert.Contains(t, html, testCase.expectedBasePath, "actual: %v", html)
+
+			asset := httpGet("static/asset.txt")
+			assert.Contains(t, asset, "some asset", "actual: %v", asset)
+		})
+	}
 }
 
-func TestNewStaticAssetsHandlerWithConfig(t *testing.T) {
+func TestNewStaticAssetsHandlerSubstitutions(t *testing.T) {
+	testCases := []struct {
+		base     string
+		expected string
+	}{
+		{base: "", expected: `<base href="/"`},
+		{base: "/", expected: `<base href="/"`},
+		{base: "/jaeger", expected: `<base href="/jaeger/"`},
+	}
+	for _, testCase := range testCases {
+		handler, err := NewStaticAssetsHandler(
+			"fixture",
+			StaticAssetsHandlerOptions{
+				UIConfigPath: "fixture/ui-config.json",
+				BasePath:     testCase.base,
+			})
+		require.NoError(t, err)
+		require.NotNil(t, handler)
+		html := string(handler.indexHTML)
+		assert.Contains(t, html, `JAEGER_CONFIG = {"x":"y"};`, "actual: %v", html)
+		assert.Contains(t, html, testCase.expected, "actual: %v", html)
+	}
+}
+
+func TestNewStaticAssetsHandlerErrors(t *testing.T) {
 	_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/invalid-config"})
 	assert.Error(t, err)
 
-	handler, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/ui-config.json"})
-	require.NoError(t, err)
-	require.NotNil(t, handler)
-	html := string(handler.indexHTML)
-	assert.True(t, strings.Contains(html, `JAEGER_CONFIG = {"x":"y"};`), "actual: %v", html)
+	for _, base := range []string{"x", "x/", "/x/"} {
+		_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/ui-config.json", BasePath: base})
+		require.Error(t, err, base)
+		assert.Contains(t, err.Error(), "Invalid base path")
+	}
 }
 
 func TestLoadUIConfig(t *testing.T) {
diff --git a/jaeger-ui b/jaeger-ui
index 5520c89d9b8..8d62be8b909 160000
--- a/jaeger-ui
+++ b/jaeger-ui
@@ -1 +1 @@
-Subproject commit 5520c89d9b84f9e86e8413f578158e2780bb8807
+Subproject commit 8d62be8b909d35b1fe378386e8dca59b96023234

From 44611cd265416d8270091191b34fa701a53be08e Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <ys@uber.com>
Date: Mon, 19 Mar 2018 21:08:04 -0400
Subject: [PATCH 4/7] Remove unnecessary test

Signed-off-by: Yuri Shkuro <ys@uber.com>
---
 cmd/query/app/static_handler_test.go | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/cmd/query/app/static_handler_test.go b/cmd/query/app/static_handler_test.go
index 3ee5ea1de1e..c512bfc0c12 100644
--- a/cmd/query/app/static_handler_test.go
+++ b/cmd/query/app/static_handler_test.go
@@ -132,30 +132,6 @@ func TestRegisterStaticHandler(t *testing.T) {
 	}
 }
 
-func TestNewStaticAssetsHandlerSubstitutions(t *testing.T) {
-	testCases := []struct {
-		base     string
-		expected string
-	}{
-		{base: "", expected: `<base href="/"`},
-		{base: "/", expected: `<base href="/"`},
-		{base: "/jaeger", expected: `<base href="/jaeger/"`},
-	}
-	for _, testCase := range testCases {
-		handler, err := NewStaticAssetsHandler(
-			"fixture",
-			StaticAssetsHandlerOptions{
-				UIConfigPath: "fixture/ui-config.json",
-				BasePath:     testCase.base,
-			})
-		require.NoError(t, err)
-		require.NotNil(t, handler)
-		html := string(handler.indexHTML)
-		assert.Contains(t, html, `JAEGER_CONFIG = {"x":"y"};`, "actual: %v", html)
-		assert.Contains(t, html, testCase.expected, "actual: %v", html)
-	}
-}
-
 func TestNewStaticAssetsHandlerErrors(t *testing.T) {
 	_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/invalid-config"})
 	assert.Error(t, err)

From 240d4ac1f1cfe83439f7ff836cc5987d7835d3aa Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <ys@uber.com>
Date: Mon, 19 Mar 2018 21:32:05 -0400
Subject: [PATCH 5/7] clean-up tests

Signed-off-by: Yuri Shkuro <ys@uber.com>
---
 cmd/query/app/static_handler.go      |  6 ++-
 cmd/query/app/static_handler_test.go | 60 ++++++++++------------------
 cmd/query/main.go                    |  3 +-
 3 files changed, 25 insertions(+), 44 deletions(-)

diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go
index 6569ff9ab46..f8623bc8eb7 100644
--- a/cmd/query/app/static_handler.go
+++ b/cmd/query/app/static_handler.go
@@ -29,7 +29,8 @@ import (
 )
 
 var (
-	staticRootFiles = []string{"favicon.ico"}
+	favoriteIcon    = "favicon.ico"
+	staticRootFiles = []string{favoriteIcon}
 	configPattern   = regexp.MustCompile("JAEGER_CONFIG *= *DEFAULT_CONFIG;")
 	basePathPattern = regexp.MustCompile(`<base href="/"`)
 	basePathReplace = `<base href="%s/"`
@@ -132,7 +133,6 @@ func loadUIConfig(uiConfig string) (map[string]interface{}, error) {
 
 // RegisterRoutes registers routes for this handler on the given router
 func (sH *StaticAssetsHandler) RegisterRoutes(router *mux.Router) {
-	// router.PathPrefix("/static").Handler(http.FileServer(http.Dir(sH.staticAssetsRoot)))
 	router.PathPrefix("/static").Handler(sH.fileHandler())
 	for _, file := range staticRootFiles {
 		router.Path("/" + file).HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -150,6 +150,8 @@ func (sH *StaticAssetsHandler) fileHandler() http.Handler {
 	}
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if strings.HasPrefix(r.URL.Path, base) {
+			// gorilla Subroute() is a bit odd, it keeps the base path in the URL,
+			// which prevents the FileServer from locating the files, so we strip the prefix.
 			r.URL.Path = r.URL.Path[len(base):]
 		}
 		fs.ServeHTTP(w, r)
diff --git a/cmd/query/app/static_handler_test.go b/cmd/query/app/static_handler_test.go
index c512bfc0c12..61eccc3e2a5 100644
--- a/cmd/query/app/static_handler_test.go
+++ b/cmd/query/app/static_handler_test.go
@@ -29,24 +29,6 @@ import (
 	"github.com/jaegertracing/jaeger/pkg/testutils"
 )
 
-func TestStaticAssetsHandler(t *testing.T) {
-	r := mux.NewRouter()
-	handler, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{})
-	require.NoError(t, err)
-	handler.RegisterRoutes(r)
-	server := httptest.NewServer(r)
-	defer server.Close()
-
-	httpClient = &http.Client{
-		Timeout: 2 * time.Second,
-	}
-
-	resp, err := httpClient.Get(server.URL)
-	assert.NoError(t, err)
-	defer resp.Body.Close()
-	assert.Equal(t, http.StatusOK, resp.StatusCode)
-}
-
 func TestDefaultStaticAssetsRoot(t *testing.T) {
 	handler, err := NewStaticAssetsHandler("", StaticAssetsHandlerOptions{})
 	assert.Nil(t, handler)
@@ -77,17 +59,20 @@ func TestRegisterStaticHandlerNotCreated(t *testing.T) {
 
 func TestRegisterStaticHandler(t *testing.T) {
 	testCases := []struct {
-		subroute         bool
-		baseURL          string
-		basePath         string
-		expectedBasePath string
+		basePath         string // input to the test
+		subroute         bool   // should we create a subroute?
+		baseURL          string // expected URL prefix
+		expectedBaseHTML string // substring to match in the home page
 	}{
-		{baseURL: "/", basePath: "", expectedBasePath: `<base href="/"`},
-		{baseURL: "/", basePath: "/", expectedBasePath: `<base href="/"`},
-		{baseURL: "/jaeger/", basePath: "/jaeger", expectedBasePath: `<base href="/jaeger/"`, subroute: true},
+		{basePath: "", baseURL: "/", expectedBaseHTML: `<base href="/"`},
+		{basePath: "/", baseURL: "/", expectedBaseHTML: `<base href="/"`},
+		{basePath: "/jaeger", baseURL: "/jaeger/", expectedBaseHTML: `<base href="/jaeger/"`, subroute: true},
+	}
+	httpClient = &http.Client{
+		Timeout: 2 * time.Second,
 	}
 	for _, testCase := range testCases {
-		t.Run("", func(t *testing.T) {
+		t.Run("basePath="+testCase.basePath, func(t *testing.T) {
 			logger, buf := testutils.NewLogger()
 			r := mux.NewRouter()
 			if testCase.subroute {
@@ -98,33 +83,28 @@ func TestRegisterStaticHandler(t *testing.T) {
 				BasePath:     testCase.basePath,
 				UIConfig:     "fixture/ui-config.json",
 			})
-			assert.Empty(t, buf.String())
+			assert.Empty(t, buf.String(), "no logs during construction")
 
 			server := httptest.NewServer(r)
 			defer server.Close()
-			httpClient = &http.Client{
-				Timeout: 2 * time.Second,
-			}
 
 			httpGet := func(path string) string {
 				resp, err := httpClient.Get(fmt.Sprintf("%s%s%s", server.URL, testCase.baseURL, path))
 				require.NoError(t, err)
 				defer resp.Body.Close()
 
-				respByteArray, _ := ioutil.ReadAll(resp.Body)
-				respString := string(respByteArray)
-
-				assert.NoError(t, err)
-				assert.Equal(t, http.StatusOK, resp.StatusCode)
-				return respString
+				respByteArray, err := ioutil.ReadAll(resp.Body)
+				require.NoError(t, err)
+				require.Equal(t, http.StatusOK, resp.StatusCode)
+				return string(respByteArray)
 			}
 
-			respString := httpGet("favicon.ico")
+			respString := httpGet(favoriteIcon)
 			assert.Contains(t, respString, "Test Favicon") // this text is present in fixtures/favicon.ico
 
-			html := httpGet("") // get index.html
+			html := httpGet("") // get home page
 			assert.Contains(t, html, `JAEGER_CONFIG = {"x":"y"};`, "actual: %v", html)
-			assert.Contains(t, html, testCase.expectedBasePath, "actual: %v", html)
+			assert.Contains(t, html, testCase.expectedBaseHTML, "actual: %v", html)
 
 			asset := httpGet("static/asset.txt")
 			assert.Contains(t, asset, "some asset", "actual: %v", asset)
@@ -138,7 +118,7 @@ func TestNewStaticAssetsHandlerErrors(t *testing.T) {
 
 	for _, base := range []string{"x", "x/", "/x/"} {
 		_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/ui-config.json", BasePath: base})
-		require.Error(t, err, base)
+		require.Error(t, err, "basePath=%s", base)
 		assert.Contains(t, err.Error(), "Invalid base path")
 	}
 }
diff --git a/cmd/query/main.go b/cmd/query/main.go
index b31662c0581..c328055794b 100644
--- a/cmd/query/main.go
+++ b/cmd/query/main.go
@@ -130,8 +130,7 @@ func main() {
 			}
 
 			portStr := ":" + strconv.Itoa(queryOpts.Port)
-			loggingHandler := handlers.LoggingHandler(os.Stdout, r)
-			compressHandler := handlers.CompressHandler(loggingHandler)
+			compressHandler := handlers.CompressHandler(r)
 			recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true)
 
 			go func() {

From de4221ad9052abe5147ca2a8dda6ca5cb67ebbc1 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <ys@uber.com>
Date: Mon, 19 Mar 2018 21:37:41 -0400
Subject: [PATCH 6/7] Fix error msg

Signed-off-by: Yuri Shkuro <ys@uber.com>
---
 cmd/query/app/static_handler.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/query/app/static_handler.go b/cmd/query/app/static_handler.go
index f8623bc8eb7..7af5d18207c 100644
--- a/cmd/query/app/static_handler.go
+++ b/cmd/query/app/static_handler.go
@@ -34,7 +34,7 @@ var (
 	configPattern   = regexp.MustCompile("JAEGER_CONFIG *= *DEFAULT_CONFIG;")
 	basePathPattern = regexp.MustCompile(`<base href="/"`)
 	basePathReplace = `<base href="%s/"`
-	errBadBasePath  = "Invalid base path '%s'. Must start with / but not end with /, e.g. /jaeger/ui."
+	errBadBasePath  = "Invalid base path '%s'. Must start but not end with a slash '/', e.g. '/jaeger/ui'"
 )
 
 // RegisterStaticHandler adds handler for static assets to the router.

From 7a019dfaa1f35bb22bb83b8d6f62320c9a5cef39 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <ys@uber.com>
Date: Fri, 23 Mar 2018 14:07:51 -0400
Subject: [PATCH 7/7] Bump ui

Signed-off-by: Yuri Shkuro <ys@uber.com>
---
 jaeger-ui | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/jaeger-ui b/jaeger-ui
index 8d62be8b909..e20963bb40a 160000
--- a/jaeger-ui
+++ b/jaeger-ui
@@ -1 +1 @@
-Subproject commit 8d62be8b909d35b1fe378386e8dca59b96023234
+Subproject commit e20963bb40a40ce49191d4794edb765f4adae6a2