Handle variations in the user_statistics collecting #4306

glinton · 2018-06-15T21:00:48Z

There are differences in the user_statistics table (if it exists) between MariaDB, Percona, and MySQL. This should allow the stats to be collected properly.

MariaDB has two different table definitions:

https://mariadb.com/kb/en/library/information-schema-user_statistics-table/

Percona has it's own:

https://www.percona.com/doc/percona-server/LATEST/diagnostics/user_stats.html#USER_STATISTICS

MySQL doesn't have the table, but there may be a patch:

https://www.byte.nl/blog/enhancing-mysql-with-user-statistics

Resolves #2910
Closes #3382

Required for all PRs:

Signed CLA.
Associated README.md updated.
Has appropriate unit tests.

MariaDB has two different table definitions: - https://mariadb.com/kb/en/library/information-schema-user_statistics-table/ Percona has it's own: - https://www.percona.com/doc/percona-server/LATEST/diagnostics/user_stats.html#USER_STATISTICS MySQL doesn't have the table, but there may be a patch: - https://www.byte.nl/blog/enhancing-mysql-with-user-statistics Resolves #2910 Closes #3382

danielnelson · 2018-06-15T21:43:14Z

plugins/inputs/mysql/mysql.go

-
-				acc.AddFields("mysql_user_stats", fields, tags)
+			// disable collecting if table is not found (suppresses repeat errors)
+			if strings.Contains(err.Error(), "nknown table 'user_statistics'") {


I generally leave out capital letters when matching error strings in case the error changes. (nknown is intended)

Okay, personally I would probably just ToLower it, but ideally this type of error matching will be uncommon.

I did find an interesting link off of http://go-database-sql.org/errors.html#identifying-specific-database-errors to a package of error codes for mysql, this might be helpful, though I'm unsure if these work on all mysql flavors.

danielnelson · 2018-06-18T00:44:26Z

plugins/inputs/mysql/mysql.go

@@ -790,72 +790,13 @@ func (m *Mysql) gatherGlobalStatuses(db *sql.DB, serv string, acc telegraf.Accum

 	// gather connection metrics from user_statistics for each user
 	if m.GatherUserStatistics {
-		conn_rows, err := db.Query("select user, total_connections, concurrent_connections, connected_time, busy_time, cpu_time, bytes_received, bytes_sent, binlog_bytes_written, rows_fetched, rows_updated, table_rows_read, select_commands, update_commands, other_commands, commit_transactions, rollback_transactions, denied_connections, lost_connections, access_denied, empty_queries, total_ssl_connections FROM INFORMATION_SCHEMA.USER_STATISTICS GROUP BY user")
+		err = m.GatherUserStatisticsStatuses(db, serv, acc)


Will this result in this function being called twice per interval, once from gatherServer and once here?

It didn't appear to in testing (unless I missed it), but all the logic was already happening at this location anyways.

What about L455 though?

danielnelson · 2018-06-18T01:08:09Z

plugins/inputs/mysql/mysql.go

 		}
 		acc.AddFields("mysql_user_stats", fields, tags)
 	}
 	return nil
 }

+// convertColumns converts selected column names to lowercase.
+func convertColumns(s []string, e error) ([]string, error) {


Maybe rename columnsToLower or lowerColumns?

danielnelson · 2018-06-18T01:14:44Z

plugins/inputs/mysql/mysql.go

-		count                  uint32
-	)
+
+	cols, err := convertColumns(rows.Columns())


This is just an observation. I never knew you could pass multiple arguments like this, interesting. I'm not sure I like how it becomes the outer functions job to deal with an upstream error, but I like how it can compress the error handling needed in this function.

danielnelson · 2018-06-18T01:26:24Z

plugins/inputs/mysql/mysql.go

