Skip to content
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.

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

Why there is no endpoint info in wai-middleware-prometheus metrics? #28

Open
qrilka opened this issue Dec 14, 2017 · 6 comments
Open

Why there is no endpoint info in wai-middleware-prometheus metrics? #28

qrilka opened this issue Dec 14, 2017 · 6 comments

Comments

@qrilka
Copy link
Contributor

qrilka commented Dec 14, 2017

It seems to me that use of wai-middleware-prometheus is quite limited as usually people use multiple endpoints and measuring average value for all of them (while the nature of those endpoints could be very different) sounds to be at least odd.
Do I miss some way to get per endpoint metrics or I should create a PR adding extra label with endpoint information?

@fimad
Copy link
Owner

fimad commented Dec 15, 2017

There are currently two options.

If you've already architected your server as a composition of Wai Application's then you can use instrumentApp to instrument each component application with specific label.

If you have a monolithic application you can use instrumentIO, though that won't include method and response code.

If there is a use case not covered by those methods then I'd be happy to take a PR adding additional APIs that support your use case.

@qrilka
Copy link
Contributor Author

qrilka commented Dec 18, 2017

@fimad but is it assumed that every endpoint should be in a separate Wai Application? I don't remember any application using design like this.
I suppose I could add 1 extra middleware and use instrumentIO with #26 fixed but it looks odd to me that wai-middleware-prometheus doesn't allow me to instrument wai app with granularity of endpoint and not complete wai app.

@fimad
Copy link
Owner

fimad commented Dec 19, 2017

Assuming #26 is fixed it seems like instrumentIO does what you are looking for? It does not make assumptions about how you structure your endpoints.

@qrilka
Copy link
Contributor Author

qrilka commented Dec 19, 2017

It does. My question was rather about instrumentApp sitting inside of prometheus for which I don't see a good use case but if you do feel free to close this issue

@qrilka
Copy link
Contributor Author

qrilka commented Dec 19, 2017

Other thing is that using instrumentIO gives a metric named http_request_duration_seconds_bucket while having name and signature talking just about some IO a. Could be quite confusing.

@MaxGabriel
Copy link
Contributor

This is the setup we did. I use a template haskell function to generate a fixed string for each Yesod endpoint to use as a label in prometheus, along with the HTTP method. Then in a Wai middleware, I log the timing for that endpoint.

module Mercury.Yesod.Routes.Metrics.TH (mkRouteToMetricName) where

import ClassyPrelude
import qualified Data.Text as T
import Language.Haskell.TH.Syntax
import Yesod.Core (Route (..))
import Yesod.Routes.TH.Types

-- | Template Haskell to generate a function named routeToMetricName.
--
-- For a route like HomeR, this function returns "HomeR".
--
-- For routes with parents, this function returns e.g. "APIR_MercuryR_AccountCardsR".
mkRouteToMetricName :: Name -> [ResourceTree a] -> Q [Dec]
mkRouteToMetricName appName ress = do
  let fnName = mkName "routeToMetricName"
      t1 `arrow` t2 = ArrowT `AppT` t1 `AppT` t2

  clauses <- mapM (goTree id []) ress

  return
    [ SigD fnName ((ConT ''Route `AppT` ConT appName) `arrow` ConT ''Text),
      FunD fnName $ concat clauses
    ]

-- This code was primarily copied from https://github.com/yesodweb/yesod/blob/e7cf662af7971c5558130de45c0be2d47b99324a/yesod-core/src/Yesod/Routes/TH/RouteAttrs.hs
-- Then modified to support the use case of building up metric names
-- (I usually would use more verbose names, possibly rewrite this? Somewhat nice to just be copying library code though)

goTree :: (Pat -> Pat) -> [String] -> ResourceTree a -> Q [Clause]
goTree front names (ResourceLeaf res) = return <$> goRes front names res
goTree front names (ResourceParent name _check pieces trees) =
  concat <$> mapM (goTree front' newNames) trees
  where
    ignored = (replicate toIgnore WildP ++) . return
    toIgnore = length $ filter isDynamic pieces
    isDynamic Dynamic {} = True
    isDynamic Static {} = False
    front' = front . ConP (mkName name) . ignored
    newNames = names <> [name]

goRes :: (Pat -> Pat) -> [String] -> Resource a -> Q Clause
goRes front names Resource {..} =
  return $
    Clause
      [front $ RecP (mkName resourceName) []]
      (NormalB $ toText $ intercalate "_" (names <> [resourceName]))
      []
  where
    toText s = VarE 'T.pack `AppE` LitE (StringL s)
module Mercury.Yesod.Routes.Metrics.Function (routeToMetricName) where

import App -- Open import to import all routes
import Mercury.Yesod.Routes.Metrics.TH (mkRouteToMetricName)

-- Generates routeToMetricName
$(mkRouteToMetricName ''App resourcesApp)
-- | Module to track metrics on all incoming requests
module Mercury.Network.Wai.Middleware.Metrics (collectMetrics) where

import App (App (..))
import ClassyPrelude
import Data.String.Conversions (cs)
import qualified Data.Text.Lazy as DTL
import qualified Data.Text.Lazy.Builder as TLB
import qualified Data.Text.Lazy.Builder.Int as TLBI
import Mercury.Timing (getElapsedSeconds, getStartTime)
import Mercury.Yesod.Routes.Metrics.Function (routeToMetricName)
import Metrics
import Network.HTTP.Types.Status (Status (..))
import Network.Wai (Middleware)
import qualified Network.Wai as Wai
import Prometheus (incCounter, observe, withLabel)
import Yesod.Core (Route, parseRoute) -- NB: Avoid importing unqualified from Yesod, since Yesod names conflict with Wai ones

collectMetrics :: App -> Middleware
collectMetrics app waiApp req sendResponse = do
  -- Technically parseRoute takes a query string
  -- But, afaict it isn't actually used to create the route
  -- And to pass it in I need to do a little parsing, so skipping for now
  let mRoute :: Maybe (Route App) = parseRoute (Wai.pathInfo req, [])
      routeMetricName = maybe "404" routeToMetricName mRoute
      Metrics {..} = appMetrics app
      methodText = cs $ Wai.requestMethod req

  start <- getStartTime
  withLabel incomingHTTPTotal (routeMetricName, methodText) incCounter

  waiApp req $ \(res :: Wai.Response) -> do
    let httpCodeText = DTL.toStrict $ TLB.toLazyText $ TLBI.decimal $ statusCode $ Wai.responseStatus res
    withLabel outgoingHTTPTotal (routeMetricName, methodText, httpCodeText) incCounter

    elapsed <- getElapsedSeconds start
    withLabel incomingHTTPSeconds (routeMetricName, methodText, httpCodeText) (`observe` elapsed)
    withLabel incomingHTTPHistogramSeconds (routeMetricName, methodText, httpCodeText) (`observe` elapsed)

    sendResponse res

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants