Skip to content

Commit 3912f18

Browse files
Add a clickhouse_cluster tag alongside clickhouse_node (DataDog#24711)
* Add a clickhouse_cluster tag alongside clickhouse_node Resolves the ClickHouse cluster once per check instance and attaches it as an instance-level tag, giving DBM a dimension one level above clickhouse_node so per-node data can be rolled up per cluster. Unlike node, the cluster is constant for a given instance: the DBM jobs read either local system tables or clusterAllReplicas('<cluster>', system.*), which spans exactly one named cluster. So it needs no per-row field — setting it on the tag manager before the query manager is built lets it flow through the existing payload tags/ddtags plumbing to query metrics, samples, completions, errors, parts and merges, FQT and instance metadata. Resolution prefers the {cluster} macro, which is only ever set when an operator deliberately configured a cluster, then falls back to system.clusters. The fallback excludes the sample clusters ClickHouse ships in its own default config (test_shard_localhost and friends, present through at least 21.x) — they are is_local and would otherwise mislabel every stock instance. When neither source answers, no tag is emitted rather than guessing 'default'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add changelog entry Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Exclude the ClickHouse Cloud all_groups pseudo-cluster ClickHouse Cloud reports two is_local rows in system.clusters: the real cluster ('default') and an internal 'all_groups.default' entry spanning every service group. 'all_groups.default' sorts first alphabetically, so the fallback query picked it and tagged Cloud instances clickhouse_cluster:all_groups.default while their data is actually collected from 'default' via clusterAllReplicas. Caught by running the built Agent against a real ClickHouse Cloud service, which also confirms the macro path behaves as intended there: getMacro('cluster') raises NO_ELEMENTS_IN_CONFIG (code 139) and resolution falls through to system.clusters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Resolve the cluster without polluting customer metrics The cluster-resolution probes had server-side side effects that surfaced as spurious metrics and failed test_check across every ClickHouse version: - getMacro('cluster') raises when the macro is undefined, incrementing FailedQuery/FailedSelectQuery -> clickhouse.query.failed and clickhouse.select.query.select.failed. - The fallback's LIKE '<prefix>%' makes ClickHouse compile a re2 regex, incrementing RegexpCreated -> clickhouse.compilation.regex. Both counters were zero on master and only fired because of these probes, so they polluted the customer's own metrics on the first run of every clusterless instance. Read the macro from system.macros (returns zero rows instead of raising) and filter with startsWith instead of LIKE. Verified against live ClickHouse 18 through latest that both counters stay at zero. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Expect the clickhouse_cluster tag in test_custom_queries ClickHouse ships a built-in is_local `default` cluster on some versions (18, 22.7+, 24.8, 25.x) and ClickHouse Cloud reports one too, so once a cluster resolves, every emitted metric carries a clickhouse_cluster tag. The exact-tag assertion in test_custom_queries didn't account for it and failed on those versions with "Needed at least 1 candidates ... got 0". Append the tag to the expected set when check.cluster_name is set, keeping the assertion correct on versions that resolve no cluster (19, 20, 21.8). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Mark compilation.function.execute as an optional test metric CompiledFunctionExecute -> clickhouse.compilation.function.execute is a JIT execution counter that only appears in system.events once a compiled function is actually run, so its presence is nondeterministic. It is mapped in queries.py and documented in metadata.csv but was never in the test metric lists, so assert_all_metrics_covered flagged it the moment it fired (seen in the v18 E2E run). Add it to OPTIONAL_METRICS for both the legacy and advanced metric sets so any run tolerates it whether or not the counter fired. Verified it is not triggered by the cluster-resolution queries this PR adds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1f72c8d commit 3912f18

7 files changed

Lines changed: 235 additions & 15 deletions

File tree

clickhouse/changelog.d/24711.added

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add a `clickhouse_cluster` tag to ClickHouse DBM metrics and events.

clickhouse/datadog_checks/clickhouse/clickhouse.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,13 @@
2323
from .statement_samples import ClickhouseStatementSamples
2424
from .statements import ClickhouseStatementMetrics
2525
from .table_metrics import ClickhouseTableMetrics
26-
from .utils import ErrorSanitizer, cluster_aware_query
26+
from .utils import (
27+
CLUSTER_MACRO_QUERY,
28+
CLUSTER_NAME_QUERY,
29+
CLUSTER_TAG,
30+
ErrorSanitizer,
31+
cluster_aware_query,
32+
)
2733

2834
try:
2935
import datadog_agent
@@ -60,6 +66,8 @@ def __init__(self, name, init_config, instances):
6066
self._resolved_hostname = None
6167
self._database_hostname = None
6268
self._dbms_version = None
69+
self._cluster_name = None
70+
self._cluster_name_resolved = False
6371

6472
# Track last emission time for database instance metadata (rate limiting)
6573
self._database_instance_last_emitted = 0
@@ -242,6 +250,12 @@ def _send_database_instance_metadata(self):
242250
def check(self, _):
243251
self.connect()
244252
self._server_version = self.select_version()
253+
254+
# Must run before the query manager is built and before the DBM jobs are handed
255+
# self.tags below, since both snapshot the tag list.
256+
if self.cluster_name:
257+
self.tag_manager.set_tag(CLUSTER_TAG, self.cluster_name, replace=True)
258+
245259
if self._query_manager is None or self._query_manager_version != self._server_version:
246260
self._query_manager = self._build_query_manager()
247261
self._query_manager_version = self._server_version
@@ -357,6 +371,32 @@ def database_hostname(self) -> str:
357371
self._database_hostname = resolve_db_host(self._config.server)
358372
return self._database_hostname
359373

374+
@property
375+
def cluster_name(self) -> str | None:
376+
"""The cluster this instance belongs to, or None when it cannot be determined.
377+
378+
Requires a live client, so this resolves on the first check run rather than at
379+
init. The "not found" outcome is cached too: a deployment without a cluster
380+
should not re-query on every run.
381+
"""
382+
if not self._cluster_name_resolved:
383+
self._cluster_name = self._resolve_cluster_name()
384+
self._cluster_name_resolved = True
385+
return self._cluster_name
386+
387+
def _resolve_cluster_name(self) -> str | None:
388+
for query in (CLUSTER_MACRO_QUERY, CLUSTER_NAME_QUERY):
389+
try:
390+
rows = self.execute_query_raw(query)
391+
except Exception as e:
392+
self.log.debug('Unable to resolve cluster name with %r: %s', query, e)
393+
continue
394+
if rows and rows[0] and rows[0][0]:
395+
return str(rows[0][0])
396+
# Deliberately no 'default' fallback: an absent tag is better than a wrong one.
397+
self.log.debug('No ClickHouse cluster name found; %s tag will not be emitted', CLUSTER_TAG)
398+
return None
399+
360400
@property
361401
def database_identifier_template(self) -> str:
362402
return self._config.database_identifier.template

clickhouse/datadog_checks/clickhouse/utils.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,49 @@ def compact_query(query):
3030
# Tag added to per-node metrics when collecting from all replicas in single endpoint mode.
3131
CLUSTER_NODE_TAG = 'clickhouse_node'
3232

33+
# Tag identifying the cluster this instance belongs to, one level above CLUSTER_NODE_TAG.
34+
CLUSTER_TAG = 'clickhouse_cluster'
35+
36+
# The {cluster} macro used by ON CLUSTER DDL. Tried first because it is only ever set when an
37+
# operator deliberately configured a cluster, which makes it the highest-signal source.
38+
# Read from system.macros rather than getMacro('cluster'): the function raises server-side when the
39+
# macro is undefined, which increments FailedQuery/FailedSelectQuery and pollutes the customer's
40+
# clickhouse.query.failed metrics. Selecting from the table returns zero rows instead, no error.
41+
CLUSTER_MACRO_QUERY = "SELECT substitution FROM system.macros WHERE macro = 'cluster'"
42+
43+
# Sample clusters that ClickHouse ships in its own default config (present through at least 21.x,
44+
# gone by 24.8). They are indistinguishable from real clusters in system.clusters and would
45+
# otherwise mislabel every stock instance, so they are excluded by name.
46+
BUILTIN_SAMPLE_CLUSTERS = (
47+
'test_cluster_one_shard_three_replicas_localhost',
48+
'test_cluster_two_shards',
49+
'test_cluster_two_shards_internal_replication',
50+
'test_cluster_two_shards_localhost',
51+
'test_shard_localhost',
52+
'test_shard_localhost_secure',
53+
'test_unavailable_shard',
54+
)
55+
56+
# ClickHouse Cloud exposes an internal 'all_groups.<cluster>' entry spanning every service group
57+
# alongside the real cluster, and both are is_local. It sorts first alphabetically, so without this
58+
# filter Cloud instances would be tagged all_groups.default while their data is actually collected
59+
# from 'default' via clusterAllReplicas.
60+
CLUSTER_GROUP_PREFIX = 'all_groups.'
61+
62+
# Fallback covering ClickHouse Cloud (returns 'default') and any deployment with remote_servers
63+
# configured but no {cluster} macro. ORDER BY keeps the result stable when a node is a member of
64+
# more than one cluster. The prefix filter uses startsWith rather than LIKE '<prefix>%': ClickHouse
65+
# compiles a re2 regex for that LIKE pattern (bumping RegexpCreated, which surfaces as
66+
# clickhouse.compilation.regex), whereas startsWith is a plain prefix comparison.
67+
CLUSTER_NAME_QUERY = (
68+
"SELECT cluster FROM system.clusters "
69+
"WHERE is_local AND cluster NOT IN ({excluded}) AND NOT startsWith(cluster, '{prefix}') "
70+
"ORDER BY cluster LIMIT 1".format(
71+
excluded=', '.join(f"'{name}'" for name in BUILTIN_SAMPLE_CLUSTERS),
72+
prefix=CLUSTER_GROUP_PREFIX,
73+
)
74+
)
75+
3376

3477
def cluster_aware_query(base: dict) -> dict:
3578
"""Build a cluster-aware variant that reads all replicas and tags each row per node.

clickhouse/tests/advanced_metrics.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,9 @@
304304

305305
# These are the common metrics that are not always available.
306306
OPTIONAL_METRICS = [
307+
# JIT execution counter; only appears once a compiled function is actually run, so it is optional.
308+
'clickhouse.compilation.function.execute.count',
309+
'clickhouse.compilation.function.execute.total',
307310
'clickhouse.asynchronous_metrics.AsynchronousHeavyMetricsCalculationTimeSpent',
308311
'clickhouse.asynchronous_metrics.AsynchronousHeavyMetricsUpdateInterval',
309312
'clickhouse.asynchronous_metrics.AsynchronousMetricsCalculationTimeSpent',

clickhouse/tests/metrics.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@
9191
'clickhouse.cache_dictionary.update_queue.keys',
9292
'clickhouse.compilation.llvm.attempt.count',
9393
'clickhouse.compilation.llvm.attempt.total',
94+
# JIT execution counter; only appears once a compiled function is actually run, so it is optional.
95+
'clickhouse.compilation.function.execute.count',
96+
'clickhouse.compilation.function.execute.total',
9497
'clickhouse.compilation.size.count',
9598
'clickhouse.compilation.size.total',
9699
'clickhouse.compilation.time',

clickhouse/tests/test_clickhouse.py

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import pytest
55

66
from datadog_checks.clickhouse import ClickhouseCheck
7+
from datadog_checks.clickhouse.utils import CLUSTER_TAG
78
from datadog_checks.dev.utils import get_metadata_metrics
89

910
from . import common
@@ -46,19 +47,21 @@ def test_custom_queries(aggregator, instance, dd_run_check):
4647
check = ClickhouseCheck('clickhouse', {}, [instance])
4748
dd_run_check(check)
4849

49-
aggregator.assert_metric(
50-
'clickhouse.settings.changed',
51-
metric_type=0,
52-
tags=[
53-
'server:{}'.format(instance['server']),
54-
'port:{}'.format(instance['port']),
55-
'db:default',
56-
'foo:bar',
57-
'test:clickhouse',
58-
'database_hostname:{}'.format(check.database_hostname),
59-
'database_instance:{}:{}:default'.format(instance['server'], instance['port']),
60-
],
61-
)
50+
expected_tags = [
51+
'server:{}'.format(instance['server']),
52+
'port:{}'.format(instance['port']),
53+
'db:default',
54+
'foo:bar',
55+
'test:clickhouse',
56+
'database_hostname:{}'.format(check.database_hostname),
57+
'database_instance:{}:{}:default'.format(instance['server'], instance['port']),
58+
]
59+
# ClickHouse ships a built-in is_local `default` cluster on some versions (and Cloud reports one
60+
# too), so every metric carries the clickhouse_cluster tag when a cluster resolves.
61+
if check.cluster_name:
62+
expected_tags.append('{}:{}'.format(CLUSTER_TAG, check.cluster_name))
63+
64+
aggregator.assert_metric('clickhouse.settings.changed', metric_type=0, tags=expected_tags)
6265

6366

6467
@pytest.mark.skipif(

clickhouse/tests/test_unit.py

Lines changed: 128 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,14 @@
77

88
from datadog_checks.base import ConfigurationError
99
from datadog_checks.clickhouse import ClickhouseCheck, advanced_queries, queries
10-
from datadog_checks.clickhouse.utils import cluster_aware_query
10+
from datadog_checks.clickhouse.utils import (
11+
BUILTIN_SAMPLE_CLUSTERS,
12+
CLUSTER_GROUP_PREFIX,
13+
CLUSTER_MACRO_QUERY,
14+
CLUSTER_NAME_QUERY,
15+
CLUSTER_TAG,
16+
cluster_aware_query,
17+
)
1118

1219
from .utils import ensure_csv_safe, parse_described_metrics, raise_error
1320

@@ -455,3 +462,123 @@ def test_get_queries_uses_base_queries_for_direct_connection(instance, use_advan
455462
check._server_version = '24.8'
456463

457464
assert all('clusterAllReplicas' not in q['query'] for q in check.get_queries())
465+
466+
467+
def make_cluster_name_check(query_results):
468+
"""Build a check whose execute_query_raw replays query_results keyed by SQL."""
469+
check = ClickhouseCheck('clickhouse', {}, [BASE_INSTANCE])
470+
471+
def execute(query):
472+
result = query_results[query]
473+
if isinstance(result, Exception):
474+
raise result
475+
return result
476+
477+
check.execute_query_raw = mock.Mock(side_effect=execute)
478+
return check
479+
480+
481+
def test_cluster_name_prefers_the_macro():
482+
check = make_cluster_name_check({CLUSTER_MACRO_QUERY: [['macro_cluster']]})
483+
484+
assert check.cluster_name == 'macro_cluster'
485+
# system.clusters must not be consulted once the macro answers.
486+
check.execute_query_raw.assert_called_once_with(CLUSTER_MACRO_QUERY)
487+
488+
489+
@pytest.mark.parametrize(
490+
'macro_result',
491+
[
492+
pytest.param([], id='macro-not-set'),
493+
pytest.param([['']], id='empty-value'),
494+
pytest.param(Error('query failed'), id='query-error'),
495+
],
496+
)
497+
def test_cluster_name_falls_back_to_system_clusters(macro_result):
498+
check = make_cluster_name_check({CLUSTER_MACRO_QUERY: macro_result, CLUSTER_NAME_QUERY: [['prod_cluster']]})
499+
500+
assert check.cluster_name == 'prod_cluster'
501+
502+
503+
def test_cluster_name_query_excludes_builtin_sample_clusters():
504+
"""ClickHouse <=21.x ships test_* clusters in its default config.
505+
506+
They are is_local and would otherwise mislabel every stock instance, so the
507+
fallback query filters them out by name rather than picking one.
508+
"""
509+
for name in BUILTIN_SAMPLE_CLUSTERS:
510+
assert f"'{name}'" in CLUSTER_NAME_QUERY
511+
512+
assert 'test_cluster_two_shards_localhost' in BUILTIN_SAMPLE_CLUSTERS
513+
assert 'test_shard_localhost' in BUILTIN_SAMPLE_CLUSTERS
514+
515+
516+
def test_cluster_name_query_excludes_cloud_group_pseudo_cluster():
517+
"""ClickHouse Cloud reports both 'all_groups.default' and 'default' as is_local.
518+
519+
The group entry sorts first, so without the filter Cloud instances would be
520+
tagged all_groups.default while their data comes from 'default' via
521+
clusterAllReplicas.
522+
"""
523+
assert f"NOT startsWith(cluster, '{CLUSTER_GROUP_PREFIX}')" in CLUSTER_NAME_QUERY
524+
525+
526+
def test_cluster_name_absent_when_both_sources_fail():
527+
check = make_cluster_name_check(
528+
{
529+
CLUSTER_MACRO_QUERY: Error('query failed'),
530+
CLUSTER_NAME_QUERY: [],
531+
}
532+
)
533+
534+
# No 'default' fallback: a wrong cluster name is worse than an absent one.
535+
assert check.cluster_name is None
536+
537+
538+
def test_cluster_name_is_cached_including_the_absent_case():
539+
check = make_cluster_name_check({CLUSTER_MACRO_QUERY: [], CLUSTER_NAME_QUERY: []})
540+
541+
assert check.cluster_name is None
542+
assert check.cluster_name is None
543+
544+
assert check.execute_query_raw.call_count == 2 # one attempt per source, not per access
545+
546+
547+
def test_check_tags_with_cluster(instance):
548+
check = ClickhouseCheck('clickhouse', {}, [instance])
549+
with mock.patch.object(ClickhouseCheck, 'cluster_name', new_callable=mock.PropertyMock) as cluster_name:
550+
cluster_name.return_value = 'prod_cluster'
551+
with mock.patch('clickhouse_connect.get_client'):
552+
check.check({})
553+
554+
assert f'{CLUSTER_TAG}:prod_cluster' in check.tags
555+
556+
557+
def test_can_connect_carries_cluster_tag_from_the_second_run(aggregator, instance):
558+
"""The tag needs a live client, so connect() on the first run predates it.
559+
560+
The tag persists on the tag manager afterwards, so every later run — and every
561+
other surface on the first run, since they all follow the resolution point —
562+
does carry it.
563+
"""
564+
check = ClickhouseCheck('clickhouse', {}, [instance])
565+
with mock.patch.object(ClickhouseCheck, 'cluster_name', new_callable=mock.PropertyMock) as cluster_name:
566+
cluster_name.return_value = 'prod_cluster'
567+
with mock.patch('clickhouse_connect.get_client'):
568+
check.check({})
569+
first_run_tags = list(check.tags)
570+
check.check({})
571+
572+
assert f'{CLUSTER_TAG}:prod_cluster' not in aggregator.service_checks('clickhouse.can_connect')[0].tags
573+
assert f'{CLUSTER_TAG}:prod_cluster' in first_run_tags
574+
aggregator.assert_service_check('clickhouse.can_connect', tags=check.tags)
575+
576+
577+
def test_check_omits_cluster_tag_when_unresolved(instance):
578+
check = ClickhouseCheck('clickhouse', {}, [instance])
579+
with mock.patch.object(ClickhouseCheck, 'cluster_name', new_callable=mock.PropertyMock) as cluster_name:
580+
cluster_name.return_value = None
581+
with mock.patch('clickhouse_connect.get_client'):
582+
check.check({})
583+
584+
assert not any(tag.startswith(f'{CLUSTER_TAG}:') for tag in check.tags)

0 commit comments

Comments
 (0)