[sqlserver] Group perf counter rows by counter name before dispatch

Every metric object scanned the whole sys.dm_os_performance_counters result
set, and with autodiscovery both the metric count and the row count grow with
the database count, so the dispatch cost grew with the square of the number of
databases. Group the rows by counter name once per run instead.

Also cache the counter type for counters that need no base counter, which was
costing a round trip per counter per database on every metric list rebuild, and
rebuild instance_per_type_metrics from scratch so counter names for metrics that
are no longer collected stop being queried.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: eric-weaver

sqlserver/datadog_checks/sqlserver/metrics.py

@@ -96,29 +96,27 @@ class SqlSimpleMetric(BaseSqlServerMetric):
 
     @classmethod
     def fetch_all_values(cls, cursor, counters_list, logger, databases=None, engine_edition=None):
-        return cls._fetch_generic_values(cursor, counters_list, logger)
+        rows, _ = cls._fetch_generic_values(cursor, counters_list, logger)
+        # The name columns are nchar(128), so every value arrives blank-padded. Strip once here and group by
+        # counter name so each metric only walks its own rows: with autodiscovery both the number of metrics
+        # and the number of rows grow with the database count, making a per-metric scan quadratic.
+        results = defaultdict(list)
+        for counter_name, instance_name, object_name, cntr_value in rows:
+            results[counter_name.strip()].append((instance_name.strip(), object_name.strip(), cntr_value))
+        return results, None
 
-    def fetch_metric(self, rows, columns, values_cache=None):
-        for counter_name_long, instance_name_long, object_name, cntr_value in rows:
-            counter_name = counter_name_long.strip()
-            instance_name = instance_name_long.strip()
-            object_name = object_name.strip()
-            if counter_name.strip() == self.sql_name:
-                matched = False
+    def fetch_metric(self, results, columns, values_cache=None):
+        for instance_name, object_name, cntr_value in results.get(self.sql_name, ()):
+            if (self.instance == ALL_INSTANCES and instance_name != "_Total") or (
+                (instance_name == self.instance or instance_name == self.physical_db_name)
+                and (not self.object_name or object_name == self.object_name)
+            ):
                 metric_tags = list(self.tags)
-
-                if (self.instance == ALL_INSTANCES and instance_name != "_Total") or (
-                    (instance_name == self.instance or instance_name == self.physical_db_name)
-                    and (not self.object_name or object_name == self.object_name)
-                ):
-                    matched = True
-
-                if matched:
-                    if self.instance == ALL_INSTANCES:
-                        metric_tags.append('{}:{}'.format(self.tag_by, instance_name.strip()))
-                    self.report_function(self.metric_name, cntr_value, tags=metric_tags)
-                    if self.instance != ALL_INSTANCES:
-                        break
+                if self.instance == ALL_INSTANCES:
+                    metric_tags.append('{}:{}'.format(self.tag_by, instance_name))
+                self.report_function(self.metric_name, cntr_value, tags=metric_tags)
+                if self.instance != ALL_INSTANCES:
+                    break
 
 
 class SqlFractionMetric(BaseSqlServerMetric):

sqlserver/datadog_checks/sqlserver/sqlserver.py

@@ -701,15 +701,19 @@ def _make_metric_list_to_collect(self, custom_metrics):
         self.instance_metrics = metrics_to_collect
         self.log.debug("metrics to collect %s", metrics_to_collect)
 
-        # create an organized grouping of metric names to their metric classes
+        # create an organized grouping of metric names to their metric classes. Build it up locally and swap it in
+        # at the end so a rebuild drops names that are no longer collected without ever leaving this mapping out of
+        # sync with `instance_metrics`.
+        per_type_metrics = defaultdict(set)
         for m in metrics_to_collect:
             cls = m.__class__.__name__
             name = m.sql_name or m.column
             self.log.debug("Adding metric class %s named %s", cls, name)
 
-            self.instance_per_type_metrics[cls].add(name)
+            per_type_metrics[cls].add(name)
             if m.base_name:
-                self.instance_per_type_metrics[cls].add(m.base_name)
+                per_type_metrics[cls].add(m.base_name)
+        self.instance_per_type_metrics = per_type_metrics
 
     def _add_performance_counters(self, metrics, metrics_to_collect, tags, db=None, physical_database_name=None):
         if db is not None:
@@ -783,6 +787,10 @@ def get_sql_counter_type(self, counter_name):
                         )
                 except Exception as e:
                     self.log.warning("Could not get counter_name of base for metric: %s", e)
+            else:
+                # Counters that need no base counter can be cached right away. Base-requiring counters are only
+                # cached once their base is resolved, so a transient lookup failure is retried instead of pinned.
+                self._sql_counter_types[counter_name] = (sql_counter_type, base_name)
 
         return sql_counter_type, base_name
 

