feat(postgresql): add disable_pg_stat_database and disable_pg_stat_bgwriter config options#1430
Conversation
There was a problem hiding this comment.
Pull request overview
Adds configurability to the PostgreSQL input so operators can skip collecting built-in metrics from pg_stat_database and/or pg_stat_bgwriter (and, on PG17+, the compatibility metrics from pg_stat_checkpointer). This helps reduce load/permissions requirements in environments where these views are undesirable to query.
Changes:
- Add
disable_pg_stat_databaseanddisable_pg_stat_bgwriterinstance config options and gate the corresponding built-in metric queries. - Refactor metric collection logic into
gatherMetrics()and introduce sqlmock-based unit tests for default/disabled/PG17 paths. - Document the new options in the input README and example TOML, and add the sqlmock dependency.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| inputs/postgresql/postgresql.go | Adds two config flags and conditionally skips pg_stat_database / pg_stat_bgwriter (+ PG17 pg_stat_checkpointer) collection; factors collection into gatherMetrics(). |
| inputs/postgresql/postgresql_test.go | New unit tests using go-sqlmock to verify query/metric behavior for default and disabled configurations and PG17 split behavior. |
| inputs/postgresql/README.md | Documents the new disable flags and clarifies PG17 behavior. |
| conf/input.postgresql/postgresql.toml | Adds commented configuration examples for the new flags. |
| go.mod / go.sum | Adds github.com/DATA-DOG/go-sqlmock dependency for tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ins := newTestInstance() | ||
| ins.db = db | ||
| ins.DisablePgStatBgwriter = true | ||
|
|
There was a problem hiding this comment.
The DisablePgStatBgwriter tests only cover the <17 code path. Since PostgreSQL 17+ also collects compatibility metrics from pg_stat_checkpointer, add a test case with Version=170000 and DisablePgStatBgwriter=true to assert that neither pg_stat_bgwriter nor pg_stat_checkpointer are queried and no related metrics are emitted.
…writer config options
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -218,89 +230,100 @@ func (ins *Instance) Gather(slist *types.SampleList) { | |||
| ins.Version = version | |||
| } | |||
There was a problem hiding this comment.
gatherMetrics always queries server_version_num when ins.Version == 0, even if both disable_pg_stat_bgwriter is true and enable_statement_metrics is false (and there are no other version-dependent code paths). This adds an unnecessary query per gather and means disabling both built-in metric groups can still trigger a version query. Consider only querying the version when it’s actually needed (e.g., when !DisablePgStatBgwriter || EnableStatementMetrics).
No description provided.