Skip to content
New issue

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

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

Already on GitHub? # to your account

Log PoolRequest and PoolRequestFullfilled observations #3903

Closed
taimoorzaeem opened this issue Feb 5, 2025 · 0 comments · Fixed by #3925
Closed

Log PoolRequest and PoolRequestFullfilled observations #3903

taimoorzaeem opened this issue Feb 5, 2025 · 0 comments · Fixed by #3925

Comments

@taimoorzaeem
Copy link
Collaborator

Discussed in #3899

Originally posted by taimoorzaeem February 4, 2025

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:

)
_ -> mempty
where

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

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 -> 
   "Trying to borrow a connection from pool"
 PoolRequestFullfilled -> 
   "Borrowed a connection from the pool"

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)

# for free to join this conversation on GitHub. Already have an account? # to comment
Development

Successfully merging a pull request may close this issue.

1 participant