sqlserver/hatch.toml

@@ -43,6 +43,19 @@ COMPOSE_FOLDER = "compose"
 TZ="UTC"
 PIP_EXTRA_INDEX_URL = "https://datadoghq.dev/ci-wheels/bin"
 
+# Benchmarks run against synthetic result sets and need no database, so this env only exists to make
+# `ddev test -b sqlserver` available. Standard `ddev test` runs skip benchmark environments.
+# The version variables are set because the shared test helpers read them at import time; the values
+# are arbitrary since no server is contacted.
+[envs.bench.env-vars]
+ODBCSYSINI = "{root}{/}tests{/}odbc"
+COMPOSE_FOLDER = "compose"
+TZ="UTC"
+PIP_EXTRA_INDEX_URL = "https://datadoghq.dev/ci-wheels/bin"
+SQLSERVER_YEAR = "2022"
+SQLSERVER_MAJOR_VERSION = "16"
+SQLSERVER_ENGINE_EDITION = "express"
+
 [envs.default.overrides]
 env.GITHUB_ACTIONS.e2e-env = { value = false, if = ["true"], platform = ["windows"] }
 platform.windows.env-vars = [

sqlserver/tests/test_bench.py

@@ -0,0 +1,130 @@
+# (C) Datadog, Inc. 2026-present
+# All rights reserved
+# Licensed under a 3-clause BSD style license (see LICENSE)
+import logging
+
+import pytest
+
+from datadog_checks.sqlserver.const import INSTANCE_METRICS, INSTANCE_METRICS_DATABASE
+from datadog_checks.sqlserver.metrics import SqlSimpleMetric
+
+# Databases on the server that are always present, plus the aggregate instance the DMV reports.
+SERVER_DATABASES = ['master', 'tempdb', 'msdb', 'model', '_Total']
+
+# The name columns of sys.dm_os_performance_counters are nchar(128), so values come back blank-padded.
+COUNTER_NAME_WIDTH = 128
+
+# A real logger rather than a mock: mocks record every call, which would accumulate across benchmark
+# rounds and distort both the timing and the memory profile.
+benchmark_logger = logging.getLogger(__name__)
+benchmark_logger.addHandler(logging.NullHandler())
+benchmark_logger.setLevel(logging.WARNING)
+
+
+class StubCursor:
+    """Returns a fixed result set, standing in for the cursor `fetch_all_values` queries."""
+
+    description = [('counter_name',), ('instance_name',), ('object_name',), ('cntr_value',)]
+
+    def __init__(self, rows):
+        self._rows = rows
+
+    def execute(self, *args, **kwargs):
+        pass
+
+    def fetchall(self):
+        return self._rows
+
+
+def _discard_metric(*args, **kwargs):
+    pass
+
+
+def _row(counter_name, instance_name, object_name, cntr_value):
+    return (
+        counter_name.ljust(COUNTER_NAME_WIDTH),
+        instance_name.ljust(COUNTER_NAME_WIDTH),
+        object_name.ljust(COUNTER_NAME_WIDTH),
+        cntr_value,
+    )
+
+
+def _build_dispatch_inputs(num_databases):
+    """Build the metric objects and result set the check would hold for `num_databases` databases.
+
+    Mirrors `_add_performance_counters`: instance-level counters once, then the database-scoped
+    counters once per autodiscovered database. Every counter is treated as a simple metric so the
+    benchmark isolates the `SqlSimpleMetric` dispatch.
+    """
+    databases = ['tenant_db_{:04d}'.format(i) for i in range(num_databases)]
+    base_tags = ['database_hostname:sql-1', 'database_instance:sql-1', 'port:1433']
+
+    metrics = []
+    for name, counter_name, instance_name, object_name in INSTANCE_METRICS:
+        metrics.append(
+            SqlSimpleMetric(
+                {
+                    'name': name,
+                    'counter_name': counter_name,
+                    'instance_name': instance_name,
+                    'object_name': object_name,
+                    'tags': base_tags,
+                },
+                None,
+                _discard_metric,
+                None,
+                benchmark_logger,
+            )
+        )
+    for database in databases:
+        database_tags = base_tags + ['database:{}'.format(database)]
+        for name, counter_name, _, object_name in INSTANCE_METRICS_DATABASE:
+            metrics.append(
+                SqlSimpleMetric(
+                    {
+                        'name': name,
+                        'counter_name': counter_name,
+                        'instance_name': database,
+                        'object_name': object_name,
+                        'physical_db_name': database,
+                        'tags': database_tags,
+                    },
+                    None,
+                    _discard_metric,
+                    None,
+                    benchmark_logger,
+                )
+            )
+
+    # The query filters on counter name only, so database-scoped counters return a row for every
+    # database on the server regardless of which ones autodiscovery selected.
+    rows = [
+        _row(counter_name, instance_name, object_name or 'SQLServer:Generic', 1)
+        for _, counter_name, instance_name, object_name in INSTANCE_METRICS
+    ]
+    for _, counter_name, _, object_name in INSTANCE_METRICS_DATABASE:
+        rows.extend(
+            _row(counter_name, database, object_name or 'SQLServer:Databases', 1)
+            for database in databases + SERVER_DATABASES
+        )
+
+    counter_names = sorted({counter_name for _, counter_name, _, _ in INSTANCE_METRICS + INSTANCE_METRICS_DATABASE})
+    return metrics, rows, counter_names
+
+
+@pytest.mark.parametrize('num_databases', [50, 100, 250, 500])
+def test_simple_metric_dispatch(benchmark, num_databases):
+    """Time one check run's worth of perf-counter dispatch as the database count grows.
+
+    Both the number of metric objects and the number of rows scale with the database count, so this
+    is where a per-metric scan of the full result set becomes quadratic. Compare the growth across
+    the parameters rather than absolute times.
+    """
+    metrics, rows, counter_names = _build_dispatch_inputs(num_databases)
+
+    def dispatch():
+        results, columns = SqlSimpleMetric.fetch_all_values(StubCursor(rows), counter_names, benchmark_logger)
+        for metric in metrics:
+            metric.fetch_metric(results, columns)
+
+    benchmark(dispatch)

sqlserver/tests/test_unit.py

@@ -19,6 +19,7 @@
     ENGINE_EDITION_AZURE_MANAGED_INSTANCE,
     ENGINE_EDITION_SQL_DATABASE,
     ENGINE_EDITION_STANDARD,
+    PERF_COUNTER_BULK_COUNT,
     STATIC_INFO_ENGINE_EDITION,
     STATIC_INFO_FULL_SERVERNAME,
     STATIC_INFO_INSTANCENAME,
@@ -27,7 +28,7 @@
     STATIC_INFO_SERVERNAME,
     STATIC_INFO_VERSION,
 )
