From 48091f4a57837a0b02379de279dab3a8bb856ba7 Mon Sep 17 00:00:00 2001 From: "eric.weaver" Date: Tue, 18 Aug 2026 13:50:24 +0000 Subject: [PATCH 1/5] [sqlserver] Share one performance counter query and index rows by instance The three metric classes that read sys.dm_os_performance_counters each issued their own query, and scanning that table costs about the same whether it returns two rows or thousands: at 1000 autodiscovered databases the two fraction queries spent 110ms between them to return four rows. They now share a single fetch over the union of their counter names. Grouping rows by counter name left the dispatch quadratic, because every per-database metric still scanned the rows of its own counter looking for its instance. Indexing by instance name as well turns that scan into a lookup, which takes the dispatch from 234ms to 3ms at 1000 databases. Together these cut the steady-state check from 2938ms to 2423ms at 1000 databases, measured on the same instance, and the emitted series and tag sets are unchanged. A fraction metric whose base counter is missing for its instance is now skipped rather than raising, which previously abandoned the remaining metrics for that run. Co-authored-by: Cursor --- sqlserver/datadog_checks/sqlserver/metrics.py | 171 +++++++++--------- .../datadog_checks/sqlserver/sqlserver.py | 74 +++++--- sqlserver/tests/test_unit.py | 99 ++++++++-- 3 files changed, 217 insertions(+), 127 deletions(-) diff --git a/sqlserver/datadog_checks/sqlserver/metrics.py b/sqlserver/datadog_checks/sqlserver/metrics.py index b31ca82e1ae8c..da71e9a3c278a 100644 --- a/sqlserver/datadog_checks/sqlserver/metrics.py +++ b/sqlserver/datadog_checks/sqlserver/metrics.py @@ -87,119 +87,114 @@ def fetch_metric(self, rows, columns, values_cache=None): # https://docs.microsoft.com/en-us/sql/relational-databases/system-dynamic-management-views/sys-dm-os-performance-counters-transact-sql -class SqlSimpleMetric(BaseSqlServerMetric): +class SqlPerfCounterMetric(BaseSqlServerMetric): + """Base class for the metrics read from sys.dm_os_performance_counters. + + Subclasses differ only in how they turn counter values into metrics, so they share a single query and a + single snapshot of the table: scanning it costs the same whether two rows come back or thousands, so a + query per subclass is pure overhead. + """ + TABLE = 'sys.dm_os_performance_counters' - DEFAULT_METRIC_TYPE = None # can be either rate or gauge QUERY_BASE = """select counter_name, instance_name, object_name, cntr_value from {table} where counter_name in ({{placeholders}})""".format(table=TABLE) - OPERATION_NAME = 'simple_metrics' + OPERATION_NAME = 'perf_counter_metrics' @classmethod def fetch_all_values(cls, cursor, counters_list, logger, databases=None, engine_edition=None): 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) + # The name columns are nchar(128), so every value arrives blank-padded. Strip once here and index by + # counter name and instance name so each metric can look its own rows up directly: with autodiscovery + # both the number of metrics and the number of rows grow with the database count, so scanning rows per + # metric is quadratic. + results = defaultdict(lambda: 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)) + results[counter_name.strip()][instance_name.strip()].append((object_name.strip(), cntr_value)) return results, None + def _configured_instance_names(self): + """The instance names this metric collects from, in precedence order. + + The physical database name is a fallback for the logical one and differs from it only on Azure SQL + Database; skip it when the two match so the metric is not reported twice. + """ + if self.physical_db_name and self.physical_db_name != self.instance: + return (self.instance, self.physical_db_name) + return (self.instance,) + + +class SqlSimpleMetric(SqlPerfCounterMetric): + DEFAULT_METRIC_TYPE = None # can be either rate or gauge + 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: - 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 + counters_by_instance = results.get(self.sql_name) + if not counters_by_instance: + return + if self.instance == ALL_INSTANCES: + for instance_name, counters in counters_by_instance.items(): + if instance_name == "_Total": + continue + for _object_name, cntr_value in counters: + metric_tags = list(self.tags) + metric_tags.append('{}:{}'.format(self.tag_by, instance_name)) + self.report_function(self.metric_name, cntr_value, tags=metric_tags) + return -class SqlFractionMetric(BaseSqlServerMetric): - TABLE = 'sys.dm_os_performance_counters' - DEFAULT_METRIC_TYPE = 'gauge' - QUERY_BASE = """select counter_name, cntr_type, cntr_value, instance_name, object_name - from {table} - where counter_name in ({{placeholders}}) - order by cntr_type;""".format(table=TABLE) - OPERATION_NAME = 'fraction_metrics' + for instance_name in self._configured_instance_names(): + for object_name, cntr_value in counters_by_instance.get(instance_name, ()): + if not self.object_name or object_name == self.object_name: + self.report_function(self.metric_name, cntr_value, tags=list(self.tags)) + return - @classmethod - def fetch_all_values(cls, cursor, counters_list, logger, databases=None, engine_edition=None): - placeholders = ', '.join('?' for _ in counters_list) - query = cls.QUERY_BASE.format(placeholders=placeholders) - logger.debug("%s: fetch_all executing query: %s, %s", cls.__name__, query, str(counters_list)) - cursor.execute(query, counters_list) - rows = cursor.fetchall() - results = defaultdict(list) - - for counter_name, cntr_type, cntr_value, instance_name, object_name in rows: - counter_result = { - 'cntr_type': cntr_type, - 'cntr_value': cntr_value, - 'instance_name': instance_name.strip(), - 'object_name': object_name.strip(), - } - logger.debug("Adding new counter_result %s", str(counter_result)) - results[counter_name.strip()].append(counter_result) - return results, None +class SqlFractionMetric(SqlPerfCounterMetric): + DEFAULT_METRIC_TYPE = 'gauge' def fetch_metric(self, results, columns, values_cache=None): if not self.base_name: self.log.error('Skipping counter. Missing base counter name') return - num_counters = results.get(self.sql_name.strip()) - base_counters = results.get(self.base_name.strip()) - if not num_counters or not base_counters: + numerators = results.get(self.sql_name.strip()) + bases = results.get(self.base_name.strip()) + if not numerators or not bases: self.log.error( - 'Skipping counter. Missing numerator and/or base counters \nsql_name=%s \nbase_name=%s \nresults=%s', + 'Skipping counter. Missing numerator and/or base counters \nsql_name=%s \nbase_name=%s', self.sql_name, self.base_name, - str(results), ) return - base_by_key = {} - - # let's organize each base counter by key - for base in base_counters: - key = '{}::{}'.format(base['instance_name'], base['object_name']) - if base_by_key.get(key): - self.log.warning('Found duplicate base counters for key:%s', key) - base_by_key[key] = base - - for numerator in num_counters: - instance_name = numerator['instance_name'] - object_name = numerator['object_name'] - if ( - self.instance != ALL_INSTANCES - and instance_name != self.instance - and instance_name != self.physical_db_name - ): - continue - if self.object_name and self.object_name != object_name: - continue - key = '{}::{}'.format(numerator['instance_name'], numerator['object_name']) - corresponding_base = base_by_key.get(key) - - if not corresponding_base: - self.log.warning( - 'Could not find corresponding base counter for sql_name: %s base_name: %s', - self.sql_name, - self.base_name, - ) - - metric_tags = list(self.tags) - if self.instance == ALL_INSTANCES: - metric_tags.append('{}:{}'.format(self.tag_by, instance_name)) - self.report_fraction( - numerator['cntr_value'], corresponding_base['cntr_value'], metric_tags, previous_values=values_cache - ) + if self.instance == ALL_INSTANCES: + instance_names = list(numerators) + else: + instance_names = self._configured_instance_names() + + for instance_name in instance_names: + for object_name, cntr_value in numerators.get(instance_name, ()): + if self.object_name and self.object_name != object_name: + continue + base_value = self._base_value(bases.get(instance_name), object_name) + if base_value is None: + self.log.warning( + 'Could not find corresponding base counter for sql_name: %s base_name: %s', + self.sql_name, + self.base_name, + ) + continue + + metric_tags = list(self.tags) + if self.instance == ALL_INSTANCES: + metric_tags.append('{}:{}'.format(self.tag_by, instance_name)) + self.report_fraction(cntr_value, base_value, metric_tags, previous_values=values_cache) + + @staticmethod + def _base_value(base_counters, object_name): + """The value of the base counter recorded under the same object as the numerator, if there is one.""" + for base_object_name, base_value in base_counters or (): + if base_object_name == object_name: + return base_value + return None def report_fraction(self, value, base, metric_tags, previous_values): try: @@ -220,8 +215,6 @@ class SqlIncrFractionMetric(SqlFractionMetric): the current value and the base value (denominator) between two collection points that are one second apart. """ - OPERATION_NAME = 'incr_fraction_metrics' - def report_fraction(self, value, base, metric_tags, previous_values): # return if nil is passed as the values cache, as this should be instantiated # at check instantiation diff --git a/sqlserver/datadog_checks/sqlserver/sqlserver.py b/sqlserver/datadog_checks/sqlserver/sqlserver.py index 16e6c48f0f878..e689d581237a2 100644 --- a/sqlserver/datadog_checks/sqlserver/sqlserver.py +++ b/sqlserver/datadog_checks/sqlserver/sqlserver.py @@ -997,31 +997,7 @@ def load_basic_metrics(self, cursor): if self.autodiscover_databases(cursor) or not self.instance_metrics: self._make_metric_list_to_collect(self._config.custom_metrics) - instance_results = {} - engine_edition = self.static_info_cache.get(STATIC_INFO_ENGINE_EDITION, "") - # Execute the `fetch_all` operations first to minimize the database calls - for cls, metric_names in self.instance_per_type_metrics.items(): - if not metric_names: - instance_results[cls] = None, None - else: - try: - db_names = [d.name for d in self.databases] or [ - self.instance.get("database", self.connection.DEFAULT_DATABASE) - ] - metric_cls = getattr(metrics, cls) - with tracked_query(self, operation=metric_cls.OPERATION_NAME): - rows, cols = metric_cls.fetch_all_values( - cursor, - list(metric_names), - self.log, - databases=db_names, - engine_edition=engine_edition, - ) - except Exception as e: - self.log.error("Error running `fetch_all` for metrics %s - skipping. Error: %s", cls, e) - rows, cols = None, None - - instance_results[cls] = rows, cols + instance_results = self._fetch_instance_results(cursor) for metric in self.instance_metrics: key = metric.__class__.__name__ @@ -1035,6 +1011,54 @@ def load_basic_metrics(self, cursor): else: metric.fetch_metric(rows, cols) + def _fetch_instance_results(self, cursor): + """Run the `fetch_all` of every metric class in use, keyed by class name. + + Executing them up front keeps the number of database calls down, and the performance counter classes + share one call between them because they all read the same snapshot of a table that is expensive to + scan however few rows are wanted from it. + """ + instance_results = {} + engine_edition = self.static_info_cache.get(STATIC_INFO_ENGINE_EDITION, "") + perf_counter_classes = [] + perf_counter_names = set() + + for cls, metric_names in self.instance_per_type_metrics.items(): + if not metric_names: + instance_results[cls] = None, None + elif issubclass(getattr(metrics, cls), metrics.SqlPerfCounterMetric): + perf_counter_classes.append(cls) + perf_counter_names.update(metric_names) + else: + instance_results[cls] = self._fetch_all_values(cursor, cls, metric_names, engine_edition) + + if perf_counter_classes: + perf_counter_results = self._fetch_all_values( + cursor, metrics.SqlPerfCounterMetric.__name__, perf_counter_names, engine_edition + ) + for cls in perf_counter_classes: + instance_results[cls] = perf_counter_results + + return instance_results + + def _fetch_all_values(self, cursor, cls, metric_names, engine_edition): + try: + db_names = [d.name for d in self.databases] or [ + self.instance.get("database", self.connection.DEFAULT_DATABASE) + ] + metric_cls = getattr(metrics, cls) + with tracked_query(self, operation=metric_cls.OPERATION_NAME): + return metric_cls.fetch_all_values( + cursor, + list(metric_names), + self.log, + databases=db_names, + engine_edition=engine_edition, + ) + except Exception as e: + self.log.error("Error running `fetch_all` for metrics %s - skipping. Error: %s", cls, e) + return None, None + def collect_metrics(self): """Fetch the metrics from all the associated database tables.""" with self.connection.open_managed_default_connection(KEY_PREFIX): diff --git a/sqlserver/tests/test_unit.py b/sqlserver/tests/test_unit.py index c949e307068c5..0dfedb066bc4b 100644 --- a/sqlserver/tests/test_unit.py +++ b/sqlserver/tests/test_unit.py @@ -28,7 +28,7 @@ STATIC_INFO_SERVERNAME, STATIC_INFO_VERSION, ) -from datadog_checks.sqlserver.metrics import SqlFractionMetric, SqlSimpleMetric +from datadog_checks.sqlserver.metrics import DEFAULT_PERFORMANCE_TABLE, 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 ( @@ -781,12 +781,11 @@ def test_autodiscovery_resets_database_metrics_on_db_addition(instance_autodisco ], ) def test_SqlFractionMetric_base(caplog, base_name): - Row = namedtuple('Row', ['counter_name', 'cntr_type', 'cntr_value', 'instance_name', 'object_name']) fetchall_results = [ - Row('Buffer cache hit ratio', 537003264, 33453, '', 'SQLServer:Buffer Manager'), - Row('Buffer cache hit ratio base', 1073939712, 33531, '', 'SQLServer:Buffer Manager'), - Row('some random counter', 1073939712, 1111, '', 'SQLServer:Buffer Manager'), - Row('some random counter base', 1073939712, 33531, '', 'SQLServer:Buffer Manager'), + _padded_counter_row('Buffer cache hit ratio', '', 'SQLServer:Buffer Manager', 33453), + _padded_counter_row('Buffer cache hit ratio base', '', 'SQLServer:Buffer Manager', 33531), + _padded_counter_row('some random counter', '', 'SQLServer:Buffer Manager', 1111), + _padded_counter_row('some random counter base', '', 'SQLServer:Buffer Manager', 33531), ] mock_cursor = mock.MagicMock() mock_cursor.fetchall.return_value = fetchall_results @@ -823,14 +822,13 @@ def test_SqlFractionMetric_base(caplog, base_name): def test_SqlFractionMetric_group_by_instance(caplog): - Row = namedtuple('Row', ['counter_name', 'cntr_type', 'cntr_value', 'instance_name', 'object_name']) fetchall_results = [ - Row('Buffer cache hit ratio', 537003264, 33453, '', 'SQLServer:Buffer Manager'), - Row('Buffer cache hit ratio base', 1073939712, 33531, '', 'SQLServer:Buffer Manager'), - Row('Foo counter', 537003264, 1, 'bar', 'SQLServer:Buffer Manager'), - Row('Foo counter base', 1073939712, 50, 'bar', 'SQLServer:Buffer Manager'), - Row('Foo counter', 537003264, 5, 'zoo', 'SQLServer:Buffer Manager'), - Row('Foo counter base', 1073939712, 100, 'zoo', 'SQLServer:Buffer Manager'), + _padded_counter_row('Buffer cache hit ratio', '', 'SQLServer:Buffer Manager', 33453), + _padded_counter_row('Buffer cache hit ratio base', '', 'SQLServer:Buffer Manager', 33531), + _padded_counter_row('Foo counter', 'bar', 'SQLServer:Buffer Manager', 1), + _padded_counter_row('Foo counter base', 'bar', 'SQLServer:Buffer Manager', 50), + _padded_counter_row('Foo counter', 'zoo', 'SQLServer:Buffer Manager', 5), + _padded_counter_row('Foo counter base', 'zoo', 'SQLServer:Buffer Manager', 100), ] mock_cursor = mock.MagicMock() mock_cursor.fetchall.return_value = fetchall_results @@ -882,6 +880,48 @@ def _padded_counter_row( return (counter_name.ljust(128), instance_name.ljust(128), object_name.ljust(128), cntr_value) +def test_SqlFractionMetric_skips_instances_without_a_base_counter(): + """An instance whose base counter is missing must be skipped, and the others still reported. + + A base counter recorded for one instance says nothing about another, and dividing by it would report a + wrong value for every instance that has no base of its own. + """ + fetchall_results = [ + _padded_counter_row('Foo counter', 'bar', 'SQLServer:Buffer Manager', 1), + _padded_counter_row('Foo counter', 'zoo', 'SQLServer:Buffer Manager', 5), + _padded_counter_row('Foo counter base', 'zoo', 'SQLServer:Buffer Manager', 100), + ] + mock_cursor = mock.MagicMock() + mock_cursor.fetchall.return_value = fetchall_results + + report_function = mock.MagicMock() + metric_obj = SqlFractionMetric( + cfg_instance={ + 'name': 'sqlserver.test.metric', + 'counter_name': 'Foo counter', + 'instance_name': 'ALL', + 'physical_db_name': None, + 'tags': ['optional:tag1'], + 'hostname': 'stubbed.hostname', + 'tag_by': 'db', + }, + base_name='Foo counter base', + report_function=report_function, + column=None, + logger=mock.MagicMock(), + ) + results, columns = SqlFractionMetric.fetch_all_values( + mock_cursor, ['Foo counter', 'Foo counter base'], mock.MagicMock() + ) + metric_obj.fetch_metric(results, columns) + + assert report_function.call_args_list == [ + mock.call( + 'sqlserver.test.metric', 0.05, raw=True, hostname='stubbed.hostname', tags=['optional:tag1', 'db:zoo'] + ) + ] + + SIMPLE_METRIC_ROWS = [ _padded_counter_row('Processes blocked', '', 'SQLServer:General Statistics', 1), _padded_counter_row('Cache Pages', '_Total', 'SQLServer:Plan Cache', 10), @@ -1008,6 +1048,39 @@ def test_get_sql_counter_type_caches_counters_without_a_base(instance_docker): assert mock_cursor.execute.call_count == 1 +def test_performance_counter_metrics_share_a_single_query(instance_docker): + """The performance counter table must be read once per run, for the counters of every class at once. + + Scanning sys.dm_os_performance_counters costs about as much whether it returns two rows or thousands, so + a query per metric class multiplies the most expensive part of the collection without returning more data. + """ + check = SQLServer(CHECK_NAME, {}, [instance_docker]) + check.databases = {Database('master')} + check.instance_per_type_metrics = { + 'SqlSimpleMetric': {'Transactions/sec'}, + 'SqlFractionMetric': {'Buffer cache hit ratio', 'Buffer cache hit ratio base'}, + 'SqlIncrFractionMetric': {'Average Latch Wait Time (ms)', 'Average Latch Wait Time Base'}, + 'SqlOsWaitStat': {'LCK_M_S'}, + } + mock_cursor = mock.MagicMock() + mock_cursor.fetchall.return_value = [] + + check._fetch_instance_results(mock_cursor) + + executed = [call.args for call in mock_cursor.execute.call_args_list] + perf_counter_queries = [args for args in executed if DEFAULT_PERFORMANCE_TABLE in args[0]] + assert len(perf_counter_queries) == 1 + assert sorted(perf_counter_queries[0][1]) == [ + 'Average Latch Wait Time (ms)', + 'Average Latch Wait Time Base', + 'Buffer cache hit ratio', + 'Buffer cache hit ratio base', + 'Transactions/sec', + ] + # metrics from other tables keep their own query + assert len([args for args in executed if 'sys.dm_os_wait_stats' in args[0]]) == 1 + + def _mock_database_list(): Row = namedtuple('Row', 'name') fetchall_results = [ From 1d828b9bd439d0b93f5d5947769757d3c2650536 Mon Sep 17 00:00:00 2001 From: "eric.weaver" Date: Tue, 18 Aug 2026 20:02:11 +0000 Subject: [PATCH 2/5] Add changelog entry Co-authored-by: Cursor --- sqlserver/changelog.d/24908.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 sqlserver/changelog.d/24908.fixed diff --git a/sqlserver/changelog.d/24908.fixed b/sqlserver/changelog.d/24908.fixed new file mode 100644 index 0000000000000..47d70a7e352cd --- /dev/null +++ b/sqlserver/changelog.d/24908.fixed @@ -0,0 +1 @@ +Further speed up performance counter metric collection on instances with many autodiscovered databases, and skip fraction metrics whose base counter is missing for an instance instead of ending the collection early. From 2115ba6a24d9084c81d5dc36b55c954173c43bf9 Mon Sep 17 00:00:00 2001 From: "eric.weaver" Date: Tue, 18 Aug 2026 20:11:43 +0000 Subject: [PATCH 3/5] Build the instance tag once per instance rather than per row The tag depends only on the instance name, so constructing it in the row loop put it a level below where it belongs. The rows of one counter and instance are almost always a single row, so this is for the reader rather than for speed. Co-authored-by: Cursor --- sqlserver/datadog_checks/sqlserver/metrics.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/sqlserver/datadog_checks/sqlserver/metrics.py b/sqlserver/datadog_checks/sqlserver/metrics.py index da71e9a3c278a..c2eb7a8df7f83 100644 --- a/sqlserver/datadog_checks/sqlserver/metrics.py +++ b/sqlserver/datadog_checks/sqlserver/metrics.py @@ -135,9 +135,8 @@ def fetch_metric(self, results, columns, values_cache=None): for instance_name, counters in counters_by_instance.items(): if instance_name == "_Total": continue + metric_tags = [*self.tags, '{}:{}'.format(self.tag_by, instance_name)] for _object_name, cntr_value in counters: - metric_tags = list(self.tags) - metric_tags.append('{}:{}'.format(self.tag_by, instance_name)) self.report_function(self.metric_name, cntr_value, tags=metric_tags) return @@ -171,6 +170,11 @@ def fetch_metric(self, results, columns, values_cache=None): instance_names = self._configured_instance_names() for instance_name in instance_names: + if self.instance == ALL_INSTANCES: + metric_tags = [*self.tags, '{}:{}'.format(self.tag_by, instance_name)] + else: + metric_tags = list(self.tags) + for object_name, cntr_value in numerators.get(instance_name, ()): if self.object_name and self.object_name != object_name: continue @@ -183,9 +187,6 @@ def fetch_metric(self, results, columns, values_cache=None): ) continue - metric_tags = list(self.tags) - if self.instance == ALL_INSTANCES: - metric_tags.append('{}:{}'.format(self.tag_by, instance_name)) self.report_fraction(cntr_value, base_value, metric_tags, previous_values=values_cache) @staticmethod From 6f5fac9d1aeba68488852e7aed6e8aa93a2d9543 Mon Sep 17 00:00:00 2001 From: "eric.weaver" Date: Tue, 18 Aug 2026 20:39:27 +0000 Subject: [PATCH 4/5] Expect the merged perf counter operation in the internal telemetry assertions The three tracked operations became one, so the integration tests asserting a dd.sqlserver.operation.time series per operation name were looking for tags no longer emitted. Co-authored-by: Cursor --- sqlserver/tests/common.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sqlserver/tests/common.py b/sqlserver/tests/common.py index 99f56e41a9864..51bd9ff3ed8f2 100644 --- a/sqlserver/tests/common.py +++ b/sqlserver/tests/common.py @@ -254,9 +254,7 @@ def get_local_driver(): } OPERATION_TIME_METRICS = [ - 'simple_metrics', - 'fraction_metrics', - 'incr_fraction_metrics', + 'perf_counter_metrics', ] OPERATION_TIME_METRIC_NAME = 'dd.sqlserver.operation.time' From 8b800c902d6d82169c417e4a6ab305329937d4b8 Mon Sep 17 00:00:00 2001 From: "eric.weaver" Date: Tue, 18 Aug 2026 21:08:42 +0000 Subject: [PATCH 5/5] Annotate the new perf counter helpers Names the nested index the dispatch reads from, so the shape is stated once rather than inferred from the loops that walk it. Co-authored-by: Cursor --- sqlserver/datadog_checks/sqlserver/metrics.py | 21 +++++++++++++++---- .../datadog_checks/sqlserver/sqlserver.py | 14 ++++++++++--- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/sqlserver/datadog_checks/sqlserver/metrics.py b/sqlserver/datadog_checks/sqlserver/metrics.py index c2eb7a8df7f83..2ff7239107932 100644 --- a/sqlserver/datadog_checks/sqlserver/metrics.py +++ b/sqlserver/datadog_checks/sqlserver/metrics.py @@ -9,10 +9,11 @@ Collection of metric classes for specific SQL Server tables. """ -from __future__ import division +from __future__ import annotations, division from collections import defaultdict from functools import partial +from typing import Any # Queries ALL_INSTANCES = 'ALL' @@ -86,6 +87,11 @@ def fetch_metric(self, rows, columns, values_cache=None): raise NotImplementedError +# A snapshot of sys.dm_os_performance_counters, indexed for lookup by the two columns a metric knows about +# itself: counter name, then instance name, then the (object name, counter value) of each row underneath. +PerfCounterIndex = dict[str, dict[str, list[tuple[str, int]]]] + + # https://docs.microsoft.com/en-us/sql/relational-databases/system-dynamic-management-views/sys-dm-os-performance-counters-transact-sql class SqlPerfCounterMetric(BaseSqlServerMetric): """Base class for the metrics read from sys.dm_os_performance_counters. @@ -101,7 +107,14 @@ class SqlPerfCounterMetric(BaseSqlServerMetric): OPERATION_NAME = 'perf_counter_metrics' @classmethod - def fetch_all_values(cls, cursor, counters_list, logger, databases=None, engine_edition=None): + def fetch_all_values( + cls, + cursor: Any, + counters_list: list[str], + logger: Any, + databases: list[str] | None = None, + engine_edition: str | None = None, + ) -> tuple[PerfCounterIndex, None]: 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 index by # counter name and instance name so each metric can look its own rows up directly: with autodiscovery @@ -112,7 +125,7 @@ def fetch_all_values(cls, cursor, counters_list, logger, databases=None, engine_ results[counter_name.strip()][instance_name.strip()].append((object_name.strip(), cntr_value)) return results, None - def _configured_instance_names(self): + def _configured_instance_names(self) -> tuple[str, ...]: """The instance names this metric collects from, in precedence order. The physical database name is a fallback for the logical one and differs from it only on Azure SQL @@ -190,7 +203,7 @@ def fetch_metric(self, results, columns, values_cache=None): self.report_fraction(cntr_value, base_value, metric_tags, previous_values=values_cache) @staticmethod - def _base_value(base_counters, object_name): + def _base_value(base_counters: list[tuple[str, int]] | None, object_name: str) -> int | None: """The value of the base counter recorded under the same object as the numerator, if there is one.""" for base_object_name, base_value in base_counters or (): if base_object_name == object_name: diff --git a/sqlserver/datadog_checks/sqlserver/sqlserver.py b/sqlserver/datadog_checks/sqlserver/sqlserver.py index e689d581237a2..cad081b04610e 100644 --- a/sqlserver/datadog_checks/sqlserver/sqlserver.py +++ b/sqlserver/datadog_checks/sqlserver/sqlserver.py @@ -2,11 +2,13 @@ # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) -from __future__ import division +from __future__ import annotations, division import functools import time from collections import defaultdict +from collections.abc import Iterable +from typing import Any from cachetools import TTLCache @@ -128,6 +130,10 @@ KEY_PREFIX = "dbm-sqlserver-" +# What a metric class's `fetch_all_values` hands back: the rows it fetched, in whatever shape that class +# dispatches from, and their column names. Classes that index their own rows return None for the columns. +MetricFetchResult = tuple[Any, list[str] | None] + class SQLServer(DatabaseCheck): DBMS = "sqlserver" @@ -1011,7 +1017,7 @@ def load_basic_metrics(self, cursor): else: metric.fetch_metric(rows, cols) - def _fetch_instance_results(self, cursor): + def _fetch_instance_results(self, cursor: Any) -> dict[str, MetricFetchResult]: """Run the `fetch_all` of every metric class in use, keyed by class name. Executing them up front keeps the number of database calls down, and the performance counter classes @@ -1041,7 +1047,9 @@ def _fetch_instance_results(self, cursor): return instance_results - def _fetch_all_values(self, cursor, cls, metric_names, engine_edition): + def _fetch_all_values( + self, cursor: Any, cls: str, metric_names: Iterable[str], engine_edition: str + ) -> MetricFetchResult: try: db_names = [d.name for d in self.databases] or [ self.instance.get("database", self.connection.DEFAULT_DATABASE)