[Dev] Should we log PoolRequest and PoolRequestFullfilled observations?

[taimoorzaeem]

Problem

While working on #3802, we noticed in comment#3802 that we are not logging all observations. Namely PoolRequest and PoolRequestFullfilled (because we are only using them for metrics). Infact, we are just logging empty here for them here:

postgrest/src/PostgREST/Observation.hs

Lines 140 to 142 in db85e64

	)
	_ -> mempty
	where

This never gets executed because we intercept it in the Logger.hs:

postgrest/src/PostgREST/Logger.hs

Lines 91 to 94 in db85e64

	PoolRequest ->
	pure ()
	PoolRequestFullfilled ->
	pure ()

Solution

I think this needs a refactor and one way to solve this is to log these observations for loglevel >= debug. So it should be something like:

-- Logger.hs
  o@PoolRequest ->
    when (logLevel >= LogDebug) $ do
      logWithZTime loggerState $ observationMessage o
  o@PoolRequestFullfilled ->
    when (logLevel >= LogDebug) $ do
      logWithZTime loggerState $ observationMessage o

and

-- Observation.hs
 PoolRequest -> 
   "Connection requested"
 PoolRequestFullfilled -> 
   "Connection request fullfilled"

[steve-chavez]

I think this needs a refactor and one way to solve this is to log these observations for loglevel >= debug

Yes, that sounds fine. Maybe like:

-- Observation.hs
 PoolRequest -> 
   "Trying to borrow a connection from pool"
 PoolRequestFullfilled -> 
   "Borrowed a connection from the pool"

postgrest/src/PostgREST/Observation.hs

Lines 140 to 142 in db85e64

	)
	_ -> mempty
	where

What would be more important is to remove that mempty there, this to ensure we don't log empty lines accidentally like what happened on #3802 (comment)