Skip to content

Commit

Permalink
Report known mysql database configuration errors as warnings (#11221)
Browse files Browse the repository at this point in the history
* Add warning reporting on mysql database configuration errors

* Fix test

* Add ddagenthostname to metrics payloads

* Introduce error codes

* Only emit warnings once per check

* Remove extra debug warning and fix report_warnings race condition

* New Dict instead of Clearing

* Fix type annotation
  • Loading branch information
alexandre-normand authored Jan 29, 2022
1 parent 6ee9859 commit c562b0d
Show file tree
Hide file tree
Showing 6 changed files with 213 additions and 28 deletions.
15 changes: 15 additions & 0 deletions mysql/datadog_checks/mysql/mysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
)
from .statement_samples import MySQLStatementSamples
from .statements import MySQLStatementMetrics
from .util import DatabaseConfigurationError
from .version_utils import get_version

try:
Expand Down Expand Up @@ -97,6 +98,7 @@ def __init__(self, name, init_config, instances):
self.check_initializations.append(self._query_manager.compile_queries)
self.innodb_stats = InnoDBMetrics()
self.check_initializations.append(self._config.configuration_checks)
self._warnings_by_code = {}
self._statement_metrics = MySQLStatementMetrics(self, self._config, self._get_connection_args())
self._statement_samples = MySQLStatementSamples(self, self._config, self._get_connection_args())

Expand Down Expand Up @@ -179,6 +181,7 @@ def check(self, _):
raise e
finally:
self._conn = None
self._report_warnings()

def cancel(self):
self._statement_samples.cancel()
Expand Down Expand Up @@ -972,3 +975,15 @@ def _compute_synthetic_results(self, results):
self._qcache_hits = int(results['Qcache_hits'])
self._qcache_inserts = int(results['Qcache_inserts'])
self._qcache_not_cached = int(results['Qcache_not_cached'])

def record_warning(self, code, message):
# type: (DatabaseConfigurationError, str) -> None
self._warnings_by_code[code] = message

def _report_warnings(self):
messages = self._warnings_by_code.values()
# Reset the warnings for the next check run
self._warnings_by_code = {}

for warning in messages:
self.warning(warning)
72 changes: 61 additions & 11 deletions mysql/datadog_checks/mysql/statement_samples.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
)
from datadog_checks.base.utils.serialization import json

from .util import DatabaseConfigurationError, warning_with_tags

SUPPORTED_EXPLAIN_STATEMENTS = frozenset({'select', 'table', 'delete', 'insert', 'replace', 'update', 'with'})

# unless a specific table is configured, we try all of the events_statements tables in descending order of
Expand Down Expand Up @@ -200,6 +202,15 @@
}
)

PYMYSQL_MISSING_EXPLAIN_STATEMENT_PROC_ERRORS = frozenset(
{
pymysql.constants.ER.ACCESS_DENIED_ERROR,
pymysql.constants.ER.DBACCESS_DENIED_ERROR,
pymysql.constants.ER.SP_DOES_NOT_EXIST,
pymysql.constants.ER.PROCACCESS_DENIED_ERROR,
}
)


