Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sqlserver/changelog.d/24952.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Restrict instance-level file stats, database stats, and database backup metrics to the databases selected by autodiscovery.
16 changes: 16 additions & 0 deletions sqlserver/datadog_checks/sqlserver/database_metrics/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from datadog_checks.sqlserver.config import SQLServerConfig
from datadog_checks.sqlserver.const import STATIC_INFO_ENGINE_EDITION, STATIC_INFO_MAJOR_VERSION, STATIC_INFO_RDS

SQLSERVER_PARAMETER_LIMIT = 2100


class SqlserverDatabaseMetricsBase:
def __init__(
Expand Down Expand Up @@ -57,6 +59,20 @@ def queries(self) -> List[dict]:
def databases(self) -> Optional[List[str]]:
return self._databases

def _database_filters(self, column: str) -> list[tuple[str, tuple[str, ...]]]:
# None disables filtering; an empty list means autodiscovery selected no databases.
if self.databases is None:
return [("", ())]
if not self.databases:
return [("1 = 0", ())]
Comment thread
jasonmp85 marked this conversation as resolved.

filters = []
for start in range(0, len(self.databases), SQLSERVER_PARAMETER_LIMIT):
params = tuple(self.databases[start : start + SQLSERVER_PARAMETER_LIMIT])
placeholders = ", ".join("?" for _ in params)
filters.append((f"{column} IN ({placeholders})", params))
return filters

@property
def query_executors(self) -> List[QueryExecutor]:
'''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,19 @@ def collection_interval(self) -> int:
def queries(self):
# make a copy of the query to avoid modifying the original
# in case different instances have different collection intervals
query = DATABASE_BACKUP_METRICS_QUERY.copy()
query['collection_interval'] = self.collection_interval
return [query]
queries = []
for database_filter, params in self._database_filters("sys.databases.name"):
query = DATABASE_BACKUP_METRICS_QUERY.copy()
if database_filter:
query['query'] = query['query'].replace(
" group by sys.databases.name",
f" where {database_filter}\n group by sys.databases.name",
)
if params:
query['params'] = params
query['collection_interval'] = self.collection_interval
queries.append(query)
return queries

def __repr__(self) -> str:
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,15 @@ def enabled(self):

@property
def queries(self):
return [DATABASE_STATS_METRICS_QUERY]
queries = []
for database_filter, params in self._database_filters("name"):
query = DATABASE_STATS_METRICS_QUERY.copy()
if database_filter:
query['query'] += f" WHERE {database_filter}"
if params:
query['params'] = params
queries.append(query)
return queries

def __repr__(self) -> str:
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ def enabled(self):

@property
def queries(self):
return [self.__get_query_file_stats()]
return [
self.__get_query_file_stats(database_filter, params)
for database_filter, params in self._database_filters("DB_NAME(fs.database_id)")
]

def __get_query_file_stats(self) -> dict:
def __get_query_file_stats(self, database_filter: str, params: tuple[str, ...]) -> dict:
"""
Construct the dm_io_virtual_file_stats QueryExecutor configuration based on the SQL Server major version
:return: a QueryExecutor query config object
Expand Down Expand Up @@ -65,9 +68,12 @@ def __get_query_file_stats(self) -> dict:
sql_columns.append("fs.{}".format(column))
metric_columns.append(column_definitions[column])

query_filter = ""
query_filters = []
if database_filter:
query_filters.append(database_filter)
if self.major_version >= 16:
query_filter = "WHERE DB_NAME(fs.database_id) not like 'model_%'"
query_filters.append("DB_NAME(fs.database_id) not like 'model_%'")
query_filter = f"WHERE {' AND '.join(query_filters)}" if query_filters else ""

query = """
SELECT
Expand All @@ -93,10 +99,10 @@ def __get_query_file_stats(self) -> dict:
{sql_columns}
FROM sys.dm_io_virtual_file_stats(DB_ID(), NULL) fs
LEFT JOIN sys.database_files df
ON df.file_id = fs.file_id;
ON df.file_id = fs.file_id {filter};
"""

return {
query_config = {
"name": "sys.dm_io_virtual_file_stats",
"query": query.strip().format(sql_columns=", ".join(sql_columns), filter=query_filter),
"columns": [
Expand All @@ -107,3 +113,6 @@ def __get_query_file_stats(self) -> dict:
]
+ metric_columns,
}
if params:
query_config["params"] = params
return query_config
21 changes: 16 additions & 5 deletions sqlserver/datadog_checks/sqlserver/sqlserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -586,14 +586,15 @@ def autodiscover_databases(self, cursor):

self.log.debug("Resulting filtered databases: %s", filtered_dbs)
self._ad_last_check = now
if filtered_dbs != self.databases:
databases_changed = filtered_dbs != self.databases
if databases_changed:
self.log.debug("Databases updated from previous autodiscovery check.")
if self._ad_initial_discovery_done and self._database_metrics is not None:
self.log.info("Invalidating database metrics cache due to database list change.")
self._database_metrics = None
self._ad_initial_discovery_done = True
self.databases = filtered_dbs
return True
self._ad_initial_discovery_done = True
return databases_changed
return False

def _get_autodiscovery_query_cached(self, cursor):
Expand Down Expand Up @@ -978,11 +979,21 @@ def database_metrics(self):

self._database_metrics = []
# list of database names to collect metrics for
db_names = [d.name for d in self.databases] or [self.instance.get('database', self.connection.DEFAULT_DATABASE)]
autodiscovered_db_names = [d.name for d in self.databases]
db_names = autodiscovered_db_names or [self.instance.get('database', self.connection.DEFAULT_DATABASE)]

# instance level metrics
for database_metric_class in self._instance_level_database_metrics:
self._database_metrics.append(self._new_database_metric_executor(database_metric_class))
filter_databases = self._config.autodiscovery and database_metric_class in (
SqlserverFileStatsMetrics,
SqlserverDatabaseStatsMetrics,
SqlserverDatabaseBackupMetrics,
)
self._database_metrics.append(
self._new_database_metric_executor(
database_metric_class, autodiscovered_db_names if filter_databases else None
)
)

# database level metrics
for database_metric_class in self._database_level_database_metrics:
Expand Down
69 changes: 69 additions & 0 deletions sqlserver/tests/test_database_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,75 @@
STATIC_INFO_MAJOR_VERSION: SQLSERVER_MAJOR_VERSION,
}

AUTODISCOVERY_FILTERED_INSTANCE_METRICS = [
'sqlserver.files.size_on_disk',
'sqlserver.database.user_access',
'sqlserver.database.backup_count',
]
SQLSERVER_PARAMETER_LIMIT = 2100


@pytest.mark.unit
@pytest.mark.parametrize(
'database_metric_class',
[SqlserverFileStatsMetrics, SqlserverDatabaseStatsMetrics, SqlserverDatabaseBackupMetrics],
)
def test_instance_level_database_metrics_stay_within_parameter_limit(
init_config,
instance_docker_metrics,
database_metric_class,
):
databases = [f'database_{index}' for index in range(SQLSERVER_PARAMETER_LIMIT + 1)]
sqlserver_check = SQLServer(CHECK_NAME, init_config, [instance_docker_metrics])
database_metrics = database_metric_class(
config=sqlserver_check._config,
new_query_executor=sqlserver_check._new_query_executor,
server_static_info=STATIC_SERVER_INFO,
execute_query_handler=sqlserver_check.execute_query_raw,
databases=databases,
)

query_params = [query.get('params', ()) for query in database_metrics.queries]

assert all(len(params) <= SQLSERVER_PARAMETER_LIMIT for params in query_params)
assert [database for params in query_params for database in params] == databases


@pytest.mark.integration
@pytest.mark.usefixtures('dd_environment')
@pytest.mark.parametrize('metric_name', AUTODISCOVERY_FILTERED_INSTANCE_METRICS)
def test_instance_level_database_metrics_respect_autodiscovery(
aggregator,
dd_run_check,
init_config,
instance_docker_metrics,
metric_name,
):
instance_docker_metrics['database_autodiscovery'] = True
instance_docker_metrics['autodiscovery_include'] = ['master']

sqlserver_check = SQLServer(CHECK_NAME, init_config, [instance_docker_metrics])
dd_run_check(sqlserver_check)

aggregator.assert_metric_has_tag(metric_name, 'db:master')
aggregator.assert_metric_has_tag(metric_name, 'db:msdb', count=0)


@pytest.mark.integration
@pytest.mark.usefixtures('dd_environment')
def test_instance_level_database_metrics_remain_unfiltered_without_autodiscovery(
aggregator,
dd_run_check,
init_config,
instance_docker_metrics,
):
sqlserver_check = SQLServer(CHECK_NAME, init_config, [instance_docker_metrics])
dd_run_check(sqlserver_check)

for metric_name in AUTODISCOVERY_FILTERED_INSTANCE_METRICS:
aggregator.assert_metric_has_tag(metric_name, 'db:master')
aggregator.assert_metric_has_tag(metric_name, 'db:msdb')


@pytest.mark.integration
@pytest.mark.usefixtures('dd_environment')
Expand Down
26 changes: 26 additions & 0 deletions sqlserver/tests/test_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
STATIC_INFO_SERVERNAME,
STATIC_INFO_VERSION,
)
from datadog_checks.sqlserver.database_metrics import SqlserverDatabaseStatsMetrics
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
Expand Down Expand Up @@ -773,6 +774,31 @@ def test_autodiscovery_resets_database_metrics_on_db_addition(instance_autodisco
assert check._database_metrics is None


def test_autodiscovery_resets_database_metrics_after_initial_empty_result(instance_autodiscovery):
"""Database metric executors must refresh when an initially empty discovery later finds a database."""
_, mock_cursor = _mock_database_list()
instance_autodiscovery['autodiscovery_include'] = ['newdb$']
check = SQLServer(CHECK_NAME, {}, [instance_autodiscovery])

changed = check.autodiscover_databases(mock_cursor)
assert changed is False
assert check.databases == set()

assert check.database_metrics

Row = namedtuple('Row', 'name')
mock_cursor.fetchall.return_value = iter([Row('newdb')])
check._ad_last_check = 0

changed = check.autodiscover_databases(mock_cursor)
assert changed is True
assert check.databases == {Database('newdb')}
database_stats_metrics = next(
metric for metric in check.database_metrics if isinstance(metric, SqlserverDatabaseStatsMetrics)
)
assert [query.get('params') for query in database_stats_metrics.queries] == [('newdb',)]


@pytest.mark.parametrize(
'base_name',
[
Expand Down
Loading