Skip to content

Fix versioned metrics #2316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pcapriotti merged 2 commits into from
Apr 27, 2022
Merged

Fix versioned metrics #2316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e1b5c1
Apply versionMiddleware last
pcapriotti Apr 25, 2022
826f440
Add CHANGELOG entry
pcapriotti Apr 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog.d/5-internal/fix-versioned-metrics
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.

4 changes: 2 additions & 2 deletions services/brig/src/Brig/Run.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions services/cannon/src/Cannon/Run.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions services/cargohold/src/CargoHold/Run.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions services/galley/src/Galley/Run.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions services/gundeck/src/Gundeck/Run.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions services/proxy/src/Proxy/Run.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions services/spar/src/Spar/Run.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down