Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate ignore_missing_database #17364

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Deprecate ignore_missing_database #17364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 0 additions & 47 deletions sqlserver/datadog_checks/sqlserver/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,6 @@ def close_cursor(self, cursor):
except Exception as e:
self.log.warning("Could not close adodbapi cursor\n%s", e)

def check_database(self):
with self.open_managed_default_database():
db_exists, context = self._check_db_exists()

return db_exists, context

def check_database_conns(self, db_name):
self.open_db_connections(None, db_name=db_name, is_default=False)
self.close_db_connections(None, db_name)
Expand Down Expand Up @@ -338,47 +332,6 @@ def close_db_connections(self, db_key, db_name=None, key_prefix=None):
except Exception as e:
self.log.warning("Could not close adodbapi db connection\n%s", e)

def _check_db_exists(self):
"""
Check for existence of a database, but take into consideration whether the db is case-sensitive or not.

If not case-sensitive, then we normalize the database name to lowercase on both sides and check.
If case-sensitive, then we only accept exact-name matches.

If the check fails, then we won't do any checks if `ignore_missing_database` is enabled, or we will fail
with a ConfigurationError otherwise.
"""

_, host, _, _, database, _ = self._get_access_info(self.DEFAULT_DB_KEY)
context = "{} - {}".format(host, database)
if self.existing_databases is None:
cursor = self.get_cursor(None, self.DEFAULT_DATABASE)

try:
self.existing_databases = {}
cursor.execute(DATABASE_EXISTS_QUERY)
for row in cursor.fetchall():
# collation_name can be NULL if db offline, in that case assume its case_insensitive
case_insensitive = not row.collation_name or 'CI' in row.collation_name
self.existing_databases[row.name.lower()] = (
case_insensitive,
row.name,
)

except Exception as e:
self.log.error("Failed to check if database %s exists: %s", database, e)
return False, context
finally:
self.close_cursor(cursor)

exists = False
if database.lower() in self.existing_databases:
case_insensitive, cased_name = self.existing_databases[database.lower()]
if case_insensitive or database == cased_name:
exists = True

return exists, context

def _get_connector(self, init_config, instance_config):
'''
Get the connector to use for the instance.
Expand Down
15 changes: 0 additions & 15 deletions sqlserver/datadog_checks/sqlserver/sqlserver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,21 +332,6 @@ def initialize_connection(self):
def make_metric_list_to_collect(self):
# Pre-process the list of metrics to collect
try:
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:
Expand Down
6 changes: 0 additions & 6 deletions sqlserver/tests/test_metrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ 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(
Expand Down Expand Up @@ -248,8 +247,6 @@ def test_check_index_usage_metrics(

tags = instance_docker_metrics.get('tags', [])

check_sqlserver_can_connect(aggregator, instance_docker_metrics['host'], sqlserver_check.resolved_hostname, tags)

for metric_name in DATABASE_INDEX_METRICS:
expected_tags = tags + [
'db:{}'.format(instance_docker_metrics['database']),
Expand Down Expand Up @@ -389,7 +386,6 @@ 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()
Expand Down Expand Up @@ -425,8 +421,6 @@ def test_check_incr_fraction_metrics(

tags = instance_docker_metrics.get('tags', [])

check_sqlserver_can_connect(aggregator, instance_docker_metrics['host'], sqlserver_check.resolved_hostname, tags)

for metric_name in INCR_FRACTION_METRICS:
key = "{}:{}".format(metric_name, "".join(tags))
if previous_value[key] == sqlserver_check.sqlserver_incr_fraction_metric_previous_values[key]:
Expand Down
74 changes: 0 additions & 74 deletions sqlserver/tests/test_unit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ 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.open_managed_default_connection',
Expand All @@ -52,79 +51,6 @@ def test_missing_db(instance_docker, dd_run_check):
check.initialize_connection()
check.make_metric_list_to_collect()

instance['ignore_missing_database'] = True
with mock.patch('datadog_checks.sqlserver.connection.Connection.check_database', return_value=(False, 'db')):
check = SQLServer(CHECK_NAME, {}, [instance])
check.initialize_connection()
check.make_metric_list_to_collect()
dd_run_check(check)
assert check.do_check is False


@mock.patch('datadog_checks.sqlserver.connection.Connection.open_managed_default_database')
@mock.patch('datadog_checks.sqlserver.connection.Connection.get_cursor')
def test_db_exists(get_cursor, mock_connect, instance_docker_defaults, dd_run_check):
Row = namedtuple('Row', 'name,collation_name')
db_results = [
Row('master', 'SQL_Latin1_General_CP1_CI_AS'),
Row('tempdb', 'SQL_Latin1_General_CP1_CI_AS'),
Row('AdventureWorks2017', 'SQL_Latin1_General_CP1_CI_AS'),
Row('CaseSensitive2018', 'SQL_Latin1_General_CP1_CS_AS'),
Row('OfflineDB', None),
]

mock_connect.__enter__ = mock.Mock(return_value='foo')

mock_results = mock.MagicMock()
mock_results.fetchall.return_value = db_results
get_cursor.return_value = mock_results

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])
check.initialize_connection()
check.make_metric_list_to_collect()
assert check.do_check is True

# check mixed case against mixed case but case-insensitive db
instance['database'] = 'AdventureWORKS2017'
check = SQLServer(CHECK_NAME, {}, [instance])
check.initialize_connection()
check.make_metric_list_to_collect()
assert check.do_check is True

# check case sensitive but matched db
instance['database'] = 'CaseSensitive2018'
check = SQLServer(CHECK_NAME, {}, [instance])
check.initialize_connection()
check.make_metric_list_to_collect()
assert check.do_check is True

# check case sensitive but mismatched db
instance['database'] = 'cASEsENSITIVE2018'
check = SQLServer(CHECK_NAME, {}, [instance])
check.initialize_connection()
check.make_metric_list_to_collect()
assert check.do_check is False

# check offline but exists db
instance['database'] = 'Offlinedb'
check = SQLServer(CHECK_NAME, {}, [instance])
check.initialize_connection()
check.make_metric_list_to_collect()
assert check.do_check is True


@mock.patch('datadog_checks.sqlserver.connection.Connection.open_managed_default_database')
@mock.patch('datadog_checks.sqlserver.connection.Connection.get_cursor')
def test_azure_cross_database_queries_excluded(get_cursor, mock_connect, instance_docker_defaults, dd_run_check):
Expand Down