[DBMON-6784] Fix Clickhouse database_hostname usage (#24247)

* Fix database_hostname usage

* Add changelog

* Fixup tests

Author: eric-weaver

clickhouse/changelog.d/24247.fixed

@@ -0,0 +1 @@
+Fix the `database_hostname` tag and metadata to always report the resolved database host instead of the `reported_hostname` override.

clickhouse/datadog_checks/clickhouse/clickhouse.py

@@ -57,6 +57,7 @@ def __init__(self, name, init_config, instances):
 
         # DBM-related properties (computed lazily)
         self._resolved_hostname = None
+        self._database_hostname = None
         self._database_identifier = None
         self._agent_hostname = None
         self._dbms_version = None
@@ -155,7 +156,7 @@ def _add_core_tags(self):
         self.tag_manager.set_tag("server", self._config.server, replace=True)
         self.tag_manager.set_tag("port", str(self._config.port), replace=True)
         self.tag_manager.set_tag("db", self._config.db, replace=True)
-        self.tag_manager.set_tag("database_hostname", self.reported_hostname, replace=True)
+        self.tag_manager.set_tag("database_hostname", self.database_hostname, replace=True)
         self.tag_manager.set_tag("database_instance", self.database_identifier, replace=True)
 
     def validate_config(self):
@@ -227,7 +228,7 @@ def _send_database_instance_metadata(self):
                 "host": self.reported_hostname,
                 "port": self._config.port,
                 "database_instance": self.database_identifier,
-                "database_hostname": self.reported_hostname,
+                "database_hostname": self.database_hostname,
                 "agent_version": datadog_agent.get_version(),
                 "ddagenthostname": self.agent_hostname,
                 "dbms": "clickhouse",
@@ -353,6 +354,12 @@ def reported_hostname(self) -> str | None:
                 self._resolved_hostname = resolve_db_host(self._config.server)
         return self._resolved_hostname
 
+    @property
+    def database_hostname(self) -> str:
+        if self._database_hostname is None:
+            self._database_hostname = resolve_db_host(self._config.server)
+        return self._database_hostname
+
     @property
     def agent_hostname(self):
         """Get the agent hostname."""

clickhouse/tests/test_clickhouse.py

@@ -18,7 +18,7 @@ def test_check(aggregator, instance, dd_run_check):
     server_tag = 'server:{}'.format(instance['server'])
     port_tag = 'port:{}'.format(instance['port'])
     metrics = common.get_metrics(CLICKHOUSE_VERSION)
-    db_hostname_tag = 'database_hostname:{}'.format(check.reported_hostname)
+    db_hostname_tag = 'database_hostname:{}'.format(check.database_hostname)
     db_instance_tag = 'database_instance:{}:{}:default'.format(instance['server'], instance['port'])
 
     for metric in metrics:
@@ -55,7 +55,7 @@ def test_custom_queries(aggregator, instance, dd_run_check):
             'db:default',
             'foo:bar',
             'test:clickhouse',
-            'database_hostname:{}'.format(check.reported_hostname),
+            'database_hostname:{}'.format(check.database_hostname),
             'database_instance:{}:{}:default'.format(instance['server'], instance['port']),
         ],
     )
@@ -99,8 +99,12 @@ def test_version_metadata(instance, datadog_agent, dd_run_check):
     )
 
 
-def test_database_instance_metadata(aggregator, instance, datadog_agent, dd_run_check):
+@pytest.mark.parametrize('reported_hostname', [None, 'forced-clickhouse-host'])
+def test_database_instance_metadata(aggregator, instance, datadog_agent, dd_run_check, reported_hostname):
     """Test that database_instance metadata is sent correctly."""
+    if reported_hostname:
+        instance['reported_hostname'] = reported_hostname
+
     check = ClickhouseCheck('clickhouse', {}, [instance])
     check.check_id = 'test:456'
     dd_run_check(check)
@@ -115,6 +119,13 @@ def test_database_instance_metadata(aggregator, instance, datadog_agent, dd_run_
     assert event['dbms'] == 'clickhouse'
     assert event['kind'] == 'database_instance'
     assert event['database_instance'] == check.database_identifier
+    # database_hostname always reports the resolved host, independent of the reported_hostname override
+    assert event['database_hostname'] == check.database_hostname
+    # host follows the reported_hostname override when one is configured
+    assert event['host'] == check.reported_hostname
+    if reported_hostname:
+        assert event['host'] == reported_hostname
+        assert event['database_hostname'] != reported_hostname
     assert event['collection_interval'] == 300
     assert 'metadata' in event
     assert 'dbm' in event['metadata']

clickhouse/tests/test_unit.py

@@ -361,10 +361,23 @@ def test_reported_hostname_loopback_substitutes_agent_hostname(loopback):
         assert check.reported_hostname == 'my-agent-host'
 
 
-def test_reported_hostname_non_loopback():
+@pytest.mark.parametrize(
+    'reported_hostname, expected_reported_hostname',
+    [
+        pytest.param(None, 'resolved-host', id='no-override'),
+        pytest.param('custom-host', 'custom-host', id='with-override'),
+    ],
+)
+def test_database_hostname_ignores_reported_hostname_override(reported_hostname, expected_reported_hostname):
+    instance = {**BASE_INSTANCE}
+    if reported_hostname:
+        instance['reported_hostname'] = reported_hostname
     with mock.patch(
-        'datadog_checks.clickhouse.clickhouse.resolve_db_host', return_value=BASE_INSTANCE['server']
+        'datadog_checks.clickhouse.clickhouse.resolve_db_host', return_value='resolved-host'
     ) as mock_resolve:
-        check = ClickhouseCheck('clickhouse', {}, [BASE_INSTANCE])
-        assert check.reported_hostname == BASE_INSTANCE['server']
-        mock_resolve.assert_called_once_with(BASE_INSTANCE['server'])
+        check = ClickhouseCheck('clickhouse', {}, [instance])
+        # database_hostname always resolves the real host, regardless of the override
+        assert check.database_hostname == 'resolved-host'
+        # reported_hostname honors the override when configured, otherwise the resolved host
+        assert check.reported_hostname == expected_reported_hostname
+        mock_resolve.assert_called_with(BASE_INSTANCE['server'])