Submit database_hostname with database_instance (DataDog#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
bhargavnariyanicrest · Nov 25, 2024 · 2ae8902 · 2ae8902
1 parent 0dc1f49
commit 2ae8902
Show file tree

Hide file tree

Showing 16 changed files with 107 additions and 22 deletions.
diff --git a/mysql/changelog.d/18969.added 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
@@ -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
@@ -3,24 +3,35 @@
 # 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 = [
     'port:' + str(common.PORT),
     '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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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",
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Submit database_hostname with database instance and metrics for MySQL, Postgres, and SQLServer