Skip to content

Commit

Permalink
use resolved hostname for all metrics (#11634)
Browse files Browse the repository at this point in the history
When DBM is enabled we want all telemetry, both standard metrics as well as DBM events, to have the hostname set to the resolved hostname. This ensures that when an agent is monitoring a remote host (i.e. RDS or Azure managed) all SQL Server telemetry is tagged with the hostname of the remote host and not the agent's hostname.
  • Loading branch information
djova authored Mar 9, 2022
1 parent 137582a commit e908636
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 7 deletions.
5 changes: 4 additions & 1 deletion sqlserver/datadog_checks/sqlserver/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ def __init__(self, cfg_instance, base_name, report_function, column, logger):
self.datadog_name = cfg_instance['name']
self.sql_name = cfg_instance.get('counter_name', '')
self.base_name = base_name
self.report_function = partial(report_function, raw=True)
partial_kwargs = {}
if 'hostname' in cfg_instance:
partial_kwargs['hostname'] = cfg_instance['hostname']
self.report_function = partial(report_function, raw=True, **partial_kwargs)
self.instance = cfg_instance.get('instance_name', '')
self.object_name = cfg_instance.get('object_name', '')
self.tags = cfg_instance.get('tags', [])
Expand Down
5 changes: 3 additions & 2 deletions sqlserver/datadog_checks/sqlserver/sqlserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ def _make_metric_list_to_collect(self, custom_metrics):

# Load database files
for name, column, metric_type in DATABASE_FILES_IO:
cfg = {'name': name, 'column': column, 'tags': tags}

cfg = {'name': name, 'column': column, 'tags': tags, 'hostname': self.resolved_hostname}
metrics_to_collect.append(SqlFileStats(cfg, None, getattr(self, metric_type), column, self.log))

# Load AlwaysOn metrics
Expand Down Expand Up @@ -575,6 +574,8 @@ def typed_metric(self, cfg_inst, table, base_name=None, user_type=None, sql_type
metric_type_str, cls = metrics.TABLE_MAPPING[table]
metric_type = getattr(self, metric_type_str)

cfg_inst['hostname'] = self.resolved_hostname

return cls(cfg_inst, base_name, metric_type, column, self.log)

def check(self, _):
Expand Down
5 changes: 3 additions & 2 deletions sqlserver/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def get_local_driver():
}


def assert_metrics(aggregator, expected_tags, dbm_enabled=False):
def assert_metrics(aggregator, expected_tags, dbm_enabled=False, hostname=None):
"""
Boilerplate asserting all the expected metrics and service checks.
Make sure ALL custom metric is tagged by database.
Expand All @@ -170,7 +170,8 @@ def assert_metrics(aggregator, expected_tags, dbm_enabled=False):
dbm_excluded_metrics = [m[0] for m in DBM_MIGRATED_METRICS]
expected_metrics = [m for m in EXPECTED_METRICS if m not in dbm_excluded_metrics]
for mname in expected_metrics:
aggregator.assert_metric(mname)
assert hostname is not None, "hostname must be explicitly specified for all metrics"
aggregator.assert_metric(mname, hostname=hostname)
aggregator.assert_service_check('sqlserver.can_connect', status=SQLServer.OK, tags=expected_tags)
aggregator.assert_all_metrics_covered()
aggregator.assert_no_duplicate_metrics()
2 changes: 1 addition & 1 deletion sqlserver/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def test_check_docker(aggregator, dd_run_check, init_config, instance_docker):
'sqlserver_host:{}'.format(instance_docker.get('host')),
'db:master',
]
assert_metrics(aggregator, expected_tags)
assert_metrics(aggregator, expected_tags, hostname=sqlserver_check.resolved_hostname)


@pytest.mark.integration
Expand Down
2 changes: 1 addition & 1 deletion sqlserver/tests/test_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,4 @@ def test_check_local(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', []) + ['sqlserver_host:{}'.format(DOCKER_SERVER), 'db:master']
assert_metrics(aggregator, expected_tags)
assert_metrics(aggregator, expected_tags, hostname=sqlserver_check.resolved_hostname)

0 comments on commit e908636

Please sign in to comment.