From 2ae890214eba9c1f5a6d5efed25abe06b2a60f02 Mon Sep 17 00:00:00 2001 From: Seth Samuel Date: Mon, 25 Nov 2024 14:21:00 -0500 Subject: [PATCH] Submit database_hostname with database_instance (#18969) * database_hostname * Lint * Add database_hostname to tags * _database_hostname * Tests * Changelog * MySQL tests * Tests * Lint * Fix * Fix * Fix * Mysql * Lint * Fix * Python * Test * Flaky * Test * Lint * Revert * Stub * WIP * Fix * Lint --- mysql/changelog.d/18969.added | 1 + mysql/datadog_checks/mysql/mysql.py | 11 ++++++ mysql/tests/tags.py | 19 ++++++++-- mysql/tests/test_metadata.py | 1 + mysql/tests/test_mysql.py | 16 ++++---- mysql/tests/test_query_activity.py | 2 + mysql/tests/test_statements.py | 2 + mysql/tests/test_unit.py | 38 +++++++++++++++++-- postgres/changelog.d/18969.added | 1 + postgres/datadog_checks/postgres/postgres.py | 11 ++++++ postgres/tests/common.py | 2 +- postgres/tests/test_e2e.py | 3 ++ postgres/tests/test_pg_integration.py | 7 +--- postgres/tests/test_unit.py | 2 + sqlserver/changelog.d/18969.added | 1 + .../datadog_checks/sqlserver/sqlserver.py | 12 ++++++ 16 files changed, 107 insertions(+), 22 deletions(-) create mode 100644 mysql/changelog.d/18969.added create mode 100644 postgres/changelog.d/18969.added create mode 100644 sqlserver/changelog.d/18969.added diff --git a/mysql/changelog.d/18969.added b/mysql/changelog.d/18969.added new file mode 100644 index 0000000000000..dfc29ca0f7645 --- /dev/null +++ b/mysql/changelog.d/18969.added @@ -0,0 +1 @@ +Submit database_hostname with database instance and metrics for MySQL, Postgres, and SQLServer diff --git a/mysql/datadog_checks/mysql/mysql.py b/mysql/datadog_checks/mysql/mysql.py index 69e40133a0a09..23f571a09368a 100644 --- a/mysql/datadog_checks/mysql/mysql.py +++ b/mysql/datadog_checks/mysql/mysql.py @@ -109,6 +109,7 @@ def __init__(self, name, init_config, instances): self.is_mariadb = None self._resolved_hostname = None self._agent_hostname = None + self._database_hostname = None self._is_aurora = None self._config = MySQLConfig(self.instance, init_config) self.tags = self._config.tags @@ -170,7 +171,16 @@ def agent_hostname(self): self._agent_hostname = datadog_agent.get_hostname() return self._agent_hostname + @property + def database_hostname(self): + # type: () -> str + if self._database_hostname is None: + self._database_hostname = self.resolve_db_host() + return self._database_hostname + def set_resource_tags(self): + self.tags.append("database_hostname:{}".format(self.database_hostname)) + if self.cloud_metadata.get("gcp") is not None: self.tags.append( "dd.internal.resource:gcp_sql_database_instance:{}:{}".format( @@ -1303,6 +1313,7 @@ def _send_database_instance_metadata(self): event = { "host": self.resolved_hostname, "port": self._config.port, + "database_hostname": self.database_hostname, "agent_version": datadog_agent.get_version(), "dbms": "mysql", "kind": "database_instance", diff --git a/mysql/tests/tags.py b/mysql/tests/tags.py index 0543af7c2bbdf..4cd89f39fcc81 100644 --- a/mysql/tests/tags.py +++ b/mysql/tests/tags.py @@ -3,12 +3,16 @@ # Licensed under a 3-clause BSD style license (see LICENSE) from . import common -DATABASE_INSTANCE_RESOURCE_TAG = 'dd.internal.resource:database_instance:{hostname}' + +def database_instance_resource_tags(hostname): + return [f'dd.internal.resource:database_instance:{hostname}', f'database_hostname:{hostname}'] + + METRIC_TAGS = ['tag1:value1', 'tag2:value2'] METRIC_TAGS_WITH_RESOURCE = [ 'tag1:value1', 'tag2:value2', - DATABASE_INSTANCE_RESOURCE_TAG.format(hostname='stubbed.hostname'), + *database_instance_resource_tags('stubbed.hostname'), 'dbms_flavor:{}'.format(common.MYSQL_FLAVOR.lower()), ] SC_TAGS = [ @@ -16,11 +20,18 @@ 'tag1:value1', 'tag2:value2', ] -SC_TAGS_MIN = ['port:' + str(common.PORT), DATABASE_INSTANCE_RESOURCE_TAG.format(hostname='stubbed.hostname')] +SC_TAGS_MIN = [ + 'port:' + str(common.PORT), + *database_instance_resource_tags('stubbed.hostname'), +] SC_TAGS_REPLICA = [ 'port:' + str(common.SLAVE_PORT), 'tag1:value1', 'tag2:value2', 'dd.internal.resource:database_instance:stubbed.hostname', + 'database_hostname:stubbed.hostname', +] +SC_FAILURE_TAGS = [ + 'port:unix_socket', + *database_instance_resource_tags('stubbed.hostname'), ] -SC_FAILURE_TAGS = ['port:unix_socket', DATABASE_INSTANCE_RESOURCE_TAG.format(hostname='stubbed.hostname')] diff --git a/mysql/tests/test_metadata.py b/mysql/tests/test_metadata.py index d49451c026e0c..648fe032a6d36 100644 --- a/mysql/tests/test_metadata.py +++ b/mysql/tests/test_metadata.py @@ -676,6 +676,7 @@ def test_collect_schemas(aggregator, dd_run_check, dbm_instance): assert schema_event.get("dbms_version") is not None assert (schema_event.get("flavor") == "MariaDB") or (schema_event.get("flavor") == "MySQL") assert sorted(schema_event["tags"]) == [ + 'database_hostname:stubbed.hostname', 'dbms_flavor:{}'.format(common.MYSQL_FLAVOR.lower()), 'dd.internal.resource:database_instance:stubbed.hostname', 'port:13306', diff --git a/mysql/tests/test_mysql.py b/mysql/tests/test_mysql.py index 4073428187122..f8214ee777781 100644 --- a/mysql/tests/test_mysql.py +++ b/mysql/tests/test_mysql.py @@ -101,7 +101,7 @@ def test_complex_config(aggregator, dd_run_check, instance_complex): _assert_complex_config( aggregator, - tags.SC_TAGS + [tags.DATABASE_INSTANCE_RESOURCE_TAG.format(hostname='stubbed.hostname')], + tags.SC_TAGS + tags.database_instance_resource_tags('stubbed.hostname'), tags.METRIC_TAGS_WITH_RESOURCE, ) aggregator.assert_metrics_using_metadata( @@ -116,8 +116,8 @@ def test_e2e(dd_agent_check, dd_default_hostname, instance_complex): aggregator = dd_agent_check(instance_complex) _assert_complex_config( aggregator, - tags.SC_TAGS + [tags.DATABASE_INSTANCE_RESOURCE_TAG.format(hostname=dd_default_hostname)], - tags.METRIC_TAGS + ['dbms_flavor:{}'.format(MYSQL_FLAVOR.lower())], + tags.SC_TAGS + tags.database_instance_resource_tags(dd_default_hostname), + tags.METRIC_TAGS + [f'database_hostname:{dd_default_hostname}', 'dbms_flavor:{}'.format(MYSQL_FLAVOR.lower())], hostname=dd_default_hostname, e2e=True, ) @@ -414,12 +414,10 @@ def test_correct_hostname(dbm_enabled, reported_hostname, expected_hostname, agg with mock.patch('datadog_checks.mysql.MySql.resolve_db_host', return_value='resolved.hostname') as resolve_db_host: mysql_check = MySql(common.CHECK_NAME, {}, [instance_basic]) dd_run_check(mysql_check) - if reported_hostname: - assert resolve_db_host.called is False, 'Expected resolve_db_host.called to be False' - else: - assert resolve_db_host.called is True + assert resolve_db_host.called is True expected_tags = [ + 'database_hostname:{}'.format(mysql_check.database_hostname), 'server:{}'.format(HOST), 'port:{}'.format(PORT), 'dd.internal.resource:database_instance:{}'.format(expected_hostname), @@ -710,7 +708,8 @@ def test_set_resources(aggregator, dd_run_check, instance_basic, cloud_metadata, for m in metric_names: aggregator.assert_metric_has_tag("mysql.net.connections", m) aggregator.assert_metric_has_tag( - "mysql.net.connections", tags.DATABASE_INSTANCE_RESOURCE_TAG.format(hostname=mysql_check.resolved_hostname) + "mysql.net.connections", + f'dd.internal.resource:database_instance:{mysql_check.resolved_hostname}', ) @@ -793,6 +792,7 @@ def test_propagate_agent_tags( expected_tags = ( instance_basic.get('tags', []) + [ + 'database_hostname:stubbed.hostname', 'server:{}'.format(HOST), 'port:{}'.format(PORT), 'dd.internal.resource:database_instance:forced_hostname', diff --git a/mysql/tests/test_query_activity.py b/mysql/tests/test_query_activity.py index 5189f4b23be6e..6571e60c6d820 100644 --- a/mysql/tests/test_query_activity.py +++ b/mysql/tests/test_query_activity.py @@ -113,6 +113,7 @@ def _run_blocking(conn): assert activity['ddsource'] == 'mysql' assert activity['ddagentversion'], "missing agent version" assert set(activity['ddtags']) == { + 'database_hostname:stubbed.hostname', 'tag1:value1', 'tag2:value2', 'port:13306', @@ -525,6 +526,7 @@ def test_events_wait_current_disabled_no_warning_azure_flexible_server( # directly to metrics-intake, so they should also be properly tagged with a resource def _expected_dbm_job_err_tags(dbm_instance): return dbm_instance['tags'] + [ + 'database_hostname:stubbed.hostname', 'job:query-activity', 'port:{}'.format(PORT), 'dd.internal.resource:database_instance:stubbed.hostname', diff --git a/mysql/tests/test_statements.py b/mysql/tests/test_statements.py index 0f667e164c2af..8269073ea517a 100644 --- a/mysql/tests/test_statements.py +++ b/mysql/tests/test_statements.py @@ -854,6 +854,7 @@ def test_async_job_cancel(aggregator, dd_run_check, dbm_instance): def _expected_dbm_instance_tags(dbm_instance): return dbm_instance.get('tags', []) + [ + 'database_hostname:{}'.format('stubbed.hostname'), 'server:{}'.format(common.HOST), 'port:{}'.format(common.PORT), 'dbms_flavor:{}'.format(MYSQL_FLAVOR.lower()), @@ -864,6 +865,7 @@ def _expected_dbm_instance_tags(dbm_instance): # directly to metrics-intake, so they should also be properly tagged with a resource def _expected_dbm_job_err_tags(dbm_instance): return dbm_instance['tags'] + [ + 'database_hostname:{}'.format('stubbed.hostname'), 'port:{}'.format(common.PORT), 'server:{}'.format(common.HOST), 'dd.internal.resource:database_instance:stubbed.hostname', diff --git a/mysql/tests/test_unit.py b/mysql/tests/test_unit.py index 94ae3912e4197..ac84bbf83d4fb 100644 --- a/mysql/tests/test_unit.py +++ b/mysql/tests/test_unit.py @@ -298,14 +298,44 @@ def cursor(self): @pytest.mark.parametrize( 'disable_generic_tags, hostname, expected_tags', [ - (True, None, {'port:unix_socket', 'dd.internal.resource:database_instance:stubbed.hostname'}), + ( + True, + None, + { + 'port:unix_socket', + 'database_hostname:stubbed.hostname', + 'dd.internal.resource:database_instance:stubbed.hostname', + }, + ), ( False, None, - {'port:unix_socket', 'server:localhost', 'dd.internal.resource:database_instance:stubbed.hostname'}, + { + 'port:unix_socket', + 'server:localhost', + 'database_hostname:stubbed.hostname', + 'dd.internal.resource:database_instance:stubbed.hostname', + }, + ), + ( + True, + 'foo', + { + 'port:unix_socket', + 'database_hostname:stubbed.hostname', + 'dd.internal.resource:database_instance:stubbed.hostname', + }, + ), + ( + False, + 'foo', + { + 'port:unix_socket', + 'server:foo', + 'database_hostname:stubbed.hostname', + 'dd.internal.resource:database_instance:stubbed.hostname', + }, ), - (True, 'foo', {'port:unix_socket', 'dd.internal.resource:database_instance:stubbed.hostname'}), - (False, 'foo', {'port:unix_socket', 'server:foo', 'dd.internal.resource:database_instance:stubbed.hostname'}), ], ) def test_service_check(disable_generic_tags, expected_tags, hostname): diff --git a/postgres/changelog.d/18969.added b/postgres/changelog.d/18969.added new file mode 100644 index 0000000000000..dfc29ca0f7645 --- /dev/null +++ b/postgres/changelog.d/18969.added @@ -0,0 +1 @@ +Submit database_hostname with database instance and metrics for MySQL, Postgres, and SQLServer diff --git a/postgres/datadog_checks/postgres/postgres.py b/postgres/datadog_checks/postgres/postgres.py index 28f6e32a73197..3f20801a9a399 100644 --- a/postgres/datadog_checks/postgres/postgres.py +++ b/postgres/datadog_checks/postgres/postgres.py @@ -97,6 +97,7 @@ def __init__(self, name, init_config, instances): super(PostgreSql, self).__init__(name, init_config, instances) self._resolved_hostname = None self._agent_hostname = None + self._database_hostname = None self._db = None self.version = None self.raw_version = None @@ -168,6 +169,8 @@ def _build_autodiscovery(self): return discovery def set_resource_tags(self): + self.tags.append("database_hostname:{}".format(self.database_hostname)) + if self.cloud_metadata.get("gcp") is not None: self.tags.append( "dd.internal.resource:gcp_sql_database_instance:{}:{}".format( @@ -476,6 +479,13 @@ def agent_hostname(self): self._agent_hostname = datadog_agent.get_hostname() return self._agent_hostname + @property + def database_hostname(self): + # type: () -> str + if self._database_hostname is None: + self._database_hostname = self.resolve_db_host() + return self._database_hostname + def resolve_db_host(self): return agent_host_resolver(self._config.host) @@ -912,6 +922,7 @@ def _send_database_instance_metadata(self): event = { "host": self.resolved_hostname, "port": self._config.port, + "database_hostname": self.database_hostname, "agent_version": datadog_agent.get_version(), "dbms": "postgres", "kind": "database_instance", diff --git a/postgres/tests/common.py b/postgres/tests/common.py index f051d673bacc3..df966caf3201a 100644 --- a/postgres/tests/common.py +++ b/postgres/tests/common.py @@ -151,7 +151,7 @@ def _get_expected_tags( role='master', **kwargs, ): - base_tags = pg_instance['tags'] + [f'port:{pg_instance["port"]}'] + base_tags = pg_instance['tags'] + [f'port:{pg_instance["port"]}'] + [f'database_hostname:{check.database_hostname}'] if role: base_tags.append(f'replication_role:{role}') if with_db: diff --git a/postgres/tests/test_e2e.py b/postgres/tests/test_e2e.py index 461f020e80c98..3a0ae87e7b798 100644 --- a/postgres/tests/test_e2e.py +++ b/postgres/tests/test_e2e.py @@ -2,6 +2,8 @@ # All rights reserved # Licensed under Simplified BSD License (see LICENSE) +import socket + import pytest from .common import _get_expected_tags, check_bgw_metrics, check_common_metrics @@ -23,6 +25,7 @@ def test_e2e(check, dd_agent_check, pg_instance): cur.execute("SHOW cluster_name;") check.cluster_name = cur.fetchone()[0] + check._database_hostname = socket.gethostname().lower() expected_tags = _get_expected_tags(check, pg_instance, with_host=False) check_bgw_metrics(aggregator, expected_tags) check_common_metrics(aggregator, expected_tags=expected_tags, count=None) diff --git a/postgres/tests/test_pg_integration.py b/postgres/tests/test_pg_integration.py index 53fad5c8d29c9..b5c27a830530f 100644 --- a/postgres/tests/test_pg_integration.py +++ b/postgres/tests/test_pg_integration.py @@ -463,7 +463,7 @@ def test_activity_vacuum_excluded(aggregator, integration_check, pg_instance): thread.join() -@pytest.mark.flaky(max_runs=5) +@pytest.mark.flaky(max_runs=10) def test_backend_transaction_age(aggregator, integration_check, pg_instance): pg_instance['collect_activity_metrics'] = True check = integration_check(pg_instance) @@ -731,10 +731,7 @@ def test_correct_hostname(dbm_enabled, reported_hostname, expected_hostname, agg ) as resolve_db_host: check = PostgreSql('test_instance', {}, [pg_instance]) check.run() - if reported_hostname: - assert resolve_db_host.called is False, 'Expected resolve_db_host.called to be False' - else: - assert resolve_db_host.called is True + assert resolve_db_host.called is True expected_tags_no_db = _get_expected_tags(check, pg_instance, server=HOST) expected_tags_with_db = expected_tags_no_db + ['db:datadog_test'] diff --git a/postgres/tests/test_unit.py b/postgres/tests/test_unit.py index 1fa3164fa1f22..79ea811764ca7 100644 --- a/postgres/tests/test_unit.py +++ b/postgres/tests/test_unit.py @@ -142,6 +142,7 @@ def test_query_timeout_connection_string(aggregator, integration_check, pg_insta 'port:5432', 'foo:bar', 'dd.internal.resource:database_instance:stubbed.hostname', + 'database_hostname:stubbed.hostname', }, ), ( @@ -152,6 +153,7 @@ def test_query_timeout_connection_string(aggregator, integration_check, pg_insta 'port:5432', 'server:localhost', 'dd.internal.resource:database_instance:stubbed.hostname', + 'database_hostname:stubbed.hostname', }, ), ], diff --git a/sqlserver/changelog.d/18969.added b/sqlserver/changelog.d/18969.added new file mode 100644 index 0000000000000..dfc29ca0f7645 --- /dev/null +++ b/sqlserver/changelog.d/18969.added @@ -0,0 +1 @@ +Submit database_hostname with database instance and metrics for MySQL, Postgres, and SQLServer diff --git a/sqlserver/datadog_checks/sqlserver/sqlserver.py b/sqlserver/datadog_checks/sqlserver/sqlserver.py index c9ba81885363d..6016f5fa6f95f 100644 --- a/sqlserver/datadog_checks/sqlserver/sqlserver.py +++ b/sqlserver/datadog_checks/sqlserver/sqlserver.py @@ -116,6 +116,7 @@ def __init__(self, name, init_config, instances): self._resolved_hostname = None self._agent_hostname = None + self._database_hostname = None self.connection = None self.failed_connections = {} self.instance_metrics = [] @@ -202,6 +203,8 @@ def set_resolved_hostname_metadata(self): self.set_metadata("resolved_hostname", self.resolved_hostname) def set_resource_tags(self): + self.tags.append("database_hostname:{}".format(self.database_hostname)) + if self._config.cloud_metadata.get("gcp") is not None: self.tags.append( "dd.internal.resource:gcp_sql_database_instance:{}:{}".format( @@ -284,6 +287,14 @@ def set_resolved_hostname(self): def resolved_hostname(self): return self._resolved_hostname + @property + def database_hostname(self): + # type: () -> str + if self._database_hostname is None: + host, _ = split_sqlserver_host_port(self.instance.get("host")) + self._database_hostname = resolve_db_host(host) + return self._database_hostname + def load_static_information(self): engine_edition_reloaded = False expected_keys = {STATIC_INFO_VERSION, STATIC_INFO_MAJOR_VERSION, STATIC_INFO_ENGINE_EDITION, STATIC_INFO_RDS} @@ -959,6 +970,7 @@ def _send_database_instance_metadata(self): if self.resolved_hostname not in self._database_instance_emitted: event = { "host": self.resolved_hostname, + "database_hostname": self.database_hostname, "agent_version": datadog_agent.get_version(), "dbms": "sqlserver", "kind": "database_instance",