Skip to content

Commit

Permalink
Allow proxysql checks to specify stats database name (#6835)
Browse files Browse the repository at this point in the history
* Allow proxysql checks to specify stats database name

The previosuly-hardcoded stats database is correct if you're connected as an admin user, but doesn't exist if you're connected as a stats-only user.

From a security point of view, it's probably better for datadog to connect as a stats user, since those aren't granted access to sensitive data. The existing checks should all work as a stats user, but for those users the 'stats' database is called 'main' instead.

This change drops the hardcoded database name from queries and specifies a default database at connection time instead, with the default set to 'stats'. This should be backwards-compatible with the old behavior.
  • Loading branch information
tabacco authored Jun 23, 2020
1 parent 8c1c822 commit 71fa47a
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 6 deletions.
9 changes: 9 additions & 0 deletions proxysql/assets/configuration/spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ files:
value:
type: string
example: <PROXYSQL_ADMIN_PASSWORD>
- name: database_name
required: false
description: |
Sets the name of the database to query for stats. For proxysql admin users, this is "stats" (default).
For stats users (defined in Proxysql's admin-stats_credentials), this is "main".
See https://github.com/sysown/proxysql/issues/1661 for more details.
value:
type: string
example: stats
- name: tls_verify
value:
example: false
Expand Down
7 changes: 7 additions & 0 deletions proxysql/datadog_checks/proxysql/data/conf.yaml.example
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ instances:
#
password: <PROXYSQL_ADMIN_PASSWORD>

## @param database_name - string - optional - default: stats
## Sets the name of the database to query for stats. For proxysql admin users, this is "stats" (default).
## For stats users (defined in Proxysql's admin-stats_credentials), this is "main".
## See https://github.com/sysown/proxysql/issues/1661 for more details.
#
# database_name: stats

## @param tls_verify - boolean - optional - default: false
## Instructs the check to use SSL when connecting to ProxySQL
#
Expand Down
2 changes: 2 additions & 0 deletions proxysql/datadog_checks/proxysql/proxysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def __init__(self, name, init_config, instances):
if not all((self.host, self.port, self.user, self.password)):
raise ConfigurationError("ProxySQL host, port, username and password are needed")

self.database_name = self.instance.get("database_name", "stats")
self.tls_verify = self.instance.get("tls_verify", False)
self.validate_hostname = self.instance.get("validate_hostname", True)
self.tls_ca_cert = self.instance.get("tls_ca_cert")
Expand Down Expand Up @@ -102,6 +103,7 @@ def connect(self):
user=self.user,
port=self.port,
passwd=self.password,
database=self.database_name,
connect_timeout=self.connect_timeout,
read_timeout=self.read_timeout,
ssl=ssl_context,
Expand Down
12 changes: 6 additions & 6 deletions proxysql/datadog_checks/proxysql/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
STATS_MYSQL_GLOBAL = Query(
{
'name': 'stats_mysql_global',
'query': 'SELECT * FROM stats.stats_mysql_global',
'query': 'SELECT * FROM stats_mysql_global',
'columns': [
{
'name': 'Variable_Name',
Expand Down Expand Up @@ -112,7 +112,7 @@
'name': 'stats_mysql_commands_counters',
'query': 'SELECT Command, Total_time_us, Total_cnt, cnt_100us, cnt_500us, cnt_1ms, cnt_5ms, cnt_10ms, '
'cnt_50ms, cnt_100ms, cnt_500ms, cnt_1s, cnt_5s, cnt_10s, cnt_INFs FROM '
'stats.stats_mysql_commands_counters',
'stats_mysql_commands_counters',
'columns': [
# the type of SQL command that has been executed. Examples: FLUSH, INSERT, KILL, SELECT FOR UPDATE, etc.
{'name': 'sql_command', 'type': 'tag'},
Expand Down Expand Up @@ -145,7 +145,7 @@
'name': 'stats_mysql_connection_pool',
# Need explicit selections as some columns are unusable.
'query': 'SELECT hostgroup, srv_host, srv_port, status, ConnUsed, ConnFree, ConnOK, ConnERR, Queries, '
'Bytes_data_sent, Bytes_data_recv, Latency_us FROM stats.stats_mysql_connection_pool',
'Bytes_data_sent, Bytes_data_recv, Latency_us FROM stats_mysql_connection_pool',
'columns': [
# the hostgroup in which the backend server belongs. Note that a single backend server can belong to more
# than one hostgroup
Expand Down Expand Up @@ -190,7 +190,7 @@
STATS_MYSQL_USERS = Query(
{
'name': 'stats_mysql_users',
'query': 'SELECT username, frontend_connections, frontend_max_connections FROM stats.stats_mysql_users',
'query': 'SELECT username, frontend_connections, frontend_max_connections FROM stats_mysql_users',
'columns': [
{'name': 'username', 'type': 'tag'},
{'name': 'frontend.user_connections', 'type': 'gauge'},
Expand All @@ -202,7 +202,7 @@
STATS_MEMORY_METRICS = Query(
{
'name': 'stats_memory_metrics',
'query': 'SELECT * FROM stats.stats_memory_metrics',
'query': 'SELECT * FROM stats_memory_metrics',
'columns': [
{
'name': 'Variable_Name',
Expand Down Expand Up @@ -242,7 +242,7 @@
STATS_MYSQL_QUERY_RULES = Query(
{
'name': 'stats_mysql_query_rules',
'query': 'SELECT rule_id, hits FROM stats.stats_mysql_query_rules',
'query': 'SELECT rule_id, hits FROM stats_mysql_query_rules',
'columns': [{'name': 'rule_id', 'type': 'tag'}, {'name': 'query_rules.rule_hits', 'type': 'rate'}],
}
)
Expand Down
1 change: 1 addition & 0 deletions proxysql/tests/compose/proxysql.cnf
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ datadir="/var/lib/proxysql"
admin_variables=
{
admin_credentials="admin:admin;proxy:proxy"
stats_credentials="stats:stats;proxystats:proxystats"
mysql_ifaces="0.0.0.0:6032"
}

Expand Down
24 changes: 24 additions & 0 deletions proxysql/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
MYSQL_PASS = 'pass'
PROXY_ADMIN_USER = 'proxy'
PROXY_ADMIN_PASS = 'proxy'
PROXY_STATS_USER = 'proxystats'
PROXY_STATS_PASS = 'proxystats'
MYSQL_DATABASE = 'test'
PROXY_MAIN_DATABASE = 'main'
PROXYSQL_VERSION = os.environ['PROXYSQL_VERSION']

BASIC_INSTANCE = {
Expand All @@ -45,6 +48,22 @@
],
}

INSTANCE_ALL_METRICS_STATS = {
'host': DOCKER_HOST,
'port': PROXY_ADMIN_PORT,
'username': PROXY_STATS_USER,
'password': PROXY_STATS_PASS,
'database_name': PROXY_MAIN_DATABASE,
'tags': ["application:test"],
'additional_metrics': [
'command_counters_metrics',
'connection_pool_metrics',
'users_metrics',
'memory_metrics',
'query_rules_metrics',
],
}


@pytest.fixture
def instance_basic():
Expand All @@ -56,6 +75,11 @@ def instance_all_metrics(instance_basic):
return deepcopy(INSTANCE_ALL_METRICS)


@pytest.fixture()
def instance_stats_user(instance_basic):
return deepcopy(INSTANCE_ALL_METRICS_STATS)


@pytest.fixture(scope='session')
def dd_environment():
compose_file = os.path.join(get_here(), 'compose', 'docker-compose.yml')
Expand Down
8 changes: 8 additions & 0 deletions proxysql/tests/test_proxysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,14 @@ def test_all_metrics(aggregator, instance_all_metrics, dd_run_check):
_assert_all_metrics(aggregator)


@pytest.mark.integration
@pytest.mark.usefixtures('dd_environment')
def test_all_metrics_stats_user(aggregator, instance_stats_user, dd_run_check):
check = get_check(instance_stats_user)
dd_run_check(check)
_assert_all_metrics(aggregator)


@pytest.mark.e2e
def test_e2e(dd_agent_check):
aggregator = dd_agent_check(rate=True)
Expand Down

0 comments on commit 71fa47a

Please sign in to comment.