Run database-scoped collectors against every discovered database - #1378
Conversation
24d4d3d to
747fa8b
Compare
collector.NewRuntime only ever built one PostgresCollector, bound to DataSourceNames[0]. This meant per-database collectors like stat_user_tables, statio_user_tables and statio_user_indexes silently never ran for any database beyond the first configured DSN, whether that list came from a static, comma-separated DATA_SOURCE_NAME or from --auto-discover-databases. Naively running every collector against every discovered database isn't safe either: most collectors (bgwriter, wal, stat_activity, stat_statements, locks, ...) read catalog views or functions that already report on the whole instance from any single connection, so running them again from a second database would re-report the same rows and the registry would panic on the duplicate metric (as already happened once, for stat_activity, in prometheus-community#533). This adds an explicit scope to each registered collector (clusterScope vs databaseScope) and a multiInstanceCollector that fans the database-scoped ones out across every DSN exporter.Exporter currently targets (static list or, with autodiscovery enabled, freshly enumerated databases), while cluster-scoped collectors still only ever run once, against the originally configured DSN. The target list is re-evaluated on every scrape, so databases created or dropped after startup are picked up without restarting the exporter, matching the autodiscovery behavior already used by the legacy exporter.Exporter metrics. Also gives statio_user_indexes a datname label, matching its sibling per-database collectors: without it, index metrics from two databases with a same-named index would collide once run concurrently. Signed-off-by: Matthew Wimpelberg <120263653+mwimpelberg28@users.noreply.github.com>
747fa8b to
22e8e25
Compare
…-collectors-per-discovered-database Signed-off-by: Matthew Wimpelberg <120263653+mwimpelberg28@users.noreply.github.com> # Conflicts: # collector/collector.go # collector/runtime.go
|
Hi @SuperQ @wrouesnel @sysadmind — this has been open for a bit with no review yet. Also flagging that the Summary of the change: Happy to address any feedback. Thanks for taking a look! |
collector.NewRuntime only ever built one PostgresCollector, bound to DataSourceNames[0]. This meant per-database collectors like stat_user_tables, statio_user_tables and statio_user_indexes silently never ran for any database beyond the first configured DSN, whether that list came from a static, comma-separated DATA_SOURCE_NAME or from --auto-discover-databases.
Naively running every collector against every discovered database isn't safe either: most collectors (bgwriter, wal, stat_activity, stat_statements, locks, ...) read catalog views or functions that already report on the whole instance from any single connection, so running them again from a second database would re-report the same rows and the registry would panic on the duplicate metric (as already happened once, for stat_activity, in #533).
This adds an explicit scope to each registered collector (clusterScope vs databaseScope) and a multiInstanceCollector that fans the database-scoped ones out across every DSN exporter.Exporter currently targets (static list or, with autodiscovery enabled, freshly enumerated databases), while cluster-scoped collectors still only ever run once, against the originally configured DSN. The target list is re-evaluated on every scrape, so databases created or dropped after startup are picked up without restarting the exporter, matching the autodiscovery behavior already used by the legacy exporter.Exporter metrics.
Also gives statio_user_indexes a datname label, matching its sibling per-database collectors: without it, index metrics from two databases with a same-named index would collide once run concurrently.