From c26aeb871df5fb23d872bb93b95b353cc009cee7 Mon Sep 17 00:00:00 2001 From: GeorgeJahad Date: Fri, 4 Oct 2019 12:18:34 -0700 Subject: [PATCH] Remove package level vars from sqlserver and mysql input plugins (#6468) --- plugins/inputs/mysql/mysql.go | 25 +++++---- plugins/inputs/mysql/mysql_test.go | 48 +++++++++++++++++ plugins/inputs/sqlserver/sqlserver.go | 29 +++++----- plugins/inputs/sqlserver/sqlserver_test.go | 61 +++++++++++++++++++++- 4 files changed, 133 insertions(+), 30 deletions(-) diff --git a/plugins/inputs/mysql/mysql.go b/plugins/inputs/mysql/mysql.go index 0516e22b7..4b6bae1ad 100644 --- a/plugins/inputs/mysql/mysql.go +++ b/plugins/inputs/mysql/mysql.go @@ -39,9 +39,12 @@ type Mysql struct { IntervalSlow string `toml:"interval_slow"` MetricVersion int `toml:"metric_version"` tls.ClientConfig + lastT time.Time + initDone bool + scanIntervalSlow uint32 } -var sampleConfig = ` +const sampleConfig = ` ## specify servers via a url matching: ## [username[:password]@][protocol[(address)]]/[?tls=[true|false|skip-verify|custom]] ## see https://github.com/go-sql-driver/mysql#dsn-data-source-name @@ -123,7 +126,7 @@ var sampleConfig = ` # insecure_skip_verify = false ` -var defaultTimeout = time.Second * time.Duration(5) +const defaultTimeout = time.Second * time.Duration(5) func (m *Mysql) SampleConfig() string { return sampleConfig @@ -133,21 +136,16 @@ func (m *Mysql) Description() string { return "Read metrics from one or many mysql servers" } -var ( - localhost = "" - lastT time.Time - initDone = false - scanIntervalSlow uint32 -) +const localhost = "" func (m *Mysql) InitMysql() { if len(m.IntervalSlow) > 0 { interval, err := time.ParseDuration(m.IntervalSlow) if err == nil && interval.Seconds() >= 1.0 { - scanIntervalSlow = uint32(interval.Seconds()) + m.scanIntervalSlow = uint32(interval.Seconds()) } } - initDone = true + m.initDone = true } func (m *Mysql) Gather(acc telegraf.Accumulator) error { @@ -156,7 +154,7 @@ func (m *Mysql) Gather(acc telegraf.Accumulator) error { return m.gatherServer(localhost, acc) } // Initialise additional query intervals - if !initDone { + if !m.initDone { m.InitMysql() } @@ -184,6 +182,7 @@ func (m *Mysql) Gather(acc telegraf.Accumulator) error { return nil } +// These are const but can't be declared as such because golang doesn't allow const maps var ( // status counter generalThreadStates = map[string]uint32{ @@ -426,12 +425,12 @@ func (m *Mysql) gatherServer(serv string, acc telegraf.Accumulator) error { // Global Variables may be gathered less often if len(m.IntervalSlow) > 0 { - if uint32(time.Since(lastT).Seconds()) >= scanIntervalSlow { + if uint32(time.Since(m.lastT).Seconds()) >= m.scanIntervalSlow { err = m.gatherGlobalVariables(db, serv, acc) if err != nil { return err } - lastT = time.Now() + m.lastT = time.Now() } } diff --git a/plugins/inputs/mysql/mysql_test.go b/plugins/inputs/mysql/mysql_test.go index b4983ba0e..be9c338bf 100644 --- a/plugins/inputs/mysql/mysql_test.go +++ b/plugins/inputs/mysql/mysql_test.go @@ -26,6 +26,54 @@ func TestMysqlDefaultsToLocal(t *testing.T) { assert.True(t, acc.HasMeasurement("mysql")) } +func TestMysqlMultipleInstances(t *testing.T) { + // Invoke Gather() from two separate configurations and + // confirm they don't interfere with each other + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + testServer := "root@tcp(127.0.0.1:3306)/?tls=false" + m := &Mysql{ + Servers: []string{testServer}, + IntervalSlow: "30s", + } + + var acc, acc2 testutil.Accumulator + err := m.Gather(&acc) + require.NoError(t, err) + assert.True(t, acc.HasMeasurement("mysql")) + // acc should have global variables + assert.True(t, acc.HasMeasurement("mysql_variables")) + + m2 := &Mysql{ + Servers: []string{testServer}, + } + err = m2.Gather(&acc2) + require.NoError(t, err) + assert.True(t, acc2.HasMeasurement("mysql")) + // acc2 should not have global variables + assert.False(t, acc2.HasMeasurement("mysql_variables")) +} + +func TestMysqlMultipleInits(t *testing.T) { + m := &Mysql{ + IntervalSlow: "30s", + } + m2 := &Mysql{} + + m.InitMysql() + assert.True(t, m.initDone) + assert.False(t, m2.initDone) + assert.Equal(t, m.scanIntervalSlow, uint32(30)) + assert.Equal(t, m2.scanIntervalSlow, uint32(0)) + + m2.InitMysql() + assert.True(t, m.initDone) + assert.True(t, m2.initDone) + assert.Equal(t, m.scanIntervalSlow, uint32(30)) + assert.Equal(t, m2.scanIntervalSlow, uint32(0)) +} + func TestMysqlGetDSNTag(t *testing.T) { tests := []struct { input string diff --git a/plugins/inputs/sqlserver/sqlserver.go b/plugins/inputs/sqlserver/sqlserver.go index 38a1b16e0..2aaccd871 100644 --- a/plugins/inputs/sqlserver/sqlserver.go +++ b/plugins/inputs/sqlserver/sqlserver.go @@ -12,10 +12,12 @@ import ( // SQLServer struct type SQLServer struct { - Servers []string `toml:"servers"` - QueryVersion int `toml:"query_version"` - AzureDB bool `toml:"azuredb"` - ExcludeQuery []string `toml:"exclude_query"` + Servers []string `toml:"servers"` + QueryVersion int `toml:"query_version"` + AzureDB bool `toml:"azuredb"` + ExcludeQuery []string `toml:"exclude_query"` + queries MapQuery + isInitialized bool } // Query struct @@ -28,14 +30,9 @@ type Query struct { // MapQuery type type MapQuery map[string]Query -var queries MapQuery +const defaultServer = "Server=.;app name=telegraf;log=1;" -// Initialized flag -var isInitialized = false - -var defaultServer = "Server=.;app name=telegraf;log=1;" - -var sampleConfig = ` +const sampleConfig = ` ## Specify instances to monitor with a list of connection strings. ## All connection parameters are optional. ## By default, the host is localhost, listening on default port, TCP 1433. @@ -89,8 +86,8 @@ type scanner interface { } func initQueries(s *SQLServer) { - queries = make(MapQuery) - + s.queries = make(MapQuery) + queries := s.queries // If this is an AzureDB instance, grab some extra metrics if s.AzureDB { queries["AzureDBResourceStats"] = Query{Script: sqlAzureDBResourceStats, ResultByRow: false} @@ -124,12 +121,12 @@ func initQueries(s *SQLServer) { } // Set a flag so we know that queries have already been initialized - isInitialized = true + s.isInitialized = true } // Gather collect data from SQL Server func (s *SQLServer) Gather(acc telegraf.Accumulator) error { - if !isInitialized { + if !s.isInitialized { initQueries(s) } @@ -140,7 +137,7 @@ func (s *SQLServer) Gather(acc telegraf.Accumulator) error { var wg sync.WaitGroup for _, serv := range s.Servers { - for _, query := range queries { + for _, query := range s.queries { wg.Add(1) go func(serv string, query Query) { defer wg.Done() diff --git a/plugins/inputs/sqlserver/sqlserver_test.go b/plugins/inputs/sqlserver/sqlserver_test.go index 063af7595..b493fb13c 100644 --- a/plugins/inputs/sqlserver/sqlserver_test.go +++ b/plugins/inputs/sqlserver/sqlserver_test.go @@ -1,6 +1,7 @@ package sqlserver import ( + "github.com/stretchr/testify/assert" "strconv" "strings" "testing" @@ -14,7 +15,7 @@ func TestSqlServer_ParseMetrics(t *testing.T) { var acc testutil.Accumulator - queries = make(MapQuery) + queries := make(MapQuery) queries["PerformanceCounters"] = Query{Script: mockPerformanceCounters, ResultByRow: true} queries["WaitStatsCategorized"] = Query{Script: mockWaitStatsCategorized, ResultByRow: false} queries["CPUHistory"] = Query{Script: mockCPUHistory, ResultByRow: false} @@ -81,6 +82,64 @@ func TestSqlServer_ParseMetrics(t *testing.T) { } } +func TestSqlServer_MultipleInstance(t *testing.T) { + // Invoke Gather() from two separate configurations and + // confirm they don't interfere with each other + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + testServer := "Server=127.0.0.1;Port=1433;User Id=SA;Password=ABCabc01;app name=telegraf;log=1" + s := &SQLServer{ + Servers: []string{testServer}, + ExcludeQuery: []string{"MemoryClerk"}, + } + s2 := &SQLServer{ + Servers: []string{testServer}, + ExcludeQuery: []string{"DatabaseSize"}, + } + + var acc, acc2 testutil.Accumulator + err := s.Gather(&acc) + require.NoError(t, err) + assert.Equal(t, s.isInitialized, true) + assert.Equal(t, s2.isInitialized, false) + + err = s2.Gather(&acc2) + require.NoError(t, err) + assert.Equal(t, s.isInitialized, true) + assert.Equal(t, s2.isInitialized, true) + + // acc includes size metrics, and excludes memory metrics + assert.False(t, acc.HasMeasurement("Memory breakdown (%)")) + assert.True(t, acc.HasMeasurement("Log size (bytes)")) + + // acc2 includes memory metrics, and excludes size metrics + assert.True(t, acc2.HasMeasurement("Memory breakdown (%)")) + assert.False(t, acc2.HasMeasurement("Log size (bytes)")) +} + +func TestSqlServer_MultipleInit(t *testing.T) { + + s := &SQLServer{} + s2 := &SQLServer{ + ExcludeQuery: []string{"DatabaseSize"}, + } + + initQueries(s) + _, ok := s.queries["DatabaseSize"] + // acc includes size metrics + assert.True(t, ok) + assert.Equal(t, s.isInitialized, true) + assert.Equal(t, s2.isInitialized, false) + + initQueries(s2) + _, ok = s2.queries["DatabaseSize"] + // acc2 excludes size metrics + assert.False(t, ok) + assert.Equal(t, s.isInitialized, true) + assert.Equal(t, s2.isInitialized, true) +} + const mockPerformanceMetrics = `measurement;servername;type;Point In Time Recovery;Available physical memory (bytes);Average pending disk IO;Average runnable tasks;Average tasks;Buffer pool rate (bytes/sec);Connection memory per connection (bytes);Memory grant pending;Page File Usage (%);Page lookup per batch request;Page split per batch request;Readahead per page read;Signal wait (%);Sql compilation per batch request;Sql recompilation per batch request;Total target memory ratio Performance metrics;WIN8-DEV;Performance metrics;0;6353158144;0;0;7;2773;415061;0;25;229371;130;10;18;188;52;14`