From f6da4aeaf814de193e2281ac732d43b931158c4c Mon Sep 17 00:00:00 2001 From: root Date: Mon, 19 Feb 2024 18:33:23 +0000 Subject: [PATCH 01/32] DBMON-2051: Adding a deadlock metric to Maria DB --- .../datadog_checks/base/utils/db/core.py | 4 +- mysql/datadog_checks/mysql/mysql.py | 6 +- mysql/datadog_checks/mysql/queries.py | 15 + mysql/metadata.csv | 1 + mysql/tests/common.py | 5 +- mysql/tests/conftest.py | 59 +-- mysql/tests/conftest_patched.py | 478 ++++++++++++++++++ mysql/tests/test_query_activity.py | 56 +- mysql/tests/variables.py | 1 + 9 files changed, 573 insertions(+), 52 deletions(-) create mode 100644 mysql/tests/conftest_patched.py diff --git a/datadog_checks_base/datadog_checks/base/utils/db/core.py b/datadog_checks_base/datadog_checks/base/utils/db/core.py index 5ad3e4776cf3c..61bd7c7a15641 100644 --- a/datadog_checks_base/datadog_checks/base/utils/db/core.py +++ b/datadog_checks_base/datadog_checks/base/utils/db/core.py @@ -56,8 +56,9 @@ def compile_queries(self): method = getattr(self.submitter, submission_method) # Save each method in the initializer -> callable format column_transformers[transformer_name] = create_submission_transformer(method) - + print("we are compiling queries", self.queries) for query in self.queries: + print("we are compiling query wit data and name", query.query_data, query.name) query.compile(column_transformers, EXTRA_TRANSFORMERS.copy()) def execute(self, extra_tags=None): @@ -77,6 +78,7 @@ def execute(self, extra_tags=None): continue query_name = query.name + print("Query Name: ", query_name) query_columns = query.column_transformers extra_transformers = query.extra_transformers query_tags = query.base_tags diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index 1c213e7ed2154..6a75d35e36d5b 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -57,6 +57,7 @@ from .metadata import MySQLMetadata from .queries import ( QUERY_USER_CONNECTIONS, + QUERY_DEADLOCKS, SQL_95TH_PERCENTILE, SQL_AVG_QUERY_RUN_TIME, SQL_GROUP_REPLICATION_MEMBER, @@ -294,10 +295,13 @@ def check(self, _): self.check_userstat_enabled(db) # Metric collection + #lets assume my query is a metric too if not self._config.only_custom_queries: self._collect_metrics(db, tags=tags) self._collect_system_metrics(self._config.host, db, tags) if self.runtime_queries: + print("will execute runtime queries") + #BORIS my query is it a runtime query that is only run when the nofig hasnt only_custom_queries set to true self.runtime_queries.execute(extra_tags=tags) if self._config.dbm_enabled: @@ -344,7 +348,7 @@ def runtime_queries(self): return self._runtime_queries queries = [] - + queries.extend([QUERY_DEADLOCKS]) if self.performance_schema_enabled: queries.extend([QUERY_USER_CONNECTIONS]) diff --git a/mysql/datadog_checks/mysql/queries.py b/mysql/datadog_checks/mysql/queries.py index d7496c81cf226..5e043b21dc57d 100644 --- a/mysql/datadog_checks/mysql/queries.py +++ b/mysql/datadog_checks/mysql/queries.py @@ -74,6 +74,21 @@ SELECT plugin_status FROM information_schema.plugins WHERE plugin_name='group_replication'""" +QUERY_DEADLOCKS = { + 'name': 'information_schema.INNODB_METRICS.lock_deadlocks', + 'query': """ + SELECT + COUNT(*) as deadlocks + FROM + information_schema.INNODB_METRICS + WHERE + NAME='lock_deadlocks' + """.strip(), + 'columns': [{'name': 'mysql.innodb.deadlocks', 'type': 'gauge'}], + #Boris name ? + #Boris name ? + +} QUERY_USER_CONNECTIONS = { 'name': 'performance_schema.threads', 'query': """ diff --git a/mysql/metadata.csv b/mysql/metadata.csv index ff36b13c65f17..0dd835daddd90 100644 --- a/mysql/metadata.csv +++ b/mysql/metadata.csv @@ -12,6 +12,7 @@ mysql.innodb.buffer_pool_utilization,gauge,,fraction,,The utilization of the Inn mysql.innodb.current_row_locks,gauge,,lock,,The number of current row locks.,-1,mysql,current row locks, mysql.innodb.data_reads,gauge,,read,second,The rate of data reads.,0,mysql,data reads, mysql.innodb.data_writes,gauge,,write,second,The rate of data writes.,0,mysql,data writes, +mysql.innodb.deadlocks,gauge,,lock,,The number of current row deadlocks.,0,mysql,number of deadlocks, mysql.innodb.mutex_os_waits,gauge,,event,second,The rate of mutex OS waits.,0,mysql,mutex os waits, mysql.innodb.mutex_spin_rounds,gauge,,event,second,The rate of mutex spin rounds.,0,mysql,mutex spin rounds, mysql.innodb.mutex_spin_waits,gauge,,event,second,The rate of mutex spin waits.,0,mysql,mutex spin waits, diff --git a/mysql/tests/common.py b/mysql/tests/common.py index a2c58554c2e9d..21b13ac9e5034 100644 --- a/mysql/tests/common.py +++ b/mysql/tests/common.py @@ -16,10 +16,7 @@ MYSQL_REPLICATION = os.getenv('MYSQL_REPLICATION') MYSQL_VERSION_IS_LATEST = os.getenv('MYSQL_VERSION', '').endswith('latest') -if MYSQL_VERSION_IS_LATEST is False: - MYSQL_VERSION_PARSED = parse_version(os.getenv('MYSQL_VERSION', '').split('-')[0]) -else: - MYSQL_VERSION_PARSED = parse_version(str(maxsize)) +MYSQL_VERSION_PARSED = parse_version("10.5") CHECK_NAME = 'mysql' # adding flavor to differentiate mariadb from mysql diff --git a/mysql/tests/conftest.py b/mysql/tests/conftest.py index b78d0da59a0d2..ea644095f970e 100644 --- a/mysql/tests/conftest.py +++ b/mysql/tests/conftest.py @@ -24,54 +24,11 @@ @pytest.fixture(scope='session') def config_e2e(instance_basic): - instance = copy.deepcopy(instance_basic) - instance['dbm'] = True - logs_path = _mysql_logs_path() - - return { - 'init_config': {}, - 'instances': [instance], - 'logs': [ - {'type': 'file', 'path': '{}/mysql.log'.format(logs_path), 'source': 'mysql', 'service': 'local_mysql'}, - { - 'type': 'file', - 'path': '{}/mysql_slow.log'.format(logs_path), - 'source': 'mysql', - 'service': 'local_mysql', - 'log_processing_rules': [ - {'type': 'multi_line', 'name': 'log_starts_with_time', 'pattern': '# Time: '}, - ], - }, - ], - } - + pass @pytest.fixture(scope='session') def dd_environment(config_e2e): - logs_path = _mysql_logs_path() - - with TempDir('logs') as logs_host_path: - # for Ubuntu - os.chmod(logs_host_path, 0o770) - - e2e_metadata = {'docker_volumes': ['{}:{}'.format(logs_host_path, logs_path)]} - - with docker_run( - os.path.join(common.HERE, 'compose', COMPOSE_FILE), - env_vars={ - 'MYSQL_DOCKER_REPO': _mysql_docker_repo(), - 'MYSQL_PORT': str(common.PORT), - 'MYSQL_SLAVE_PORT': str(common.SLAVE_PORT), - 'MYSQL_CONF_PATH': _mysql_conf_path(), - 'MYSQL_LOGS_HOST_PATH': logs_host_path, - 'MYSQL_LOGS_PATH': logs_path, - 'WAIT_FOR_IT_SCRIPT_PATH': _wait_for_it_script(), - }, - conditions=_get_warmup_conditions(), - attempts=2, - attempts_wait=10, - ): - yield config_e2e, e2e_metadata + pass @pytest.fixture(scope='session') @@ -461,6 +418,18 @@ def populate_database(): cur.execute("GRANT SELECT, UPDATE ON testdb.users TO 'bob'@'%';") cur.execute("GRANT SELECT, UPDATE ON testdb.users TO 'fred'@'%';") cur.close() + #Boris later come up with a better way to do this probably + # deadlock can be created just with testdb.users table + cur = conn.cursor() + cur.execute("CREATE TABLE testdb.accounts (id INT PRIMARY KEY, balance INT);") + cur.execute("CREATE TABLE transactions (id INT PRIMARY KEY,from_account INT,to_account INT,amount INT);") + cur.execute("INSERT INTO testdb.accounts (id,balance) VALUES(1,1000);") + cur.execute("INSERT INTO testdb.accounts (id,balance) VALUES(2,1000);") + cur.execute("GRANT SELECT, UPDATE ON testdb.accounts TO 'bob'@'%';") + cur.execute("GRANT SELECT, UPDATE ON testdb.accounts TO 'fred'@'%';") + cur.execute("GRANT SELECT, UPDATE ON testdb.transactions TO 'bob'@'%';") + cur.execute("GRANT SELECT, UPDATE ON testdb.transactions TO 'fred'@'%';") + cur.close() _create_explain_procedure(conn, "testdb") diff --git a/mysql/tests/conftest_patched.py b/mysql/tests/conftest_patched.py new file mode 100644 index 0000000000000..49cc1b954e60b --- /dev/null +++ b/mysql/tests/conftest_patched.py @@ -0,0 +1,478 @@ +# (C) Datadog, Inc. 2018-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +import copy +import logging +import os + +import mock +import pymysql +import pytest + +from datadog_checks.dev import TempDir, WaitFor, docker_run +from datadog_checks.dev.conditions import CheckDockerLogs + +from . import common, tags +from .common import MYSQL_REPLICATION + +logger = logging.getLogger(__name__) + +MYSQL_FLAVOR = os.getenv('MYSQL_FLAVOR') +MYSQL_VERSION = os.getenv('MYSQL_VERSION') +COMPOSE_FILE = os.getenv('COMPOSE_FILE') + + +@pytest.fixture(scope='session') +def config_e2e(instance_basic): + pass + +@pytest.fixture(scope='session') +def dd_environment(config_e2e): + pass + + +@pytest.fixture(scope='session') +def instance_basic(): + return { + 'host': common.HOST, + 'username': common.USER, + 'password': common.PASS, + 'port': common.PORT, + 'disable_generic_tags': 'true', + } + + +@pytest.fixture +def instance_complex(): + return { + 'host': common.HOST, + 'username': common.USER, + 'password': common.PASS, + 'port': common.PORT, + 'disable_generic_tags': 'true', + 'options': { + 'replication': True, + 'extra_status_metrics': True, + 'extra_innodb_metrics': True, + 'extra_performance_metrics': True, + 'schema_size_metrics': True, + 'table_size_metrics': True, + 'system_table_size_metrics': True, + 'table_row_stats_metrics': True, + }, + 'tags': tags.METRIC_TAGS, + 'queries': [ + { + 'query': "SELECT * from testdb.users where name='Alice' limit 1;", + 'metric': 'alice.age', + 'type': 'gauge', + 'field': 'age', + }, + { + 'query': "SELECT * from testdb.users where name='Bob' limit 1;", + 'metric': 'bob.age', + 'type': 'gauge', + 'field': 'age', + }, + ], + } + + +@pytest.fixture +def instance_additional_status(): + return { + 'host': common.HOST, + 'username': common.USER, + 'password': common.PASS, + 'port': common.PORT, + 'tags': tags.METRIC_TAGS, + 'disable_generic_tags': 'true', + 'additional_status': [ + { + 'name': "Innodb_rows_read", + 'metric_name': "mysql.innodb.rows_read", + 'type': "rate", + }, + { + 'name': "Innodb_row_lock_time", + 'metric_name': "mysql.innodb.row_lock_time", + 'type': "rate", + }, + ], + } + + +@pytest.fixture +def instance_additional_variable(): + return { + 'host': common.HOST, + 'username': common.USER, + 'password': common.PASS, + 'port': common.PORT, + 'tags': tags.METRIC_TAGS, + 'disable_generic_tags': 'true', + 'additional_variable': [ + { + 'name': "long_query_time", + 'metric_name': "mysql.performance.long_query_time", + 'type': "gauge", + }, + { + 'name': "innodb_flush_log_at_trx_commit", + 'metric_name': "mysql.performance.innodb_flush_log_at_trx_commit", + 'type': "gauge", + }, + ], + } + + +@pytest.fixture +def instance_status_already_queried(): + return { + 'host': common.HOST, + 'username': common.USER, + 'password': common.PASS, + 'port': common.PORT, + 'tags': tags.METRIC_TAGS, + 'disable_generic_tags': 'true', + 'additional_status': [ + { + 'name': "Open_files", + 'metric_name': "mysql.performance.open_files_test", + 'type': "gauge", + }, + ], + } + + +@pytest.fixture +def instance_var_already_queried(): + return { + 'host': common.HOST, + 'username': common.USER, + 'password': common.PASS, + 'port': common.PORT, + 'tags': tags.METRIC_TAGS, + 'disable_generic_tags': 'true', + 'additional_variable': [ + { + 'name': "Key_buffer_size", + 'metric_name': "mysql.myisam.key_buffer_size", + 'type': "gauge", + }, + ], + } + + +@pytest.fixture +def instance_invalid_var(): + return { + 'host': common.HOST, + 'username': common.USER, + 'password': common.PASS, + 'port': common.PORT, + 'tags': tags.METRIC_TAGS, + 'disable_generic_tags': 'true', + 'additional_status': [ + { + 'name': "longer_query_time", + 'metric_name': "mysql.performance.longer_query_time", + 'type': "gauge", + }, + { + 'name': "innodb_flush_log_at_trx_commit", + 'metric_name': "mysql.performance.innodb_flush_log_at_trx_commit", + 'type': "gauge", + }, + ], + } + + +@pytest.fixture +def instance_custom_queries(): + return { + 'host': common.HOST, + 'username': common.USER, + 'password': common.PASS, + 'port': common.PORT, + 'tags': tags.METRIC_TAGS, + 'disable_generic_tags': 'true', + 'custom_queries': [ + { + 'query': "SELECT name,age from testdb.users where name='Alice' limit 1;", + 'columns': [{}, {'name': 'alice.age', 'type': 'gauge'}], + }, + { + 'query': "SELECT name,age from testdb.users where name='Bob' limit 1;", + 'columns': [{}, {'name': 'bob.age', 'type': 'gauge'}], + }, + ], + } + + +@pytest.fixture(scope='session') +def instance_error(): + return {'host': common.HOST, 'username': 'unknown', 'password': common.PASS, 'disable_generic_tags': 'true'} + + +@pytest.fixture(scope='session') +def version_metadata(): + parts = MYSQL_VERSION.split('-') + version = parts[0].split('.') + major, minor = version[:2] + patch = version[2] if len(version) > 2 else mock.ANY + + flavor = "MariaDB" if MYSQL_FLAVOR == "mariadb" else "MySQL" + + return { + 'version.scheme': 'semver', + 'version.major': major, + 'version.minor': minor, + 'version.patch': patch, + 'version.raw': mock.ANY, + 'version.build': mock.ANY, + 'flavor': flavor, + 'resolved_hostname': 'forced_hostname', + } + + +def _get_warmup_conditions(): + if MYSQL_REPLICATION == 'group': + return [ + CheckDockerLogs('node1', "X Plugin ready for connections. Bind-address: '::' port: 33060"), + CheckDockerLogs('node2', "X Plugin ready for connections. Bind-address: '::' port: 33060"), + CheckDockerLogs('node3', "X Plugin ready for connections. Bind-address: '::' port: 33060"), + init_group_replication, + populate_database, + ] + return [ + WaitFor(init_master, wait=2), + WaitFor(init_slave, wait=2), + CheckDockerLogs('mysql-slave', ["ready for connections", "mariadb successfully initialized"]), + populate_database, + ] + + +def init_group_replication(): + logger.debug("initializing group replication") + import time + + time.sleep(5) + conns = [pymysql.connect(host=common.HOST, port=p, user='root', password='mypass') for p in common.PORTS_GROUP] + _add_dog_user(conns[0]) + _add_bob_user(conns[0]) + _add_fred_user(conns[0]) + _init_datadog_sample_collection(conns[0]) + + cur_primary = conns[0].cursor() + cur_primary.execute("SET @@GLOBAL.group_replication_bootstrap_group=1;") + cur_primary.execute("create user 'repl'@'%';") + cur_primary.execute("GRANT REPLICATION SLAVE ON *.* TO repl@'%';") + cur_primary.execute("flush privileges;") + cur_primary.execute("change master to master_user='root' for channel 'group_replication_recovery';") + cur_primary.execute("START GROUP_REPLICATION;") + cur_primary.execute("SET @@GLOBAL.group_replication_bootstrap_group=0;") + cur_primary.execute("SELECT * FROM performance_schema.replication_group_members;") + + # Node 2 and 3 + for c in conns[1:]: + cur = c.cursor() + cur.execute("change master to master_user='repl' for channel 'group_replication_recovery';") + cur.execute("START GROUP_REPLICATION;") + + +def _init_datadog_sample_collection(conn): + logger.debug("initializing datadog sample collection") + cur = conn.cursor() + cur.execute("CREATE DATABASE datadog") + cur.execute("GRANT CREATE TEMPORARY TABLES ON `datadog`.* TO 'dog'@'%'") + cur.execute("GRANT EXECUTE on `datadog`.* TO 'dog'@'%'") + _create_explain_procedure(conn, "datadog") + _create_explain_procedure(conn, "mysql") + _create_enable_consumers_procedure(conn) + + +def _create_explain_procedure(conn, schema): + logger.debug("creating explain procedure in schema=%s", schema) + cur = conn.cursor() + cur.execute( + """ + CREATE PROCEDURE {schema}.explain_statement(IN query TEXT) + SQL SECURITY DEFINER + BEGIN + SET @explain := CONCAT('EXPLAIN FORMAT=json ', query); + PREPARE stmt FROM @explain; + EXECUTE stmt; + DEALLOCATE PREPARE stmt; + END; + """.format( + schema=schema + ) + ) + if schema != 'datadog': + cur.execute("GRANT EXECUTE ON PROCEDURE {schema}.explain_statement to 'dog'@'%'".format(schema=schema)) + cur.close() + + +def _create_enable_consumers_procedure(conn): + logger.debug("creating enable_events_statements_consumers procedure") + cur = conn.cursor() + cur.execute( + """ + CREATE PROCEDURE datadog.enable_events_statements_consumers() + SQL SECURITY DEFINER + BEGIN + UPDATE performance_schema.setup_consumers SET enabled='YES' WHERE name LIKE 'events_statements_%'; + UPDATE performance_schema.setup_consumers SET enabled='YES' WHERE name = 'events_waits_current'; + END; + """ + ) + cur.close() + + +def init_master(): + logger.debug("initializing master") + conn = pymysql.connect(host=common.HOST, port=common.PORT, user='root') + _add_dog_user(conn) + _add_bob_user(conn) + _add_fred_user(conn) + _init_datadog_sample_collection(conn) + + +@pytest.fixture +def root_conn(): + conn = pymysql.connect( + host=common.HOST, port=common.PORT, user='root', password='mypass' if MYSQL_REPLICATION == 'group' else None + ) + yield conn + conn.close() + + +def init_slave(): + logger.debug("initializing slave") + pymysql.connect(host=common.HOST, port=common.SLAVE_PORT, user=common.USER, passwd=common.PASS) + + +def _add_dog_user(conn): + cur = conn.cursor() + cur.execute("SELECT count(*) FROM mysql.user WHERE User = 'dog' and Host = '%'") + if cur.fetchone()[0] == 0: + # gracefully handle retries due to partial failure later on + cur.execute("CREATE USER 'dog'@'%' IDENTIFIED BY 'dog'") + cur.execute("GRANT PROCESS ON *.* TO 'dog'@'%'") + cur.execute("GRANT REPLICATION CLIENT ON *.* TO 'dog'@'%'") + cur.execute("GRANT SELECT ON performance_schema.* TO 'dog'@'%'") + + # refactor try older mysql.user table first. if this fails, go to newer ALTER USER + try: + cur.execute("UPDATE mysql.user SET max_user_connections = 0 WHERE user='dog' AND host='%'") + cur.execute("FLUSH PRIVILEGES") + # need to get better exception in order to raise errors in the future + except Exception: + if MYSQL_FLAVOR == 'mariadb': + cur.execute("GRANT SLAVE MONITOR ON *.* TO 'dog'@'%'") + cur.execute("ALTER USER 'dog'@'%' WITH MAX_USER_CONNECTIONS 0") + + +def _add_bob_user(conn): + cur = conn.cursor() + cur.execute("CREATE USER 'bob'@'%' IDENTIFIED BY 'bob'") + cur.execute("GRANT USAGE on *.* to 'bob'@'%'") + + +def _add_fred_user(conn): + cur = conn.cursor() + cur.execute("CREATE USER 'fred'@'%' IDENTIFIED BY 'fred'") + cur.execute("GRANT USAGE on *.* to 'fred'@'%'") + + +@pytest.fixture +def bob_conn(): + conn = pymysql.connect(host=common.HOST, port=common.PORT, user='bob', password='bob') + yield conn + conn.close() + + +@pytest.fixture +def fred_conn(): + conn = pymysql.connect(host=common.HOST, port=common.PORT, user='fred', password='fred') + yield conn + conn.close() + + +def populate_database(): + logger.debug("populating database") + conn = pymysql.connect( + host=common.HOST, port=common.PORT, user='root', password='mypass' if MYSQL_REPLICATION == 'group' else None + ) + + cur = conn.cursor() + + cur.execute("USE mysql;") + cur.execute("CREATE DATABASE testdb;") + cur.execute("USE testdb;") + cur.execute("CREATE TABLE testdb.users (id INT NOT NULL UNIQUE KEY, name VARCHAR(20), age INT);") + cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(1,'Alice',25);") + cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(2,'Bob',20);") + cur.execute("GRANT SELECT ON testdb.users TO 'dog'@'%';") + cur.execute("GRANT SELECT, UPDATE ON testdb.users TO 'bob'@'%';") + cur.execute("GRANT SELECT, UPDATE ON testdb.users TO 'fred'@'%';") + cur.close() + _create_explain_procedure(conn, "testdb") + + +def _wait_for_it_script(): + """ + FIXME: relying on the filesystem layout is a bad idea, the testing helper + should expose its path through the api instead + """ + script = os.path.join(common.TESTS_HELPER_DIR, 'scripts', 'wait-for-it.sh') + return os.path.abspath(script) + + +def _mysql_conf_path(): + """ + Return the path to a local MySQL configuration file suited for the current environment. + """ + if MYSQL_FLAVOR == 'mysql': + filename = 'mysql.conf' + elif MYSQL_FLAVOR == 'mariadb': + filename = 'mariadb.conf' + else: + raise ValueError('Unsupported MySQL flavor: {}'.format(MYSQL_FLAVOR)) + + conf = os.path.join(common.HERE, 'compose', filename) + return os.path.abspath(conf) + + +def _mysql_logs_path(): + """ + Return the path to the MySQL logs directory in the MySQL container. + + Should be kept in sync with the log paths set in the local MySQL configuration files + (which don't support interpolation of environment variables). + """ + if MYSQL_FLAVOR == 'mysql': + if MYSQL_VERSION == '8.0' or MYSQL_VERSION == 'latest': + return '/opt/bitnami/mysql/logs' + else: + return '/var/log/mysql' + elif MYSQL_FLAVOR == 'mariadb': + return '/opt/bitnami/mariadb/logs' + else: + raise ValueError('Unsupported MySQL flavor: {}'.format(MYSQL_FLAVOR)) + + +def _mysql_docker_repo(): + if MYSQL_FLAVOR == 'mysql': + # The image for testing Mysql 5.5 is located at `jfullaondo/mysql-replication` or `bergerx/mysql-replication` + # Warning: This image is a bit flaky on CI (it has been removed + # https://github.com/DataDog/integrations-core/pull/4669) + if MYSQL_VERSION in ('5.6', '5.7'): + return 'bergerx/mysql-replication' + elif MYSQL_VERSION == '8.0' or MYSQL_VERSION == 'latest': + return 'bitnami/mysql' + elif MYSQL_FLAVOR == 'mariadb': + return 'bitnami/mariadb' + else: + raise ValueError('Unsupported MySQL flavor: {}'.format(MYSQL_FLAVOR)) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index 308585dcf2a62..427c9db87b6b5 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -48,6 +48,60 @@ def dbm_instance(instance_complex): instance_complex['collect_settings'] = {'enabled': False} return copy(instance_complex) +@pytest.mark.integration +@pytest.mark.usefixtures('dd_environment') +def test_deadlocks(aggregator, dd_run_check, dbm_instance): + check = MySql(CHECK_NAME, {}, [dbm_instance]) + #Justin suggested to change timeout instead of using a long sleep + # Iguess his idea is to change timeout on how long it ccan wait on asquiring a lock. + #Boris makes sense like in Nenands example make sure that there were no deadlocks before the test + #BORIS also simplify deadlock thing can be like in Nenads example + # https://github.com/DataDog/integrations-core/pull/13374/files#diff-369b313fc4346aa906cef597e161569db8e0c52c876dea0a7fe90cd619a0da62 + def run_deadlock_1(conn): + conn.begin() + conn.cursor().execute("""START TRANSACTION; + UPDATE accounts SET balance = balance - 100 WHERE id = 1; + SLEEP(3); + UPDATE accounts SET balance = balance + 100 WHERE id = 2; + COMMIT;""") +#ADD SLEEP + conn.commit() + def run_deadlock_2(conn): + conn.begin() + conn.cursor().execute("""START TRANSACTION; + UPDATE accounts SET balance = balance - 50 WHERE id = 2; + SLEEP(3); + UPDATE accounts SET balance = balance + 50 WHERE id = 1; + COMMIT;""") + conn.commit() + + bob_conn = _get_conn_for_user('bob') + fred_conn = _get_conn_for_user('fred') + + executor = concurrent.futures.thread.ThreadPoolExecutor(1) + # bob's query will block until the TX is completed + executor.submit(run_deadlock_1, bob_conn) + # fred's query will get blocked by bob's TX + time.sleep(1.5) + executor.submit(run_deadlock_2, fred_conn) + + dd_run_check(check) + bob_conn.close() + fred_conn.close() + + executor.shutdown() + + # get result of my query in aggregator and assert + # dbm_activity = aggregator.get_event_platform_events("dbm-activity") + + deadlock_metric = aggregator.metrics("mysql.innodb.deadlocks") + if len(deadlock_metric)>0: + for_debug = deadlock_metric[0].value + else: + for_debug = 0 + + assert for_debug == 1, "should have collected exactly one activity payload" + @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') @@ -156,7 +210,7 @@ def test_activity_reported_hostname(aggregator, dbm_instance, dd_run_check, repo dbm_instance['reported_hostname'] = reported_hostname check = MySql(CHECK_NAME, {}, [dbm_instance]) - dd_run_check(check) + dd_run_check(check)#Boris why 2 times dd_run_check? dd_run_check(check) dbm_activity = aggregator.get_event_platform_events("dbm-activity") diff --git a/mysql/tests/variables.py b/mysql/tests/variables.py index 193856391853b..00d8b19d95f87 100644 --- a/mysql/tests/variables.py +++ b/mysql/tests/variables.py @@ -90,6 +90,7 @@ 'mysql.innodb.buffer_pool_read_requests', 'mysql.innodb.buffer_pool_reads', 'mysql.innodb.buffer_pool_utilization', + 'mysql.innodb.deadlocks', #Boris may be was a bad name ] COMPLEX_INNODB_VARS = [ From f0a7bcf3c64942e2051ba5b72f6a8d76fc8e3300 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Tue, 20 Feb 2024 09:07:49 +0000 Subject: [PATCH 02/32] removing print outs --- mysql/datadog_checks/mysql/queries.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mysql/datadog_checks/mysql/queries.py b/mysql/datadog_checks/mysql/queries.py index 5e043b21dc57d..08220897082e2 100644 --- a/mysql/datadog_checks/mysql/queries.py +++ b/mysql/datadog_checks/mysql/queries.py @@ -89,6 +89,7 @@ #Boris name ? } + QUERY_USER_CONNECTIONS = { 'name': 'performance_schema.threads', 'query': """ From 402fa6fb07ec999fe48e925b7458b1600882f8c4 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Tue, 20 Feb 2024 09:09:24 +0000 Subject: [PATCH 03/32] removed prints --- datadog_checks_base/datadog_checks/base/utils/db/core.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/utils/db/core.py b/datadog_checks_base/datadog_checks/base/utils/db/core.py index 61bd7c7a15641..5ad3e4776cf3c 100644 --- a/datadog_checks_base/datadog_checks/base/utils/db/core.py +++ b/datadog_checks_base/datadog_checks/base/utils/db/core.py @@ -56,9 +56,8 @@ def compile_queries(self): method = getattr(self.submitter, submission_method) # Save each method in the initializer -> callable format column_transformers[transformer_name] = create_submission_transformer(method) - print("we are compiling queries", self.queries) + for query in self.queries: - print("we are compiling query wit data and name", query.query_data, query.name) query.compile(column_transformers, EXTRA_TRANSFORMERS.copy()) def execute(self, extra_tags=None): @@ -78,7 +77,6 @@ def execute(self, extra_tags=None): continue query_name = query.name - print("Query Name: ", query_name) query_columns = query.column_transformers extra_transformers = query.extra_transformers query_tags = query.base_tags From cdf98190d6123c692455cafc4908bc05d6fd6dd2 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Tue, 20 Feb 2024 10:25:53 +0000 Subject: [PATCH 04/32] trying to create a proper deadlock --- mysql/tests/test_query_activity.py | 46 +++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index 427c9db87b6b5..71a13b3fbe9f4 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -51,7 +51,24 @@ def dbm_instance(instance_complex): @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') def test_deadlocks(aggregator, dd_run_check, dbm_instance): + #set up + check = MySql(CHECK_NAME, {}, [dbm_instance]) + deadlock_query = """ + SELECT + COUNT(*) as deadlocks + FROM + information_schema.INNODB_METRICS + WHERE + NAME='lock_deadlocks'""" + dd_run_check(check) + deadlocks_start = 0 + deadlock_metric_start = aggregator.metrics("mysql.innodb.deadlocks") + if len(deadlock_metric_start)>0: + deadlocks_start = deadlock_metric_start[0].value + + #act + #Justin suggested to change timeout instead of using a long sleep # Iguess his idea is to change timeout on how long it ccan wait on asquiring a lock. #Boris makes sense like in Nenands example make sure that there were no deadlocks before the test @@ -74,6 +91,32 @@ def run_deadlock_2(conn): UPDATE accounts SET balance = balance + 50 WHERE id = 1; COMMIT;""") conn.commit() + + # cur.execute("CREATE TABLE testdb.users (id INT NOT NULL UNIQUE KEY, name VARCHAR(20), age INT);") + #cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(1,'Alice',25);") + #cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(2,'Bob',20);") +#Boris do we need to check that current value is 25 and then set it back in a tear down ? + def run_first_deadlock_query(conn): + conn.begin() + conn.cursor().execute(""" + START TRANSACTION; + UPDATE testdb.users SET age = 25 WHERE id = 1; + SLEEP(3); + UPDATE testdb.users SET age = 20 WHERE id = 2; + COMMIT; + """) + conn.commit() + + def run_second_deadlock_query(conn): + conn.begin() + conn.cursor().execute(""" + START TRANSACTION; + UPDATE testdb.users SET age = 20 WHERE id = 2; + SLEEP(3); + UPDATE testdb.users SET age = 25 WHERE id = 1; + COMMIT; + """) + conn.commit() bob_conn = _get_conn_for_user('bob') fred_conn = _get_conn_for_user('fred') @@ -100,7 +143,8 @@ def run_deadlock_2(conn): else: for_debug = 0 - assert for_debug == 1, "should have collected exactly one activity payload" + #assert + assert for_debug - deadlocks_start == 1, "there should be one deadlock" @pytest.mark.integration From 89728deacefd0f000ef01fc5235b7d0e0ac6f9c7 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Tue, 20 Feb 2024 12:42:37 +0000 Subject: [PATCH 05/32] Fix thread pool limit --- mysql/datadog_checks/mysql/mysql.py | 2 +- mysql/tests/test_query_activity.py | 48 +++++++++++------------------ 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index 6a75d35e36d5b..d5ed444ef1902 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -386,7 +386,7 @@ def _get_connection_args(self): connection_args = { 'ssl': ssl, 'connect_timeout': self._config.connect_timeout, - 'autocommit': True, + 'autocommit': False, } if self._config.charset: connection_args['charset'] = self._config.charset diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index 71a13b3fbe9f4..de2c6660a1601 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -74,59 +74,48 @@ def test_deadlocks(aggregator, dd_run_check, dbm_instance): #Boris makes sense like in Nenands example make sure that there were no deadlocks before the test #BORIS also simplify deadlock thing can be like in Nenads example # https://github.com/DataDog/integrations-core/pull/13374/files#diff-369b313fc4346aa906cef597e161569db8e0c52c876dea0a7fe90cd619a0da62 - def run_deadlock_1(conn): - conn.begin() - conn.cursor().execute("""START TRANSACTION; - UPDATE accounts SET balance = balance - 100 WHERE id = 1; - SLEEP(3); - UPDATE accounts SET balance = balance + 100 WHERE id = 2; - COMMIT;""") -#ADD SLEEP - conn.commit() - def run_deadlock_2(conn): - conn.begin() - conn.cursor().execute("""START TRANSACTION; - UPDATE accounts SET balance = balance - 50 WHERE id = 2; - SLEEP(3); - UPDATE accounts SET balance = balance + 50 WHERE id = 1; - COMMIT;""") - conn.commit() + # cur.execute("CREATE TABLE testdb.users (id INT NOT NULL UNIQUE KEY, name VARCHAR(20), age INT);") #cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(1,'Alice',25);") #cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(2,'Bob',20);") -#Boris do we need to check that current value is 25 and then set it back in a tear down ? + #Boris do we need to check that current value is 25 and then set it back in a tear down ? + #or may be do select for update as it also should be blocking def run_first_deadlock_query(conn): conn.begin() conn.cursor().execute(""" + SET autocommit=0; START TRANSACTION; - UPDATE testdb.users SET age = 25 WHERE id = 1; - SLEEP(3); - UPDATE testdb.users SET age = 20 WHERE id = 2; + UPDATE testdb.users SET age = 20 WHERE id = 1; + SELECT SLEEP(3); + UPDATE testdb.users SET age = 25 WHERE id = 2; COMMIT; + SET autocommit=0; """) conn.commit() def run_second_deadlock_query(conn): conn.begin() conn.cursor().execute(""" + SET autocommit=0; START TRANSACTION; - UPDATE testdb.users SET age = 20 WHERE id = 2; - SLEEP(3); - UPDATE testdb.users SET age = 25 WHERE id = 1; + UPDATE testdb.users SET age = 25 WHERE id = 2; + SELECT SLEEP(10); + UPDATE testdb.users SET age = 20 WHERE id = 1; COMMIT; + SET autocommit=0; """) conn.commit() - bob_conn = _get_conn_for_user('bob') - fred_conn = _get_conn_for_user('fred') + bob_conn = _get_conn_for_user('bob', False) + fred_conn = _get_conn_for_user('fred', False) - executor = concurrent.futures.thread.ThreadPoolExecutor(1) + executor = concurrent.futures.thread.ThreadPoolExecutor(2) # bob's query will block until the TX is completed - executor.submit(run_deadlock_1, bob_conn) + executor.submit(run_first_deadlock_query, bob_conn) # fred's query will get blocked by bob's TX time.sleep(1.5) - executor.submit(run_deadlock_2, fred_conn) + executor.submit(run_second_deadlock_query, fred_conn) dd_run_check(check) bob_conn.close() @@ -575,7 +564,6 @@ def _expected_dbm_job_err_tags(dbm_instance): 'dd.internal.resource:database_instance:stubbed.hostname', ] - def _get_conn_for_user(user, _autocommit=False): return pymysql.connect(host=HOST, port=PORT, user=user, password=user, autocommit=_autocommit) From af655a4cbd86c5daf0be482e4ffa9551bc54c35e Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Tue, 20 Feb 2024 13:10:48 +0000 Subject: [PATCH 06/32] Add a check --- mysql/tests/test_query_activity.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index de2c6660a1601..71543d6fb9b87 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -24,6 +24,9 @@ from .common import CHECK_NAME, HOST, MYSQL_VERSION_PARSED, PORT +#Boris t check if query runs +from . import tags + ACTIVITY_JSON_PLANS_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "activity") @@ -84,32 +87,32 @@ def test_deadlocks(aggregator, dd_run_check, dbm_instance): def run_first_deadlock_query(conn): conn.begin() conn.cursor().execute(""" - SET autocommit=0; START TRANSACTION; - UPDATE testdb.users SET age = 20 WHERE id = 1; + UPDATE testdb.users SET age = 31 WHERE id = 1; SELECT SLEEP(3); - UPDATE testdb.users SET age = 25 WHERE id = 2; + UPDATE testdb.users SET age = 32 WHERE id = 2; COMMIT; - SET autocommit=0; """) conn.commit() def run_second_deadlock_query(conn): conn.begin() conn.cursor().execute(""" - SET autocommit=0; START TRANSACTION; - UPDATE testdb.users SET age = 25 WHERE id = 2; - SELECT SLEEP(10); - UPDATE testdb.users SET age = 20 WHERE id = 1; + UPDATE testdb.users SET age = 32 WHERE id = 2; + SELECT SLEEP(3); + UPDATE testdb.users SET age = 31 WHERE id = 1; COMMIT; - SET autocommit=0; """) conn.commit() + bob_conn = _get_conn_for_user('bob', False) fred_conn = _get_conn_for_user('fred', False) + autocom1 = bob_conn.get_autocommit() + print(autocom1) + executor = concurrent.futures.thread.ThreadPoolExecutor(2) # bob's query will block until the TX is completed executor.submit(run_first_deadlock_query, bob_conn) @@ -133,6 +136,9 @@ def run_second_deadlock_query(conn): for_debug = 0 #assert + dd_run_check(check) + aggregator.assert_metric('alice.age', value=31, tags=tags.METRIC_TAGS_WITH_RESOURCE) + aggregator.assert_metric('bob.age', value=32, tags=tags.METRIC_TAGS_WITH_RESOURCE) assert for_debug - deadlocks_start == 1, "there should be one deadlock" From 9f04c816bf988bd0b9b3c7e1f5df4e40ae641f73 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Tue, 20 Feb 2024 13:25:43 +0000 Subject: [PATCH 07/32] Add wait to let update db threads to finish --- mysql/tests/test_query_activity.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index 71543d6fb9b87..c6714418d3f1a 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -93,6 +93,9 @@ def run_first_deadlock_query(conn): UPDATE testdb.users SET age = 32 WHERE id = 2; COMMIT; """) + results = dict(conn.cursor().fetchall()) + print(results) + conn.commit() def run_second_deadlock_query(conn): @@ -119,7 +122,7 @@ def run_second_deadlock_query(conn): # fred's query will get blocked by bob's TX time.sleep(1.5) executor.submit(run_second_deadlock_query, fred_conn) - + time.sleep(10) dd_run_check(check) bob_conn.close() fred_conn.close() @@ -137,8 +140,8 @@ def run_second_deadlock_query(conn): #assert dd_run_check(check) - aggregator.assert_metric('alice.age', value=31, tags=tags.METRIC_TAGS_WITH_RESOURCE) - aggregator.assert_metric('bob.age', value=32, tags=tags.METRIC_TAGS_WITH_RESOURCE) + aggregator.assert_metric('alice.age', value=31) + aggregator.assert_metric('bob.age', value=32) assert for_debug - deadlocks_start == 1, "there should be one deadlock" From c87e65c55b5f7b2e18863422c8ddc51bf26ae532 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Tue, 20 Feb 2024 13:28:05 +0000 Subject: [PATCH 08/32] Add select query --- mysql/tests/test_query_activity.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index c6714418d3f1a..cd98780824764 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -89,10 +89,15 @@ def run_first_deadlock_query(conn): conn.cursor().execute(""" START TRANSACTION; UPDATE testdb.users SET age = 31 WHERE id = 1; - SELECT SLEEP(3); + SELECT SLEEP(0); UPDATE testdb.users SET age = 32 WHERE id = 2; COMMIT; """) + conn.cursor().execute(""" + START TRANSACTION; + SELECT age FROM testdb.users WHERE id = 1; + COMMIT; + """) results = dict(conn.cursor().fetchall()) print(results) @@ -103,7 +108,7 @@ def run_second_deadlock_query(conn): conn.cursor().execute(""" START TRANSACTION; UPDATE testdb.users SET age = 32 WHERE id = 2; - SELECT SLEEP(3); + SELECT SLEEP(0); UPDATE testdb.users SET age = 31 WHERE id = 1; COMMIT; """) From e1c2b7a70a283f330cc8afd6fdbcd5263679c0ef Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Tue, 20 Feb 2024 20:23:56 +0000 Subject: [PATCH 09/32] fix query --- mysql/datadog_checks/mysql/queries.py | 2 +- mysql/tests/conftest.py | 12 +++++ mysql/tests/test_query_activity.py | 69 ++++++++++++++------------- 3 files changed, 48 insertions(+), 35 deletions(-) diff --git a/mysql/datadog_checks/mysql/queries.py b/mysql/datadog_checks/mysql/queries.py index 08220897082e2..1665455aabfdc 100644 --- a/mysql/datadog_checks/mysql/queries.py +++ b/mysql/datadog_checks/mysql/queries.py @@ -78,7 +78,7 @@ 'name': 'information_schema.INNODB_METRICS.lock_deadlocks', 'query': """ SELECT - COUNT(*) as deadlocks + count as deadlocks FROM information_schema.INNODB_METRICS WHERE diff --git a/mysql/tests/conftest.py b/mysql/tests/conftest.py index ea644095f970e..651007a9494ca 100644 --- a/mysql/tests/conftest.py +++ b/mysql/tests/conftest.py @@ -430,6 +430,18 @@ def populate_database(): cur.execute("GRANT SELECT, UPDATE ON testdb.transactions TO 'bob'@'%';") cur.execute("GRANT SELECT, UPDATE ON testdb.transactions TO 'fred'@'%';") cur.close() + cur = conn.cursor() + cur.execute("UPDATE testdb.users SET age = 31 WHERE id = 1;") + cur.close() + cur = conn.cursor() + + try: + cur.execute("SELECT age FROM testdb.users WHERE id = 1;") + except Exception as e: + + print("Error occurred:", e) + results = dict(cur.fetchall()) + print(results) _create_explain_procedure(conn, "testdb") diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index cd98780824764..5421aaca054cf 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -57,13 +57,6 @@ def test_deadlocks(aggregator, dd_run_check, dbm_instance): #set up check = MySql(CHECK_NAME, {}, [dbm_instance]) - deadlock_query = """ - SELECT - COUNT(*) as deadlocks - FROM - information_schema.INNODB_METRICS - WHERE - NAME='lock_deadlocks'""" dd_run_check(check) deadlocks_start = 0 deadlock_metric_start = aggregator.metrics("mysql.innodb.deadlocks") @@ -84,34 +77,38 @@ def test_deadlocks(aggregator, dd_run_check, dbm_instance): #cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(2,'Bob',20);") #Boris do we need to check that current value is 25 and then set it back in a tear down ? #or may be do select for update as it also should be blocking - def run_first_deadlock_query(conn): + def check_if_age_updated(conn): conn.begin() - conn.cursor().execute(""" - START TRANSACTION; - UPDATE testdb.users SET age = 31 WHERE id = 1; - SELECT SLEEP(0); - UPDATE testdb.users SET age = 32 WHERE id = 2; - COMMIT; - """) - conn.cursor().execute(""" - START TRANSACTION; - SELECT age FROM testdb.users WHERE id = 1; - COMMIT; - """) - results = dict(conn.cursor().fetchall()) + cursor = conn.cursor() + try: + cursor.execute("SELECT age FROM testdb.users WHERE id = 1;") + except Exception as e: + print("Error occurred:", e) + results = cursor.fetchall() print(results) conn.commit() - + def run_first_deadlock_query(conn): + conn.begin() + try: + conn.cursor().execute("START TRANSACTION;") + conn.cursor().execute("UPDATE testdb.users SET age = 32 WHERE id = 1;") + conn.cursor().execute("SELECT SLEEP(3);") + conn.cursor().execute("UPDATE testdb.users SET age = 32 WHERE id = 2;") + conn.cursor().execute("COMMIT;") + except Exception as e: + print("Error occurred:", e) + conn.commit() def run_second_deadlock_query(conn): conn.begin() - conn.cursor().execute(""" - START TRANSACTION; - UPDATE testdb.users SET age = 32 WHERE id = 2; - SELECT SLEEP(0); - UPDATE testdb.users SET age = 31 WHERE id = 1; - COMMIT; - """) + try: + conn.cursor().execute("START TRANSACTION;") + conn.cursor().execute("UPDATE testdb.users SET age = 32 WHERE id = 2;") + conn.cursor().execute("SELECT SLEEP(3);") + conn.cursor().execute("UPDATE testdb.users SET age = 32 WHERE id = 1;") + conn.cursor().execute("COMMIT;") + except Exception as e: + print("Error occurred:", e) conn.commit() @@ -124,10 +121,15 @@ def run_second_deadlock_query(conn): executor = concurrent.futures.thread.ThreadPoolExecutor(2) # bob's query will block until the TX is completed executor.submit(run_first_deadlock_query, bob_conn) - # fred's query will get blocked by bob's TX + time.sleep(1.5) executor.submit(run_second_deadlock_query, fred_conn) - time.sleep(10) + time.sleep(8) + + + + + #check_if_age_updated(bob_conn) dd_run_check(check) bob_conn.close() fred_conn.close() @@ -144,9 +146,8 @@ def run_second_deadlock_query(conn): for_debug = 0 #assert - dd_run_check(check) - aggregator.assert_metric('alice.age', value=31) - aggregator.assert_metric('bob.age', value=32) + + assert for_debug - deadlocks_start == 1, "there should be one deadlock" From 6b1eb22b25d6d7c4c66398ffb813648f30c36dec Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Wed, 21 Feb 2024 13:39:43 +0000 Subject: [PATCH 10/32] revert changes to conftest --- mysql/tests/conftest.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/mysql/tests/conftest.py b/mysql/tests/conftest.py index 651007a9494ca..ea644095f970e 100644 --- a/mysql/tests/conftest.py +++ b/mysql/tests/conftest.py @@ -430,18 +430,6 @@ def populate_database(): cur.execute("GRANT SELECT, UPDATE ON testdb.transactions TO 'bob'@'%';") cur.execute("GRANT SELECT, UPDATE ON testdb.transactions TO 'fred'@'%';") cur.close() - cur = conn.cursor() - cur.execute("UPDATE testdb.users SET age = 31 WHERE id = 1;") - cur.close() - cur = conn.cursor() - - try: - cur.execute("SELECT age FROM testdb.users WHERE id = 1;") - except Exception as e: - - print("Error occurred:", e) - results = dict(cur.fetchall()) - print(results) _create_explain_procedure(conn, "testdb") From 0c6b2309027abbdf7b70626e2e212a4e73c9e5dd Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Wed, 21 Feb 2024 15:42:53 +0000 Subject: [PATCH 11/32] fix test --- mysql/tests/test_query_activity.py | 47 ++++-------------------------- 1 file changed, 5 insertions(+), 42 deletions(-) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index 5421aaca054cf..d2361a3df7b25 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -51,6 +51,8 @@ def dbm_instance(instance_complex): instance_complex['collect_settings'] = {'enabled': False} return copy(instance_complex) +#Boris exclude MariaDB +#make test shorter mark as flacky ? @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') def test_deadlocks(aggregator, dd_run_check, dbm_instance): @@ -63,31 +65,7 @@ def test_deadlocks(aggregator, dd_run_check, dbm_instance): if len(deadlock_metric_start)>0: deadlocks_start = deadlock_metric_start[0].value - #act - - #Justin suggested to change timeout instead of using a long sleep - # Iguess his idea is to change timeout on how long it ccan wait on asquiring a lock. - #Boris makes sense like in Nenands example make sure that there were no deadlocks before the test - #BORIS also simplify deadlock thing can be like in Nenads example - # https://github.com/DataDog/integrations-core/pull/13374/files#diff-369b313fc4346aa906cef597e161569db8e0c52c876dea0a7fe90cd619a0da62 - - - # cur.execute("CREATE TABLE testdb.users (id INT NOT NULL UNIQUE KEY, name VARCHAR(20), age INT);") - #cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(1,'Alice',25);") - #cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(2,'Bob',20);") - #Boris do we need to check that current value is 25 and then set it back in a tear down ? - #or may be do select for update as it also should be blocking - def check_if_age_updated(conn): - conn.begin() - cursor = conn.cursor() - try: - cursor.execute("SELECT age FROM testdb.users WHERE id = 1;") - except Exception as e: - print("Error occurred:", e) - results = cursor.fetchall() - print(results) - - conn.commit() +#refacrtor these 2 functions def run_first_deadlock_query(conn): conn.begin() try: @@ -115,39 +93,24 @@ def run_second_deadlock_query(conn): bob_conn = _get_conn_for_user('bob', False) fred_conn = _get_conn_for_user('fred', False) - autocom1 = bob_conn.get_autocommit() - print(autocom1) - executor = concurrent.futures.thread.ThreadPoolExecutor(2) - # bob's query will block until the TX is completed executor.submit(run_first_deadlock_query, bob_conn) - time.sleep(1.5) executor.submit(run_second_deadlock_query, fred_conn) time.sleep(8) - - - - #check_if_age_updated(bob_conn) - dd_run_check(check) bob_conn.close() fred_conn.close() + dd_run_check(check) executor.shutdown() - # get result of my query in aggregator and assert - # dbm_activity = aggregator.get_event_platform_events("dbm-activity") - deadlock_metric = aggregator.metrics("mysql.innodb.deadlocks") if len(deadlock_metric)>0: - for_debug = deadlock_metric[0].value + for_debug = deadlock_metric[1].value else: for_debug = 0 - #assert - - assert for_debug - deadlocks_start == 1, "there should be one deadlock" From 478063b52080d755e3ab9e6852691720d4ab65a1 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Wed, 21 Feb 2024 18:07:20 +0000 Subject: [PATCH 12/32] Add events --- mysql/tests/test_query_activity.py | 74 +++++++++++++++++------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index d2361a3df7b25..bf9ee01d0c807 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -24,8 +24,8 @@ from .common import CHECK_NAME, HOST, MYSQL_VERSION_PARSED, PORT -#Boris t check if query runs -from . import tags +#Boris not sure if needed +import threading ACTIVITY_JSON_PLANS_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "activity") @@ -52,66 +52,74 @@ def dbm_instance(instance_complex): return copy(instance_complex) #Boris exclude MariaDB -#make test shorter mark as flacky ? +#mark as flacky(Nenand's suggestion) ? +#Boris test on different versions of MySQL @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') def test_deadlocks(aggregator, dd_run_check, dbm_instance): - #set up - check = MySql(CHECK_NAME, {}, [dbm_instance]) dd_run_check(check) deadlocks_start = 0 deadlock_metric_start = aggregator.metrics("mysql.innodb.deadlocks") - if len(deadlock_metric_start)>0: - deadlocks_start = deadlock_metric_start[0].value + assert len(deadlock_metric_start) == 1, "there should be one deadlock metric" + deadlocks_start = deadlock_metric_start[0].value + + first_query = "SELECT * FROM testdb.users WHERE id = 1 FOR UPDATE;" + second_query = "SELECT * FROM testdb.users WHERE id = 2 FOR UPDATE;" -#refacrtor these 2 functions - def run_first_deadlock_query(conn): + def run_first_deadlock_query(conn, event1, event2): conn.begin() + #Boris change for with ? if exception is caught in this thread it doesnt matter + # but better for debugging try: conn.cursor().execute("START TRANSACTION;") - conn.cursor().execute("UPDATE testdb.users SET age = 32 WHERE id = 1;") - conn.cursor().execute("SELECT SLEEP(3);") - conn.cursor().execute("UPDATE testdb.users SET age = 32 WHERE id = 2;") + conn.cursor().execute(first_query) + event1.set() + event2.wait() + conn.cursor().execute(second_query) conn.cursor().execute("COMMIT;") except Exception as e: - print("Error occurred:", e) - conn.commit() - def run_second_deadlock_query(conn): + #Exception is expected due to a deadlock + pass + conn.commit() + + def run_second_deadlock_query(conn, event1, event2): conn.begin() try: + event1.wait() conn.cursor().execute("START TRANSACTION;") - conn.cursor().execute("UPDATE testdb.users SET age = 32 WHERE id = 2;") - conn.cursor().execute("SELECT SLEEP(3);") - conn.cursor().execute("UPDATE testdb.users SET age = 32 WHERE id = 1;") + conn.cursor().execute(second_query) + event2.set() + conn.cursor().execute(first_query) conn.cursor().execute("COMMIT;") except Exception as e: - print("Error occurred:", e) - conn.commit() + #Exception is expected due to a deadlock + pass + conn.commit() - - bob_conn = _get_conn_for_user('bob', False) - fred_conn = _get_conn_for_user('fred', False) + bob_conn = _get_conn_for_user('bob') + fred_conn = _get_conn_for_user('fred') executor = concurrent.futures.thread.ThreadPoolExecutor(2) - executor.submit(run_first_deadlock_query, bob_conn) - time.sleep(1.5) - executor.submit(run_second_deadlock_query, fred_conn) - time.sleep(8) + + event1 = threading.Event() + event2 = threading.Event() + + executor.submit(run_first_deadlock_query, bob_conn, event1, event2) + executor.submit(run_second_deadlock_query, fred_conn, event1, event2) + #Boris give some time for DB to update or superfluous ? + time.sleep(0.5) bob_conn.close() fred_conn.close() + dd_run_check(check) executor.shutdown() - deadlock_metric = aggregator.metrics("mysql.innodb.deadlocks") - if len(deadlock_metric)>0: - for_debug = deadlock_metric[1].value - else: - for_debug = 0 + deadlock_metric_end = aggregator.metrics("mysql.innodb.deadlocks") - assert for_debug - deadlocks_start == 1, "there should be one deadlock" + assert len(deadlock_metric_end) == 2 and deadlock_metric_end[1].value - deadlocks_start == 1, "there should be one new deadlock" @pytest.mark.integration From 03efd98e77da27ca7bd4ca12e89ea637d1177976 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Wed, 21 Feb 2024 18:21:36 +0000 Subject: [PATCH 13/32] remove obsolete diff --- mysql/datadog_checks/mysql/mysql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index d5ed444ef1902..6a75d35e36d5b 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -386,7 +386,7 @@ def _get_connection_args(self): connection_args = { 'ssl': ssl, 'connect_timeout': self._config.connect_timeout, - 'autocommit': False, + 'autocommit': True, } if self._config.charset: connection_args['charset'] = self._config.charset From deeddbb13fff2e3f71189c3230adabad11b2b618 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 09:56:46 +0000 Subject: [PATCH 14/32] cleaned up pr --- mysql/datadog_checks/mysql/mysql.py | 6 +---- mysql/datadog_checks/mysql/queries.py | 3 --- mysql/tests/conftest.py | 12 --------- mysql/tests/test_query_activity.py | 38 +++++++++++++-------------- mysql/tests/variables.py | 2 +- 5 files changed, 20 insertions(+), 41 deletions(-) diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index 6a75d35e36d5b..1461ec0aae7da 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -295,13 +295,10 @@ def check(self, _): self.check_userstat_enabled(db) # Metric collection - #lets assume my query is a metric too if not self._config.only_custom_queries: self._collect_metrics(db, tags=tags) self._collect_system_metrics(self._config.host, db, tags) if self.runtime_queries: - print("will execute runtime queries") - #BORIS my query is it a runtime query that is only run when the nofig hasnt only_custom_queries set to true self.runtime_queries.execute(extra_tags=tags) if self._config.dbm_enabled: @@ -347,8 +344,7 @@ def runtime_queries(self): if self._runtime_queries: return self._runtime_queries - queries = [] - queries.extend([QUERY_DEADLOCKS]) + queries = [QUERY_DEADLOCKS] if self.performance_schema_enabled: queries.extend([QUERY_USER_CONNECTIONS]) diff --git a/mysql/datadog_checks/mysql/queries.py b/mysql/datadog_checks/mysql/queries.py index 1665455aabfdc..68aa8b884018a 100644 --- a/mysql/datadog_checks/mysql/queries.py +++ b/mysql/datadog_checks/mysql/queries.py @@ -85,9 +85,6 @@ NAME='lock_deadlocks' """.strip(), 'columns': [{'name': 'mysql.innodb.deadlocks', 'type': 'gauge'}], - #Boris name ? - #Boris name ? - } QUERY_USER_CONNECTIONS = { diff --git a/mysql/tests/conftest.py b/mysql/tests/conftest.py index ea644095f970e..49cc1b954e60b 100644 --- a/mysql/tests/conftest.py +++ b/mysql/tests/conftest.py @@ -418,18 +418,6 @@ def populate_database(): cur.execute("GRANT SELECT, UPDATE ON testdb.users TO 'bob'@'%';") cur.execute("GRANT SELECT, UPDATE ON testdb.users TO 'fred'@'%';") cur.close() - #Boris later come up with a better way to do this probably - # deadlock can be created just with testdb.users table - cur = conn.cursor() - cur.execute("CREATE TABLE testdb.accounts (id INT PRIMARY KEY, balance INT);") - cur.execute("CREATE TABLE transactions (id INT PRIMARY KEY,from_account INT,to_account INT,amount INT);") - cur.execute("INSERT INTO testdb.accounts (id,balance) VALUES(1,1000);") - cur.execute("INSERT INTO testdb.accounts (id,balance) VALUES(2,1000);") - cur.execute("GRANT SELECT, UPDATE ON testdb.accounts TO 'bob'@'%';") - cur.execute("GRANT SELECT, UPDATE ON testdb.accounts TO 'fred'@'%';") - cur.execute("GRANT SELECT, UPDATE ON testdb.transactions TO 'bob'@'%';") - cur.execute("GRANT SELECT, UPDATE ON testdb.transactions TO 'fred'@'%';") - cur.close() _create_explain_procedure(conn, "testdb") diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index bf9ee01d0c807..75f45f953e5a8 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -11,6 +11,7 @@ from copy import copy from datetime import datetime from os import environ +from threading import Event import mock import pymysql @@ -24,12 +25,8 @@ from .common import CHECK_NAME, HOST, MYSQL_VERSION_PARSED, PORT -#Boris not sure if needed -import threading - ACTIVITY_JSON_PLANS_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "activity") - @pytest.fixture(autouse=True) def stop_orphaned_threads(): # make sure we shut down any orphaned threads and create a new Executor for each test @@ -51,9 +48,6 @@ def dbm_instance(instance_complex): instance_complex['collect_settings'] = {'enabled': False} return copy(instance_complex) -#Boris exclude MariaDB -#mark as flacky(Nenand's suggestion) ? -#Boris test on different versions of MySQL @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') def test_deadlocks(aggregator, dd_run_check, dbm_instance): @@ -61,7 +55,13 @@ def test_deadlocks(aggregator, dd_run_check, dbm_instance): dd_run_check(check) deadlocks_start = 0 deadlock_metric_start = aggregator.metrics("mysql.innodb.deadlocks") + assert len(deadlock_metric_start) == 1, "there should be one deadlock metric" + + if environ.get('MYSQL_FLAVOR') == 'mariadb': + #Skip the test below as count is not updated in MariaDB + return + deadlocks_start = deadlock_metric_start[0].value first_query = "SELECT * FROM testdb.users WHERE id = 1 FOR UPDATE;" @@ -69,8 +69,6 @@ def test_deadlocks(aggregator, dd_run_check, dbm_instance): def run_first_deadlock_query(conn, event1, event2): conn.begin() - #Boris change for with ? if exception is caught in this thread it doesnt matter - # but better for debugging try: conn.cursor().execute("START TRANSACTION;") conn.cursor().execute(first_query) @@ -102,21 +100,21 @@ def run_second_deadlock_query(conn, event1, event2): executor = concurrent.futures.thread.ThreadPoolExecutor(2) - event1 = threading.Event() - event2 = threading.Event() - - executor.submit(run_first_deadlock_query, bob_conn, event1, event2) - executor.submit(run_second_deadlock_query, fred_conn, event1, event2) - #Boris give some time for DB to update or superfluous ? - time.sleep(0.5) - + event1 = Event() + event2 = Event() + + futures_first_query = executor.submit(run_first_deadlock_query, bob_conn, event1, event2) + futures_second_query = executor.submit(run_second_deadlock_query, fred_conn, event1, event2) + futures_first_query.result() + futures_second_query.result() + # Make sure innodb is updated. + time.sleep(0.3) bob_conn.close() fred_conn.close() + executor.shutdown() dd_run_check(check) - executor.shutdown() - deadlock_metric_end = aggregator.metrics("mysql.innodb.deadlocks") assert len(deadlock_metric_end) == 2 and deadlock_metric_end[1].value - deadlocks_start == 1, "there should be one new deadlock" @@ -229,7 +227,7 @@ def test_activity_reported_hostname(aggregator, dbm_instance, dd_run_check, repo dbm_instance['reported_hostname'] = reported_hostname check = MySql(CHECK_NAME, {}, [dbm_instance]) - dd_run_check(check)#Boris why 2 times dd_run_check? + dd_run_check(check) dd_run_check(check) dbm_activity = aggregator.get_event_platform_events("dbm-activity") diff --git a/mysql/tests/variables.py b/mysql/tests/variables.py index 00d8b19d95f87..22f3b854ddaf7 100644 --- a/mysql/tests/variables.py +++ b/mysql/tests/variables.py @@ -90,7 +90,7 @@ 'mysql.innodb.buffer_pool_read_requests', 'mysql.innodb.buffer_pool_reads', 'mysql.innodb.buffer_pool_utilization', - 'mysql.innodb.deadlocks', #Boris may be was a bad name + 'mysql.innodb.deadlocks', ] COMPLEX_INNODB_VARS = [ From dc5693dad015f1bdb268d26c0c0e3d5cb93607ae Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 13:06:13 +0000 Subject: [PATCH 15/32] moved tests to the end --- mysql/tests/test_query_activity.py | 143 ++++++++++++++--------------- 1 file changed, 71 insertions(+), 72 deletions(-) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index 75f45f953e5a8..fbbc42e6d5995 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -48,78 +48,6 @@ def dbm_instance(instance_complex): instance_complex['collect_settings'] = {'enabled': False} return copy(instance_complex) -@pytest.mark.integration -@pytest.mark.usefixtures('dd_environment') -def test_deadlocks(aggregator, dd_run_check, dbm_instance): - check = MySql(CHECK_NAME, {}, [dbm_instance]) - dd_run_check(check) - deadlocks_start = 0 - deadlock_metric_start = aggregator.metrics("mysql.innodb.deadlocks") - - assert len(deadlock_metric_start) == 1, "there should be one deadlock metric" - - if environ.get('MYSQL_FLAVOR') == 'mariadb': - #Skip the test below as count is not updated in MariaDB - return - - deadlocks_start = deadlock_metric_start[0].value - - first_query = "SELECT * FROM testdb.users WHERE id = 1 FOR UPDATE;" - second_query = "SELECT * FROM testdb.users WHERE id = 2 FOR UPDATE;" - - def run_first_deadlock_query(conn, event1, event2): - conn.begin() - try: - conn.cursor().execute("START TRANSACTION;") - conn.cursor().execute(first_query) - event1.set() - event2.wait() - conn.cursor().execute(second_query) - conn.cursor().execute("COMMIT;") - except Exception as e: - #Exception is expected due to a deadlock - pass - conn.commit() - - def run_second_deadlock_query(conn, event1, event2): - conn.begin() - try: - event1.wait() - conn.cursor().execute("START TRANSACTION;") - conn.cursor().execute(second_query) - event2.set() - conn.cursor().execute(first_query) - conn.cursor().execute("COMMIT;") - except Exception as e: - #Exception is expected due to a deadlock - pass - conn.commit() - - bob_conn = _get_conn_for_user('bob') - fred_conn = _get_conn_for_user('fred') - - executor = concurrent.futures.thread.ThreadPoolExecutor(2) - - event1 = Event() - event2 = Event() - - futures_first_query = executor.submit(run_first_deadlock_query, bob_conn, event1, event2) - futures_second_query = executor.submit(run_second_deadlock_query, fred_conn, event1, event2) - futures_first_query.result() - futures_second_query.result() - # Make sure innodb is updated. - time.sleep(0.3) - bob_conn.close() - fred_conn.close() - executor.shutdown() - - dd_run_check(check) - - deadlock_metric_end = aggregator.metrics("mysql.innodb.deadlocks") - - assert len(deadlock_metric_end) == 2 and deadlock_metric_end[1].value - deadlocks_start == 1, "there should be one new deadlock" - - @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') @pytest.mark.parametrize( @@ -548,6 +476,77 @@ def _expected_dbm_job_err_tags(dbm_instance): 'dd.internal.resource:database_instance:stubbed.hostname', ] +@pytest.mark.integration +@pytest.mark.usefixtures('dd_environment') +def test_deadlocks(aggregator, dd_run_check, dbm_instance): + check = MySql(CHECK_NAME, {}, [dbm_instance]) + dd_run_check(check) + deadlocks_start = 0 + deadlock_metric_start = aggregator.metrics("mysql.innodb.deadlocks") + + assert len(deadlock_metric_start) == 1, "there should be one deadlock metric" + + if environ.get('MYSQL_FLAVOR') == 'mariadb': + #Skip the test below as deadlock count is not updated in MariaDB + return + + deadlocks_start = deadlock_metric_start[0].value + + first_query = "SELECT * FROM testdb.users WHERE id = 1 FOR UPDATE;" + second_query = "SELECT * FROM testdb.users WHERE id = 2 FOR UPDATE;" + + def run_first_deadlock_query(conn, event1, event2): + conn.begin() + try: + conn.cursor().execute("START TRANSACTION;") + conn.cursor().execute(first_query) + event1.set() + event2.wait() + conn.cursor().execute(second_query) + conn.cursor().execute("COMMIT;") + except Exception as e: + #Exception is expected due to a deadlock + pass + conn.commit() + + def run_second_deadlock_query(conn, event1, event2): + conn.begin() + try: + event1.wait() + conn.cursor().execute("START TRANSACTION;") + conn.cursor().execute(second_query) + event2.set() + conn.cursor().execute(first_query) + conn.cursor().execute("COMMIT;") + except Exception as e: + #Exception is expected due to a deadlock + pass + conn.commit() + + bob_conn = _get_conn_for_user('bob') + fred_conn = _get_conn_for_user('fred') + + executor = concurrent.futures.thread.ThreadPoolExecutor(2) + + event1 = Event() + event2 = Event() + + futures_first_query = executor.submit(run_first_deadlock_query, bob_conn, event1, event2) + futures_second_query = executor.submit(run_second_deadlock_query, fred_conn, event1, event2) + futures_first_query.result() + futures_second_query.result() + # Make sure innodb is updated. + time.sleep(0.3) + bob_conn.close() + fred_conn.close() + executor.shutdown() + + dd_run_check(check) + + deadlock_metric_end = aggregator.metrics("mysql.innodb.deadlocks") + + assert len(deadlock_metric_end) == 2 and deadlock_metric_end[1].value - deadlocks_start == 1, "there should be one new deadlock" + def _get_conn_for_user(user, _autocommit=False): return pymysql.connect(host=HOST, port=PORT, user=user, password=user, autocommit=_autocommit) From 6c7dc2e6bdb826679f309d6c3f00d6649ec13e31 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 13:08:46 +0000 Subject: [PATCH 16/32] Put back empty lines --- mysql/tests/test_query_activity.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index fbbc42e6d5995..b3efd4f2cafc4 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -27,6 +27,7 @@ ACTIVITY_JSON_PLANS_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "activity") + @pytest.fixture(autouse=True) def stop_orphaned_threads(): # make sure we shut down any orphaned threads and create a new Executor for each test @@ -48,6 +49,7 @@ def dbm_instance(instance_complex): instance_complex['collect_settings'] = {'enabled': False} return copy(instance_complex) + @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') @pytest.mark.parametrize( From 1492844b19fefc917c6121d2eb5ff0fcc9735340 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 13:21:32 +0000 Subject: [PATCH 17/32] put back empty line --- mysql/datadog_checks/mysql/mysql.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index 1461ec0aae7da..40be19e0c5b9f 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -345,6 +345,7 @@ def runtime_queries(self): return self._runtime_queries queries = [QUERY_DEADLOCKS] + if self.performance_schema_enabled: queries.extend([QUERY_USER_CONNECTIONS]) From 7441ad0558138f9ace2a0e8ff9cf6ceb5ff03e3e Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 13:36:03 +0000 Subject: [PATCH 18/32] added a changelog --- datadog_checks_base/changelog.d/16900.added | 1 + 1 file changed, 1 insertion(+) create mode 100644 datadog_checks_base/changelog.d/16900.added diff --git a/datadog_checks_base/changelog.d/16900.added b/datadog_checks_base/changelog.d/16900.added new file mode 100644 index 0000000000000..0efc9f0e9a8c8 --- /dev/null +++ b/datadog_checks_base/changelog.d/16900.added @@ -0,0 +1 @@ +[DBMON-2051] collect deadlock metric for MySQL From edd227cc65d9d5b6794eddde8fef3da78bddb07e Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 13:43:18 +0000 Subject: [PATCH 19/32] changed pr number in changelog --- datadog_checks_base/changelog.d/{16900.added => 16904.added} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename datadog_checks_base/changelog.d/{16900.added => 16904.added} (100%) diff --git a/datadog_checks_base/changelog.d/16900.added b/datadog_checks_base/changelog.d/16904.added similarity index 100% rename from datadog_checks_base/changelog.d/16900.added rename to datadog_checks_base/changelog.d/16904.added From 8abc8bf6d2376a7029bec95d6d836da2ae9f4828 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 13:45:47 +0000 Subject: [PATCH 20/32] removed patched config --- mysql/tests/conftest_patched.py | 478 -------------------------------- 1 file changed, 478 deletions(-) delete mode 100644 mysql/tests/conftest_patched.py diff --git a/mysql/tests/conftest_patched.py b/mysql/tests/conftest_patched.py deleted file mode 100644 index 49cc1b954e60b..0000000000000 --- a/mysql/tests/conftest_patched.py +++ /dev/null @@ -1,478 +0,0 @@ -# (C) Datadog, Inc. 2018-present -# All rights reserved -# Licensed under a 3-clause BSD style license (see LICENSE) -import copy -import logging -import os - -import mock -import pymysql -import pytest - -from datadog_checks.dev import TempDir, WaitFor, docker_run -from datadog_checks.dev.conditions import CheckDockerLogs - -from . import common, tags -from .common import MYSQL_REPLICATION - -logger = logging.getLogger(__name__) - -MYSQL_FLAVOR = os.getenv('MYSQL_FLAVOR') -MYSQL_VERSION = os.getenv('MYSQL_VERSION') -COMPOSE_FILE = os.getenv('COMPOSE_FILE') - - -@pytest.fixture(scope='session') -def config_e2e(instance_basic): - pass - -@pytest.fixture(scope='session') -def dd_environment(config_e2e): - pass - - -@pytest.fixture(scope='session') -def instance_basic(): - return { - 'host': common.HOST, - 'username': common.USER, - 'password': common.PASS, - 'port': common.PORT, - 'disable_generic_tags': 'true', - } - - -@pytest.fixture -def instance_complex(): - return { - 'host': common.HOST, - 'username': common.USER, - 'password': common.PASS, - 'port': common.PORT, - 'disable_generic_tags': 'true', - 'options': { - 'replication': True, - 'extra_status_metrics': True, - 'extra_innodb_metrics': True, - 'extra_performance_metrics': True, - 'schema_size_metrics': True, - 'table_size_metrics': True, - 'system_table_size_metrics': True, - 'table_row_stats_metrics': True, - }, - 'tags': tags.METRIC_TAGS, - 'queries': [ - { - 'query': "SELECT * from testdb.users where name='Alice' limit 1;", - 'metric': 'alice.age', - 'type': 'gauge', - 'field': 'age', - }, - { - 'query': "SELECT * from testdb.users where name='Bob' limit 1;", - 'metric': 'bob.age', - 'type': 'gauge', - 'field': 'age', - }, - ], - } - - -@pytest.fixture -def instance_additional_status(): - return { - 'host': common.HOST, - 'username': common.USER, - 'password': common.PASS, - 'port': common.PORT, - 'tags': tags.METRIC_TAGS, - 'disable_generic_tags': 'true', - 'additional_status': [ - { - 'name': "Innodb_rows_read", - 'metric_name': "mysql.innodb.rows_read", - 'type': "rate", - }, - { - 'name': "Innodb_row_lock_time", - 'metric_name': "mysql.innodb.row_lock_time", - 'type': "rate", - }, - ], - } - - -@pytest.fixture -def instance_additional_variable(): - return { - 'host': common.HOST, - 'username': common.USER, - 'password': common.PASS, - 'port': common.PORT, - 'tags': tags.METRIC_TAGS, - 'disable_generic_tags': 'true', - 'additional_variable': [ - { - 'name': "long_query_time", - 'metric_name': "mysql.performance.long_query_time", - 'type': "gauge", - }, - { - 'name': "innodb_flush_log_at_trx_commit", - 'metric_name': "mysql.performance.innodb_flush_log_at_trx_commit", - 'type': "gauge", - }, - ], - } - - -@pytest.fixture -def instance_status_already_queried(): - return { - 'host': common.HOST, - 'username': common.USER, - 'password': common.PASS, - 'port': common.PORT, - 'tags': tags.METRIC_TAGS, - 'disable_generic_tags': 'true', - 'additional_status': [ - { - 'name': "Open_files", - 'metric_name': "mysql.performance.open_files_test", - 'type': "gauge", - }, - ], - } - - -@pytest.fixture -def instance_var_already_queried(): - return { - 'host': common.HOST, - 'username': common.USER, - 'password': common.PASS, - 'port': common.PORT, - 'tags': tags.METRIC_TAGS, - 'disable_generic_tags': 'true', - 'additional_variable': [ - { - 'name': "Key_buffer_size", - 'metric_name': "mysql.myisam.key_buffer_size", - 'type': "gauge", - }, - ], - } - - -@pytest.fixture -def instance_invalid_var(): - return { - 'host': common.HOST, - 'username': common.USER, - 'password': common.PASS, - 'port': common.PORT, - 'tags': tags.METRIC_TAGS, - 'disable_generic_tags': 'true', - 'additional_status': [ - { - 'name': "longer_query_time", - 'metric_name': "mysql.performance.longer_query_time", - 'type': "gauge", - }, - { - 'name': "innodb_flush_log_at_trx_commit", - 'metric_name': "mysql.performance.innodb_flush_log_at_trx_commit", - 'type': "gauge", - }, - ], - } - - -@pytest.fixture -def instance_custom_queries(): - return { - 'host': common.HOST, - 'username': common.USER, - 'password': common.PASS, - 'port': common.PORT, - 'tags': tags.METRIC_TAGS, - 'disable_generic_tags': 'true', - 'custom_queries': [ - { - 'query': "SELECT name,age from testdb.users where name='Alice' limit 1;", - 'columns': [{}, {'name': 'alice.age', 'type': 'gauge'}], - }, - { - 'query': "SELECT name,age from testdb.users where name='Bob' limit 1;", - 'columns': [{}, {'name': 'bob.age', 'type': 'gauge'}], - }, - ], - } - - -@pytest.fixture(scope='session') -def instance_error(): - return {'host': common.HOST, 'username': 'unknown', 'password': common.PASS, 'disable_generic_tags': 'true'} - - -@pytest.fixture(scope='session') -def version_metadata(): - parts = MYSQL_VERSION.split('-') - version = parts[0].split('.') - major, minor = version[:2] - patch = version[2] if len(version) > 2 else mock.ANY - - flavor = "MariaDB" if MYSQL_FLAVOR == "mariadb" else "MySQL" - - return { - 'version.scheme': 'semver', - 'version.major': major, - 'version.minor': minor, - 'version.patch': patch, - 'version.raw': mock.ANY, - 'version.build': mock.ANY, - 'flavor': flavor, - 'resolved_hostname': 'forced_hostname', - } - - -def _get_warmup_conditions(): - if MYSQL_REPLICATION == 'group': - return [ - CheckDockerLogs('node1', "X Plugin ready for connections. Bind-address: '::' port: 33060"), - CheckDockerLogs('node2', "X Plugin ready for connections. Bind-address: '::' port: 33060"), - CheckDockerLogs('node3', "X Plugin ready for connections. Bind-address: '::' port: 33060"), - init_group_replication, - populate_database, - ] - return [ - WaitFor(init_master, wait=2), - WaitFor(init_slave, wait=2), - CheckDockerLogs('mysql-slave', ["ready for connections", "mariadb successfully initialized"]), - populate_database, - ] - - -def init_group_replication(): - logger.debug("initializing group replication") - import time - - time.sleep(5) - conns = [pymysql.connect(host=common.HOST, port=p, user='root', password='mypass') for p in common.PORTS_GROUP] - _add_dog_user(conns[0]) - _add_bob_user(conns[0]) - _add_fred_user(conns[0]) - _init_datadog_sample_collection(conns[0]) - - cur_primary = conns[0].cursor() - cur_primary.execute("SET @@GLOBAL.group_replication_bootstrap_group=1;") - cur_primary.execute("create user 'repl'@'%';") - cur_primary.execute("GRANT REPLICATION SLAVE ON *.* TO repl@'%';") - cur_primary.execute("flush privileges;") - cur_primary.execute("change master to master_user='root' for channel 'group_replication_recovery';") - cur_primary.execute("START GROUP_REPLICATION;") - cur_primary.execute("SET @@GLOBAL.group_replication_bootstrap_group=0;") - cur_primary.execute("SELECT * FROM performance_schema.replication_group_members;") - - # Node 2 and 3 - for c in conns[1:]: - cur = c.cursor() - cur.execute("change master to master_user='repl' for channel 'group_replication_recovery';") - cur.execute("START GROUP_REPLICATION;") - - -def _init_datadog_sample_collection(conn): - logger.debug("initializing datadog sample collection") - cur = conn.cursor() - cur.execute("CREATE DATABASE datadog") - cur.execute("GRANT CREATE TEMPORARY TABLES ON `datadog`.* TO 'dog'@'%'") - cur.execute("GRANT EXECUTE on `datadog`.* TO 'dog'@'%'") - _create_explain_procedure(conn, "datadog") - _create_explain_procedure(conn, "mysql") - _create_enable_consumers_procedure(conn) - - -def _create_explain_procedure(conn, schema): - logger.debug("creating explain procedure in schema=%s", schema) - cur = conn.cursor() - cur.execute( - """ - CREATE PROCEDURE {schema}.explain_statement(IN query TEXT) - SQL SECURITY DEFINER - BEGIN - SET @explain := CONCAT('EXPLAIN FORMAT=json ', query); - PREPARE stmt FROM @explain; - EXECUTE stmt; - DEALLOCATE PREPARE stmt; - END; - """.format( - schema=schema - ) - ) - if schema != 'datadog': - cur.execute("GRANT EXECUTE ON PROCEDURE {schema}.explain_statement to 'dog'@'%'".format(schema=schema)) - cur.close() - - -def _create_enable_consumers_procedure(conn): - logger.debug("creating enable_events_statements_consumers procedure") - cur = conn.cursor() - cur.execute( - """ - CREATE PROCEDURE datadog.enable_events_statements_consumers() - SQL SECURITY DEFINER - BEGIN - UPDATE performance_schema.setup_consumers SET enabled='YES' WHERE name LIKE 'events_statements_%'; - UPDATE performance_schema.setup_consumers SET enabled='YES' WHERE name = 'events_waits_current'; - END; - """ - ) - cur.close() - - -def init_master(): - logger.debug("initializing master") - conn = pymysql.connect(host=common.HOST, port=common.PORT, user='root') - _add_dog_user(conn) - _add_bob_user(conn) - _add_fred_user(conn) - _init_datadog_sample_collection(conn) - - -@pytest.fixture -def root_conn(): - conn = pymysql.connect( - host=common.HOST, port=common.PORT, user='root', password='mypass' if MYSQL_REPLICATION == 'group' else None - ) - yield conn - conn.close() - - -def init_slave(): - logger.debug("initializing slave") - pymysql.connect(host=common.HOST, port=common.SLAVE_PORT, user=common.USER, passwd=common.PASS) - - -def _add_dog_user(conn): - cur = conn.cursor() - cur.execute("SELECT count(*) FROM mysql.user WHERE User = 'dog' and Host = '%'") - if cur.fetchone()[0] == 0: - # gracefully handle retries due to partial failure later on - cur.execute("CREATE USER 'dog'@'%' IDENTIFIED BY 'dog'") - cur.execute("GRANT PROCESS ON *.* TO 'dog'@'%'") - cur.execute("GRANT REPLICATION CLIENT ON *.* TO 'dog'@'%'") - cur.execute("GRANT SELECT ON performance_schema.* TO 'dog'@'%'") - - # refactor try older mysql.user table first. if this fails, go to newer ALTER USER - try: - cur.execute("UPDATE mysql.user SET max_user_connections = 0 WHERE user='dog' AND host='%'") - cur.execute("FLUSH PRIVILEGES") - # need to get better exception in order to raise errors in the future - except Exception: - if MYSQL_FLAVOR == 'mariadb': - cur.execute("GRANT SLAVE MONITOR ON *.* TO 'dog'@'%'") - cur.execute("ALTER USER 'dog'@'%' WITH MAX_USER_CONNECTIONS 0") - - -def _add_bob_user(conn): - cur = conn.cursor() - cur.execute("CREATE USER 'bob'@'%' IDENTIFIED BY 'bob'") - cur.execute("GRANT USAGE on *.* to 'bob'@'%'") - - -def _add_fred_user(conn): - cur = conn.cursor() - cur.execute("CREATE USER 'fred'@'%' IDENTIFIED BY 'fred'") - cur.execute("GRANT USAGE on *.* to 'fred'@'%'") - - -@pytest.fixture -def bob_conn(): - conn = pymysql.connect(host=common.HOST, port=common.PORT, user='bob', password='bob') - yield conn - conn.close() - - -@pytest.fixture -def fred_conn(): - conn = pymysql.connect(host=common.HOST, port=common.PORT, user='fred', password='fred') - yield conn - conn.close() - - -def populate_database(): - logger.debug("populating database") - conn = pymysql.connect( - host=common.HOST, port=common.PORT, user='root', password='mypass' if MYSQL_REPLICATION == 'group' else None - ) - - cur = conn.cursor() - - cur.execute("USE mysql;") - cur.execute("CREATE DATABASE testdb;") - cur.execute("USE testdb;") - cur.execute("CREATE TABLE testdb.users (id INT NOT NULL UNIQUE KEY, name VARCHAR(20), age INT);") - cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(1,'Alice',25);") - cur.execute("INSERT INTO testdb.users (id,name,age) VALUES(2,'Bob',20);") - cur.execute("GRANT SELECT ON testdb.users TO 'dog'@'%';") - cur.execute("GRANT SELECT, UPDATE ON testdb.users TO 'bob'@'%';") - cur.execute("GRANT SELECT, UPDATE ON testdb.users TO 'fred'@'%';") - cur.close() - _create_explain_procedure(conn, "testdb") - - -def _wait_for_it_script(): - """ - FIXME: relying on the filesystem layout is a bad idea, the testing helper - should expose its path through the api instead - """ - script = os.path.join(common.TESTS_HELPER_DIR, 'scripts', 'wait-for-it.sh') - return os.path.abspath(script) - - -def _mysql_conf_path(): - """ - Return the path to a local MySQL configuration file suited for the current environment. - """ - if MYSQL_FLAVOR == 'mysql': - filename = 'mysql.conf' - elif MYSQL_FLAVOR == 'mariadb': - filename = 'mariadb.conf' - else: - raise ValueError('Unsupported MySQL flavor: {}'.format(MYSQL_FLAVOR)) - - conf = os.path.join(common.HERE, 'compose', filename) - return os.path.abspath(conf) - - -def _mysql_logs_path(): - """ - Return the path to the MySQL logs directory in the MySQL container. - - Should be kept in sync with the log paths set in the local MySQL configuration files - (which don't support interpolation of environment variables). - """ - if MYSQL_FLAVOR == 'mysql': - if MYSQL_VERSION == '8.0' or MYSQL_VERSION == 'latest': - return '/opt/bitnami/mysql/logs' - else: - return '/var/log/mysql' - elif MYSQL_FLAVOR == 'mariadb': - return '/opt/bitnami/mariadb/logs' - else: - raise ValueError('Unsupported MySQL flavor: {}'.format(MYSQL_FLAVOR)) - - -def _mysql_docker_repo(): - if MYSQL_FLAVOR == 'mysql': - # The image for testing Mysql 5.5 is located at `jfullaondo/mysql-replication` or `bergerx/mysql-replication` - # Warning: This image is a bit flaky on CI (it has been removed - # https://github.com/DataDog/integrations-core/pull/4669) - if MYSQL_VERSION in ('5.6', '5.7'): - return 'bergerx/mysql-replication' - elif MYSQL_VERSION == '8.0' or MYSQL_VERSION == 'latest': - return 'bitnami/mysql' - elif MYSQL_FLAVOR == 'mariadb': - return 'bitnami/mariadb' - else: - raise ValueError('Unsupported MySQL flavor: {}'.format(MYSQL_FLAVOR)) From ffd08e10a1475404b7c62cdba5758053eecd4639 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 16:14:17 +0000 Subject: [PATCH 21/32] reverting back conftest --- mysql/tests/conftest.py | 47 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/mysql/tests/conftest.py b/mysql/tests/conftest.py index 49cc1b954e60b..b78d0da59a0d2 100644 --- a/mysql/tests/conftest.py +++ b/mysql/tests/conftest.py @@ -24,11 +24,54 @@ @pytest.fixture(scope='session') def config_e2e(instance_basic): - pass + instance = copy.deepcopy(instance_basic) + instance['dbm'] = True + logs_path = _mysql_logs_path() + + return { + 'init_config': {}, + 'instances': [instance], + 'logs': [ + {'type': 'file', 'path': '{}/mysql.log'.format(logs_path), 'source': 'mysql', 'service': 'local_mysql'}, + { + 'type': 'file', + 'path': '{}/mysql_slow.log'.format(logs_path), + 'source': 'mysql', + 'service': 'local_mysql', + 'log_processing_rules': [ + {'type': 'multi_line', 'name': 'log_starts_with_time', 'pattern': '# Time: '}, + ], + }, + ], + } + @pytest.fixture(scope='session') def dd_environment(config_e2e): - pass + logs_path = _mysql_logs_path() + + with TempDir('logs') as logs_host_path: + # for Ubuntu + os.chmod(logs_host_path, 0o770) + + e2e_metadata = {'docker_volumes': ['{}:{}'.format(logs_host_path, logs_path)]} + + with docker_run( + os.path.join(common.HERE, 'compose', COMPOSE_FILE), + env_vars={ + 'MYSQL_DOCKER_REPO': _mysql_docker_repo(), + 'MYSQL_PORT': str(common.PORT), + 'MYSQL_SLAVE_PORT': str(common.SLAVE_PORT), + 'MYSQL_CONF_PATH': _mysql_conf_path(), + 'MYSQL_LOGS_HOST_PATH': logs_host_path, + 'MYSQL_LOGS_PATH': logs_path, + 'WAIT_FOR_IT_SCRIPT_PATH': _wait_for_it_script(), + }, + conditions=_get_warmup_conditions(), + attempts=2, + attempts_wait=10, + ): + yield config_e2e, e2e_metadata @pytest.fixture(scope='session') From 5c4e7bac32b2b2fc910077ba914458c7b06f017c Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 16:17:58 +0000 Subject: [PATCH 22/32] reverted common py --- mysql/tests/common.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mysql/tests/common.py b/mysql/tests/common.py index 21b13ac9e5034..a2c58554c2e9d 100644 --- a/mysql/tests/common.py +++ b/mysql/tests/common.py @@ -16,7 +16,10 @@ MYSQL_REPLICATION = os.getenv('MYSQL_REPLICATION') MYSQL_VERSION_IS_LATEST = os.getenv('MYSQL_VERSION', '').endswith('latest') -MYSQL_VERSION_PARSED = parse_version("10.5") +if MYSQL_VERSION_IS_LATEST is False: + MYSQL_VERSION_PARSED = parse_version(os.getenv('MYSQL_VERSION', '').split('-')[0]) +else: + MYSQL_VERSION_PARSED = parse_version(str(maxsize)) CHECK_NAME = 'mysql' # adding flavor to differentiate mariadb from mysql From 49582d0816211a1f62c550f5bb688aca3789a982 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 17:14:19 +0000 Subject: [PATCH 23/32] applied linter --- mysql/datadog_checks/mysql/mysql.py | 2 +- mysql/tests/test_query_activity.py | 34 ++++++++++++++++------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index 40be19e0c5b9f..b29aedd604b6f 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -56,8 +56,8 @@ from .innodb_metrics import InnoDBMetrics from .metadata import MySQLMetadata from .queries import ( - QUERY_USER_CONNECTIONS, QUERY_DEADLOCKS, + QUERY_USER_CONNECTIONS, SQL_95TH_PERCENTILE, SQL_AVG_QUERY_RUN_TIME, SQL_GROUP_REPLICATION_MEMBER, diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index b3efd4f2cafc4..dcbac99588279 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -478,20 +478,21 @@ def _expected_dbm_job_err_tags(dbm_instance): 'dd.internal.resource:database_instance:stubbed.hostname', ] + @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') def test_deadlocks(aggregator, dd_run_check, dbm_instance): check = MySql(CHECK_NAME, {}, [dbm_instance]) dd_run_check(check) deadlocks_start = 0 - deadlock_metric_start = aggregator.metrics("mysql.innodb.deadlocks") + deadlock_metric_start = aggregator.metrics("mysql.innodb.deadlocks") assert len(deadlock_metric_start) == 1, "there should be one deadlock metric" if environ.get('MYSQL_FLAVOR') == 'mariadb': - #Skip the test below as deadlock count is not updated in MariaDB + # Skip the test below as deadlock count is not updated in MariaDB return - + deadlocks_start = deadlock_metric_start[0].value first_query = "SELECT * FROM testdb.users WHERE id = 1 FOR UPDATE;" @@ -501,35 +502,35 @@ def run_first_deadlock_query(conn, event1, event2): conn.begin() try: conn.cursor().execute("START TRANSACTION;") - conn.cursor().execute(first_query) + conn.cursor().execute(first_query) event1.set() event2.wait() conn.cursor().execute(second_query) conn.cursor().execute("COMMIT;") - except Exception as e: - #Exception is expected due to a deadlock + except Exception: + # Exception is expected due to a deadlock pass - conn.commit() - + conn.commit() + def run_second_deadlock_query(conn, event1, event2): conn.begin() try: event1.wait() conn.cursor().execute("START TRANSACTION;") - conn.cursor().execute(second_query) + conn.cursor().execute(second_query) event2.set() conn.cursor().execute(first_query) conn.cursor().execute("COMMIT;") - except Exception as e: - #Exception is expected due to a deadlock + except Exception: + # Exception is expected due to a deadlock pass - conn.commit() + conn.commit() bob_conn = _get_conn_for_user('bob') fred_conn = _get_conn_for_user('fred') executor = concurrent.futures.thread.ThreadPoolExecutor(2) - + event1 = Event() event2 = Event() @@ -545,9 +546,12 @@ def run_second_deadlock_query(conn, event1, event2): dd_run_check(check) - deadlock_metric_end = aggregator.metrics("mysql.innodb.deadlocks") + deadlock_metric_end = aggregator.metrics("mysql.innodb.deadlocks") + + assert ( + len(deadlock_metric_end) == 2 and deadlock_metric_end[1].value - deadlocks_start == 1 + ), "there should be one new deadlock" - assert len(deadlock_metric_end) == 2 and deadlock_metric_end[1].value - deadlocks_start == 1, "there should be one new deadlock" def _get_conn_for_user(user, _autocommit=False): return pymysql.connect(host=HOST, port=PORT, user=user, password=user, autocommit=_autocommit) From daac79f8a928c77f552d75203048d693df1c3544 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Thu, 22 Feb 2024 17:17:50 +0000 Subject: [PATCH 24/32] Added the correct changelog --- datadog_checks_base/changelog.d/16904.added | 1 - mysql/changelog.d/16904.added | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 datadog_checks_base/changelog.d/16904.added create mode 100644 mysql/changelog.d/16904.added diff --git a/datadog_checks_base/changelog.d/16904.added b/datadog_checks_base/changelog.d/16904.added deleted file mode 100644 index 0efc9f0e9a8c8..0000000000000 --- a/datadog_checks_base/changelog.d/16904.added +++ /dev/null @@ -1 +0,0 @@ -[DBMON-2051] collect deadlock metric for MySQL diff --git a/mysql/changelog.d/16904.added b/mysql/changelog.d/16904.added new file mode 100644 index 0000000000000..61c57a75b76e6 --- /dev/null +++ b/mysql/changelog.d/16904.added @@ -0,0 +1 @@ +DBMON-2051: Adding a deadlock metric to Maria DB From ee10ca3e9f2ccb7c9281320025a8b449243c8477 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Fri, 23 Feb 2024 09:24:57 +0000 Subject: [PATCH 25/32] add a check for innnodb --- mysql/datadog_checks/mysql/mysql.py | 20 ++++++++++++++++---- mysql/metadata.csv | 2 +- mysql/tests/test_query_activity.py | 15 +++++++++++---- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index b29aedd604b6f..872b71acb95d0 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -141,6 +141,8 @@ def __init__(self, name, init_config, instances): # go through the agent internal metrics submission processing those tags self._non_internal_tags = copy.deepcopy(self.tags) self.set_resource_tags() + with self._connect() as db: + self._is_innodb_engine_enabled = self._is_innodb_engine_enabled(db) def execute_query_raw(self, query): with closing(self._conn.cursor(pymysql.cursors.SSCursor)) as cursor: @@ -214,6 +216,12 @@ def _check_performance_schema_enabled(self, db): return self.performance_schema_enabled + def _check_if_engine_is_innodb(self, db): + with closing(db.cursor()) as cursor: + cursor.execute(SQL_INNODB_ENGINES) + results = dict(cursor.fetchall()) + return results.get('InnoDB', 'NO') == 'YES' + def check_userstat_enabled(self, db): with closing(db.cursor()) as cursor: cursor.execute("SHOW VARIABLES LIKE 'userstat'") @@ -344,7 +352,10 @@ def runtime_queries(self): if self._runtime_queries: return self._runtime_queries - queries = [QUERY_DEADLOCKS] + queries = [] + + if self._is_innodb_engine_enabled: + queries.extend([QUERY_DEADLOCKS]) if self.performance_schema_enabled: queries.extend([QUERY_USER_CONNECTIONS]) @@ -446,9 +457,10 @@ def _collect_metrics(self, db, tags): with tracked_query(self, operation="variables_metrics"): results.update(self._get_stats_from_variables(db)) - if not is_affirmative( - self._config.options.get('disable_innodb_metrics', False) - ) and self._is_innodb_engine_enabled(db): + if ( + not is_affirmative(self._config.options.get('disable_innodb_metrics', False)) + and self._is_innodb_engine_enabled + ): with tracked_query(self, operation="innodb_metrics"): results.update(self.innodb_stats.get_stats_from_innodb_status(db)) self.innodb_stats.process_innodb_stats(results, self._config.options, metrics) diff --git a/mysql/metadata.csv b/mysql/metadata.csv index 0dd835daddd90..8cb4756965cac 100644 --- a/mysql/metadata.csv +++ b/mysql/metadata.csv @@ -12,7 +12,7 @@ mysql.innodb.buffer_pool_utilization,gauge,,fraction,,The utilization of the Inn mysql.innodb.current_row_locks,gauge,,lock,,The number of current row locks.,-1,mysql,current row locks, mysql.innodb.data_reads,gauge,,read,second,The rate of data reads.,0,mysql,data reads, mysql.innodb.data_writes,gauge,,write,second,The rate of data writes.,0,mysql,data writes, -mysql.innodb.deadlocks,gauge,,lock,,The number of current row deadlocks.,0,mysql,number of deadlocks, +mysql.innodb.deadlocks,gauge,,lock,,"The number of current row deadlocks.",0,mysql,number of deadlocks, mysql.innodb.mutex_os_waits,gauge,,event,second,The rate of mutex OS waits.,0,mysql,mutex os waits, mysql.innodb.mutex_spin_rounds,gauge,,event,second,The rate of mutex spin rounds.,0,mysql,mutex spin rounds, mysql.innodb.mutex_spin_waits,gauge,,event,second,The rate of mutex spin waits.,0,mysql,mutex spin waits, diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index dcbac99588279..56f7dc3f47de2 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -479,6 +479,17 @@ def _expected_dbm_job_err_tags(dbm_instance): ] +@pytest.mark.integration +@pytest.mark.usefixtures('dd_environment') +def test_if_deadlock_metric_is_collected(aggregator, dd_run_check, dbm_instance): + check = MySql(CHECK_NAME, {}, [dbm_instance]) + dd_run_check(check) + deadlock_metric = aggregator.metrics("mysql.innodb.deadlocks") + + assert len(deadlock_metric) == 1, "there should be one deadlock metric" + + +@pytest.mark.skipif(environ.get('MYSQL_FLAVOR') == 'mariadb', reason='Deadock count is not updated in MariaDB') @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') def test_deadlocks(aggregator, dd_run_check, dbm_instance): @@ -489,10 +500,6 @@ def test_deadlocks(aggregator, dd_run_check, dbm_instance): assert len(deadlock_metric_start) == 1, "there should be one deadlock metric" - if environ.get('MYSQL_FLAVOR') == 'mariadb': - # Skip the test below as deadlock count is not updated in MariaDB - return - deadlocks_start = deadlock_metric_start[0].value first_query = "SELECT * FROM testdb.users WHERE id = 1 FOR UPDATE;" From e295f28aa51158785c291c87a67c23deb316c50e Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Fri, 23 Feb 2024 09:31:54 +0000 Subject: [PATCH 26/32] removed superfluous method --- mysql/datadog_checks/mysql/mysql.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index 872b71acb95d0..ecc430c49b950 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -216,12 +216,6 @@ def _check_performance_schema_enabled(self, db): return self.performance_schema_enabled - def _check_if_engine_is_innodb(self, db): - with closing(db.cursor()) as cursor: - cursor.execute(SQL_INNODB_ENGINES) - results = dict(cursor.fetchall()) - return results.get('InnoDB', 'NO') == 'YES' - def check_userstat_enabled(self, db): with closing(db.cursor()) as cursor: cursor.execute("SHOW VARIABLES LIKE 'userstat'") From 8e89bf4359239179d862468c59d8092bf777da3b Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Fri, 23 Feb 2024 09:34:41 +0000 Subject: [PATCH 27/32] removed whitespace --- mysql/datadog_checks/mysql/queries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql/datadog_checks/mysql/queries.py b/mysql/datadog_checks/mysql/queries.py index 68aa8b884018a..d2c92b7d77a34 100644 --- a/mysql/datadog_checks/mysql/queries.py +++ b/mysql/datadog_checks/mysql/queries.py @@ -79,7 +79,7 @@ 'query': """ SELECT count as deadlocks - FROM + FROM information_schema.INNODB_METRICS WHERE NAME='lock_deadlocks' From 8bce95701485f27b6d886c8c0cd26a493294e2c0 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Fri, 23 Feb 2024 14:02:30 +0000 Subject: [PATCH 28/32] changed to monotonic_count --- mysql/metadata.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql/metadata.csv b/mysql/metadata.csv index 8cb4756965cac..6e2ea9bc3a3f8 100644 --- a/mysql/metadata.csv +++ b/mysql/metadata.csv @@ -12,7 +12,7 @@ mysql.innodb.buffer_pool_utilization,gauge,,fraction,,The utilization of the Inn mysql.innodb.current_row_locks,gauge,,lock,,The number of current row locks.,-1,mysql,current row locks, mysql.innodb.data_reads,gauge,,read,second,The rate of data reads.,0,mysql,data reads, mysql.innodb.data_writes,gauge,,write,second,The rate of data writes.,0,mysql,data writes, -mysql.innodb.deadlocks,gauge,,lock,,"The number of current row deadlocks.",0,mysql,number of deadlocks, +mysql.innodb.deadlocks,monotonic_count,,lock,,"The number of deadlocks.",0,mysql,number of deadlocks, mysql.innodb.mutex_os_waits,gauge,,event,second,The rate of mutex OS waits.,0,mysql,mutex os waits, mysql.innodb.mutex_spin_rounds,gauge,,event,second,The rate of mutex spin rounds.,0,mysql,mutex spin rounds, mysql.innodb.mutex_spin_waits,gauge,,event,second,The rate of mutex spin waits.,0,mysql,mutex spin waits, From e72f952c3c284ef52634a82f6211ddbc4dcf5102 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Mon, 26 Feb 2024 12:41:17 +0000 Subject: [PATCH 29/32] removed check from constructor --- mysql/datadog_checks/mysql/mysql.py | 28 ++++++++++++++-------------- mysql/tests/test_query_activity.py | 11 ++++++++--- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index ecc430c49b950..4d1e6b98cb5a9 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -141,8 +141,7 @@ def __init__(self, name, init_config, instances): # go through the agent internal metrics submission processing those tags self._non_internal_tags = copy.deepcopy(self.tags) self.set_resource_tags() - with self._connect() as db: - self._is_innodb_engine_enabled = self._is_innodb_engine_enabled(db) + self._is_innodb_engine_enabled = None def execute_query_raw(self, query): with closing(self._conn.cursor(pymysql.cursors.SSCursor)) as cursor: @@ -300,8 +299,8 @@ def check(self, _): if not self._config.only_custom_queries: self._collect_metrics(db, tags=tags) self._collect_system_metrics(self._config.host, db, tags) - if self.runtime_queries: - self.runtime_queries.execute(extra_tags=tags) + if self._get_runtime_queries(db): + self._get_runtime_queries(db).execute(extra_tags=tags) if self._config.dbm_enabled: dbm_tags = list(set(self.service_check_tags) | set(tags)) @@ -338,8 +337,7 @@ def _new_query_executor(self, queries): track_operation_time=True, ) - @property - def runtime_queries(self): + def _get_runtime_queries(self, db): """ Initializes runtime queries which depend on outside factors (e.g. permission checks) to load first. """ @@ -348,7 +346,7 @@ def runtime_queries(self): queries = [] - if self._is_innodb_engine_enabled: + if self._check_innodb_engine_enabled(db): queries.extend([QUERY_DEADLOCKS]) if self.performance_schema_enabled: @@ -451,10 +449,9 @@ def _collect_metrics(self, db, tags): with tracked_query(self, operation="variables_metrics"): results.update(self._get_stats_from_variables(db)) - if ( - not is_affirmative(self._config.options.get('disable_innodb_metrics', False)) - and self._is_innodb_engine_enabled - ): + if not is_affirmative( + self._config.options.get('disable_innodb_metrics', False) + ) and self._check_innodb_engine_enabled(db): with tracked_query(self, operation="innodb_metrics"): results.update(self.innodb_stats.get_stats_from_innodb_status(db)) self.innodb_stats.process_innodb_stats(results, self._config.options, metrics) @@ -1006,18 +1003,21 @@ def _get_binary_log_stats(self, db): self.warning("Privileges error accessing the BINARY LOGS (must grant REPLICATION CLIENT): %s", e) return None - def _is_innodb_engine_enabled(self, db): + def _check_innodb_engine_enabled(self, db): # Whether InnoDB engine is available or not can be found out either # from the output of SHOW ENGINES or from information_schema.ENGINES # table. Later is chosen because that involves no string parsing. + if self._is_innodb_engine_enabled is not None: + return self._is_innodb_engine_enabled try: with closing(db.cursor()) as cursor: cursor.execute(SQL_INNODB_ENGINES) - return cursor.rowcount > 0 + self._is_innodb_engine_enabled = cursor.rowcount > 0 except (pymysql.err.InternalError, pymysql.err.OperationalError, pymysql.err.NotSupportedError) as e: self.warning("Possibly innodb stats unavailable - error querying engines table: %s", e) - return False + self._is_innodb_engine_enabled = False + return self._is_innodb_engine_enabled def _get_replica_stats(self, db, is_mariadb, replication_channel): replica_results = defaultdict(dict) diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index 56f7dc3f47de2..413cf3bbd7688 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -489,7 +489,10 @@ def test_if_deadlock_metric_is_collected(aggregator, dd_run_check, dbm_instance) assert len(deadlock_metric) == 1, "there should be one deadlock metric" -@pytest.mark.skipif(environ.get('MYSQL_FLAVOR') == 'mariadb', reason='Deadock count is not updated in MariaDB') +@pytest.mark.skipif( + environ.get('MYSQL_FLAVOR') == 'mariadb' or MYSQL_VERSION_PARSED < parse_version('8.0'), + reason='Deadock count is not updated in MariaDB or older MySQL versions', +) @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') def test_deadlocks(aggregator, dd_run_check, dbm_instance): @@ -514,8 +517,9 @@ def run_first_deadlock_query(conn, event1, event2): event2.wait() conn.cursor().execute(second_query) conn.cursor().execute("COMMIT;") - except Exception: + except Exception as e: # Exception is expected due to a deadlock + print(e) pass conn.commit() @@ -528,8 +532,9 @@ def run_second_deadlock_query(conn, event1, event2): event2.set() conn.cursor().execute(first_query) conn.cursor().execute("COMMIT;") - except Exception: + except Exception as e: # Exception is expected due to a deadlock + print(e) pass conn.commit() From 126881e86acd56832ff462cc595c9e286f7d3bc2 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Mon, 26 Feb 2024 19:34:01 +0000 Subject: [PATCH 30/32] modified variable names --- mysql/datadog_checks/mysql/mysql.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index 4d1e6b98cb5a9..2752c149ee922 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -136,12 +136,12 @@ def __init__(self, name, init_config, instances): ttl=self._config.database_instance_collection_interval, ) # type: TTLCache - self._runtime_queries = None + self._runtime_queries_cached = None # Keep a copy of the tags without the internal resource tags so they can be used for paths that don't # go through the agent internal metrics submission processing those tags self._non_internal_tags = copy.deepcopy(self.tags) self.set_resource_tags() - self._is_innodb_engine_enabled = None + self._is_innodb_engine_enabled_cached = None def execute_query_raw(self, query): with closing(self._conn.cursor(pymysql.cursors.SSCursor)) as cursor: @@ -341,8 +341,8 @@ def _get_runtime_queries(self, db): """ Initializes runtime queries which depend on outside factors (e.g. permission checks) to load first. """ - if self._runtime_queries: - return self._runtime_queries + if self._runtime_queries_cached: + return self._runtime_queries_cached queries = [] @@ -352,10 +352,10 @@ def _get_runtime_queries(self, db): if self.performance_schema_enabled: queries.extend([QUERY_USER_CONNECTIONS]) - self._runtime_queries = self._new_query_executor(queries) - self._runtime_queries.compile_queries() + self._runtime_queries_cached = self._new_query_executor(queries) + self._runtime_queries_cached.compile_queries() self.log.debug("initialized runtime queries") - return self._runtime_queries + return self._runtime_queries_cached def _set_qcache_stats(self): host_key = self._get_host_key() @@ -1007,17 +1007,17 @@ def _check_innodb_engine_enabled(self, db): # Whether InnoDB engine is available or not can be found out either # from the output of SHOW ENGINES or from information_schema.ENGINES # table. Later is chosen because that involves no string parsing. - if self._is_innodb_engine_enabled is not None: - return self._is_innodb_engine_enabled + if self._is_innodb_engine_enabled_cached is not None: + return self._is_innodb_engine_enabled_cached try: with closing(db.cursor()) as cursor: cursor.execute(SQL_INNODB_ENGINES) - self._is_innodb_engine_enabled = cursor.rowcount > 0 + self._is_innodb_engine_enabled_cached = cursor.rowcount > 0 except (pymysql.err.InternalError, pymysql.err.OperationalError, pymysql.err.NotSupportedError) as e: self.warning("Possibly innodb stats unavailable - error querying engines table: %s", e) - self._is_innodb_engine_enabled = False - return self._is_innodb_engine_enabled + self._is_innodb_engine_enabled_cached = False + return self._is_innodb_engine_enabled_cached def _get_replica_stats(self, db, is_mariadb, replication_channel): replica_results = defaultdict(dict) From 77d44a10b68851cc0a08a635f19eb42772f98226 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Tue, 27 Feb 2024 18:40:17 +0000 Subject: [PATCH 31/32] fixed changelog and test error --- mysql/changelog.d/16904.added | 2 +- mysql/datadog_checks/mysql/queries.py | 2 +- mysql/metadata.csv | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mysql/changelog.d/16904.added b/mysql/changelog.d/16904.added index 61c57a75b76e6..729aa77053e6e 100644 --- a/mysql/changelog.d/16904.added +++ b/mysql/changelog.d/16904.added @@ -1 +1 @@ -DBMON-2051: Adding a deadlock metric to Maria DB +DBMON-2051: Adding a deadlock metric to MySQL databases. diff --git a/mysql/datadog_checks/mysql/queries.py b/mysql/datadog_checks/mysql/queries.py index d2c92b7d77a34..987aa0a3f89a1 100644 --- a/mysql/datadog_checks/mysql/queries.py +++ b/mysql/datadog_checks/mysql/queries.py @@ -84,7 +84,7 @@ WHERE NAME='lock_deadlocks' """.strip(), - 'columns': [{'name': 'mysql.innodb.deadlocks', 'type': 'gauge'}], + 'columns': [{'name': 'mysql.innodb.deadlocks', 'type': 'monotonic_count'}], } QUERY_USER_CONNECTIONS = { diff --git a/mysql/metadata.csv b/mysql/metadata.csv index 6e2ea9bc3a3f8..4c7451bb3f40d 100644 --- a/mysql/metadata.csv +++ b/mysql/metadata.csv @@ -12,7 +12,7 @@ mysql.innodb.buffer_pool_utilization,gauge,,fraction,,The utilization of the Inn mysql.innodb.current_row_locks,gauge,,lock,,The number of current row locks.,-1,mysql,current row locks, mysql.innodb.data_reads,gauge,,read,second,The rate of data reads.,0,mysql,data reads, mysql.innodb.data_writes,gauge,,write,second,The rate of data writes.,0,mysql,data writes, -mysql.innodb.deadlocks,monotonic_count,,lock,,"The number of deadlocks.",0,mysql,number of deadlocks, +mysql.innodb.deadlocks,count,,lock,,"The number of deadlocks.",0,mysql,number of deadlocks, mysql.innodb.mutex_os_waits,gauge,,event,second,The rate of mutex OS waits.,0,mysql,mutex os waits, mysql.innodb.mutex_spin_rounds,gauge,,event,second,The rate of mutex spin rounds.,0,mysql,mutex spin rounds, mysql.innodb.mutex_spin_waits,gauge,,event,second,The rate of mutex spin waits.,0,mysql,mutex spin waits, From e513c14fc53964476b8570f2c6ca9b024d2ccbbe Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Wed, 28 Feb 2024 09:15:35 +0000 Subject: [PATCH 32/32] improved the changelog --- mysql/changelog.d/16904.added | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql/changelog.d/16904.added b/mysql/changelog.d/16904.added index 729aa77053e6e..898fc888b9942 100644 --- a/mysql/changelog.d/16904.added +++ b/mysql/changelog.d/16904.added @@ -1 +1 @@ -DBMON-2051: Adding a deadlock metric to MySQL databases. +Adding a deadlock metric to MySQL databases.