Skip to content

Commit

Permalink
implement disable generic tags for sqlserver (#10290)
Browse files Browse the repository at this point in the history
* implement disable generic tags

* change tags

* change Windows expected tags

* fix style

* Update sqlserver/assets/configuration/spec.yaml

Co-authored-by: ruthnaebeck <19349244+ruthnaebeck@users.noreply.github.com>

* change desc

* Bump base check requirement

Co-authored-by: ruthnaebeck <19349244+ruthnaebeck@users.noreply.github.com>
  • Loading branch information
HantingZhang2 and ruthnaebeck authored Oct 1, 2021
1 parent e50bfd1 commit c8a934f
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 12 deletions.
6 changes: 6 additions & 0 deletions sqlserver/assets/configuration/spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,12 @@ files:
type: boolean
example: false
- template: instances/default
overrides:
disable_generic_tags.hidden: False
disable_generic_tags.enabled: True
disable_generic_tags.description: |
Generic tags such as `host` are replaced by `sqlserver_host` to avoid
getting mixed with other integraton tags.
- template: logs
example:
Expand Down
6 changes: 6 additions & 0 deletions sqlserver/datadog_checks/sqlserver/data/conf.yaml.example
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,12 @@ instances:
#
# empty_default_hostname: false

## @param disable_generic_tags - boolean - optional - default: false
## Generic tags such as `host` are replaced by `sqlserver_host` to avoid
## getting mixed with other integraton tags.
#
disable_generic_tags: true

## Log Section
##
## type - required - Type of log input source (tcp / udp / file / windows_event)
Expand Down
7 changes: 5 additions & 2 deletions sqlserver/datadog_checks/sqlserver/sqlserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,13 @@ def initialize_connection(self):

def handle_service_check(self, status, host, database, message=None, is_default=True):
custom_tags = self.instance.get("tags", [])
disable_generic_tags = self.instance.get('disable_generic_tags', False)
if custom_tags is None:
custom_tags = []

service_check_tags = ['host:{}'.format(host), 'db:{}'.format(database)]
if disable_generic_tags:
service_check_tags = ['sqlserver_host:{}'.format(host), 'db:{}'.format(database)]
else:
service_check_tags = ['host:{}'.format(host), 'sqlserver_host:{}'.format(host), 'db:{}'.format(database)]
service_check_tags.extend(custom_tags)
service_check_tags = list(set(service_check_tags))

Expand Down
2 changes: 1 addition & 1 deletion sqlserver/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def get_dependencies():
return f.readlines()


CHECKS_BASE_REQ = 'datadog-checks-base>=16.7.0'
CHECKS_BASE_REQ = 'datadog-checks-base>=21.2.0'

setup(
name='datadog-sqlserver',
Expand Down
4 changes: 4 additions & 0 deletions sqlserver/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def get_local_driver():
'include_fci_metrics': True,
'include_ao_metrics': False,
'include_master_files_metrics': True,
'disable_generic_tags': True,
}

INSTANCE_AO_DOCKER_SECONDARY = {
Expand All @@ -92,6 +93,7 @@ def get_local_driver():
'password': 'Password123',
'tags': ['optional:tag1'],
'include_ao_metrics': True,
'disable_generic_tags': True,
}

CUSTOM_QUERY_A = {
Expand All @@ -113,6 +115,7 @@ def get_local_driver():
'host': LOCAL_SERVER,
'username': 'sa',
'password': 'Password12!',
'disable_generic_tags': True,
}
INSTANCE_SQL2017 = INSTANCE_SQL2017_DEFAULTS.copy()
INSTANCE_SQL2017.update(
Expand All @@ -124,6 +127,7 @@ def get_local_driver():
'include_fci_metrics': True,
'include_ao_metrics': False,
'include_master_files_metrics': True,
'disable_generic_tags': True,
}
)

