-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved Mysql plugin #633
Conversation
Thanks @maksadbek, looks like you got a test failure:
Could you also squash your commits into one? I'll do a full review tomorrow |
@@ -24,6 +30,10 @@ var sampleConfig = ` | |||
# | |||
# If no servers are specified, then localhost is used as the host. | |||
servers = ["tcp(127.0.0.1:3306)/"] | |||
PerfEventsStatementsDigestTextLimit = 120 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add comments for all of these? and also change them to match the general underscore style of the config file,
ie:
perf_events_statement_digest_text_limit = 120
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, for each of these, can you explain the use-case of someone wanting to adjust these? Seems like a few of these could just be hardcoded constants within the config file, but I'm not as familiar with these metrics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I will add brief explanation of each metric and fix the names of config settings.
This config settings are changed frequently, that is why I did not hard code.
The test fails is because there is metric that tries to gather replication stats, and it only works if the database is slave. I will add the configuration settings to switch it on/off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to modify the MySQL docker image to be a slave? Or at least act like one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the configurations file(my.cnf) must be changed as follows:
bind-address = 0.0.0.0
server-id = 2
master-host = [private-IP-of-master]
master-user = [replication-username]
master-password = [replication-password]
master-connect-retry = 60
But this requires another MySQL to be a master for this slave.
@maksadbek Is this ready to go now? Please ping me when you've made updates as I don't monitor all the commits that go into PRs |
Hi @sparrc , Sorry for late reply. Which version of MySQL server is used to test ? Is there any way to install MySQL server 5.6.27 ? |
We have been testing a version of this internally for a few weeks now, and it works very well for our specific version (which is MariaDB 10.0 and 10.1) on 50+ nodes (we found a few edge cases, which have been integrated - for example we had some databases with thousands of tables that initially caused a crazy number of metrics to be produced). I think it also works well on the main developers test environment (which is upstream MySQL 5.6). My guess is that to get this merged, all versions of MySQL that are currently active (i.e. 5.5+, MariaDB, Percona and Oracle) will need to work although some of the metrics wont be available on older versions. @sparrc your advice on that is appreciated, but we are going to start thinking about how to make that work. |
One option may be to specify the various queries in the config file, which can then have versions, tags, queries, etc. associated with them, like this contributor is doing for the postgres plugin: #705 |
@sparrc we will look at that (if we cant make it elegantly figure out what is available on its own - my preference is to not have to put huge amounts of JSON on every box if at all possible). My hope is MySQL is a little bit easier than PSQL - some metrics work on all versions, and for others we can test on startup if they are available or not. Expect an update later in the week from our team when we have tried some things. Thanks! |
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
@daviesalex I did that because Postgresql have a small amount of metrics available and imply to use some additionnal extensions. By the way, the queries are now stored directly in the telegraf.conf as a toml structure |
@maksadbek just closing this to cleanup stale PRs, when you're ready please re-open as a separate PR |
Improved mysql plugin as discussed in #403