Merge branch 'master' into master

Author: tmc

.travis.yml

@@ -3,7 +3,7 @@ services:
 - docker
 language: go
 go:
-- '1.8'
+- '1.9'
 # Make sure we have p2 and the postgres client.
 before_install:
 - go get -v github.com/mattn/goveralls

Makefile

@@ -42,12 +42,14 @@ lint: tools
 fmt: tools
 	gofmt -s -w $(GO_SRC)
 
-test: tools
-	@mkdir -p $(COVERDIR)
-	@rm -f $(COVERDIR)/*
+run-tests: tools
+	mkdir -p $(COVERDIR)
+	rm -f $(COVERDIR)/*
 	for pkg in $(GO_PKGS) ; do \
 		go test -v -covermode count -coverprofile=$(COVERDIR)/$$(echo $$pkg | tr '/' '-').out $$pkg ; \
 	done
+
+test: run-tests
 	gocovmerge $(shell find $(COVERDIR) -name '*.out') > cover.test.out
 
 test-integration: postgres_exporter postgres_exporter_integration_test
@@ -74,6 +76,6 @@ tools:
 	$(MAKE) -C $(TOOLDIR)
 
 clean:
-	rm -f postgres_exporter postgres_exporter_integration_test
+	rm -rf postgres_exporter postgres_exporter_integration_test $(COVERDIR)
 
 .PHONY: tools docker-build docker lint fmt test vet push cross clean

pg_setting.go

@@ -94,7 +94,7 @@ func (s *pgSetting) normaliseUnit() (val float64, unit string, err error) {
 		return
 	case "ms", "s", "min", "h", "d":
 		unit = "seconds"
-	case "kB", "MB", "GB", "TB", "8kB", "16MB":
+	case "kB", "MB", "GB", "TB", "8kB", "16kB", "16MB":
 		unit = "bytes"
 	default:
 		err = fmt.Errorf("Unknown unit for runtime variable: %q", s.unit)
@@ -125,6 +125,8 @@ func (s *pgSetting) normaliseUnit() (val float64, unit string, err error) {
 		val *= math.Pow(2, 40)
 	case "8kB":
 		val *= math.Pow(2, 13)
+	case "16kB":
+		val *= math.Pow(2, 14)
 	case "16MB":
 		val *= math.Pow(2, 24)
 	}

pg_setting_test.go

@@ -60,6 +60,22 @@ var fixtures = []fixture{
 		d: "Desc{fqName: \"pg_settings_eight_kb_fixture_metric_bytes\", help: \"Foo foo foo [Units converted to bytes.]\", constLabels: {}, variableLabels: []}",
 		v: 139264,
 	},
+	{
+		p: pgSetting{
+			name:      "16_kb_real_fixture_metric",
+			setting:   "3.0",
+			unit:      "16kB",
+			shortDesc: "Foo foo foo",
+			vartype:   "real",
+		},
+		n: normalised{
+			val:  49152,
+			unit: "bytes",
+			err:  "",
+		},
+		d: "Desc{fqName: \"pg_settings_16_kb_real_fixture_metric_bytes\", help: \"Foo foo foo [Units converted to bytes.]\", constLabels: {}, variableLabels: []}",
+		v: 49152,
+	},
 	{
 		p: pgSetting{
 			name:      "16_mb_real_fixture_metric",

postgres_exporter.go

@@ -13,6 +13,7 @@ import (
 	"regexp"
 	"runtime"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 
@@ -28,8 +29,6 @@ import (
 // (semantic version)-(commitish) form.
 var Version = "0.0.1"
 
-var sharedDBConn *sql.DB
-
 var (
 	listenAddress = flag.String(
 		"web.listen-address", ":9187",
@@ -673,6 +672,11 @@ type Exporter struct {
 	duration, error prometheus.Gauge
 	totalScrapes    prometheus.Counter
 
+	// dbDsn is the connection string used to establish the dbConnection
+	dbDsn string
+	// dbConnection is used to allow re-using the DB connection between scrapes
+	dbConnection *sql.DB
+
 	// Last version used to calculate metric map. If mismatch on scrape,
 	// then maps are recalculated.
 	lastMapVersion semver.Version
@@ -923,8 +927,16 @@ func (e *Exporter) checkMapVersions(ch chan<- prometheus.Metric, db *sql.DB) err
 	return nil
 }
 
-func getDB(conn string) (*sql.DB, error) {
-	if sharedDBConn == nil {
+func (e *Exporter) getDB(conn string) (*sql.DB, error) {
+	// Has dsn changed?
+	if (e.dbConnection != nil) && (e.dsn != e.dbDsn) {
+		err := e.dbConnection.Close()
+		log.Warnln("Error while closing obsolete DB connection:", err)
+		e.dbConnection = nil
+		e.dbDsn = ""
+	}
+
+	if e.dbConnection == nil {
 		d, err := sql.Open("postgres", conn)
 		if err != nil {
 			return nil, err
@@ -935,10 +947,12 @@ func getDB(conn string) (*sql.DB, error) {
 		}
 		d.SetMaxOpenConns(1)
 		d.SetMaxIdleConns(1)
-		sharedDBConn = d
+		e.dbConnection = d
+		e.dbDsn = e.dsn
+		log.Infoln("Established new database connection.")
 	}
 
-	return sharedDBConn, nil
+	return e.dbConnection, nil
 }
 
 func (e *Exporter) scrape(ch chan<- prometheus.Metric) {
@@ -949,11 +963,16 @@ func (e *Exporter) scrape(ch chan<- prometheus.Metric) {
 	e.error.Set(0)
 	e.totalScrapes.Inc()
 
-	db, err := getDB(e.dsn)
+	db, err := e.getDB(e.dsn)
 	if err != nil {
 		loggableDsn := "could not parse DATA_SOURCE_NAME"
-		if pDsn, pErr := url.Parse(e.dsn); pErr != nil {
-			pDsn.User = url.UserPassword(pDsn.User.Username(), "xxx")
+		// If the DSN is parseable, log it with a blanked out password
+		pDsn, pErr := url.Parse(e.dsn)
+		if pErr == nil {
+			// Blank user info if not nil
+			if pDsn.User != nil {
+				pDsn.User = url.UserPassword(pDsn.User.Username(), "PASSWORD_REMOVED")
+			}
 			loggableDsn = pDsn.String()
 		}
 		log.Infof("Error opening connection to database (%s): %s", loggableDsn, err)
@@ -981,6 +1000,43 @@ func (e *Exporter) scrape(ch chan<- prometheus.Metric) {
 	}
 }
 
+// try to get the DataSource
+// DATA_SOURCE_NAME always wins so we do not break older versions
+// reading secrets from files wins over secrets in environment variables
+// DATA_SOURCE_NAME > DATA_SOURCE_{USER|FILE}_FILE > DATA_SOURCE_{USER|FILE}
+func getDataSource() string {
+	var dsn = os.Getenv("DATA_SOURCE_NAME")
+	if len(dsn) == 0 {
+		var user string
+		var pass string
+
+		if len(os.Getenv("DATA_SOURCE_USER_FILE")) != 0 {
+			fileContents, err := ioutil.ReadFile(os.Getenv("DATA_SOURCE_USER_FILE"))
+			if err != nil {
+				panic(err)
+			}
+			user = strings.TrimSpace(string(fileContents))
+		} else {
+			user = os.Getenv("DATA_SOURCE_USER")
+		}
+
+		if len(os.Getenv("DATA_SOURCE_PASS_FILE")) != 0 {
+			fileContents, err := ioutil.ReadFile(os.Getenv("DATA_SOURCE_PASS_FILE"))
+			if err != nil {
+				panic(err)
+			}
+			pass = strings.TrimSpace(string(fileContents))
+		} else {
+			pass = os.Getenv("DATA_SOURCE_PASS")
+		}
+
+		uri := os.Getenv("DATA_SOURCE_URI")
+		dsn = "postgresql://" + user + ":" + pass + "@" + uri
+	}
+
+	return dsn
+}
+
 func main() {
 	flag.Parse()
 
@@ -997,12 +1053,18 @@ func main() {
 		return
 	}
 
-	dsn := os.Getenv("DATA_SOURCE_NAME")
+	dsn := getDataSource()
 	if len(dsn) == 0 {
-		log.Fatal("couldn't find environment variable DATA_SOURCE_NAME")
+		log.Fatal("couldn't find environment variables describing the datasource to use")
 	}
 
 	exporter := NewExporter(dsn, *queriesPath)
+	defer func() {
+		if exporter.dbConnection != nil {
+			exporter.dbConnection.Close() // nolint: errcheck
+		}
+	}()
+
 	prometheus.MustRegister(exporter)
 
 	http.Handle(*metricPath, prometheus.Handler())
@@ -1012,7 +1074,4 @@ func main() {
 
 	log.Infof("Starting Server: %s", *listenAddress)
 	log.Fatal(http.ListenAndServe(*listenAddress, nil))
-	if sharedDBConn != nil {
-		defer sharedDBConn.Close() // nolint: errcheck
-	}
 }

postgres_exporter_integration_test.go

@@ -73,3 +73,25 @@ func (s *IntegrationSuite) TestAllNamespacesReturnResults(c *C) {
 		}
 	}
 }
+
+// TestInvalidDsnDoesntCrash tests that specifying an invalid DSN doesn't crash
+// the exporter. Related to https://github.com/wrouesnel/postgres_exporter/issues/93
+// although not a replication of the scenario.
+func (s *IntegrationSuite) TestInvalidDsnDoesntCrash(c *C) {
+	// Setup a dummy channel to consume metrics
+	ch := make(chan prometheus.Metric, 100)
+	go func() {
+		for range ch {
+		}
+	}()
+
+	// Send a bad DSN
+	exporter := NewExporter("invalid dsn", *queriesPath)
+	c.Assert(exporter, NotNil)
+	exporter.scrape(ch)
+
+	// Send a DSN to a non-listening port.
+	exporter = NewExporter("postgresql://nothing:[email protected]:1/nothing", *queriesPath)
+	c.Assert(exporter, NotNil)
+	exporter.scrape(ch)
+}

postgres_exporter_test.go

@@ -7,13 +7,13 @@ import (
 	"testing"
 
 	"github.com/blang/semver"
+	"os"
 )
 
 // Hook up gocheck into the "go test" runner.
 func Test(t *testing.T) { TestingT(t) }
 
 type FunctionalSuite struct {
-	e *Exporter
 }
 
 var _ = Suite(&FunctionalSuite{})
@@ -85,3 +85,67 @@ func (s *FunctionalSuite) TestSemanticVersionColumnDiscard(c *C) {
 		)
 	}
 }
+
+// test read username and password from file
+func (s *FunctionalSuite) TestEnvironmentSettingWithSecretsFiles(c *C) {
+
+	err := os.Setenv("DATA_SOURCE_USER_FILE", "./tests/username_file")
+	c.Assert(err, IsNil)
+	defer UnsetEnvironment(c, "DATA_SOURCE_USER_FILE")
+
+	err = os.Setenv("DATA_SOURCE_PASS_FILE", "./tests/userpass_file")
+	c.Assert(err, IsNil)
+	defer UnsetEnvironment(c, "DATA_SOURCE_PASS_FILE")
+
+	err = os.Setenv("DATA_SOURCE_URI", "localhost:5432/?sslmode=disable")
+	c.Assert(err, IsNil)
+	defer UnsetEnvironment(c, "DATA_SOURCE_URI")
+
+	var expected = "postgresql://custom_username:custom_password@localhost:5432/?sslmode=disable"
+
+	dsn := getDataSource()
+	if dsn != expected {
+		c.Errorf("Expected Username to be read from file. Found=%v, expected=%v", dsn, expected)
+	}
+}
+
+// test read DATA_SOURCE_NAME from environment
+func (s *FunctionalSuite) TestEnvironmentSettingWithDns(c *C) {
+
+	envDsn := "postgresql://user:password@localhost:5432/?sslmode=enabled"
+	err := os.Setenv("DATA_SOURCE_NAME", envDsn)
+	c.Assert(err, IsNil)
+	defer UnsetEnvironment(c, "DATA_SOURCE_NAME")
+
+	dsn := getDataSource()
+	if dsn != envDsn {
+		c.Errorf("Expected Username to be read from file. Found=%v, expected=%v", dsn, envDsn)
+	}
+}
+
+// test DATA_SOURCE_NAME is used even if username and password environment variables are set
+func (s *FunctionalSuite) TestEnvironmentSettingWithDnsAndSecrets(c *C) {
+
+	envDsn := "postgresql://userDsn:passwordDsn@localhost:55432/?sslmode=disabled"
+	err := os.Setenv("DATA_SOURCE_NAME", envDsn)
+	c.Assert(err, IsNil)
+	defer UnsetEnvironment(c, "DATA_SOURCE_NAME")
+
+	err = os.Setenv("DATA_SOURCE_USER_FILE", "./tests/username_file")
+	c.Assert(err, IsNil)
+	defer UnsetEnvironment(c, "DATA_SOURCE_USER_FILE")
+
+	err = os.Setenv("DATA_SOURCE_PASS", "envUserPass")
+	c.Assert(err, IsNil)
+	defer UnsetEnvironment(c, "DATA_SOURCE_PASS")
+
+	dsn := getDataSource()
+	if dsn != envDsn {
+		c.Errorf("Expected Username to be read from file. Found=%v, expected=%v", dsn, envDsn)
+	}
+}
+
+func UnsetEnvironment(c *C, d string) {
+	err := os.Unsetenv(d)
+	c.Assert(err, IsNil)
+}

queries.yaml

@@ -142,3 +142,4 @@ pg_stat_statements:
     - blk_write_time:
         usage: "COUNTER"
         description: "Total time the statement spent writing blocks, in milliseconds (if track_io_timing is enabled, otherwise zero)"
+

tests/username_file

@@ -0,0 +1 @@
+custom_username

tests/userpass_file

@@ -0,0 +1 @@
+custom_password

tools/.gitignore

@@ -1,4 +1,4 @@
 /pkg
-bin
-tools.deps
-metatools.deps
+/bin
+/tools.deps
+/metatools.deps