class StatementTruncationState(Enum):
"""
Expand Down Expand Up @@ -487,7 +498,6 @@ def _collect_plan_for_statement(self, row):
# - `plan_signature` - hash computed from the normalized JSON plan to group identical plan trees
# - `resource_hash` - hash computed off the raw sql text to match apm resources
# - `query_signature` - hash computed from the digest text to match query metrics

try:
statement = obfuscate_sql_with_metadata(row['sql_text'], self._obfuscate_options)
statement_digest_text = obfuscate_sql_with_metadata(row['digest_text'], self._obfuscate_options)
Expand Down Expand Up @@ -807,7 +817,7 @@ def _explain_statement(self, cursor, statement, schema, obfuscated_statement, qu
)
continue
try:
plan = self._explain_strategies[strategy](cursor, statement, obfuscated_statement)
plan = self._explain_strategies[strategy](schema, cursor, statement, obfuscated_statement)
if plan:
self._collection_strategy_cache[explain_state_cache_key] = ExplainState(
strategy=strategy, error_code=None, error_message=None
Expand Down Expand Up @@ -851,7 +861,7 @@ def _explain_statement(self, cursor, statement, schema, obfuscated_statement, qu

return None, error_states

def _run_explain(self, cursor, statement, obfuscated_statement):
def _run_explain(self, schema, cursor, statement, obfuscated_statement):
"""
Run the explain using the EXPLAIN statement
"""
Expand All @@ -863,21 +873,61 @@ def _run_explain(self, cursor, statement, obfuscated_statement):
)
return cursor.fetchone()[0]

def _run_explain_procedure(self, cursor, statement, obfuscated_statement):
def _run_explain_procedure(self, schema, cursor, statement, obfuscated_statement):
"""
Run the explain by calling the stored procedure if available.
"""
self._cursor_run(cursor, 'CALL {}(%s)'.format(self._explain_procedure), statement, obfuscated_statement)
return cursor.fetchone()[0]
try:
self._cursor_run(cursor, 'CALL {}(%s)'.format(self._explain_procedure), statement, obfuscated_statement)
return cursor.fetchone()[0]
except pymysql.err.DatabaseError as e:
if e.args[0] in PYMYSQL_MISSING_EXPLAIN_STATEMENT_PROC_ERRORS:
err_msg = e.args[1] if len(e.args) > 1 else ''
self._check.record_warning(
DatabaseConfigurationError.explain_plan_procedure_missing,
warning_with_tags(
"Unable to collect explain plans because the procedure '%s' is either undefined or not "
"granted access to in schema '%s'. See https://docs.datadoghq.com/database_monitoring/"
'setup_mysql/troubleshooting#%s for more details: (%d) %s',
self._explain_procedure,
schema,
DatabaseConfigurationError.explain_plan_procedure_missing.value,
e.args[0],
str(err_msg),
code=DatabaseConfigurationError.explain_plan_procedure_missing.value,
host=self._check.resolved_hostname,
schema=schema,
),
)
raise

def _run_fully_qualified_explain_procedure(self, cursor, statement, obfuscated_statement):
def _run_fully_qualified_explain_procedure(self, schema, cursor, statement, obfuscated_statement):
"""
Run the explain by calling the fully qualified stored procedure if available.
"""
self._cursor_run(
cursor, 'CALL {}(%s)'.format(self._fully_qualified_explain_procedure), statement, obfuscated_statement
)
return cursor.fetchone()[0]
try:
self._cursor_run(
cursor, 'CALL {}(%s)'.format(self._fully_qualified_explain_procedure), statement, obfuscated_statement
)
return cursor.fetchone()[0]
except pymysql.err.DatabaseError as e:
if e.args[0] in PYMYSQL_MISSING_EXPLAIN_STATEMENT_PROC_ERRORS:
err_msg = e.args[1] if len(e.args) > 1 else ''
self._check.record_warning(
DatabaseConfigurationError.explain_plan_fq_procedure_missing,
warning_with_tags(
"Unable to collect explain plans because the procedure '%s' is either undefined or "
'not granted access to. See https://docs.datadoghq.com/database_monitoring/setup_mysql/'
'troubleshooting#%s for more details: (%d) %s',
self._fully_qualified_explain_procedure,
DatabaseConfigurationError.explain_plan_fq_procedure_missing.value,
e.args[0],
str(err_msg),
code=DatabaseConfigurationError.explain_plan_fq_procedure_missing.value,
host=self._check.resolved_hostname,
),
)
raise

@staticmethod
def _can_explain(obfuscated_statement):
Expand Down
19 changes: 19 additions & 0 deletions mysql/datadog_checks/mysql/statements.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
from datadog_checks.base.utils.db.utils import DBMAsyncJob, default_json_event_encoding, obfuscate_sql_with_metadata
from datadog_checks.base.utils.serialization import json

from .util import DatabaseConfigurationError, warning_with_tags

try:
import datadog_agent
except ImportError:
Expand Down Expand Up @@ -109,6 +111,22 @@ def run_job(self):
self.collect_per_statement_metrics()

def collect_per_statement_metrics(self):
# Detect a database misconfiguration by checking if the performance schema is enabled since mysql
# just returns no rows without errors if the performance schema is disabled
if not self._check.performance_schema_enabled:
self._check.record_warning(
DatabaseConfigurationError.performance_schema_not_enabled,
warning_with_tags(
'Unable to collect statement metrics because the performance schema is disabled. '
'See https://docs.datadoghq.com/database_monitoring/setup_mysql/'
'troubleshooting#%s for more details',
DatabaseConfigurationError.performance_schema_not_enabled.value,
code=DatabaseConfigurationError.performance_schema_not_enabled.value,
host=self._check.resolved_hostname,
),
)
return

rows = self._collect_per_statement_metrics()
if not rows:
return
Expand All @@ -125,6 +143,7 @@ def collect_per_statement_metrics(self):
'timestamp': time.time() * 1000,
'mysql_version': self._check.version.version + '+' + self._check.version.build,
'mysql_flavor': self._check.version.flavor,
"ddagenthostname": self._check.agent_hostname,
'ddagentversion': datadog_agent.get_version(),
'min_collection_interval': self._metric_collection_interval,
'tags': self._tags,
Expand Down
22 changes: 22 additions & 0 deletions mysql/datadog_checks/mysql/util.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# All rights reserved
# Licensed under Simplified BSD License (see LICENSE)
from enum import Enum


class DatabaseConfigurationError(Enum):
"""
Denotes the possible database configuration errors
"""

explain_plan_procedure_missing = 'explain-plan-procedure-missing'
explain_plan_fq_procedure_missing = 'explain-plan-fq-procedure-missing'
performance_schema_not_enabled = 'performance-schema-not-enabled'


def warning_with_tags(warning_message, *args, **kwargs):
if args:
warning_message = warning_message % args

return "{msg}\n{tags}".format(
msg=warning_message, tags=" ".join('{key}={value}'.format(key=k, value=v) for k, v in sorted(kwargs.items()))
)
34 changes: 17 additions & 17 deletions mysql/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ def dd_environment(config_e2e):
def instance_basic():
return {
'host': common.HOST,
'user': common.USER,
'pass': common.PASS,
'username': common.USER,
'password': common.PASS,
'port': common.PORT,
'disable_generic_tags': 'true',
}
Expand All @@ -86,8 +86,8 @@ def instance_basic():
def instance_complex():
return {
'host': common.HOST,
'user': common.USER,
'pass': common.PASS,
'username': common.USER,
'password': common.PASS,
'port': common.PORT,
'disable_generic_tags': 'true',
'options': {
Expand Down Expand Up @@ -119,8 +119,8 @@ def instance_complex():
def instance_additional_status():
return {
'host': common.HOST,
'user': common.USER,
'pass': common.PASS,
'username': common.USER,
'password': common.PASS,
'port': common.PORT,
'tags': tags.METRIC_TAGS,
'disable_generic_tags': 'true',
Expand All @@ -143,8 +143,8 @@ def instance_additional_status():
def instance_additional_variable():
return {
'host': common.HOST,
'user': common.USER,
'pass': common.PASS,
'username': common.USER,
'password': common.PASS,
'port': common.PORT,
'tags': tags.METRIC_TAGS,
'disable_generic_tags': 'true',
Expand All @@ -167,8 +167,8 @@ def instance_additional_variable():
def instance_status_already_queried():
return {
'host': common.HOST,
'user': common.USER,
'pass': common.PASS,
'username': common.USER,
'password': common.PASS,
'port': common.PORT,
'tags': tags.METRIC_TAGS,
'disable_generic_tags': 'true',
Expand All @@ -186,8 +186,8 @@ def instance_status_already_queried():
def instance_var_already_queried():
return {
'host': common.HOST,
'user': common.USER,
'pass': common.PASS,
'username': common.USER,
'password': common.PASS,
'port': common.PORT,
'tags': tags.METRIC_TAGS,
'disable_generic_tags': 'true',
Expand All @@ -205,8 +205,8 @@ def instance_var_already_queried():
def instance_invalid_var():
return {
'host': common.HOST,
'user': common.USER,
'pass': common.PASS,
'username': common.USER,
'password': common.PASS,
'port': common.PORT,
'tags': tags.METRIC_TAGS,
'disable_generic_tags': 'true',
Expand All @@ -229,8 +229,8 @@ def instance_invalid_var():
def instance_custom_queries():
return {
'host': common.HOST,
'user': common.USER,
'pass': common.PASS,
'username': common.USER,
'password': common.PASS,
'port': common.PORT,
'tags': tags.METRIC_TAGS,
'disable_generic_tags': 'true',
Expand All @@ -249,7 +249,7 @@ def instance_custom_queries():

@pytest.fixture(scope='session')
def instance_error():
return {'host': common.HOST, 'user': 'unknown', 'pass': common.PASS, 'disable_generic_tags': 'true'}
return {'host': common.HOST, 'username': 'unknown', 'password': common.PASS, 'disable_generic_tags': 'true'}


@pytest.fixture(scope='session')
Expand Down
Loading

0 comments on commit c562b0d

Please sign in to comment.