Expand Down
17 changes: 11 additions & 6 deletions sqlserver/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def test_check_invalid_password(aggregator, dd_run_check, init_config, instance_
aggregator.assert_service_check(
'sqlserver.can_connect',
status=sqlserver_check.CRITICAL,
tags=['host:localhost,1433', 'db:master', 'optional:tag1'],
tags=['sqlserver_host:localhost,1433', 'db:master', 'optional:tag1'],
)


Expand All @@ -42,7 +42,10 @@ def test_check_invalid_password(aggregator, dd_run_check, init_config, instance_
def test_check_docker(aggregator, dd_run_check, init_config, instance_docker):
sqlserver_check = SQLServer(CHECK_NAME, init_config, [instance_docker])
dd_run_check(sqlserver_check)
expected_tags = instance_docker.get('tags', []) + ['host:{}'.format(instance_docker.get('host')), 'db:master']
expected_tags = instance_docker.get('tags', []) + [
'sqlserver_host:{}'.format(instance_docker.get('host')),
'db:master',
]
assert_metrics(aggregator, expected_tags)


Expand Down Expand Up @@ -230,14 +233,16 @@ def test_autodiscovery_db_service_checks(aggregator, dd_run_check, instance_auto

# verify that the old status check returns OK
aggregator.assert_service_check(
'sqlserver.can_connect', tags=['db:master', 'optional:tag1', 'host:localhost,1433'], status=SQLServer.OK
'sqlserver.can_connect',
tags=['db:master', 'optional:tag1', 'sqlserver_host:localhost,1433'],
status=SQLServer.OK,
)

# verify all databses in autodiscovery have a service check
for database in instance_autodiscovery['autodiscovery_include']:
aggregator.assert_service_check(
'sqlserver.database.can_connect',
tags=['db:{}'.format(database), 'optional:tag1', 'host:localhost,1433'],
tags=['db:{}'.format(database), 'optional:tag1', 'sqlserver_host:localhost,1433'],
status=SQLServer.OK,
)

Expand All @@ -255,13 +260,13 @@ def test_autodiscovery_exclude_db_service_checks(aggregator, dd_run_check, insta
# assert no connection is created for an excluded database
aggregator.assert_service_check(
'sqlserver.database.can_connect',
tags=['db:msdb', 'optional:tag1', 'host:localhost,1433'],
tags=['db:msdb', 'optional:tag1', 'sqlserver_host:localhost,1433'],
status=SQLServer.OK,
count=0,
)
aggregator.assert_service_check(
'sqlserver.database.can_connect',
tags=['db:master', 'optional:tag1', 'host:localhost,1433'],
tags=['db:master', 'optional:tag1', 'sqlserver_host:localhost,1433'],
status=SQLServer.OK,
)

Expand Down
4 changes: 2 additions & 2 deletions sqlserver/tests/test_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ def test_set_default_driver_conf():
def test_check_local(aggregator, dd_run_check, init_config, instance_sql2017):
sqlserver_check = SQLServer(CHECK_NAME, init_config, [instance_sql2017])
dd_run_check(sqlserver_check)
expected_tags = instance_sql2017.get('tags', []) + ['host:{}'.format(LOCAL_SERVER), 'db:master']
expected_tags = instance_sql2017.get('tags', []) + ['sqlserver_host:{}'.format(LOCAL_SERVER), 'db:master']
assert_metrics(aggregator, expected_tags)


Expand All @@ -206,5 +206,5 @@ def test_check_adoprovider(aggregator, dd_run_check, init_config, instance_sql20

sqlserver_check = SQLServer(CHECK_NAME, init_config, [instance])
dd_run_check(sqlserver_check)
expected_tags = instance.get('tags', []) + ['host:{}'.format(LOCAL_SERVER), 'db:master']
expected_tags = instance.get('tags', []) + ['sqlserver_host:{}'.format(LOCAL_SERVER), 'db:master']
assert_metrics(aggregator, expected_tags)
1 change: 0 additions & 1 deletion sqlserver/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ commands =
pip install -r requirements.in
pytest -v {posargs}
setenv =
DDEV_SKIP_GENERIC_TAGS_CHECK=true
ODBCSYSINI = {toxinidir}/tests/odbc
COMPOSE_FOLDER = compose
ha: COMPOSE_FOLDER = compose-ha

0 comments on commit c8a934f

Please sign in to comment.