-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Use stats database for proxysql backends count #12555
Use stats database for proxysql backends count #12555
Conversation
@@ -236,9 +236,9 @@ | |||
|
|||
STATS_MYSQL_BACKENDS = { | |||
'name': 'stats_mysql_backends', | |||
'query': 'SELECT hostgroup_id, status, COUNT(*) FROM runtime_mysql_servers GROUP BY hostgroup_id, status', | |||
'query': 'SELECT hostgroup, status, COUNT(*) FROM stats_mysql_connection_pool GROUP BY hostgroup, status', |
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.
I just checked and I confirm @aymeric-ledizes's analysis:
=> using the (unprivileged) stats
user, the (old) table is not accessible, and your "new" table is working fine
cf: this test on my proxysql admin socket:
$ mysql -ustats -pstats -h127.0.0.1 -P6032
mysql> show tables;
+--------------------------------------+
| tables |
+--------------------------------------+
| global_variables |
| stats_memory_metrics |
| stats_mysql_commands_counters |
| stats_mysql_connection_pool |
| stats_mysql_connection_pool_reset |
| stats_mysql_errors |
| stats_mysql_errors_reset |
| stats_mysql_free_connections |
| stats_mysql_global |
| stats_mysql_gtid_executed |
| stats_mysql_prepared_statements_info |
| stats_mysql_processlist |
| stats_mysql_query_digest |
| stats_mysql_query_digest_reset |
| stats_mysql_query_rules |
| stats_mysql_users |
| stats_proxysql_servers_checksums |
| stats_proxysql_servers_metrics |
| stats_proxysql_servers_status |
+--------------------------------------+
mysql> SELECT hostgroup_id, status, COUNT(*) FROM runtime_mysql_servers GROUP BY hostgroup_id, status;
ERROR 1045 (28000): ProxySQL Admin Error: no such table: runtime_mysql_servers
mysql> SELECT hostgroup, status, COUNT(*) FROM stats_mysql_connection_pool GROUP BY hostgroup, status;
+-----------+---------+----------+
| hostgroup | status | COUNT(*) |
+-----------+---------+----------+
| 10 | ONLINE | 1 |
| 10 | SHUNNED | 4 |
| 20 | ONLINE | 4 |
| 30 | ONLINE | 4 |
+-----------+---------+----------+
NB: of course, the "new" query also works with an "admin" user, so no regression :)
So, this change should only improve the usability & security of this integration 👍
Hello @hithwen, is this small PR ok to be merged ? |
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.
Hi! Thanks for your contribution!
I left some comments. Basically, you can simply update the query and it should be fine.
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.
Thank you!
What does this PR do?
This PR will change the query for the
proxysql.backends.count
metric from a query in a runtime table (admin-admin_credentials
required) to a query in a stats table (allowingadmin-stats_credentials
only)Motivation
Query in the
runtime_mysql_servers
table require admin credential by default. Runtime tables such asruntime_mysql_users
contains sensitive data so it is not recommended to useadmin-admin_credentials
nor grant permissions.While we can get the relevant data about backends count from the stats table
stats_mysql_connection_pool
we can update the query to this table instead.Additional Notes
Review checklist (to be filled by reviewers)
changelog/
andintegration/
labels attached