-			"total_ssl_connections":  total_ssl_connections,
+		for i := range cols {
+			if i == 0 {
+				continue // skip "user"


Percona index 0 comment applies here.

danielnelson · 2018-06-18T01:28:28Z

plugins/inputs/mysql/mysql.go

-			"access_denied":          access_denied,
-			"empty_queries":          empty_queries,
-			"total_ssl_connections":  total_ssl_connections,
+		for i := range cols {


I would use for _, column := range cols { and avoid using the index directly. I know in some situations it can be slightly slower to iterate in this manner, but I think it almost always is more readable and less error prone.

In this case, I think leaving it is best. I'll still need the iterator for the read slice and it's pretty clear where cols[i] is only used in the switch block.

danielnelson · 2018-06-18T01:28:54Z

plugins/inputs/mysql/mysql.go

+				continue // skip "user"
+			}
+			switch v := read[i].(type) {
+			case *bool:


Only include expected types and the default case can handle anything else.

Good call. Is it stylistically ok to leave out the default case since it is immediately followed by the function's return?

I usually leave the default in to catch any programming mistakes.

danielnelson · 2018-06-18T01:30:25Z

plugins/inputs/mysql/mysql.go

 		if err != nil {
 			return err
 		}

-		tags := map[string]string{"server": servtag, "user": user}
-		fields := map[string]interface{}{
+		tags := map[string]string{"server": servtag, "user": *read[0].(*string)}


User is not always index 0 (percona)

I'll make it more robust, but in all the docs I checked it would be index 0.

What about in this list of columns?

https://github.com/influxdata/telegraf/pull/4306/files#diff-d267f290fd16dce7d39997e69d775747R1041

Alias for user iirc, though now i'm not sure..

AH! I was referencing client_statistics table instead of user_statistics table

danielnelson · 2018-06-18T01:32:17Z

plugins/inputs/mysql/mysql.go

+		return err
+	}
+
+	if len(read) != len(cols) {


I would probably remove this check, since in theory it is impossible unless there is a programming error and it should. be picked up by the rows.Scan(

Resolves #4312

glinton · 2018-06-19T21:16:48Z

Closes #1645

(cherry picked from commit 54056f3)

…uxdata#4306) (cherry picked from commit 54056f3)

…uxdata#4306)

glinton added 2 commits June 15, 2018 14:45

Remove un-needed check

b5bac9f

danielnelson added area/mysql fix pr to fix corresponding bug labels Jun 18, 2018

danielnelson added this to the 1.7.1 milestone Jun 18, 2018

danielnelson reviewed Jun 18, 2018

View reviewed changes

glinton mentioned this pull request Jun 18, 2018

Expression #2 of SELECT list is not in GROUP BY clause and contains nonaggregated column #4312

Closed

glinton added 3 commits June 18, 2018 17:47

Gather user stats once. Remove 'group by' from user stat query

b534e15

Resolves #4312

Merge branch 'master' into bugfix/2910

921262f

Consolidate process list stats gathering

aa5ded1

loosi mentioned this pull request Jun 19, 2018

Fix Mysql Full GROUP BY #4313

Closed

3 tasks

Only include/check for types we know about

bf14c86

glinton force-pushed the bugfix/2910 branch from 04b1f7d to bf14c86 Compare June 19, 2018 15:00

glinton mentioned this pull request Jun 19, 2018

Allow to configure whether to gather global variables and statuses in input/mysql #1642

Closed

2 tasks

glinton added 2 commits June 19, 2018 15:40

Merge branch 'master' into bugfix/2910

0c5209a

Add missing field to percona for user_statistics

33ca597

danielnelson merged commit 54056f3 into master Jun 30, 2018

danielnelson deleted the bugfix/2910 branch June 30, 2018 01:16

danielnelson pushed a commit that referenced this pull request Jun 30, 2018

Handle mysql input variations in the user_statistics collecting (#4306)

70c20b0

(cherry picked from commit 54056f3)

danielnelson mentioned this pull request Jun 30, 2018

Percona input plugin #1645

Closed

codesmith14 pushed a commit to signalfx/telegraf that referenced this pull request Jul 5, 2018

Handle mysql input variations in the user_statistics collecting (infl…

6f54376

…uxdata#4306) (cherry picked from commit 54056f3)

otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019

Handle mysql input variations in the user_statistics collecting (infl…

a283aad

…uxdata#4306)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle variations in the user_statistics collecting #4306

Handle variations in the user_statistics collecting #4306

glinton commented Jun 15, 2018 •

edited

Loading

danielnelson Jun 15, 2018

glinton Jun 18, 2018

danielnelson Jun 18, 2018

danielnelson Jun 18, 2018

glinton Jun 18, 2018

danielnelson Jun 18, 2018

danielnelson Jun 18, 2018

glinton Jun 18, 2018

danielnelson Jun 18, 2018

danielnelson Jun 18, 2018

danielnelson Jun 18, 2018

glinton Jun 18, 2018

danielnelson Jun 18, 2018

glinton Jun 18, 2018

danielnelson Jun 18, 2018

danielnelson Jun 18, 2018

glinton Jun 18, 2018

danielnelson Jun 18, 2018

glinton Jun 18, 2018 •

edited

Loading

glinton Jun 18, 2018

danielnelson Jun 18, 2018

glinton Jun 18, 2018

glinton commented Jun 19, 2018

Handle variations in the user_statistics collecting #4306

Handle variations in the user_statistics collecting #4306

Conversation

glinton commented Jun 15, 2018 • edited Loading

Required for all PRs:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glinton Jun 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glinton commented Jun 19, 2018

glinton commented Jun 15, 2018 •

edited

Loading

glinton Jun 18, 2018 •

edited

Loading