Improved Mysql plugin #889
Conversation
shows global variables shows slave statuses shows size and count of binary log files shows information_schema.processlist stats shows perf table stats shows auto increments stats from information schema shows perf index stats shows table lock waits summary by table shows time and operations of event waits shows file event statuses shows events statements stats from perf_schema shows schema statistics refactored plugin, provided multiple fields per insert
|
I can't comment on individual lines of code because the diff is too large, but this should be changed in the config file: to underscore and lower case: |
|
@maksadbek There are lots of times throughout the code where you declare variables as unsigned integers, and then convert them later to floats. What is the reason for that? Why not instantiate as floats? (see https://github.com/Maksadbek/telegraf/blob/master/plugins/inputs/mysql/mysql.go#L1367 for one example) |
|
please just instantiate these all as int64: https://github.com/Maksadbek/telegraf/blob/master/plugins/inputs/mysql/mysql.go#L19 |
|
And lastly, you'll need to write a README for this plugin |
|
@sparrc thanks for codereview, I will fix code asap |
|
thanks @maksadbek, just to make sure, will all of the default queries work on a default configured MySQL instance? The README doesn't need to contain every single possible metric, but there should at least be thorough documentation of what each of these options collects. |
|
Hi @sparrc, yes the metrics that does not require an extra effort are turned on by defaults. Some of the queries works on MySQL 5.6 version with default configurations, as the testing MySQL is 5.5 such queries turned off by default. Okay, I will also mention this on README and write briefly descriptions for specific metrics only |
| } | ||
| fields["syncs"] = i | ||
| } | ||
| // Send any remaining fields |
There was a problem hiding this comment.
Hey @maksadbek - this will need to move outside the above for loop or it ends up sending metrics multiple times (to match the implementation in gatherGlobalVariables()) 😄
There was a problem hiding this comment.
Yep, thanks, fix it asap
|
@sparrc , I will ask Tim and Seb to respond directly, but we certainly are using this in prod and have been using it for some time. We are very happy with it; it provides us visibility into vastly more than we got with the previous collector. I've attached a sample Grafana dashboard we use to give you an idea of the metrics we can extract from this. We have had a good number of production outages that we have been able to understand as a result of data that we collect now that we did not previously collect; its extremely helpful. Our internal MySQL DBAs are extremely impressed with it, too (and wrote many of the queries in the dashboard).
|
|
@sparrc Hey Cam, As Alex mentioned we have been running this across a number of MySQL hosts for some time and pinging @maksadbek directly with our findings or any potential issues. As the graphs above are a testament this significantly improves the visibility into MySQL. We agree with the transpose of the "lock waits" tags/fields (to avoid mutation), and actually makes more sense for our queries (especially until math operators are supported in functions in InfluxDB itself). Cheers, Tim |
|
Agreed with @wrigtim, @daviesalex here, this is currently running within our Production environment and is quite useful, helping avoid confusion and delay troubleshooting our mysql problems. That being said, any formatting and/or syntax changes you would recommend are indeed welcome to ensure compatibility and sanity moving forward. |
|
Okay, thanks for the input. I'm ready to merge this but would like to see more documentation. All input plugins need to have a README going forward based off of this one: https://github.com/influxdata/telegraf/blob/master/plugins/inputs/EXAMPLE_README.md @maksadbek all metrics do not need to be documented, but you'll need to at least provide some amount of information on each of these config options (ideally you should put comments describing them into the config file too): perf_events_statements_digest_text_limit = 120
perf_events_statements_limit = 250
perf_events_statements_time_limit = 86400
table_schema_databases = []
gather_process_list = true
gather_info_schema_auto_inc = true
gather_slave_status = true
gather_binary_logs = false
gather_table_io_waits = false
gather_index_io_waits = false
gather_table_schema = false
gather_file_events_stats = false
gather_perf_events_statements = false |
|
Hi @sparrc , yes I am already writing detailed readme about each metrics, and soon push it. Okay, I will comment configs too |
|
Hi @sparrc |
|
this will be available in 0.13, thanks again @maksadbek! |
No description provided.