From a27d5ee71a5441ece6cd23e13ba79bafb02a544e Mon Sep 17 00:00:00 2001 From: Joel Meador Date: Tue, 21 Jun 2016 16:28:31 -0400 Subject: [PATCH 01/16] separate hello and authenticate functions, force connection close at end of write cycle so we don't hold open idle connections, which has the benefit of mostly removing the chance of getting hopelessly connection lost --- plugins/outputs/instrumental/instrumental.go | 41 ++++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/plugins/outputs/instrumental/instrumental.go b/plugins/outputs/instrumental/instrumental.go index 461ba9d9ee986..99b38ef759fff 100644 --- a/plugins/outputs/instrumental/instrumental.go +++ b/plugins/outputs/instrumental/instrumental.go @@ -28,8 +28,9 @@ type Instrumental struct { } const ( - DefaultHost = "collector.instrumentalapp.com" - AuthFormat = "hello version go/telegraf/1.0\nauthenticate %s\n" + DefaultHost = "collector.instrumentalapp.com" + HelloMessage = "hello version go/telegraf/1.0\n" + AuthFormat = "authenticate %s\n" ) var ( @@ -52,6 +53,13 @@ var sampleConfig = ` func (i *Instrumental) Connect() error { connection, err := net.DialTimeout("tcp", i.Host+":8000", i.Timeout.Duration) + + if err != nil { + i.conn = nil + return err + } + + err = i.hello(connection) if err != nil { i.conn = nil return err @@ -151,6 +159,11 @@ func (i *Instrumental) Write(metrics []telegraf.Metric) error { return err } + // force the connection closed after sending data + // to deal with various disconnection scenarios and eschew holding + // open idle connections en masse + i.Close() + return nil } @@ -162,19 +175,39 @@ func (i *Instrumental) SampleConfig() string { return sampleConfig } +func (i *Instrumental) hello(conn net.Conn) error { + _, err := fmt.Fprintf(conn, HelloMessage) + if err != nil { + return err + } + + response := make([]byte, 512) + if _, err = conn.Read(response); err != nil { + fmt.Println("hello err", err) + return err + } + + if string(response)[:3] != "ok\n" { + return fmt.Errorf("Hello failed: %s", response) + } + + return nil +} + func (i *Instrumental) authenticate(conn net.Conn) error { _, err := fmt.Fprintf(conn, AuthFormat, i.ApiToken) if err != nil { return err } - // The response here will either be two "ok"s or an error message. + // The response here will either be an "ok" or an error message. responses := make([]byte, 512) if _, err = conn.Read(responses); err != nil { + fmt.Println("err was not nil: ", err) return err } - if string(responses)[:6] != "ok\nok\n" { + if string(responses)[:3] != "ok\n" { return fmt.Errorf("Authentication failed: %s", responses) } From 05a0186196f03d34fd17d74651b7f495915f093a Mon Sep 17 00:00:00 2001 From: Joel Meador Date: Tue, 21 Jun 2016 16:38:20 -0400 Subject: [PATCH 02/16] update changelog, though this will need to be updated again to merge into telegraf master --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f614f4422434b..0dd2d0913baf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ should now look like: - [#1374](https://github.com/influxdata/telegraf/pull/1374): Change "default" retention policy to "". - [#1377](https://github.com/influxdata/telegraf/issues/1377): Graphite output mangling '%' character. - [#1396](https://github.com/influxdata/telegraf/pull/1396): Prometheus input plugin now supports x509 certs authentication +- [#1](https://github.com/Instrumental/telegraf/pull/1): Instrumental output has better reconnect behavior ## v1.0 beta 1 [2016-06-07] From 298d928e27c845409a76784fcfa59af49a9446eb Mon Sep 17 00:00:00 2001 From: Joel Meador Date: Tue, 21 Jun 2016 17:44:06 -0400 Subject: [PATCH 03/16] bump instrumental agent version --- plugins/outputs/instrumental/instrumental.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/outputs/instrumental/instrumental.go b/plugins/outputs/instrumental/instrumental.go index 99b38ef759fff..6dd8a01c1a304 100644 --- a/plugins/outputs/instrumental/instrumental.go +++ b/plugins/outputs/instrumental/instrumental.go @@ -29,7 +29,7 @@ type Instrumental struct { const ( DefaultHost = "collector.instrumentalapp.com" - HelloMessage = "hello version go/telegraf/1.0\n" + HelloMessage = "hello version go/telegraf/1.1\n" AuthFormat = "authenticate %s\n" ) From b64d5f7a1b37b39f099d878106790ed1013d905b Mon Sep 17 00:00:00 2001 From: Joel Meador Date: Thu, 23 Jun 2016 13:43:51 -0400 Subject: [PATCH 04/16] fix test to deal with better better connect/reconnect logic and changed ident & auth handshake --- .../outputs/instrumental/instrumental_test.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/plugins/outputs/instrumental/instrumental_test.go b/plugins/outputs/instrumental/instrumental_test.go index ceb53bac64300..dbab355d68dec 100644 --- a/plugins/outputs/instrumental/instrumental_test.go +++ b/plugins/outputs/instrumental/instrumental_test.go @@ -24,7 +24,6 @@ func TestWrite(t *testing.T) { ApiToken: "abc123token", Prefix: "my.prefix", } - i.Connect() // Default to gauge m1, _ := telegraf.NewMetric( @@ -40,10 +39,8 @@ func TestWrite(t *testing.T) { time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), ) - // Simulate a connection close and reconnect. metrics := []telegraf.Metric{m1, m2} i.Write(metrics) - i.Close() // Counter and Histogram are increments m3, _ := telegraf.NewMetric( @@ -70,7 +67,6 @@ func TestWrite(t *testing.T) { i.Write(metrics) wg.Wait() - i.Close() } func TCPServer(t *testing.T, wg *sync.WaitGroup) { @@ -82,11 +78,11 @@ func TCPServer(t *testing.T, wg *sync.WaitGroup) { tp := textproto.NewReader(reader) hello, _ := tp.ReadLine() - assert.Equal(t, "hello version go/telegraf/1.0", hello) + assert.Equal(t, "hello version go/telegraf/1.1", hello) + conn.Write([]byte("ok\n")) auth, _ := tp.ReadLine() assert.Equal(t, "authenticate abc123token", auth) - - conn.Write([]byte("ok\nok\n")) + conn.Write([]byte("ok\n")) data1, _ := tp.ReadLine() assert.Equal(t, "gauge my.prefix.192_168_0_1.mymeasurement.myfield 3.14 1289430000", data1) @@ -99,11 +95,12 @@ func TCPServer(t *testing.T, wg *sync.WaitGroup) { tp = textproto.NewReader(reader) hello, _ = tp.ReadLine() - assert.Equal(t, "hello version go/telegraf/1.0", hello) + assert.Equal(t, "hello version go/telegraf/1.1", hello) + conn.Write([]byte("ok\n")) + auth, _ = tp.ReadLine() assert.Equal(t, "authenticate abc123token", auth) - - conn.Write([]byte("ok\nok\n")) + conn.Write([]byte("ok\n")) data3, _ := tp.ReadLine() assert.Equal(t, "increment my.prefix.192_168_0_1.my_histogram 3.14 1289430000", data3) From 149dea90e5ed9f9f79976c4a178391b309975bfe Mon Sep 17 00:00:00 2001 From: "Joel \"The Merciless\" Meador" Date: Thu, 23 Jun 2016 16:51:49 -0400 Subject: [PATCH 05/16] Update CHANGELOG.md correct URL from instrumental fork to origin and put the change in the correct part of the file --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dd2d0913baf6..96000777bfebb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ should now look like: - [#1405](https://github.com/influxdata/telegraf/issues/1405): Fix memory/connection leak in prometheus input plugin. - [#1378](https://github.com/influxdata/telegraf/issues/1378): Trim BOM from config file for Windows support. - [#1339](https://github.com/influxdata/telegraf/issues/1339): Prometheus client output panic on service reload. +- [#1412](https://github.com/influxdata/telegraf/pull/1412): Instrumental output has better reconnect behavior ## v1.0 beta 2 [2016-06-21] @@ -61,7 +62,6 @@ should now look like: - [#1374](https://github.com/influxdata/telegraf/pull/1374): Change "default" retention policy to "". - [#1377](https://github.com/influxdata/telegraf/issues/1377): Graphite output mangling '%' character. - [#1396](https://github.com/influxdata/telegraf/pull/1396): Prometheus input plugin now supports x509 certs authentication -- [#1](https://github.com/Instrumental/telegraf/pull/1): Instrumental output has better reconnect behavior ## v1.0 beta 1 [2016-06-07] From bb35a5aecc41a7d4e7173ea0fa4001cb3d82cc66 Mon Sep 17 00:00:00 2001 From: Joel Meador Date: Sat, 25 Jun 2016 10:59:11 -0400 Subject: [PATCH 06/16] go fmt --- plugins/outputs/instrumental/instrumental.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/outputs/instrumental/instrumental.go b/plugins/outputs/instrumental/instrumental.go index 6dd8a01c1a304..199069377f1a1 100644 --- a/plugins/outputs/instrumental/instrumental.go +++ b/plugins/outputs/instrumental/instrumental.go @@ -28,9 +28,9 @@ type Instrumental struct { } const ( - DefaultHost = "collector.instrumentalapp.com" - HelloMessage = "hello version go/telegraf/1.1\n" - AuthFormat = "authenticate %s\n" + DefaultHost = "collector.instrumentalapp.com" + HelloMessage = "hello version go/telegraf/1.1\n" + AuthFormat = "authenticate %s\n" ) var ( From b4c8ef6e235bcf9d235c199923dce00fab7c45b8 Mon Sep 17 00:00:00 2001 From: Elijah Miller Date: Mon, 27 Jun 2016 14:56:41 -0400 Subject: [PATCH 07/16] Split out Instrumental tests for invalid metric and value. --- plugins/outputs/instrumental/instrumental_test.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/plugins/outputs/instrumental/instrumental_test.go b/plugins/outputs/instrumental/instrumental_test.go index dbab355d68dec..af9d3ecc2f44f 100644 --- a/plugins/outputs/instrumental/instrumental_test.go +++ b/plugins/outputs/instrumental/instrumental_test.go @@ -49,21 +49,28 @@ func TestWrite(t *testing.T) { map[string]interface{}{"value": float64(3.14)}, time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), ) - // We will drop metrics that simply won't be accepted by Instrumental + // We will drop metric names that won't be accepted by Instrumental m4, _ := telegraf.NewMetric( + "bad_metric_name", + map[string]string{"host": "192.168.0.1:8888", "metric_type": "counter"}, + map[string]interface{}{"value": "\" 1\""}, + time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), + ) + // We will drop metric values that won't be accepted by Instrumental + m5, _ := telegraf.NewMetric( "bad_values", map[string]string{"host": "192.168.0.1", "metric_type": "counter"}, map[string]interface{}{"value": "\" 3:30\""}, time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), ) - m5, _ := telegraf.NewMetric( + m6, _ := telegraf.NewMetric( "my_counter", map[string]string{"host": "192.168.0.1", "metric_type": "counter"}, map[string]interface{}{"value": float64(3.14)}, time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), ) - metrics = []telegraf.Metric{m3, m4, m5} + metrics = []telegraf.Metric{m3, m4, m5, m6} i.Write(metrics) wg.Wait() From 66dc5a40f643f89871b4fd7c5f58621952f73bd2 Mon Sep 17 00:00:00 2001 From: Elijah Miller Date: Mon, 27 Jun 2016 14:58:03 -0400 Subject: [PATCH 08/16] Ensure nothing remains on the wire after final test. --- plugins/outputs/instrumental/instrumental_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/outputs/instrumental/instrumental_test.go b/plugins/outputs/instrumental/instrumental_test.go index af9d3ecc2f44f..0882d806266cd 100644 --- a/plugins/outputs/instrumental/instrumental_test.go +++ b/plugins/outputs/instrumental/instrumental_test.go @@ -114,5 +114,8 @@ func TCPServer(t *testing.T, wg *sync.WaitGroup) { data4, _ := tp.ReadLine() assert.Equal(t, "increment my.prefix.192_168_0_1.my_counter 3.14 1289430000", data4) + data5, _ := tp.ReadLine() + assert.Equal(t, "", data5) + conn.Close() } From eb9744da3e882804df714f04c93dfc4627c152be Mon Sep 17 00:00:00 2001 From: Elijah Miller Date: Mon, 27 Jun 2016 15:27:30 -0400 Subject: [PATCH 09/16] Force valid metric names by replacing invalid parts with underscores. --- plugins/outputs/instrumental/instrumental.go | 16 +++++++++++++--- .../outputs/instrumental/instrumental_test.go | 10 +++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/plugins/outputs/instrumental/instrumental.go b/plugins/outputs/instrumental/instrumental.go index 199069377f1a1..39ef95267d9f6 100644 --- a/plugins/outputs/instrumental/instrumental.go +++ b/plugins/outputs/instrumental/instrumental.go @@ -34,7 +34,8 @@ const ( ) var ( - StatIncludesBadChar = regexp.MustCompile("[^[:alnum:][:blank:]-_.]") + ValueIncludesBadChar = regexp.MustCompile("[^[:digit:].]") + MetricNameReplacer = regexp.MustCompile("[^-[:alnum:]_.]") ) var sampleConfig = ` @@ -136,8 +137,17 @@ func (i *Instrumental) Write(metrics []telegraf.Metric) error { } for _, stat := range stats { - if !StatIncludesBadChar.MatchString(stat) { - points = append(points, fmt.Sprintf("%s %s", metricType, stat)) + // decompose "metric.name value time" + splitStat := strings.SplitN(stat, " ", 3) + metric := splitStat[0] + value := splitStat[1] + time := splitStat[2] + + // replace invalid components of metric name with underscore + clean_metric := MetricNameReplacer.ReplaceAllString(metric, "_") + + if !ValueIncludesBadChar.MatchString(value) { + points = append(points, fmt.Sprintf("%s %s %s %s", metricType, clean_metric, value, time)) } else if i.Debug { log.Printf("Unable to send bad stat: %s", stat) } diff --git a/plugins/outputs/instrumental/instrumental_test.go b/plugins/outputs/instrumental/instrumental_test.go index 0882d806266cd..08012f26e47c6 100644 --- a/plugins/outputs/instrumental/instrumental_test.go +++ b/plugins/outputs/instrumental/instrumental_test.go @@ -53,7 +53,7 @@ func TestWrite(t *testing.T) { m4, _ := telegraf.NewMetric( "bad_metric_name", map[string]string{"host": "192.168.0.1:8888", "metric_type": "counter"}, - map[string]interface{}{"value": "\" 1\""}, + map[string]interface{}{"value": 1}, time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), ) // We will drop metric values that won't be accepted by Instrumental @@ -111,11 +111,15 @@ func TCPServer(t *testing.T, wg *sync.WaitGroup) { data3, _ := tp.ReadLine() assert.Equal(t, "increment my.prefix.192_168_0_1.my_histogram 3.14 1289430000", data3) + data4, _ := tp.ReadLine() - assert.Equal(t, "increment my.prefix.192_168_0_1.my_counter 3.14 1289430000", data4) + assert.Equal(t, "increment my.prefix.192_168_0_1_8888.bad_metric_name 1 1289430000", data4) data5, _ := tp.ReadLine() - assert.Equal(t, "", data5) + assert.Equal(t, "increment my.prefix.192_168_0_1.my_counter 3.14 1289430000", data5) + + data6, _ := tp.ReadLine() + assert.Equal(t, "", data6) conn.Close() } From 227ffbb6d47f30e4b4560fb9ef507150d3c4d310 Mon Sep 17 00:00:00 2001 From: Elijah Miller Date: Mon, 27 Jun 2016 15:33:04 -0400 Subject: [PATCH 10/16] Multiple invalid characters being joined into a single udnerscore. --- plugins/outputs/instrumental/instrumental.go | 2 +- plugins/outputs/instrumental/instrumental_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/outputs/instrumental/instrumental.go b/plugins/outputs/instrumental/instrumental.go index 39ef95267d9f6..3d0518136ad5c 100644 --- a/plugins/outputs/instrumental/instrumental.go +++ b/plugins/outputs/instrumental/instrumental.go @@ -35,7 +35,7 @@ const ( var ( ValueIncludesBadChar = regexp.MustCompile("[^[:digit:].]") - MetricNameReplacer = regexp.MustCompile("[^-[:alnum:]_.]") + MetricNameReplacer = regexp.MustCompile("[^-[:alnum:]_.]+") ) var sampleConfig = ` diff --git a/plugins/outputs/instrumental/instrumental_test.go b/plugins/outputs/instrumental/instrumental_test.go index 08012f26e47c6..5609bb969adaa 100644 --- a/plugins/outputs/instrumental/instrumental_test.go +++ b/plugins/outputs/instrumental/instrumental_test.go @@ -52,7 +52,7 @@ func TestWrite(t *testing.T) { // We will drop metric names that won't be accepted by Instrumental m4, _ := telegraf.NewMetric( "bad_metric_name", - map[string]string{"host": "192.168.0.1:8888", "metric_type": "counter"}, + map[string]string{"host": "192.168.0.1:8888::123", "metric_type": "counter"}, map[string]interface{}{"value": 1}, time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), ) @@ -113,7 +113,7 @@ func TCPServer(t *testing.T, wg *sync.WaitGroup) { assert.Equal(t, "increment my.prefix.192_168_0_1.my_histogram 3.14 1289430000", data3) data4, _ := tp.ReadLine() - assert.Equal(t, "increment my.prefix.192_168_0_1_8888.bad_metric_name 1 1289430000", data4) + assert.Equal(t, "increment my.prefix.192_168_0_1_8888_123.bad_metric_name 1 1289430000", data4) data5, _ := tp.ReadLine() assert.Equal(t, "increment my.prefix.192_168_0_1.my_counter 3.14 1289430000", data5) From 42f1a14af7f7dc151ae1add9245efd0309400754 Mon Sep 17 00:00:00 2001 From: Elijah Miller Date: Mon, 27 Jun 2016 16:26:50 -0400 Subject: [PATCH 11/16] Adjust comment to what happens. --- plugins/outputs/instrumental/instrumental_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/outputs/instrumental/instrumental_test.go b/plugins/outputs/instrumental/instrumental_test.go index 5609bb969adaa..d363313c54fd8 100644 --- a/plugins/outputs/instrumental/instrumental_test.go +++ b/plugins/outputs/instrumental/instrumental_test.go @@ -49,7 +49,7 @@ func TestWrite(t *testing.T) { map[string]interface{}{"value": float64(3.14)}, time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC), ) - // We will drop metric names that won't be accepted by Instrumental + // We will modify metric names that won't be accepted by Instrumental m4, _ := telegraf.NewMetric( "bad_metric_name", map[string]string{"host": "192.168.0.1:8888::123", "metric_type": "counter"}, From 3d0f5d204c0c64519b45a22ceb2c40df3e5094e0 Mon Sep 17 00:00:00 2001 From: Joel Meador Date: Mon, 27 Jun 2016 16:54:56 -0400 Subject: [PATCH 12/16] undo split hello and auth commands, to reduce roundtrips --- plugins/outputs/instrumental/instrumental.go | 39 ++++--------------- .../outputs/instrumental/instrumental_test.go | 7 +--- 2 files changed, 9 insertions(+), 37 deletions(-) diff --git a/plugins/outputs/instrumental/instrumental.go b/plugins/outputs/instrumental/instrumental.go index 199069377f1a1..2fcc28cc0a752 100644 --- a/plugins/outputs/instrumental/instrumental.go +++ b/plugins/outputs/instrumental/instrumental.go @@ -28,9 +28,10 @@ type Instrumental struct { } const ( - DefaultHost = "collector.instrumentalapp.com" - HelloMessage = "hello version go/telegraf/1.1\n" - AuthFormat = "authenticate %s\n" + DefaultHost = "collector.instrumentalapp.com" + HelloMessage = "hello version go/telegraf/1.1\n" + AuthFormat = "authenticate %s\n" + HandshakeFormat = HelloMessage + AuthFormat ) var ( @@ -59,12 +60,6 @@ func (i *Instrumental) Connect() error { return err } - err = i.hello(connection) - if err != nil { - i.conn = nil - return err - } - err = i.authenticate(connection) if err != nil { i.conn = nil @@ -175,39 +170,19 @@ func (i *Instrumental) SampleConfig() string { return sampleConfig } -func (i *Instrumental) hello(conn net.Conn) error { - _, err := fmt.Fprintf(conn, HelloMessage) - if err != nil { - return err - } - - response := make([]byte, 512) - if _, err = conn.Read(response); err != nil { - fmt.Println("hello err", err) - return err - } - - if string(response)[:3] != "ok\n" { - return fmt.Errorf("Hello failed: %s", response) - } - - return nil -} - func (i *Instrumental) authenticate(conn net.Conn) error { - _, err := fmt.Fprintf(conn, AuthFormat, i.ApiToken) + _, err := fmt.Fprintf(conn, HandshakeFormat, i.ApiToken) if err != nil { return err } - // The response here will either be an "ok" or an error message. + // The response here will either be two "ok"s or an error message. responses := make([]byte, 512) if _, err = conn.Read(responses); err != nil { - fmt.Println("err was not nil: ", err) return err } - if string(responses)[:3] != "ok\n" { + if string(responses)[:6] != "ok\nok\n" { return fmt.Errorf("Authentication failed: %s", responses) } diff --git a/plugins/outputs/instrumental/instrumental_test.go b/plugins/outputs/instrumental/instrumental_test.go index dbab355d68dec..9708a25900dad 100644 --- a/plugins/outputs/instrumental/instrumental_test.go +++ b/plugins/outputs/instrumental/instrumental_test.go @@ -79,10 +79,9 @@ func TCPServer(t *testing.T, wg *sync.WaitGroup) { hello, _ := tp.ReadLine() assert.Equal(t, "hello version go/telegraf/1.1", hello) - conn.Write([]byte("ok\n")) auth, _ := tp.ReadLine() assert.Equal(t, "authenticate abc123token", auth) - conn.Write([]byte("ok\n")) + conn.Write([]byte("ok\nok\n")) data1, _ := tp.ReadLine() assert.Equal(t, "gauge my.prefix.192_168_0_1.mymeasurement.myfield 3.14 1289430000", data1) @@ -96,11 +95,9 @@ func TCPServer(t *testing.T, wg *sync.WaitGroup) { hello, _ = tp.ReadLine() assert.Equal(t, "hello version go/telegraf/1.1", hello) - conn.Write([]byte("ok\n")) - auth, _ = tp.ReadLine() assert.Equal(t, "authenticate abc123token", auth) - conn.Write([]byte("ok\n")) + conn.Write([]byte("ok\nok\n")) data3, _ := tp.ReadLine() assert.Equal(t, "increment my.prefix.192_168_0_1.my_histogram 3.14 1289430000", data3) From 398479cf18131a5949be9a36b62a8a7cd7b6fb32 Mon Sep 17 00:00:00 2001 From: mediocretes Date: Thu, 1 Sep 2016 16:32:07 -0400 Subject: [PATCH 13/16] Add ignored_databases option to postgresql configuration files, to enable easy filtering of system databases without needing to whitelist all the databases on the server. Add tests for database whitelist and blacklist. --- plugins/inputs/postgresql/postgresql.go | 12 +++- plugins/inputs/postgresql/postgresql_test.go | 72 ++++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/plugins/inputs/postgresql/postgresql.go b/plugins/inputs/postgresql/postgresql.go index da8ee80013e7f..0e7cdb5094ad0 100644 --- a/plugins/inputs/postgresql/postgresql.go +++ b/plugins/inputs/postgresql/postgresql.go @@ -17,6 +17,7 @@ import ( type Postgresql struct { Address string Databases []string + IgnoredDatabases []string OrderedColumns []string AllColumns []string sanitizedAddress string @@ -40,8 +41,12 @@ var sampleConfig = ` ## address = "host=localhost user=postgres sslmode=disable" + ## A list of databases to explicitly ignore. If not specified, metrics for all + ## databases are gathered. Do NOT use with the 'databases' option. + # ignored_databases = ["postgres", "template0", "template1"] + ## A list of databases to pull metrics about. If not specified, metrics for all - ## databases are gathered. + ## databases are gathered. Do NOT use with the 'ignore_databases' option. # databases = ["app_production", "testing"] ` @@ -73,8 +78,11 @@ func (p *Postgresql) Gather(acc telegraf.Accumulator) error { defer db.Close() - if len(p.Databases) == 0 { + if len(p.Databases) == 0 && len(p.IgnoredDatabases) == 0 { query = `SELECT * FROM pg_stat_database` + } else if len(p.IgnoredDatabases) != 0 { + query = fmt.Sprintf(`SELECT * FROM pg_stat_database WHERE datname NOT IN ('%s')`, + strings.Join(p.IgnoredDatabases, "','")) } else { query = fmt.Sprintf(`SELECT * FROM pg_stat_database WHERE datname IN ('%s')`, strings.Join(p.Databases, "','")) diff --git a/plugins/inputs/postgresql/postgresql_test.go b/plugins/inputs/postgresql/postgresql_test.go index 552b18cdb4b2a..fb9c007f2f898 100644 --- a/plugins/inputs/postgresql/postgresql_test.go +++ b/plugins/inputs/postgresql/postgresql_test.go @@ -150,3 +150,75 @@ func TestPostgresqlIgnoresUnwantedColumns(t *testing.T) { assert.False(t, acc.HasMeasurement(col)) } } + +func TestPostgresqlDatabaseWhitelistTest(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + p := &Postgresql{ + Address: fmt.Sprintf("host=%s user=postgres sslmode=disable", + testutil.GetLocalHost()), + Databases: []string{"template0"}, + } + + var acc testutil.Accumulator + + err := p.Gather(&acc) + require.NoError(t, err) + + var foundTemplate0 = false + var foundTemplate1 = false + + for _, pnt := range acc.Metrics { + if pnt.Measurement == "postgresql" { + if pnt.Tags["db"] == "template0" { + foundTemplate0 = true + } + } + if pnt.Measurement == "postgresql" { + if pnt.Tags["db"] == "template1" { + foundTemplate1 = true + } + } + } + + assert.True(t, foundTemplate0) + assert.False(t, foundTemplate1) +} + +func TestPostgresqlDatabaseBlacklistTest(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + p := &Postgresql{ + Address: fmt.Sprintf("host=%s user=postgres sslmode=disable", + testutil.GetLocalHost()), + IgnoredDatabases: []string{"template0"}, + } + + var acc testutil.Accumulator + + err := p.Gather(&acc) + require.NoError(t, err) + + var foundTemplate0 = false + var foundTemplate1 = false + + for _, pnt := range acc.Metrics { + if pnt.Measurement == "postgresql" { + if pnt.Tags["db"] == "template0" { + foundTemplate0 = true + } + } + if pnt.Measurement == "postgresql" { + if pnt.Tags["db"] == "template1" { + foundTemplate1 = true + } + } + } + + assert.False(t, foundTemplate0) + assert.True(t, foundTemplate1) +} From 51db125531b1d30778ec7641b99fdf377e62a297 Mon Sep 17 00:00:00 2001 From: mediocretes Date: Thu, 1 Sep 2016 16:33:39 -0400 Subject: [PATCH 14/16] run go fmt on new postgresql database whitelist/blacklist code --- plugins/inputs/postgresql/postgresql_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/inputs/postgresql/postgresql_test.go b/plugins/inputs/postgresql/postgresql_test.go index fb9c007f2f898..64926f61ea503 100644 --- a/plugins/inputs/postgresql/postgresql_test.go +++ b/plugins/inputs/postgresql/postgresql_test.go @@ -171,11 +171,11 @@ func TestPostgresqlDatabaseWhitelistTest(t *testing.T) { var foundTemplate1 = false for _, pnt := range acc.Metrics { - if pnt.Measurement == "postgresql" { + if pnt.Measurement == "postgresql" { if pnt.Tags["db"] == "template0" { foundTemplate0 = true } - } + } if pnt.Measurement == "postgresql" { if pnt.Tags["db"] == "template1" { foundTemplate1 = true @@ -207,11 +207,11 @@ func TestPostgresqlDatabaseBlacklistTest(t *testing.T) { var foundTemplate1 = false for _, pnt := range acc.Metrics { - if pnt.Measurement == "postgresql" { + if pnt.Measurement == "postgresql" { if pnt.Tags["db"] == "template0" { foundTemplate0 = true } - } + } if pnt.Measurement == "postgresql" { if pnt.Tags["db"] == "template1" { foundTemplate1 = true From 60d3d4065a1d157902a3ac200efc37a6dd44e321 Mon Sep 17 00:00:00 2001 From: mediocretes Date: Tue, 6 Sep 2016 17:10:41 -0400 Subject: [PATCH 15/16] add postgresql database blacklist option to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ae986ce637d3..d8960b567e14d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - [#1599](https://github.com/influxdata/telegraf/pull/1599): Add server hostname for each docker measurements. - [#1697](https://github.com/influxdata/telegraf/pull/1697): Add NATS output plugin. - [#1407](https://github.com/influxdata/telegraf/pull/1407): HTTP service listener input plugin. +- [#1699](https://github.com/influxdata/telegraf/pull/1699): Add database blacklist option for Postgresql ### Bugfixes From 5750fa589721c8d31542ef6155e0d983203e7b85 Mon Sep 17 00:00:00 2001 From: mediocretes Date: Tue, 6 Sep 2016 17:13:15 -0400 Subject: [PATCH 16/16] remove a bad merge from the changelog --- CHANGELOG.md | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8960b567e14d..65216e9f73fd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,20 +86,6 @@ consistent with the behavior of `collection_jitter`. - [#1402](https://github.com/influxdata/telegraf/pull/1402): docker-machine/boot2docker no longer required for unit tests. - [#1350](https://github.com/influxdata/telegraf/pull/1350): cgroup input plugin. - [#1369](https://github.com/influxdata/telegraf/pull/1369): Add input plugin for consuming metrics from NSQD. - -### Bugfixes - -- [#1384](https://github.com/influxdata/telegraf/pull/1384): Fix datarace in apache input plugin. -- [#1399](https://github.com/influxdata/telegraf/issues/1399): Add `read_repairs` statistics to riak plugin. -- [#1405](https://github.com/influxdata/telegraf/issues/1405): Fix memory/connection leak in prometheus input plugin. -- [#1378](https://github.com/influxdata/telegraf/issues/1378): Trim BOM from config file for Windows support. -- [#1339](https://github.com/influxdata/telegraf/issues/1339): Prometheus client output panic on service reload. -- [#1412](https://github.com/influxdata/telegraf/pull/1412): Instrumental output has better reconnect behavior - -## v1.0 beta 2 [2016-06-21] - -### Features - - [#1369](https://github.com/influxdata/telegraf/pull/1480): add ability to read redis from a socket. - [#1387](https://github.com/influxdata/telegraf/pull/1387): **Breaking Change** - Redis `role` tag renamed to `replication_role` to avoid global_tags override - [#1437](https://github.com/influxdata/telegraf/pull/1437): Fetching Galera status metrics in MySQL