-from datadog_checks.sqlserver.metrics import SqlFractionMetric
+from datadog_checks.sqlserver.metrics import SqlFractionMetric, SqlSimpleMetric
 from datadog_checks.sqlserver.schemas import KEY_PREFIX, KEY_PREFIX_PRE_2017, SQLServerSchemaCollector
 from datadog_checks.sqlserver.sqlserver import SQLConnectionError
 from datadog_checks.sqlserver.utils import (
@@ -846,6 +847,138 @@ def test_SqlFractionMetric_group_by_instance(caplog):
     )
 
 
+# sys.dm_os_performance_counters declares the name columns as nchar(128), so every value comes back
+# blank-padded. Pad the fixtures the same way to exercise the stripping.
+def _padded_counter_row(counter_name, instance_name, object_name, cntr_value):
+    return (counter_name.ljust(128), instance_name.ljust(128), object_name.ljust(128), cntr_value)
+
+
+SIMPLE_METRIC_ROWS = [
+    _padded_counter_row('Processes blocked', '', 'SQLServer:General Statistics', 1),
+    _padded_counter_row('Cache Pages', '_Total', 'SQLServer:Plan Cache', 10),
+    _padded_counter_row('Cache Pages', 'SQL Plans', 'SQLServer:Plan Cache', 11),
+    _padded_counter_row('Cache Pages', 'Object Plans', 'SQLServer:Plan Cache', 12),
+    _padded_counter_row('Transaction Delay', 'tenant_a', 'SQLServer:Database Replica', 20),
+    _padded_counter_row('Transaction Delay', 'tenant_a', 'SQLServer:Databases', 21),
+    _padded_counter_row('Log Flushes/sec', '_Total', 'SQLServer:Databases', 30),
+    _padded_counter_row('Log Flushes/sec', 'tenant_a', 'SQLServer:Databases', 31),
+    _padded_counter_row('Log Flushes/sec', 'physical_a', 'SQLServer:Databases', 32),
+]
+
+SIMPLE_METRIC_TAGS = ['optional:tag1', 'dd.internal.resource:database_instance:stubbed.hostname']
+
+
+@pytest.mark.parametrize(
+    'cfg_overrides, expected',
+    [
+        pytest.param(
+            {'counter_name': 'Processes blocked', 'instance_name': ''},
+            [(1, SIMPLE_METRIC_TAGS)],
+            id='instance level counter',
+        ),
+        pytest.param(
+            {'counter_name': 'Log Flushes/sec', 'instance_name': '_Total'},
+            [(30, SIMPLE_METRIC_TAGS)],
+            id='total instance of a per database counter',
+        ),
+        pytest.param(
+            {'counter_name': 'Log Flushes/sec', 'instance_name': 'tenant_a'},
+            [(31, SIMPLE_METRIC_TAGS)],
+            id='per database counter matched by instance_name',
+        ),
+        pytest.param(
+            {'counter_name': 'Log Flushes/sec', 'instance_name': 'tenant_b', 'physical_db_name': 'physical_a'},
+            [(32, SIMPLE_METRIC_TAGS)],
+            id='per database counter matched by physical_db_name',
+        ),
+        pytest.param(
+            {
+                'counter_name': 'Transaction Delay',
+                'instance_name': 'tenant_a',
+                'object_name': 'SQLServer:Databases',
+            },
+            [(21, SIMPLE_METRIC_TAGS)],
+            id='object_name selects among counters sharing a name',
+        ),
+        pytest.param(
+            {'counter_name': 'Transaction Delay', 'instance_name': 'tenant_a', 'object_name': 'SQLServer:Missing'},
+            [],
+            id='object_name matching nothing submits nothing',
+        ),
+        pytest.param(
+            {'counter_name': 'Cache Pages', 'instance_name': 'ALL', 'tag_by': 'plan_type'},
+            [(11, SIMPLE_METRIC_TAGS + ['plan_type:SQL Plans']), (12, SIMPLE_METRIC_TAGS + ['plan_type:Object Plans'])],
+            id='ALL instances tags each instance and skips _Total',
+        ),
+        pytest.param(
+            {
+                'counter_name': 'Transaction Delay',
+                'instance_name': 'ALL',
+                'tag_by': 'db',
+                'object_name': 'SQLServer:Missing',
+            },
+            [(20, SIMPLE_METRIC_TAGS + ['db:tenant_a']), (21, SIMPLE_METRIC_TAGS + ['db:tenant_a'])],
+            id='ALL instances ignores object_name',
+        ),
+        pytest.param(
+            {'counter_name': 'Not Collected', 'instance_name': ''},
+            [],
+            id='counter absent from the result set submits nothing',
+        ),
+        pytest.param(
+            {'counter_name': 'Log Flushes/sec', 'instance_name': 'tenant_missing'},
+            [],
+            id='instance absent from the result set submits nothing',
+        ),
+    ],
+)
+def test_SqlSimpleMetric_fetch_metric(cfg_overrides, expected):
+    mock_cursor = mock.MagicMock()
+    mock_cursor.fetchall.return_value = SIMPLE_METRIC_ROWS
+    mock_cursor.description = [('counter_name',), ('instance_name',), ('object_name',), ('cntr_value',)]
+
+    report_function = mock.MagicMock()
+    cfg_instance = {
+        'name': 'sqlserver.test.metric',
+        'tags': list(SIMPLE_METRIC_TAGS),
+        'hostname': 'stubbed.hostname',
+    }
+    cfg_instance.update(cfg_overrides)
+    metric_obj = SqlSimpleMetric(
+        cfg_instance=cfg_instance,
+        base_name=None,
+        report_function=report_function,
+        column=None,
+        logger=mock.MagicMock(),
+    )
+
+    results, columns = SqlSimpleMetric.fetch_all_values(mock_cursor, [cfg_instance['counter_name']], mock.MagicMock())
+    metric_obj.fetch_metric(results, columns)
+
+    assert report_function.call_args_list == [
+        mock.call('sqlserver.test.metric', value, raw=True, hostname='stubbed.hostname', tags=tags)
+        for value, tags in expected
+    ]
+
+
+def test_get_sql_counter_type_caches_counters_without_a_base(instance_docker):
+    """A counter type is immutable for the lifetime of the server, so it should only be queried once.
+
+    Without caching, rebuilding the metric list queries the type of every counter for every
+    autodiscovered database, which is hundreds of round trips on an instance with many databases.
+    """
+    check = SQLServer(CHECK_NAME, {}, [instance_docker])
+    mock_cursor = mock.MagicMock()
+    mock_cursor.fetchone.return_value = (PERF_COUNTER_BULK_COUNT,)
+    check._connection = mock.MagicMock()
+    check._connection.get_managed_cursor.return_value.__enter__.return_value = mock_cursor
+
+    for _ in range(3):
+        assert check.get_sql_counter_type('Transactions/sec') == (PERF_COUNTER_BULK_COUNT, None)
+
+    assert mock_cursor.execute.call_count == 1
+
+
 def _mock_database_list():
     Row = namedtuple('Row', 'name')
     fetchall_results = [