Introduce ServerTiming for queries

[develop7]

Roadmap:

• db — database response time
• render — resultset render time
• plan — ❓ ❓ ❓ do we need it? is it worth it? does it get stuck?
• jwt — authentication time

quoting #2771 (comment):

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":

• Make the Response module pure (wip on steve-chavez@edbc4f9)
• Make the Query module pure
  • This will also help implement Feature request: show SQL query in terminal(log-level=debug) #1578 (comment)
  • The Query module has functions, like failNotSingular, that are impure in nature (they need to rollback), these can be separated into another AbortQuery module.
• Then have a single function entrypoint for each module.

[develop7]

hey @steve-chavez do we need to make Response.info{Ident,Info,Proc}Response pure as well? My guess is we do, but not right now

postgrest/src/PostgREST/Response.hs

Lines 194 to 214 in a6e3eda

	infoIdentResponse :: QualifiedIdentifier -> SchemaCache -> Wai.Response
	infoIdentResponse identifier sCache =
	case HM.lookup identifier (dbTables sCache) of
	Just tbl -> respondInfo $ allowH tbl
	Nothing -> Error.errorResponseFor $ Error.ApiRequestError ApiRequestTypes.NotFound
	where
	allowH table =
	let hasPK = not . null $ tablePKCols table in
	BS.intercalate "," $
	["OPTIONS,GET,HEAD"] ++
	["POST" | tableInsertable table] ++
	["PUT" | tableInsertable table && tableUpdatable table && hasPK] ++
	["PATCH" | tableUpdatable table] ++
	["DELETE" | tableDeletable table]
	infoProcResponse :: Routine -> Wai.Response
	infoProcResponse proc | pdVolatility proc == Volatile = respondInfo "OPTIONS,POST"
	| otherwise = respondInfo "OPTIONS,GET,HEAD,POST"
	infoRootResponse :: Wai.Response
	infoRootResponse = respondInfo "OPTIONS,GET,HEAD"

[steve-chavez]

Hey @develop7, yeah so infoIdentResponse is the response for the OPTIONS method and this one is special because it doesn't have a Plan or Query stage - it doesn't execute any query.

Hm, maybe it's not that relevant for Server-Timing but I think it would be good to have it pure for completion and consistency.

[steve-chavez]

plan — ❓ ❓ ❓ do we need it? is it worth it? does it get stuck?

Yeah, so during resource embedding I believe the plan could get slow. If that's true then we might try to optimize the code or cache the results somehow (idea here).

But first we would need to confirm if that is indeed the case.

[steve-chavez]

Might make sense to combine both the ApiRequest.userApiRequest and the Plan module work into the same Server-Timing duration too.

[develop7]

Gotchu, on it

[steve-chavez]

@develop7 The current refactor seem good to merge. Maybe you'd like to open another PR for that and leave this one open as sort of an "epic"?

Otherwise you'll have to rebase each time the Response module is modified, that's currently happening on #2969

[develop7]

it doesn't execute any query

Hmm, that means we need to make the db timing optional at least here, which requires all/some metrics to be optional.

I'm wondering if we could get away with something like

data ResponseTiming =  RMJwt | RMRender | RMDb

data ServerTimings = Map ResponseTiming Double

then pass the value of ServerTimings along, throwing metrics' values in as needed; then render the header somewhere near our newly introduced pgrstResponse, wrapper around WAI.Response. That should enable us to add extra metrics relatively easily.

What do you guys think?

[develop7]

Maybe you'd like to open another PR for that and leave this one open as sort of an "epic"?

Good point, let me shuffle commits a bit then.

[steve-chavez]

Hmm, that means we need to make the db timing optional at least here, which requires all/some metrics to be optional.

True.

That should enable us to add extra metrics relatively easily.
What do you guys think?

@develop7 Sounds good!

[develop7]

Good point, let me shuffle commits a bit then.

Shuffling done, the PR got accidentally closed though