Fix versioned metrics (#2316)

* Apply versionMiddleware last This makes sure that every other middleware sees the rewritten (unversioned) path. In particular, the prometheus middleware will now only see paths it knows about, which prevents it from reporting "N/A" as the path.
wireapp · Apr 27, 2022 · 5ffb6cc · 5ffb6cc
1 parent 70ce220
commit 5ffb6cc
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 14 deletions.
diff --git a/changelog.d/5-internal/fix-versioned-metrics b/changelog.d/5-internal/fix-versioned-metrics
@@ -0,0 +1,5 @@
+Apply `versionMiddleware` last. This makes sure that every other middleware sees
+the rewritten (unversioned) path. In particular, the prometheus middleware will
+now only see paths it knows about, which prevents it from reporting "N/A" as the
+path.
+
diff --git a/services/brig/src/Brig/Run.hs b/services/brig/src/Brig/Run.hs
@@ -115,11 +115,11 @@ mkApp o = do
 
     middleware :: Env -> (RequestId -> Wai.Application) -> Wai.Application
     middleware e =
-      Metrics.servantPlusWAIPrometheusMiddleware (sitemap @BrigCanonicalEffects) (Proxy @ServantCombinedAPI)
+      versionMiddleware -- this rewrites the request, so it must be at the top (i.e. applied last)
+        . Metrics.servantPlusWAIPrometheusMiddleware (sitemap @BrigCanonicalEffects) (Proxy @ServantCombinedAPI)
         . GZip.gunzip
         . GZip.gzip GZip.def
         . catchErrors (e ^. applog) [Right $ e ^. metrics]
-        . versionMiddleware
         . lookupRequestIdMiddleware
     app e r k = runHandler e r (Server.route rtree r k) k
 

diff --git a/services/cannon/src/Cannon/Run.hs b/services/cannon/src/Cannon/Run.hs
@@ -72,10 +72,10 @@ run o = do
   s <- newSettings $ Server (o ^. cannon . host) (o ^. cannon . port) (applog e) m (Just idleTimeout)
   let middleware :: Wai.Middleware
       middleware =
-        servantPrometheusMiddleware (Proxy @CombinedAPI)
+        versionMiddleware
+          . servantPrometheusMiddleware (Proxy @CombinedAPI)
           . Gzip.gzip Gzip.def
           . catchErrors g [Right m]
-          . versionMiddleware
       app :: Application
       app = middleware (serve (Proxy @CombinedAPI) server)
       server :: Servant.Server CombinedAPI

diff --git a/services/cargohold/src/CargoHold/Run.hs b/services/cargohold/src/CargoHold/Run.hs
@@ -70,10 +70,10 @@ mkApp o = Codensity $ \k ->
   where
     middleware :: Env -> Wai.Middleware
     middleware e =
-      servantPrometheusMiddleware (Proxy @CombinedAPI)
+      versionMiddleware
+        . servantPrometheusMiddleware (Proxy @CombinedAPI)
         . GZip.gzip GZip.def
         . catchErrors (e ^. appLogger) [Right $ e ^. metrics]
-        . versionMiddleware
     servantApp :: Env -> Application
     servantApp e0 r =
       let e = set requestId (maybe def RequestId (lookupRequestId r)) e0

diff --git a/services/galley/src/Galley/Run.hs b/services/galley/src/Galley/Run.hs
@@ -89,11 +89,11 @@ mkApp o = do
         Log.flush l
         Log.close l
       middlewares =
-        servantPlusWAIPrometheusMiddleware API.sitemap (Proxy @CombinedAPI)
+        versionMiddleware
+          . servantPlusWAIPrometheusMiddleware API.sitemap (Proxy @CombinedAPI)
           . GZip.gunzip
           . GZip.gzip GZip.def
           . catchErrors l [Right m]
-          . versionMiddleware
   return (middlewares $ servantApp e, e, finalizer)
   where
     rtree = compile API.sitemap

diff --git a/services/gundeck/src/Gundeck/Run.hs b/services/gundeck/src/Gundeck/Run.hs
@@ -63,11 +63,11 @@ run o = do
   where
     middleware :: Env -> Wai.Middleware
     middleware e =
-      waiPrometheusMiddleware sitemap
+      versionMiddleware
+        . waiPrometheusMiddleware sitemap
         . GZip.gunzip
         . GZip.gzip GZip.def
         . catchErrors (e ^. applog) [Right $ e ^. monitor]
-        . versionMiddleware
 
 mkApp :: Env -> Wai.Application
 mkApp e r k = runGundeck e r (route routes r k)

diff --git a/services/proxy/src/Proxy/Run.hs b/services/proxy/src/Proxy/Run.hs
@@ -41,7 +41,7 @@ run o = do
   let rtree = compile (sitemap e)
   let app r k = runProxy e r (route rtree r k)
   let middleware =
-        waiPrometheusMiddleware (sitemap e)
+        versionMiddleware
+          . waiPrometheusMiddleware (sitemap e)
           . catchErrors (e ^. applog) [Right m]
-          . versionMiddleware
   runSettings s (middleware app) `finally` destroyEnv e
diff --git a/services/spar/src/Spar/Run.hs b/services/spar/src/Spar/Run.hs
@@ -116,7 +116,8 @@ mkApp sparCtxOpts = do
           . Bilge.port (sparCtxOpts ^. to galley . epPort)
           $ Bilge.empty
   let wrappedApp =
-        WU.heavyDebugLogging heavyLogOnly logLevel sparCtxLogger
+        versionMiddleware
+          . WU.heavyDebugLogging heavyLogOnly logLevel sparCtxLogger
           . servantPrometheusMiddleware (Proxy @API)
           . WU.catchErrors sparCtxLogger []
           -- Error 'Response's are usually not thrown as exceptions, but logged in
@@ -125,7 +126,6 @@ mkApp sparCtxOpts = do
           -- still here for errors outside the power of the 'Application', like network
           -- outages.
           . SAML.setHttpCachePolicy
-          . versionMiddleware
           . lookupRequestIdMiddleware
           $ \sparCtxRequestId -> app Env {..}
       heavyLogOnly :: (Wai.Request, LByteString) -> Maybe (Wai.Request, LByteString)