nikita-volkov / hasql-pool

A pool of connections for Hasql
http://hackage.haskell.org/package/hasql-pool
MIT License
17 stars 15 forks source link

Observability #40

Closed nikita-volkov closed 6 months ago

nikita-volkov commented 7 months ago

Adds a stream of events describing the significant state changes in the pool for monitoring purposes.

nikita-volkov commented 7 months ago

@robx Hey Rob! Can you please give a feedback on this?

steve-chavez commented 7 months ago

Many thanks for this @nikita-volkov! Just started experimenting on https://github.com/PostgREST/postgrest/pull/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 https://github.com/PostgREST/postgrest/pull/2129.

nikita-volkov commented 7 months ago

Many thanks for this @nikita-volkov! Just started experimenting on PostgREST/postgrest#3229

Great! Thanks! Looking forward for feedback!

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.

Could you clarify on what you imply by "total waiters"? Unfortunately I couldn't derive that from the link.

steve-chavez commented 7 months ago

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 poolAcquisitionTimeout, on the postgREST side this results in an API error (ref).

For this, I understand we could use the following metrics:

1) pending_requests (same idea as total waiters) https://opentelemetry.io/docs/specs/semconv/database/database-metrics/#metric-dbclientconnectionspending_requests

2) pool wait time https://opentelemetry.io/docs/specs/semconv/database/database-metrics/#metric-dbclientconnectionswait_time

nikita-volkov commented 7 months ago

Thanks for the references! I'll see what can be done.

nikita-volkov commented 7 months ago

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?

steve-chavez commented 7 months ago

@nikita-volkov You're right! Thanks for the snippet!

steve-chavez commented 7 months ago

Monitor.observeHistogram monitor "db.client.connections.wait_time" (diffUTCTime endTime startTime)

Curious, where does observeHistogram come from? I found decGauge on hoogle but not observeHistogram.

nikita-volkov commented 7 months ago

Ah. It was just pseudocode. I was implying something like this.

nikita-volkov commented 6 months ago

Thanks for cooperation guys! It's released.