diff --git a/sqlserver/assets/configuration/spec.yaml b/sqlserver/assets/configuration/spec.yaml index 11088e12ddc40..18f5bd6bc3724 100644 --- a/sqlserver/assets/configuration/spec.yaml +++ b/sqlserver/assets/configuration/spec.yaml @@ -686,7 +686,9 @@ files: type: string example: master - name: ignore_missing_database - description: If the DB specified doesn't exist on the server then don't do the check + description: | + DEPRECATED - use `database_autodiscovery` instead, when database name is uncertain. + If the DB specified doesn't exist on the server then don't do the check value: type: boolean example: false diff --git a/sqlserver/changelog.d/17300.fixed b/sqlserver/changelog.d/17300.fixed new file mode 100644 index 0000000000000..63e4d0a501aba --- /dev/null +++ b/sqlserver/changelog.d/17300.fixed @@ -0,0 +1 @@ +Remove db exist check if ignore_missing_database is set to false. In this case throw SQLConnectionError instead of ConfigurationError when database does not exist. diff --git a/sqlserver/datadog_checks/sqlserver/config.py b/sqlserver/datadog_checks/sqlserver/config.py index fa8a20b86893b..d37297c36f59d 100644 --- a/sqlserver/datadog_checks/sqlserver/config.py +++ b/sqlserver/datadog_checks/sqlserver/config.py @@ -29,6 +29,12 @@ def __init__(self, init_config, instance, log): self.include_index_usage_metrics_tempdb: bool = is_affirmative( instance.get('include_index_usage_metrics_tempdb', False) ) + self.ignore_missing_database = is_affirmative(instance.get("ignore_missing_database", False)) + if self.ignore_missing_database: + self.log.warning( + "The parameter 'ignore_missing_database' is deprecated" + "if you are unsure about the database name please use 'database_autodiscovery'" + ) # DBM self.dbm_enabled: bool = is_affirmative(instance.get('dbm', False)) diff --git a/sqlserver/datadog_checks/sqlserver/data/conf.yaml.example b/sqlserver/datadog_checks/sqlserver/data/conf.yaml.example index f28d03cb33c28..303df58f2e522 100644 --- a/sqlserver/datadog_checks/sqlserver/data/conf.yaml.example +++ b/sqlserver/datadog_checks/sqlserver/data/conf.yaml.example @@ -644,6 +644,7 @@ instances: # proc_only_if_database: master ## @param ignore_missing_database - boolean - optional - default: false + ## DEPRECATED - use `database_autodiscovery` instead, when database name is uncertain. ## If the DB specified doesn't exist on the server then don't do the check # # ignore_missing_database: false diff --git a/sqlserver/datadog_checks/sqlserver/sqlserver.py b/sqlserver/datadog_checks/sqlserver/sqlserver.py index 56c6f0b44f189..b55882a1fdd3f 100644 --- a/sqlserver/datadog_checks/sqlserver/sqlserver.py +++ b/sqlserver/datadog_checks/sqlserver/sqlserver.py @@ -11,7 +11,7 @@ import six from cachetools import TTLCache -from datadog_checks.base import AgentCheck, ConfigurationError +from datadog_checks.base import AgentCheck from datadog_checks.base.config import is_affirmative from datadog_checks.base.utils.db import QueryExecutor, QueryManager from datadog_checks.base.utils.db.utils import default_json_event_encoding, resolve_db_host, tracked_query @@ -332,31 +332,27 @@ def initialize_connection(self): def make_metric_list_to_collect(self): # Pre-process the list of metrics to collect try: - # check to see if the database exists before we try any connections to it - db_exists, context = self.connection.check_database() - if db_exists: - if self.instance.get('stored_procedure') is None: - with self.connection.open_managed_default_connection(): - with self.connection.get_managed_cursor() as cursor: - self.autodiscover_databases(cursor) - self._make_metric_list_to_collect(self._config.custom_metrics) - else: - # How much do we care that the DB doesn't exist? - ignore = is_affirmative(self.instance.get("ignore_missing_database", False)) - if ignore is not None and ignore: - # not much : we expect it. leave checks disabled - self.do_check = False - self.log.warning("Database %s does not exist. Disabling checks for this instance.", context) - else: - # yes we do. Keep trying - msg = "Database {} does not exist. Please resolve invalid database and restart agent".format( - context - ) - raise ConfigurationError(msg) - - except SQLConnectionError as e: - self.log.exception("Error connecting to database: %s", e) - except ConfigurationError: + if self._config.ignore_missing_database: + # self.connection.check_database() will try to connect to 'master'. + # If this is an Azure SQL Database this function will throw. + # For this reason we avoid calling self.connection.check_database() + # for this config as it will be a false negative. + engine_edition = self.static_info_cache.get(STATIC_INFO_ENGINE_EDITION) + if not is_azure_sql_database(engine_edition): + # Do the database exist check that will allow to disable _check as a whole + # as otherwise the first call to open_managed_default_connection will throw the + # SQLConnectionError. + db_exists, context = self.connection.check_database() + if not db_exists: + self.do_check = False + self.log.warning("Database %s does not exist. Disabling checks for this instance.", context) + return + if self.instance.get('stored_procedure') is None: + with self.connection.open_managed_default_connection(): + with self.connection.get_managed_cursor() as cursor: + self.autodiscover_databases(cursor) + self._make_metric_list_to_collect(self._config.custom_metrics) + except SQLConnectionError: raise except Exception as e: self.log.exception("Initialization exception %s", e) diff --git a/sqlserver/tests/test_metrics.py b/sqlserver/tests/test_metrics.py index b9118eb03cc75..d02d8bb29f4f0 100644 --- a/sqlserver/tests/test_metrics.py +++ b/sqlserver/tests/test_metrics.py @@ -203,6 +203,7 @@ def test_check_index_usage_metrics( ): instance_docker_metrics['database'] = 'datadog_test' instance_docker_metrics['include_index_usage_metrics'] = True + instance_docker_metrics['ignore_missing_database'] = True # Cause an index seek bob_conn.execute_with_retries( @@ -379,6 +380,7 @@ def test_check_incr_fraction_metrics( bob_conn_raw, ): instance_docker_metrics['database'] = 'datadog_test' + instance_docker_metrics['ignore_missing_database'] = True sqlserver_check = SQLServer(CHECK_NAME, init_config, [instance_docker_metrics]) sqlserver_check.run() diff --git a/sqlserver/tests/test_unit.py b/sqlserver/tests/test_unit.py index dd0af7b9c4bf4..0f65e631a01cc 100644 --- a/sqlserver/tests/test_unit.py +++ b/sqlserver/tests/test_unit.py @@ -9,7 +9,6 @@ import mock import pytest -from datadog_checks.base.errors import ConfigurationError from datadog_checks.dev import EnvVars from datadog_checks.sqlserver import SQLServer from datadog_checks.sqlserver.connection import split_sqlserver_host_port @@ -43,8 +42,12 @@ def test_get_cursor(instance_docker): def test_missing_db(instance_docker, dd_run_check): instance = copy.copy(instance_docker) instance['ignore_missing_database'] = False - with mock.patch('datadog_checks.sqlserver.connection.Connection.check_database', return_value=(False, 'db')): - with pytest.raises(ConfigurationError): + + with mock.patch( + 'datadog_checks.sqlserver.connection.Connection.open_managed_default_connection', + side_effect=SQLConnectionError(Exception("couldnt connect")), + ): + with pytest.raises(SQLConnectionError): check = SQLServer(CHECK_NAME, {}, [instance]) check.initialize_connection() check.make_metric_list_to_collect() @@ -79,13 +82,13 @@ def test_db_exists(get_cursor, mock_connect, instance_docker_defaults, dd_run_ch instance = copy.copy(instance_docker_defaults) # make sure check doesn't try to add metrics instance['stored_procedure'] = 'fake_proc' + instance['ignore_missing_database'] = True # check base case of lowercase for lowercase and case-insensitive db check = SQLServer(CHECK_NAME, {}, [instance]) check.initialize_connection() check.make_metric_list_to_collect() assert check.do_check is True - # check all caps for case insensitive db instance['database'] = 'MASTER' check = SQLServer(CHECK_NAME, {}, [instance]) @@ -110,9 +113,9 @@ def test_db_exists(get_cursor, mock_connect, instance_docker_defaults, dd_run_ch # check case sensitive but mismatched db instance['database'] = 'cASEsENSITIVE2018' check = SQLServer(CHECK_NAME, {}, [instance]) - with pytest.raises(ConfigurationError): - check.initialize_connection() - check.make_metric_list_to_collect() + check.initialize_connection() + check.make_metric_list_to_collect() + assert check.do_check is False # check offline but exists db instance['database'] = 'Offlinedb'