-
Notifications
You must be signed in to change notification settings - Fork 15
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
Observability #40
Observability #40
Conversation
@robx Hey Rob! Can you please give a feedback on this? |
Many thanks for this @nikita-volkov! Just started experimenting on PostgREST/postgrest#3229 Do you think hasql-pool could also expose metrics like "total waiters"? At one point (when hasql-pool used resource-pool) we had that one and other metrics, see PostgREST/postgrest#2129. |
-- If the action is not lightweight, it's recommended to use intermediate bufferring via channels like TBQueue. | ||
-- E.g., if the action is @'atomically' . 'writeTBQueue' yourQueue@, then reading from it and processing can be done on a separate thread. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When could this happen? When the pool size is big and there are lots of connections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm talking about the user supplied action here. It should be quick since it will block the pool processes.
library/Hasql/Pool/Observation.hs
Outdated
= ConnectionEstablishedObservation UUID | ||
| AttemptingToConnectObservation UUID | ||
| FailedToConnectObservation UUID (Maybe ByteString) | ||
| ConnectionReleasedObservation UUID ReleaseReason |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Why does the UUID need to be generated internally? (just wondering if it can be left out for simplifying the code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's for identifying the connection. Lets the user isolate events on a particular connection
Great! Thanks! Looking forward for feedback!
Could you clarify on what you imply by "total waiters"? Unfortunately I couldn't derive that from the link. |
So the motivation is to be able to prevent For this, I understand we could use the following metrics:
|
Thanks for the references! I'll see what can be done. |
I've taken a look. Seems like both of those metrics are already computable without the features involved in this PR. executeObservedSession :: Pool -> Monitor -> Session -> IO ()
executeObservedSession pool monitor session = do
Monitor.incGauge monitor "db.client.connections.pending_requests"
startTime <- getCurrentTime
Pool.use pool do
liftIO $ do
endTime <- getCurrentTime
Monitor.observeHistogram monitor "db.client.connections.wait_time" (diffUTCTime endTime startTime)
Monitor.decGauge monitor "db.client.connections.pending_requests"
session @steve-chavez Is that correct? |
@nikita-volkov You're right! Thanks for the snippet! |
Curious, where does |
Ah. It was just pseudocode. I was implying something like this. |
Thanks for cooperation guys! It's released. |
Adds a stream of events describing the significant state changes in the pool for monitoring purposes.