Log access to static assets; remove favicon test

cmd/query/app/flags.go

@@ -38,14 +38,15 @@ import (
 )
 
 const (
-	queryHTTPHostPort       = "query.http-server.host-port"
-	queryGRPCHostPort       = "query.grpc-server.host-port"
-	queryBasePath           = "query.base-path"
-	queryStaticFiles        = "query.static-files"
-	queryUIConfig           = "query.ui-config"
-	queryTokenPropagation   = "query.bearer-token-propagation"
-	queryAdditionalHeaders  = "query.additional-headers"
-	queryMaxClockSkewAdjust = "query.max-clock-skew-adjustment"
+	queryHTTPHostPort          = "query.http-server.host-port"
+	queryGRPCHostPort          = "query.grpc-server.host-port"
+	queryBasePath              = "query.base-path"
+	queryStaticFiles           = "query.static-files"
+	queryLogStaticAssetsAccess = "query.log-static-assets-access"
+	queryUIConfig              = "query.ui-config"
+	queryTokenPropagation      = "query.bearer-token-propagation"
+	queryAdditionalHeaders     = "query.additional-headers"
+	queryMaxClockSkewAdjust    = "query.max-clock-skew-adjustment"
 )
 
 var tlsGRPCFlagsConfig = tlscfg.ServerFlagsConfig{
@@ -68,6 +69,8 @@ type QueryOptions struct {
 	BasePath string
 	// StaticAssets is the path for the static assets for the UI (https://github.com/uber/jaeger-ui)
 	StaticAssets string
+	// LogStaticAssetsAccess tells static handler to log access to static assets, useful in debugging
+	LogStaticAssetsAccess bool
 	// UIConfig is the path to a configuration file for the UI
 	UIConfig string
 	// BearerTokenPropagation activate/deactivate bearer token propagation to storage
@@ -91,6 +94,7 @@ func AddFlags(flagSet *flag.FlagSet) {
 	flagSet.String(queryGRPCHostPort, ports.PortToHostPort(ports.QueryGRPC), "The host:port (e.g. 127.0.0.1:14250 or :14250) of the query's gRPC server")
 	flagSet.String(queryBasePath, "/", "The base path for all HTTP routes, e.g. /jaeger; useful when running behind a reverse proxy")
 	flagSet.String(queryStaticFiles, "", "The directory path override for the static assets for the UI")
+	flagSet.Bool(queryLogStaticAssetsAccess, false, "Log when static assets are accessed (for debugging)")
 	flagSet.String(queryUIConfig, "", "The path to the UI configuration file in JSON format")
 	flagSet.Bool(queryTokenPropagation, false, "Allow propagation of bearer token to be used by storage plugins")
 	flagSet.Duration(queryMaxClockSkewAdjust, 0, "The maximum delta by which span timestamps may be adjusted in the UI due to clock skew; set to 0s to disable clock skew adjustments")
@@ -114,6 +118,7 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q
 	}
 	qOpts.BasePath = v.GetString(queryBasePath)
 	qOpts.StaticAssets = v.GetString(queryStaticFiles)
+	qOpts.LogStaticAssetsAccess = v.GetBool(queryLogStaticAssetsAccess)
 	qOpts.UIConfig = v.GetString(queryUIConfig)
 	qOpts.BearerTokenPropagation = v.GetBool(queryTokenPropagation)
 

cmd/query/app/flags_test.go

@@ -35,6 +35,7 @@ func TestQueryBuilderFlags(t *testing.T) {
 	v, command := config.Viperize(AddFlags)
 	command.ParseFlags([]string{
 		"--query.static-files=/dev/null",
+		"--query.log-static-assets-access=true",
 		"--query.ui-config=some.json",
 		"--query.base-path=/jaeger",
 		"--query.http-server.host-port=127.0.0.1:8080",
@@ -46,6 +47,7 @@ func TestQueryBuilderFlags(t *testing.T) {
 	qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
 	require.NoError(t, err)
 	assert.Equal(t, "/dev/null", qOpts.StaticAssets)
+	assert.True(t, qOpts.LogStaticAssetsAccess)
 	assert.Equal(t, "some.json", qOpts.UIConfig)
 	assert.Equal(t, "/jaeger", qOpts.BasePath)
 	assert.Equal(t, "127.0.0.1:8080", qOpts.HTTPHostPort)

cmd/query/app/static_handler.go

@@ -37,9 +37,6 @@ import (
 )
 
 var (
-	favoriteIcon    = "favicon.ico"
-	staticRootFiles = []string{favoriteIcon}
-
 	// The following patterns are searched and replaced in the index.html as a way of customizing the UI.
 	configPattern   = regexp.MustCompile("JAEGER_CONFIG *= *DEFAULT_CONFIG;")
 	configJsPattern = regexp.MustCompile(`(?im)^\s*\/\/\s*JAEGER_CONFIG_JS.*\n.*`)
@@ -53,6 +50,7 @@ func RegisterStaticHandler(r *mux.Router, logger *zap.Logger, qOpts *QueryOption
 		BasePath:     qOpts.BasePath,
 		UIConfigPath: qOpts.UIConfig,
 		Logger:       logger,
+		LogAccess:    qOpts.LogStaticAssetsAccess,
 	})
 	if err != nil {
 		logger.Panic("Could not create static assets handler", zap.Error(err))
@@ -73,6 +71,7 @@ type StaticAssetsHandler struct {
 type StaticAssetsHandlerOptions struct {
 	BasePath     string
 	UIConfigPath string
+	LogAccess    bool
 	Logger       *zap.Logger
 	NewWatcher   func() (fswatcher.Watcher, error)
 }
@@ -259,17 +258,25 @@ func loadUIConfig(uiConfig string) (*loadedConfig, error) {
 	}
 }
 
+func (sH *StaticAssetsHandler) loggingHandler(handler http.Handler) http.Handler {
+	if !sH.options.LogAccess {
+		return handler
+	}
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		sH.options.Logger.Info("serving static asset", zap.Stringer("url", r.URL))
+		handler.ServeHTTP(w, r)
+	})
+}
+
 // RegisterRoutes registers routes for this handler on the given router
 func (sH *StaticAssetsHandler) RegisterRoutes(router *mux.Router) {
 	fileServer := http.FileServer(sH.assetsFS)
 	if sH.options.BasePath != "/" {
 		fileServer = http.StripPrefix(sH.options.BasePath+"/", fileServer)
 	}
-	router.PathPrefix("/static/").Handler(fileServer)
-	for _, file := range staticRootFiles {
-		router.Path("/" + file).Handler(fileServer)
-	}
-	router.NotFoundHandler = http.HandlerFunc(sH.notFound)
+	router.PathPrefix("/static/").Handler(sH.loggingHandler(fileServer))
+	// index.html is served by notFound handler
+	router.NotFoundHandler = sH.loggingHandler(http.HandlerFunc(sH.notFound))
 }
 
 func (sH *StaticAssetsHandler) notFound(w http.ResponseWriter, r *http.Request) {

cmd/query/app/static_handler_test.go

@@ -63,6 +63,7 @@ func TestRegisterStaticHandler(t *testing.T) {
 		basePath         string // input to the test
 		subroute         bool   // should we create a subroute?
 		baseURL          string // expected URL prefix
+		logAccess        bool
 		expectedBaseHTML string // substring to match in the home page
 		UIConfigPath     string // path to UI config
 		expectedUIConfig string // expected UI config
@@ -71,6 +72,7 @@ func TestRegisterStaticHandler(t *testing.T) {
 			basePath:         "",
 			baseURL:          "/",
 			expectedBaseHTML: `<base href="/"`,
+			logAccess:        true,
 			UIConfigPath:     "",
 			expectedUIConfig: "JAEGER_CONFIG=DEFAULT_CONFIG;",
 		},
@@ -95,15 +97,16 @@ func TestRegisterStaticHandler(t *testing.T) {
 	}
 	for _, testCase := range testCases {
 		t.Run("basePath="+testCase.basePath, func(t *testing.T) {
-			logger, _ := testutils.NewLogger()
+			logger, logBuf := testutils.NewLogger()
 			r := mux.NewRouter()
 			if testCase.subroute {
 				r = r.PathPrefix(testCase.basePath).Subrouter()
 			}
 			RegisterStaticHandler(r, logger, &QueryOptions{
-				StaticAssets: "fixture",
-				BasePath:     testCase.basePath,
-				UIConfig:     testCase.UIConfigPath,
+				StaticAssets:          "fixture",
+				BasePath:              testCase.basePath,
+				UIConfig:              testCase.UIConfigPath,
+				LogStaticAssetsAccess: testCase.logAccess,
 			})
 
 			server := httptest.NewServer(r)
@@ -121,16 +124,18 @@ func TestRegisterStaticHandler(t *testing.T) {
 				return string(respByteArray)
 			}
 
-			respString := httpGet(favoriteIcon)
-			assert.Contains(t, respString, "Test Favicon") // this text is present in fixtures/favicon.ico
-
 			html := httpGet("") // get home page
 			assert.Contains(t, html, testCase.expectedUIConfig, "actual: %v", html)
 			assert.Contains(t, html, `JAEGER_VERSION = {"gitCommit":"","gitVersion":"","buildDate":""};`, "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)
+			if testCase.logAccess {
+				assert.Contains(t, logBuf.String(), "static/asset.txt")
+			} else {
+				assert.NotContains(t, logBuf.String(), "static/asset.txt")
+			}
 		})
 	}
 }