PostgREST / postgrest

REST API for any Postgres database
https://postgrest.org
MIT License
23.43k stars 1.03k forks source link

On-demand server-timing #3410

Open steve-chavez opened 7 months ago

steve-chavez commented 7 months ago

Problem

Setting server-timing-enabled to true has a ~6.5% RPS decrease. Using GETSimple on https://github.com/PostgREST/postgrest-benchmark:

no timings = 2176.4712194 rps with timings = 2033.3584706 rps

I don't think there's a way around it since calculating the timings requires using clock time which needs a system call (clock_gettime).

Solution

Allow getting server timing headers through a new preference header.

Prefer: metrics=timings

metrics=timings would be the only value for now, but we could extend it later on.

This can later be enabled/disabled with https://github.com/PostgREST/postgrest/issues/2987.

While at it, deprecate server-timing-enabled since it has a perf impact.

develop7 commented 6 months ago

Setting server-timing-enabled to true has a ~6.5% RPS decrease.

Damn, I wonder how much slower will OTel be when enabled then.

steve-chavez commented 6 months ago

Yeah, this makes sense since server-timing is a trace and by definition:

Running traces takes too much time and consumes too many resources to trace every request an application receives. https://www.techtarget.com/searchitoperations/tip/The-3-pillars-of-observability-Logs-metrics-and-traces

Damn, I wonder how much slower will OTel be when enabled then.

Not yet familiar with OTel (and the Push model) but doesn't it do some buffering before sending the events to a collector?

I've also read that they do sampling, so not all requests are traced. This would reduce resource consumption.


It would be interesting to have an internal sampling mechanism, so we could have some server-timing traces exposed as metrics. (just an idea for now, not sure if worth it)