Server-Timing response header
#2771
Comments
steve-chavez
added
the
idea
|
Seems this might be simple with the timeit package: https://hackage.haskell.org/package/timeit-1.0.0.0/docs/System-TimeIt.html. Since the functions are so small we could also vendor the functionality. Got that from this blog post: https://chrisdone.com/posts/measuring-duration-in-haskell/ |
|
If parsing(parsec) time is high, we can look into switching to https://github.com/AndrasKovacs/flatparse edit: maybe unfeasible as flatparse looks low-level and lacking some of the combinators we use. |
steve-chavez
added
enhancement
|
Doing this for (ActionRead headersOnly, TargetIdent identifier) -> do
wrPlan <- liftEither $ Plan.wrappedReadPlan identifier conf sCache apiReq
-- timeit
resultSet <- runQuery roleIsoLvl (Plan.wrTxMode wrPlan) $ Query.readQuery wrPlan conf apiReq
-- timeit
return $ Response.readResponse headersOnly identifier apiReq resultSet
(ActionMutate MutationCreate, TargetIdent identifier) -> do
mrPlan <- liftEither $ Plan.mutateReadPlan MutationCreate apiReq identifier conf sCache
-- timeit
resultSet <- runQuery roleIsoLvl (Plan.mrTxMode mrPlan) $ Query.createQuery mrPlan apiReq conf
-- timeit
return $ Response.createResponse identifier mrPlan apiReq resultSet (ref) We would need refactoring for these. Basically have a single function exposed on the Plan, Query and Response modules and have App use those. For now I'm thinking of the following steps, the main idea is to make the Response and Query modules pure; this will also enable those to be "doctested":
For postgrest/src/PostgREST/App.hs Line 109 in 531a183 Or somewhere in the Auth module, maybe by storing it on the Vault and then retrieving it on App. |
|
@steve-chavez @laurenceisla I have sort of implemented the |
|
@taimoorzaeem I think it should be fine as a spec test, there are some examples here where we use different configs for each spec: Lines 91 to 265 in 531a183 |
@steve-chavez What exactly is the difference between |
|
@taimoorzaeem Yes, that was the idea. To be clear maybe we can name that duration Btw, since you need this for the jwt caching feat, adding only the jwt duration to |
|
According to the following blog post, there is a nice browser integration for server timing. https://www.smashingmagazine.com/2018/10/performance-server-timing/ A couple of things caught my attention.
TBD We could enable server timing without
TBD We could have an initial description like There's also a JS integration: https://developer.mozilla.org/en-US/docs/Web/API/PerformanceServerTiming Also note:
|
Doesn't look that way — the |
[…skip…] Right now, after #2971 got merged, the code looks bit better: postgrest/src/PostgREST/App.hs Lines 175 to 229 in 4c44782 Still repetitive, but, uh, more uniform. All I can think of factoring out without introducing extra indirection is On the other hand, actually delivering metrics for -- ...skip...
handleRequest AuthResult{..} conf appState authenticated prepared pgVer apiReq@ApiRequest{..} sCache serverTimingParams =
case (iAction, iTarget) of
(ActionRead headersOnly, TargetIdent identifier) -> do
(planTime, wrPlan) <- timeItT $ liftEither $ Plan.wrappedReadPlan identifier conf sCache apiReq
(rsTime, resultSet) <- timeItT $ runQuery roleIsoLvl (Plan.wrTxMode wrPlan) $ Query.readQuery wrPlan conf apiReq
(renderTime, pgrst) <- timeItT $ liftEither $ Response.readResponse wrPlan headersOnly identifier apiReq resultSet serverTimingParams
-- ...skip...Well, there's rendering the header too, but the goal at hand is still close. |
On a previous discussion we had the idea to have App do the IO and make the other modules pure, then have a single entrypoint for each module instead of having Note: previous discussion was here and is part of this big issue.
Yeah, so it can be done with some duplication but it'd be cleaner with the refactor. That being said, implementing without being DRY should be fine. We can refactor it later. |
Problem
Currently there's no way to debug the time we take to parse the request and the time we take to receive the query response. The EXPLAIN execution plan, only obtains the query time, but not the aforementioned two.
Solution
Add the standard
Server-Timingheader. This can communicate all types of metrics for a given request-response cycle.An example response would be like:
Which would mean the jwt ("jwt") decoding time was 34 ms, the parsing + planning("app") took 47.2ms, the database ("db") query time was 53 ms, and we took 15 ms to obtain the query response("render", this is relevant to #2770).
To avoid doing this work for all the requests:
Server-Timingwhenever thePrefer: server-timing=includeheader is specified.References:
The text was updated successfully, but these errors were encountered: