[DBM][SQL Server] FCI Metrics without Cluster Name (DataDog#23264)

* updated fci query

* added tests

* removed unused variable

* minor changes to the test

* added changelog

* updated comment

Author: aldrickdev

sqlserver/changelog.d/23264.fixed

@@ -0,0 +1 @@
+updates the FCI metric collection so that we can still report FCI metrics without needing a cluster name.

sqlserver/datadog_checks/sqlserver/database_metrics/fci_metrics.py

@@ -18,15 +18,17 @@
             status,
             is_current_owner
         FROM sys.dm_os_cluster_nodes
-        -- `sys.dm_hadr_cluster` does not have a related column to join on, this cross join will add the
-        -- `cluster_name` column to every row by multiplying all the rows in the left table against
-        -- all the rows in the right table. Note, there will only be one row from `sys.dm_hadr_cluster`.
-        CROSS JOIN (SELECT TOP 1 cluster_name FROM sys.dm_hadr_cluster) AS FC
+        -- `sys.dm_hadr_cluster` does not have a related column to join on. OUTER APPLY attaches the
+        -- `cluster_name` column to every row from `sys.dm_os_cluster_nodes`, preserving those rows
+        -- even when `sys.dm_hadr_cluster` returns no rows (e.g. when Always On is not enabled, in
+        -- which case `cluster_name` is NULL). Note, there will only be at most one row from
+        -- `sys.dm_hadr_cluster`.
+        OUTER APPLY (SELECT TOP 1 cluster_name FROM sys.dm_hadr_cluster) AS FC
     """.strip(),
     "columns": [
         {"name": "node_name", "type": "tag"},
         {"name": "status", "type": "tag"},
-        {"name": "failover_cluster", "type": "tag"},
+        {"name": "failover_cluster", "type": "tag_not_null"},
         {"name": "fci.status", "type": "gauge"},
         {"name": "fci.is_current_owner", "type": "gauge"},
     ],

sqlserver/tests/test_database_metrics.py

@@ -642,6 +642,72 @@ def execute_query_handler_mocked(query, db=None):
                 aggregator.assert_metric(metric_name, value=metric_value, tags=expected_tags)
 
 
+@pytest.mark.integration
+@pytest.mark.usefixtures('dd_environment')
+def test_sqlserver_fci_metrics_without_cluster_name(
+    aggregator,
+    dd_run_check,
+    init_config,
+    instance_docker_metrics,
+):
+    """When sys.dm_hadr_cluster has no rows (HADR not configured), the OUTER APPLY returns NULL
+    for cluster_name. FCI metrics should still be collected, and the failover_cluster tag should
+    be omitted thanks to the tag_not_null column type."""
+    instance_docker_metrics['database_autodiscovery'] = True
+    instance_docker_metrics['database_metrics'] = {
+        'fci_metrics': {'enabled': True},
+    }
+
+    # Columns: (node_name, status_description, cluster_name, status, is_current_owner)
+    mocked_results = [
+        ('node1', 'up', None, 0, 1),
+        ('node2', 'down', None, 1, 0),
+    ]
+
+    sqlserver_check = SQLServer(CHECK_NAME, init_config, [instance_docker_metrics])
+
+    def execute_query_handler_mocked(query, db=None):
+        return mocked_results
+
+    fci_metrics = SqlserverFciMetrics(
+        config=sqlserver_check._config,
+        new_query_executor=sqlserver_check._new_query_executor,
+        server_static_info=STATIC_SERVER_INFO,
+        execute_query_handler=execute_query_handler_mocked,
+    )
+
+    sqlserver_check._database_metrics = [fci_metrics]
+
+    dd_run_check(sqlserver_check)
+
+    base_tags = sqlserver_check._config.tags + [
+        "database_hostname:{}".format("stubbed.hostname"),
+        "database_instance:{}".format("stubbed.hostname"),
+        "dd.internal.resource:database_instance:{}".format("stubbed.hostname"),
+        "sqlserver_servername:{}".format(sqlserver_check.static_info_cache[STATIC_INFO_SERVERNAME].lower()),
+    ]
+
+    metric_names = fci_metrics.metric_names()[0]
+
+    for node_name, status, _cluster_name, *metric_values in mocked_results:
+        # Metrics should still be emitted with node_name and status tags, but without failover_cluster
+        expected_tags = [
+            f'node_name:{node_name}',
+            f'status:{status}',
+        ] + base_tags
+
+        for metric_name, metric_value in zip(metric_names, metric_values):
+            aggregator.assert_metric(metric_name, value=metric_value, tags=expected_tags)
+
+        # Verify that no failover_cluster tag was emitted (tag_not_null skips NULL values)
+        for metric_name in metric_names:
+            for metric_stub in aggregator.metrics(metric_name):
+                if f'node_name:{node_name}' in metric_stub.tags:
+                    assert all(not tag.startswith('failover_cluster:') for tag in metric_stub.tags), (
+                        "failover_cluster tag should not be present when cluster_name is NULL"
+                    )
+
+
 @pytest.mark.integration
 @pytest.mark.usefixtures('dd_environment')
 @pytest.mark.parametrize('include_primary_log_shipping_metrics', [True, False])