diff --git a/cmd/postgres_exporter/pg_setting.go b/cmd/postgres_exporter/pg_setting.go index 0f770d660..fa7a58386 100644 --- a/cmd/postgres_exporter/pg_setting.go +++ b/cmd/postgres_exporter/pg_setting.go @@ -82,6 +82,8 @@ func (s *pgSetting) metric() prometheus.Metric { return prometheus.MustNewConstMetric(desc, prometheus.GaugeValue, val) } +// TODO: fix linter override +// nolint: nakedret func (s *pgSetting) normaliseUnit() (val float64, unit string, err error) { val, err = strconv.ParseFloat(s.setting, 64) if err != nil { diff --git a/cmd/postgres_exporter/postgres_exporter.go b/cmd/postgres_exporter/postgres_exporter.go index d0741943f..dbe4e2184 100644 --- a/cmd/postgres_exporter/postgres_exporter.go +++ b/cmd/postgres_exporter/postgres_exporter.go @@ -20,6 +20,7 @@ import ( "gopkg.in/yaml.v2" "crypto/sha256" + "github.com/blang/semver" _ "github.com/lib/pq" "github.com/prometheus/client_golang/prometheus" @@ -489,7 +490,7 @@ func makeDescMap(pgVersion semver.Version, metricMaps map[string]map[string]Colu log.Debugln(columnName, "is being forced to discard due to version incompatibility.") thisMap[columnName] = MetricMap{ discard: true, - conversion: func(in interface{}) (float64, bool) { + conversion: func(_ interface{}) (float64, bool) { return math.NaN(), true }, } @@ -498,11 +499,12 @@ func makeDescMap(pgVersion semver.Version, metricMaps map[string]map[string]Colu } // Determine how to convert the column based on its usage. + // nolint: dupl switch columnMapping.usage { case DISCARD, LABEL: thisMap[columnName] = MetricMap{ discard: true, - conversion: func(in interface{}) (float64, bool) { + conversion: func(_ interface{}) (float64, bool) { return math.NaN(), true }, } @@ -577,7 +579,9 @@ func makeDescMap(pgVersion semver.Version, metricMaps map[string]map[string]Colu } // convert a string to the corresponding ColumnUsage -func stringToColumnUsage(s string) (u ColumnUsage, err error) { +func stringToColumnUsage(s string) (ColumnUsage, error) { + var u ColumnUsage + var err error switch s { case "DISCARD": u = DISCARD @@ -601,7 +605,7 @@ func stringToColumnUsage(s string) (u ColumnUsage, err error) { err = fmt.Errorf("wrong ColumnUsage given : %s", s) } - return + return u, err } // Convert database.sql types to float64s for Prometheus consumption. Null types are mapped to NaN. string and []byte @@ -793,10 +797,9 @@ func queryNamespaceMapping(ch chan<- prometheus.Metric, db *sql.DB, namespace st if !found { // I've no idea how to avoid this properly at the moment, but this is // an admin tool so you're not injecting SQL right? - // nolint: gas - rows, err = db.Query(fmt.Sprintf("SELECT * FROM %s;", namespace)) + rows, err = db.Query(fmt.Sprintf("SELECT * FROM %s;", namespace)) // nolint: gas, safesql } else { - rows, err = db.Query(query) + rows, err = db.Query(query) // nolint: safesql } if err != nil { return []error{}, errors.New(fmt.Sprintln("Error running query on database: ", namespace, err)) @@ -881,7 +884,7 @@ func queryNamespaceMappings(ch chan<- prometheus.Metric, db *sql.DB, metricMap m for namespace, mapping := range metricMap { log.Debugln("Querying namespace: ", namespace) nonFatalErrors, err := queryNamespaceMapping(ch, db, namespace, mapping, queryOverrides) - // Serious error - a namespace disappeard + // Serious error - a namespace disappeared if err != nil { namespaceErrors[namespace] = err log.Infoln(err) @@ -971,10 +974,6 @@ func (e *Exporter) getDB(conn string) (*sql.DB, error) { if err != nil { return nil, err } - err = d.Ping() - if err != nil { - return nil, err - } d.SetMaxOpenConns(1) d.SetMaxIdleConns(1) @@ -983,6 +982,15 @@ func (e *Exporter) getDB(conn string) (*sql.DB, error) { log.Infoln("Established new database connection.") } + // Always send a ping and possibly invalidate the connection if it fails + if err := e.dbConnection.Ping(); err != nil { + cerr := e.dbConnection.Close() + log.Infoln("Error while closing non-pinging DB connection:", cerr) + e.dbConnection = nil + e.psqlUp.Set(0) + return nil, err + } + return e.dbConnection, nil } @@ -1007,11 +1015,14 @@ func (e *Exporter) scrape(ch chan<- prometheus.Metric) { loggableDsn = pDsn.String() } log.Infof("Error opening connection to database (%s): %s", loggableDsn, err) + e.psqlUp.Set(0) e.error.Set(1) - e.psqlUp.Set(0) // Force "up" to 0 here. return } + // Didn't fail, can mark connection as up for this scrape. + e.psqlUp.Set(1) + // Check if map versions need to be updated if err := e.checkMapVersions(ch, db); err != nil { log.Warnln("Proceeding with outdated query maps, as the Postgres version could not be determined:", err) diff --git a/cmd/postgres_exporter/postgres_exporter_test.go b/cmd/postgres_exporter/postgres_exporter_test.go index e5a50b098..6d6aecbf3 100644 --- a/cmd/postgres_exporter/postgres_exporter_test.go +++ b/cmd/postgres_exporter/postgres_exporter_test.go @@ -3,11 +3,13 @@ package main import ( - . "gopkg.in/check.v1" "testing" - "github.com/blang/semver" + . "gopkg.in/check.v1" + "os" + + "github.com/blang/semver" ) // Hook up gocheck into the "go test" runner. @@ -45,6 +47,7 @@ func (s *FunctionalSuite) TestSemanticVersionColumnDiscard(c *C) { ) } + // nolint: dupl { // Update the map so the discard metric should be eliminated discardableMetric := testMetricMap["test_namespace"]["metric_which_discards"] @@ -65,6 +68,7 @@ func (s *FunctionalSuite) TestSemanticVersionColumnDiscard(c *C) { ) } + // nolint: dupl { // Update the map so the discard metric should be kept but has a version discardableMetric := testMetricMap["test_namespace"]["metric_which_discards"] diff --git a/cmd/postgres_exporter/tests/test-smoke b/cmd/postgres_exporter/tests/test-smoke index 00b0c3707..045e11752 100755 --- a/cmd/postgres_exporter/tests/test-smoke +++ b/cmd/postgres_exporter/tests/test-smoke @@ -108,6 +108,13 @@ smoketest_postgres() { exit 1 fi + # HACK test: check pg_up is a 1 - TODO: expand integration tests to include metric consumption + if ! grep 'pg_up.* 1' $METRICS_DIR/.metrics.single.$version.prom ; then + echo "pg_up metric was not 1 despite exporter and database being up" + kill $exporter_pid + exit 1 + fi + kill $exporter_pid docker kill $CONTAINER_NAME docker rm -v $CONTAINER_NAME diff --git a/magefile.go b/magefile.go index 1ed8be8a4..92a1bd69f 100644 --- a/magefile.go +++ b/magefile.go @@ -178,23 +178,43 @@ func init() { Log("Concurrency:", concurrency) goSrc = func() []string { results := new([]string) - filepath.Walk(".", func(path string, info os.FileInfo, err error) error { + filepath.Walk(".", func(relpath string, info os.FileInfo, err error) error { + // Ensure absolute path so globs work + path, err := filepath.Abs(relpath) + if err != nil { + panic(err) + } + // Look for files if info.IsDir() { return nil } + // Exclusions - if matched, _ := filepath.Match("*/vendor/*", path); matched { - return nil - } else if matched, _ := filepath.Match(fmt.Sprintf("%s/*", toolDir), path); matched { - return nil - } else if matched, _ := filepath.Match(fmt.Sprintf("%s/*", binDir), path); matched { + for _, exclusion := range []string{toolDir, binDir, releaseDir, coverageDir} { + if strings.HasPrefix(path, exclusion) { + if info.IsDir() { + return filepath.SkipDir + } + return nil + } + } + + if strings.Contains(path, "/vendor/") { + if info.IsDir() { + return filepath.SkipDir + } return nil - } else if matched, _ := filepath.Match(fmt.Sprintf("%s/*", releaseDir), path); matched { + } + + if strings.Contains(path, ".git") { + if info.IsDir() { + return filepath.SkipDir + } return nil } - if matched, _ := filepath.Match("*.go", path); !matched { + if !strings.HasSuffix(path, ".go") { return nil } @@ -434,7 +454,7 @@ func Lint() error { mg.Deps(Tools) args := []string{"-j", fmt.Sprintf("%v", concurrency), fmt.Sprintf("--deadline=%s", linterDeadline.String()), "--enable-all", "--line-length=120", - "--disable=gocyclo", "--disable=testify", "--disable=test", "--exclude=assets/bindata.go"} + "--disable=gocyclo", "--disable=testify", "--disable=test", "--disable=lll", "--exclude=assets/bindata.go"} return sh.RunV("gometalinter", append(args, goDirs...)...) }