From 799f3e15b22a62651ecfff4fba12f0c796a17322 Mon Sep 17 00:00:00 2001 From: Joe Adams Date: Mon, 3 Oct 2022 22:30:07 -0400 Subject: [PATCH 1/3] Fix exclude-databases for collector package The pg_database collector was not respecting the --exclude-databases flag and causing problems where databases were not accessible. This now respects the list of databases to exclude. - Adjusts the Collector create func to take a config struct instead of a logger. This allows more changes like this in the future. I figured we would need to do this at some point but I wasn't sure if we could hold off. - Split the database size collection to a separate query when database is not excluded. - Comment some probe code that was not useful/accurate Signed-off-by: Joe Adams --- cmd/postgres_exporter/main.go | 10 +++- cmd/postgres_exporter/postgres_exporter.go | 4 +- cmd/postgres_exporter/probe.go | 35 ++++++------- collector/collector.go | 16 ++++-- collector/pg_database.go | 60 +++++++++++++++++++--- collector/pg_stat_bgwriter.go | 3 +- collector/probe.go | 8 ++- 7 files changed, 97 insertions(+), 39 deletions(-) diff --git a/cmd/postgres_exporter/main.go b/cmd/postgres_exporter/main.go index 6936226c8..642301065 100644 --- a/cmd/postgres_exporter/main.go +++ b/cmd/postgres_exporter/main.go @@ -14,8 +14,10 @@ package main import ( + "fmt" "net/http" "os" + "strings" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -101,13 +103,16 @@ func main() { os.Exit(1) } + excludedDatabases := strings.Split(*excludeDatabases, ",") + logger.Log("msg", "Excluded databases", "databases", fmt.Sprintf("%v", excludedDatabases)) + opts := []ExporterOpt{ DisableDefaultMetrics(*disableDefaultMetrics), DisableSettingsMetrics(*disableSettingsMetrics), AutoDiscoverDatabases(*autoDiscoverDatabases), WithUserQueriesPath(*queriesPath), WithConstantLabels(*constantLabelsList), - ExcludeDatabases(*excludeDatabases), + ExcludeDatabases(excludedDatabases), IncludeDatabases(*includeDatabases), } @@ -128,6 +133,7 @@ func main() { pe, err := collector.NewPostgresCollector( logger, + excludedDatabases, dsn, []string{}, ) @@ -143,7 +149,7 @@ func main() { w.Write(landingPage) // nolint: errcheck }) - http.HandleFunc("/probe", handleProbe(logger)) + http.HandleFunc("/probe", handleProbe(logger, excludedDatabases)) srv := &http.Server{} if err := web.ListenAndServe(srv, webConfig, logger); err != nil { diff --git a/cmd/postgres_exporter/postgres_exporter.go b/cmd/postgres_exporter/postgres_exporter.go index 0b56e768a..bc96bb0e6 100644 --- a/cmd/postgres_exporter/postgres_exporter.go +++ b/cmd/postgres_exporter/postgres_exporter.go @@ -484,9 +484,9 @@ func AutoDiscoverDatabases(b bool) ExporterOpt { } // ExcludeDatabases allows to filter out result from AutoDiscoverDatabases -func ExcludeDatabases(s string) ExporterOpt { +func ExcludeDatabases(s []string) ExporterOpt { return func(e *Exporter) { - e.excludeDatabases = strings.Split(s, ",") + e.excludeDatabases = s } } diff --git a/cmd/postgres_exporter/probe.go b/cmd/postgres_exporter/probe.go index 167e827c2..6f0e8b8a3 100644 --- a/cmd/postgres_exporter/probe.go +++ b/cmd/postgres_exporter/probe.go @@ -16,7 +16,6 @@ package main import ( "fmt" "net/http" - "time" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -26,7 +25,7 @@ import ( "github.com/prometheus/client_golang/prometheus/promhttp" ) -func handleProbe(logger log.Logger) http.HandlerFunc { +func handleProbe(logger log.Logger, excludeDatabases []string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() conf := c.GetConfig() @@ -62,21 +61,21 @@ func handleProbe(logger log.Logger) http.HandlerFunc { // TODO(@sysadmind): Timeout - probeSuccessGauge := prometheus.NewGauge(prometheus.GaugeOpts{ - Name: "probe_success", - Help: "Displays whether or not the probe was a success", - }) - probeDurationGauge := prometheus.NewGauge(prometheus.GaugeOpts{ - Name: "probe_duration_seconds", - Help: "Returns how long the probe took to complete in seconds", - }) + // probeSuccessGauge := prometheus.NewGauge(prometheus.GaugeOpts{ + // Name: "probe_success", + // Help: "Displays whether or not the probe was a success", + // }) + // probeDurationGauge := prometheus.NewGauge(prometheus.GaugeOpts{ + // Name: "probe_duration_seconds", + // Help: "Returns how long the probe took to complete in seconds", + // }) tl := log.With(logger, "target", target) - start := time.Now() + // start := time.Now() registry := prometheus.NewRegistry() - registry.MustRegister(probeSuccessGauge) - registry.MustRegister(probeDurationGauge) + // registry.MustRegister(probeSuccessGauge) + // registry.MustRegister(probeDurationGauge) opts := []ExporterOpt{ DisableDefaultMetrics(*disableDefaultMetrics), @@ -96,10 +95,10 @@ func handleProbe(logger log.Logger) http.HandlerFunc { registry.MustRegister(exporter) // Run the probe - pc, err := collector.NewProbeCollector(tl, registry, dsn) + pc, err := collector.NewProbeCollector(tl, excludeDatabases, registry, dsn) if err != nil { - probeSuccessGauge.Set(0) - probeDurationGauge.Set(time.Since(start).Seconds()) + // probeSuccessGauge.Set(0) + // probeDurationGauge.Set(time.Since(start).Seconds()) http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -115,10 +114,6 @@ func handleProbe(logger log.Logger) http.HandlerFunc { registry.MustRegister(pc) - duration := time.Since(start).Seconds() - probeDurationGauge.Set(duration) - probeSuccessGauge.Set(1) - // TODO check success, etc h := promhttp.HandlerFor(registry, promhttp.HandlerOpts{}) h.ServeHTTP(w, r) diff --git a/collector/collector.go b/collector/collector.go index 6d1a4dd11..997fd62bf 100644 --- a/collector/collector.go +++ b/collector/collector.go @@ -28,7 +28,7 @@ import ( ) var ( - factories = make(map[string]func(logger log.Logger) (Collector, error)) + factories = make(map[string]func(collectorConfig) (Collector, error)) initiatedCollectorsMtx = sync.Mutex{} initiatedCollectors = make(map[string]Collector) collectorState = make(map[string]*bool) @@ -62,7 +62,12 @@ type Collector interface { Update(ctx context.Context, db *sql.DB, ch chan<- prometheus.Metric) error } -func registerCollector(name string, isDefaultEnabled bool, createFunc func(logger log.Logger) (Collector, error)) { +type collectorConfig struct { + logger log.Logger + excludeDatabases []string +} + +func registerCollector(name string, isDefaultEnabled bool, createFunc func(collectorConfig) (Collector, error)) { var helpDefaultState string if isDefaultEnabled { helpDefaultState = "enabled" @@ -93,7 +98,7 @@ type PostgresCollector struct { type Option func(*PostgresCollector) error // NewPostgresCollector creates a new PostgresCollector. -func NewPostgresCollector(logger log.Logger, dsn string, filters []string, options ...Option) (*PostgresCollector, error) { +func NewPostgresCollector(logger log.Logger, excludeDatabases []string, dsn string, filters []string, options ...Option) (*PostgresCollector, error) { p := &PostgresCollector{ logger: logger, } @@ -126,7 +131,10 @@ func NewPostgresCollector(logger log.Logger, dsn string, filters []string, optio if collector, ok := initiatedCollectors[key]; ok { collectors[key] = collector } else { - collector, err := factories[key](log.With(logger, "collector", key)) + collector, err := factories[key](collectorConfig{ + logger: log.With(logger, "collector", key), + excludeDatabases: excludeDatabases, + }) if err != nil { return nil, err } diff --git a/collector/pg_database.go b/collector/pg_database.go index 8fa4dab8e..1ed841ca6 100644 --- a/collector/pg_database.go +++ b/collector/pg_database.go @@ -26,11 +26,19 @@ func init() { } type PGDatabaseCollector struct { - log log.Logger + log log.Logger + excludedDatabases []string } -func NewPGDatabaseCollector(logger log.Logger) (Collector, error) { - return &PGDatabaseCollector{log: logger}, nil +func NewPGDatabaseCollector(config collectorConfig) (Collector, error) { + exclude := config.excludeDatabases + if exclude == nil { + exclude = []string{} + } + return &PGDatabaseCollector{ + log: config.logger, + excludedDatabases: exclude, + }, nil } var pgDatabase = map[string]*prometheus.Desc{ @@ -41,20 +49,49 @@ var pgDatabase = map[string]*prometheus.Desc{ ), } -func (PGDatabaseCollector) Update(ctx context.Context, db *sql.DB, ch chan<- prometheus.Metric) error { +// Update implements Collector and exposes database size. +// It is called by the Prometheus registry when collecting metrics. +// The list of databases is retrieved from pg_database and filtered +// by the excludeDatabase config parameter. The tradeoff here is that +// we have to query the list of databases and then query the size of +// each database individually. This is because we can't filter the +// list of databases in the query because the list of excluded +// databases is dynamic. +func (c PGDatabaseCollector) Update(ctx context.Context, db *sql.DB, ch chan<- prometheus.Metric) error { + // Query the list of databases rows, err := db.QueryContext(ctx, `SELECT pg_database.datname - ,pg_database_size(pg_database.datname) - FROM pg_database;`) + FROM pg_database; + `, + ) if err != nil { return err } defer rows.Close() + var databases []string + for rows.Next() { var datname string + if err := rows.Scan(&datname); err != nil { + return err + } + + // Ignore excluded databases + // Filtering is done here instead of in the query to avoid + // a complicated NOT IN query with a variable number of parameters + if sliceContains(c.excludedDatabases, datname) { + continue + } + + databases = append(databases, datname) + } + + // Query the size of the databases + for _, datname := range databases { var size int64 - if err := rows.Scan(&datname, &size); err != nil { + err = db.QueryRowContext(ctx, "SELECT pg_database_size($1)", datname).Scan(&size) + if err != nil { return err } @@ -68,3 +105,12 @@ func (PGDatabaseCollector) Update(ctx context.Context, db *sql.DB, ch chan<- pro } return nil } + +func sliceContains(slice []string, s string) bool { + for _, item := range slice { + if item == s { + return true + } + } + return false +} diff --git a/collector/pg_stat_bgwriter.go b/collector/pg_stat_bgwriter.go index 69c75653f..5456423af 100644 --- a/collector/pg_stat_bgwriter.go +++ b/collector/pg_stat_bgwriter.go @@ -18,7 +18,6 @@ import ( "database/sql" "time" - "github.com/go-kit/log" "github.com/prometheus/client_golang/prometheus" ) @@ -29,7 +28,7 @@ func init() { type PGStatBGWriterCollector struct { } -func NewPGStatBGWriterCollector(logger log.Logger) (Collector, error) { +func NewPGStatBGWriterCollector(collectorConfig) (Collector, error) { return &PGStatBGWriterCollector{}, nil } diff --git a/collector/probe.go b/collector/probe.go index c8bf8ee48..9044c40f9 100644 --- a/collector/probe.go +++ b/collector/probe.go @@ -30,7 +30,7 @@ type ProbeCollector struct { db *sql.DB } -func NewProbeCollector(logger log.Logger, registry *prometheus.Registry, dsn config.DSN) (*ProbeCollector, error) { +func NewProbeCollector(logger log.Logger, excludeDatabases []string, registry *prometheus.Registry, dsn config.DSN) (*ProbeCollector, error) { collectors := make(map[string]Collector) initiatedCollectorsMtx.Lock() defer initiatedCollectorsMtx.Unlock() @@ -45,7 +45,11 @@ func NewProbeCollector(logger log.Logger, registry *prometheus.Registry, dsn con if collector, ok := initiatedCollectors[key]; ok { collectors[key] = collector } else { - collector, err := factories[key](log.With(logger, "collector", key)) + collector, err := factories[key]( + collectorConfig{ + logger: log.With(logger, "collector", key), + excludeDatabases: excludeDatabases, + }) if err != nil { return nil, err } From 8d6ce0558c73b362625fa6eb5a273de58ecbedba Mon Sep 17 00:00:00 2001 From: Joe Adams Date: Mon, 17 Oct 2022 19:23:51 -0400 Subject: [PATCH 2/3] Remove commented code Signed-off-by: Joe Adams --- cmd/postgres_exporter/probe.go | 14 +------------- collector/replication_slots.go | 4 ++-- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/cmd/postgres_exporter/probe.go b/cmd/postgres_exporter/probe.go index 6f0e8b8a3..2a7b29ec1 100644 --- a/cmd/postgres_exporter/probe.go +++ b/cmd/postgres_exporter/probe.go @@ -61,21 +61,9 @@ func handleProbe(logger log.Logger, excludeDatabases []string) http.HandlerFunc // TODO(@sysadmind): Timeout - // probeSuccessGauge := prometheus.NewGauge(prometheus.GaugeOpts{ - // Name: "probe_success", - // Help: "Displays whether or not the probe was a success", - // }) - // probeDurationGauge := prometheus.NewGauge(prometheus.GaugeOpts{ - // Name: "probe_duration_seconds", - // Help: "Returns how long the probe took to complete in seconds", - // }) - tl := log.With(logger, "target", target) - // start := time.Now() registry := prometheus.NewRegistry() - // registry.MustRegister(probeSuccessGauge) - // registry.MustRegister(probeDurationGauge) opts := []ExporterOpt{ DisableDefaultMetrics(*disableDefaultMetrics), @@ -83,7 +71,7 @@ func handleProbe(logger log.Logger, excludeDatabases []string) http.HandlerFunc AutoDiscoverDatabases(*autoDiscoverDatabases), WithUserQueriesPath(*queriesPath), WithConstantLabels(*constantLabelsList), - ExcludeDatabases(*excludeDatabases), + ExcludeDatabases(excludeDatabases), IncludeDatabases(*includeDatabases), } diff --git a/collector/replication_slots.go b/collector/replication_slots.go index 224db3ccf..ed37441cc 100644 --- a/collector/replication_slots.go +++ b/collector/replication_slots.go @@ -29,8 +29,8 @@ type PGReplicationSlotCollector struct { log log.Logger } -func NewPGReplicationSlotCollector(logger log.Logger) (Collector, error) { - return &PGReplicationSlotCollector{log: logger}, nil +func NewPGReplicationSlotCollector(config collectorConfig) (Collector, error) { + return &PGReplicationSlotCollector{log: config.logger}, nil } var pgReplicationSlot = map[string]*prometheus.Desc{ From 55263868e0bcddab8ae13e2897b9267e3a12a006 Mon Sep 17 00:00:00 2001 From: Joe Adams Date: Sun, 5 Mar 2023 15:30:18 -0500 Subject: [PATCH 3/3] Remove more dead code Signed-off-by: Joe Adams --- cmd/postgres_exporter/probe.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/postgres_exporter/probe.go b/cmd/postgres_exporter/probe.go index 2a7b29ec1..827314a7a 100644 --- a/cmd/postgres_exporter/probe.go +++ b/cmd/postgres_exporter/probe.go @@ -85,8 +85,6 @@ func handleProbe(logger log.Logger, excludeDatabases []string) http.HandlerFunc // Run the probe pc, err := collector.NewProbeCollector(tl, excludeDatabases, registry, dsn) if err != nil { - // probeSuccessGauge.Set(0) - // probeDurationGauge.Set(time.Since(start).Seconds()) http.Error(w, err.Error(), http.StatusInternalServerError) return }