Fix pg_up never being set to 1

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 {

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)

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"]

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

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...)...)
 }