Remove package level vars from sqlserver and mysql input plugins (inf…

…luxdata#6468)

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

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

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()

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`