add pg_stat_statements plans - #1315
Conversation
Add pg_stat_statements metrics: plans_total: Number of times planned plans_seconds_total: Total time spent in planning the statement It needs the PostgreSQL setting: pg_stat_statements.track_planning Signed-off-by: Julien Francoz <julien@les-tilleuls.coop>
| statStatementsPlansSecondsTotal = prometheus.NewDesc( | ||
| prometheus.BuildFQName(namespace, statStatementsSubsystem, "plans_seconds_total"), | ||
| "Total time spent in planning the statement, in seconds", | ||
| []string{"user", "datname", "queryid"}, |
There was a problem hiding this comment.
queryID looks like a label with unbounded cardinality. Could we drop it?
There was a problem hiding this comment.
I need it, for example to get the ratio of calls by plan and find queries which are badly used (for example prepare + bind inside a loop)
There was a problem hiding this comment.
This doesn't work well with Prometheus though 😅.
Maybe that could become an exemplar label in the histogram?
There was a problem hiding this comment.
I don’t understand the problem. It’s already the same in all the other metrics from pg_stat_statements.
There was a problem hiding this comment.
Apologies, I probably should have said this from the beginning. I'm not a maintainer for this repository! If the maintainers are ok with it, that's what counts
There was a problem hiding this comment.
What I meant by "this doesn't work well with Prometheus" is that a single query will have a single ID and generate a single timeseries; another query will get a different ID and therefore become a brand-new timeseries.
All those unique IDs will become an entry in Prometheus's Posting index (good blog in case you'd like to read more). A gigantic posting index will make PromQL queries slower and increase resource usage
ArthurSens
left a comment
There was a problem hiding this comment.
Hi, getting back to this after a few months of extra experience with this exporter 😅. I'm ok with the queryID now, I understand that this is already present everywhere.
The codebase has been heavily refactored in the last few months (#1330), so a rebase will be needed, or even a fresh new PR, whatever is easier.
#1351 added configuration to preserve correct BIGINT numbers, so we also need to keep this config for the new metrics.
And last question: The collector currently selects its top statements by execution time. A planning-heavy but execution-cheap query (the main use case for this change) could therefore be excluded by the limit. Would ordering by combined planning and execution time, or making the ordering configurable, better serve this feature?
| 0 as plans_total, | ||
| 0 as plans_seconds_total, |
There was a problem hiding this comment.
Not emitting the metric at all sounds better than a 0, people might misinterpret this as the table actually returning 0.
Add pg_stat_statements metrics:
plans_total: Number of times planned
plans_seconds_total: Total time spent in planning the statement
It needs the PostgreSQL setting: pg_stat_statements.track_planning