From 6f18a498b3b7ba671cab54bf9ccc2fce2677ee7c Mon Sep 17 00:00:00 2001 From: Pierre Le Noan Date: Tue, 18 Aug 2026 15:18:00 +0200 Subject: [PATCH 1/5] sqlserver: recover DBM<>APM correlation comment for non-procedure statements SQL Server's statement_start_offset/statement_end_offset exclude a leading comment (e.g. a sqlcommenter-style /*dddbs=...*/ tag) from statement_text, so DBM<>APM correlation comments were silently dropped for ordinary statements. The only place the comment could still be recovered from is row['text'] (the untouched batch text), but that re-obfuscation only ran when the statement had stored-procedure context. Run it for every row instead, keeping procedure_signature/procedure_name assignment gated behind has_proc_context as before. SDBM-2891 --- .../datadog_checks/sqlserver/activity.py | 38 +++++++++++-------- sqlserver/tests/test_activity.py | 29 ++++++++++++++ 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/sqlserver/datadog_checks/sqlserver/activity.py b/sqlserver/datadog_checks/sqlserver/activity.py index 95ee1540fe483..9e7d08997cb8e 100644 --- a/sqlserver/datadog_checks/sqlserver/activity.py +++ b/sqlserver/datadog_checks/sqlserver/activity.py @@ -386,28 +386,36 @@ def _obfuscate_and_sanitize_row(self, row): comments = statement['metadata'].get('comments', []) row['is_proc'] = bool(row.get('procedure_name')) has_proc_context = row['is_proc'] or is_statement_proc(row.get('text', ''))[0] - if has_proc_context and row.get('text'): + if row.get('text'): + # statement_text is sliced out of the full batch text using SQL Server's own + # statement_start_offset/statement_end_offset, which excludes any comment that + # precedes the statement (e.g. a sqlcommenter-style /*dddbs=...*/ tag used for + # DBM<>APM correlation). row['text'] holds the untouched batch text, so it's the + # only place a leading comment can still be recovered from - re-obfuscate it for + # comments on every row, not just when the statement is a stored procedure call. try: - procedure_statement = obfuscate_sql_with_metadata( + full_text_statement = obfuscate_sql_with_metadata( row['text'], self._config.obfuscator_options, replace_null_character=True ) - row['procedure_signature'] = compute_sql_signature(procedure_statement['query']) - procedure_comments = procedure_statement['metadata'].get('comments', []) - if procedure_comments: - comments = list(set(comments + procedure_comments)) - if not row.get('procedure_name'): - procedures = procedure_statement['metadata'].get('procedures') - if procedures: - row['procedure_name'] = procedures[0].lower() - row['is_proc'] = True + full_text_comments = full_text_statement['metadata'].get('comments', []) + if full_text_comments: + comments = list(set(comments + full_text_comments)) + if has_proc_context: + row['procedure_signature'] = compute_sql_signature(full_text_statement['query']) + if not row.get('procedure_name'): + procedures = full_text_statement['metadata'].get('procedures') + if procedures: + row['procedure_name'] = procedures[0].lower() + row['is_proc'] = True except Exception as e: - row['procedure_signature'] = '__procedure_obfuscation_error__' - # if we fail to obfuscate the procedure text, + if has_proc_context: + row['procedure_signature'] = '__procedure_obfuscation_error__' + # if we fail to obfuscate the full text, # we should not mark query statement as failed to obfuscate if self._config.log_unobfuscated_queries: - self.log.warning("Failed to obfuscate stored procedure=[%s] | err=[%s]", repr(row['text']), e) + self.log.warning("Failed to obfuscate query text=[%s] | err=[%s]", repr(row['text']), e) else: - self.log.debug("Failed to obfuscate stored procedure | err=[%s]", e) + self.log.debug("Failed to obfuscate query text | err=[%s]", e) if 'tail_text' in row: tail_statement = obfuscate_sql_with_metadata( row['tail_text'], self._obfuscator_options_for_tail_text, replace_null_character=True diff --git a/sqlserver/tests/test_activity.py b/sqlserver/tests/test_activity.py index f8cae9cc07757..a493359eb843d 100644 --- a/sqlserver/tests/test_activity.py +++ b/sqlserver/tests/test_activity.py @@ -1056,3 +1056,32 @@ def test_sanitize_activity_row(dbm_instance, row): row = check.activity._obfuscate_and_sanitize_row(row) assert isinstance(row['query_hash'], str) assert isinstance(row['query_plan_hash'], str) + + +@pytest.mark.unit +def test_sanitize_activity_row_recovers_leading_comment_for_non_proc_statement(dbm_instance, datadog_agent): + # SQL Server's statement_start_offset/statement_end_offset exclude a comment prepended + # to a batch (e.g. sqlcommenter-style /*dddbs=...*/ used for DBM<>APM correlation), so + # `statement_text` never contains it. Before the fix, comments were only recovered from + # the untouched `text` for stored-procedure rows; this asserts recovery for a plain statement. + comment = "/*dddbs='orders-service',dde='prod'*/" + row = { + 'statement_text': "SELECT * FROM orders WHERE customer_id = @P1", + 'text': f"{comment} SELECT * FROM orders WHERE customer_id = @P1", + 'procedure_name': None, + 'query_hash': b'\xa4\xffV\x1c\xd4\x14\xbeC', + 'query_plan_hash': b'\xfe\xba\xbf\xc6_\x9bo\x83', + } + + def _obfuscate_sql(sql_query, options=None): + comments = [comment] if comment in sql_query else [] + return json.dumps({'query': sql_query, 'metadata': {'comments': comments}}) + + check = SQLServer(CHECK_NAME, {}, [dbm_instance]) + with mock.patch.object(datadog_agent, 'obfuscate_sql', passthrough=True) as mock_agent: + mock_agent.side_effect = _obfuscate_sql + row = check.activity._obfuscate_and_sanitize_row(row) + + assert row['dd_comments'] == [comment] + assert not row.get('is_proc') + assert 'procedure_signature' not in row From 0ed71ba6d1a5407186fdd14f64b06b2a4dbaf778 Mon Sep 17 00:00:00 2001 From: Pierre Le Noan Date: Tue, 18 Aug 2026 15:19:48 +0200 Subject: [PATCH 2/5] sqlserver: add changelog for #24899 SDBM-2891 --- sqlserver/changelog.d/24899.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 sqlserver/changelog.d/24899.fixed diff --git a/sqlserver/changelog.d/24899.fixed b/sqlserver/changelog.d/24899.fixed new file mode 100644 index 0000000000000..809d58896149d --- /dev/null +++ b/sqlserver/changelog.d/24899.fixed @@ -0,0 +1 @@ +Recover the DBM<>APM correlation comment (e.g. a sqlcommenter-style ``/*dddbs=...*/`` tag) for non stored-procedure statements in Query Activity. Previously, SQL Server's own ``statement_start_offset``/``statement_end_offset`` excluded any comment preceding a statement from ``statement_text``, and the check only recovered it from the full batch text when the statement had stored-procedure context, silently breaking the ``Calling Service`` correlation panel for ordinary statements. From 41c9d0f15b8f7922d414408f94acd846f663c28b Mon Sep 17 00:00:00 2001 From: Pierre Le Noan Date: Tue, 18 Aug 2026 15:44:59 +0200 Subject: [PATCH 3/5] sqlserver: address review feedback on #24899 Drop explanatory comment block, simplify changelog entry. --- sqlserver/changelog.d/24899.fixed | 2 +- sqlserver/datadog_checks/sqlserver/activity.py | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/sqlserver/changelog.d/24899.fixed b/sqlserver/changelog.d/24899.fixed index 809d58896149d..0c21d9ac7efa3 100644 --- a/sqlserver/changelog.d/24899.fixed +++ b/sqlserver/changelog.d/24899.fixed @@ -1 +1 @@ -Recover the DBM<>APM correlation comment (e.g. a sqlcommenter-style ``/*dddbs=...*/`` tag) for non stored-procedure statements in Query Activity. Previously, SQL Server's own ``statement_start_offset``/``statement_end_offset`` excluded any comment preceding a statement from ``statement_text``, and the check only recovered it from the full batch text when the statement had stored-procedure context, silently breaking the ``Calling Service`` correlation panel for ordinary statements. +Fix DBM<>APM correlation comments being dropped from Query Samples for non stored-procedure statements. diff --git a/sqlserver/datadog_checks/sqlserver/activity.py b/sqlserver/datadog_checks/sqlserver/activity.py index 9e7d08997cb8e..96732abe2f151 100644 --- a/sqlserver/datadog_checks/sqlserver/activity.py +++ b/sqlserver/datadog_checks/sqlserver/activity.py @@ -387,12 +387,6 @@ def _obfuscate_and_sanitize_row(self, row): row['is_proc'] = bool(row.get('procedure_name')) has_proc_context = row['is_proc'] or is_statement_proc(row.get('text', ''))[0] if row.get('text'): - # statement_text is sliced out of the full batch text using SQL Server's own - # statement_start_offset/statement_end_offset, which excludes any comment that - # precedes the statement (e.g. a sqlcommenter-style /*dddbs=...*/ tag used for - # DBM<>APM correlation). row['text'] holds the untouched batch text, so it's the - # only place a leading comment can still be recovered from - re-obfuscate it for - # comments on every row, not just when the statement is a stored procedure call. try: full_text_statement = obfuscate_sql_with_metadata( row['text'], self._config.obfuscator_options, replace_null_character=True From 3aeb167d97c6ad591400e66a54a9cc1dce942e38 Mon Sep 17 00:00:00 2001 From: Pierre Le Noan Date: Wed, 19 Aug 2026 11:35:50 +0200 Subject: [PATCH 4/5] sqlserver: skip redundant obfuscation RPC, dedupe comment-merge logic Address review feedback on #24899: - Only re-obfuscate row['text'] when it can add something (proc context, or text actually differs from statement_text), avoiding a doubled obfuscator RPC for every row where the two are already identical. - Extract the repeated "pull comments out of obfuscated metadata and merge" shape (full-text branch + tail-text branch) into _merge_comments, using dict.fromkeys instead of set for stable dd_comments ordering. --- sqlserver/datadog_checks/sqlserver/activity.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/sqlserver/datadog_checks/sqlserver/activity.py b/sqlserver/datadog_checks/sqlserver/activity.py index 96732abe2f151..d16913d374777 100644 --- a/sqlserver/datadog_checks/sqlserver/activity.py +++ b/sqlserver/datadog_checks/sqlserver/activity.py @@ -386,14 +386,12 @@ def _obfuscate_and_sanitize_row(self, row): comments = statement['metadata'].get('comments', []) row['is_proc'] = bool(row.get('procedure_name')) has_proc_context = row['is_proc'] or is_statement_proc(row.get('text', ''))[0] - if row.get('text'): + if row.get('text') and (has_proc_context or row['text'] != row['statement_text']): try: full_text_statement = obfuscate_sql_with_metadata( row['text'], self._config.obfuscator_options, replace_null_character=True ) - full_text_comments = full_text_statement['metadata'].get('comments', []) - if full_text_comments: - comments = list(set(comments + full_text_comments)) + comments = self._merge_comments(comments, full_text_statement['metadata']) if has_proc_context: row['procedure_signature'] = compute_sql_signature(full_text_statement['query']) if not row.get('procedure_name'): @@ -414,9 +412,7 @@ def _obfuscate_and_sanitize_row(self, row): tail_statement = obfuscate_sql_with_metadata( row['tail_text'], self._obfuscator_options_for_tail_text, replace_null_character=True ) - appended_comments = tail_statement['metadata'].get('comments', []) - if appended_comments: - comments = list(set(comments + appended_comments)) + comments = self._merge_comments(comments, tail_statement['metadata']) obfuscated_statement = statement['query'] metadata = statement['metadata'] row['dd_commands'] = metadata.get('commands', None) @@ -439,6 +435,11 @@ def _obfuscate_and_sanitize_row(self, row): def _remove_null_vals(row): return {key: val for key, val in row.items() if val is not None} + @staticmethod + def _merge_comments(comments, obfuscated_metadata): + new_comments = obfuscated_metadata.get('comments', []) + return list(dict.fromkeys(comments + new_comments)) if new_comments else comments + @staticmethod def _sanitize_row(row, obfuscated_statement=None): # rename the statement_text field to 'text' because that From 00d3676951822ec02fdebd9e646b45859d87543f Mon Sep 17 00:00:00 2001 From: Joel Marcotte Date: Wed, 19 Aug 2026 15:21:33 -0400 Subject: [PATCH 5/5] sqlserver: recover leading comments in query metrics --- sqlserver/changelog.d/24899.fixed | 2 +- .../datadog_checks/sqlserver/statements.py | 52 +++++++++---------- sqlserver/tests/test_activity.py | 11 ++-- sqlserver/tests/test_statements.py | 34 ++++++++++++ 4 files changed, 66 insertions(+), 33 deletions(-) diff --git a/sqlserver/changelog.d/24899.fixed b/sqlserver/changelog.d/24899.fixed index 0c21d9ac7efa3..ba34ea998509a 100644 --- a/sqlserver/changelog.d/24899.fixed +++ b/sqlserver/changelog.d/24899.fixed @@ -1 +1 @@ -Fix DBM<>APM correlation comments being dropped from Query Samples for non stored-procedure statements. +Fix DBM<>APM correlation comments being dropped from Query Samples and Query Metrics for non-stored-procedure statements. diff --git a/sqlserver/datadog_checks/sqlserver/statements.py b/sqlserver/datadog_checks/sqlserver/statements.py index d4a9d42cd310f..7b8e215a8ba27 100644 --- a/sqlserver/datadog_checks/sqlserver/statements.py +++ b/sqlserver/datadog_checks/sqlserver/statements.py @@ -404,7 +404,6 @@ def _normalize_queries(self, rows): if not self._should_include_query_metrics_row(row): continue # Attempt to obfuscate SQL statement with metadata - procedure_statement = None try: statement = obfuscate_sql_with_metadata( row['statement_text'], self._config.obfuscator_options, replace_null_character=True @@ -433,39 +432,40 @@ def _normalize_queries(self, rows): procedure_content = None row['is_proc'] = bool(row.get('procedure_name')) has_sproc_context = row['is_proc'] or bool(row.get('sproc_object_id')) - if (has_sproc_context and row['text']) or self.disable_secondary_tags: + needs_procedure_metadata = has_sproc_context or self.disable_secondary_tags + if row.get('text') and (needs_procedure_metadata or row['text'] != row['statement_text']): try: - procedure_statement = obfuscate_sql_with_metadata( + full_text_statement = obfuscate_sql_with_metadata( row['text'], self._config.obfuscator_options, replace_null_character=True ) - procedure_content = procedure_statement['query'] - procedure_signature = compute_sql_signature(procedure_statement['query']) - procedure_comments = procedure_statement['metadata'].get('comments', []) - if procedure_comments: - comments = list(set(comments + procedure_comments)) - if not row.get('procedure_name'): - procedures = procedure_statement['metadata'].get('procedures') - if procedures: - row['procedure_name'] = procedures[0].lower() - row['is_proc'] = True + full_text_comments = full_text_statement['metadata'].get('comments', []) + if full_text_comments: + comments = list(dict.fromkeys(comments + full_text_comments)) + if needs_procedure_metadata: + procedure_content = full_text_statement['query'] + procedure_signature = compute_sql_signature(full_text_statement['query']) + if not row.get('procedure_name'): + procedures = full_text_statement['metadata'].get('procedures') + if procedures: + row['procedure_name'] = procedures[0].lower() + row['is_proc'] = True except Exception as e: - procedure_signature = '__procedure_obfuscation_error__' - procedure_content = '__procedure_obfuscation_error__' + if needs_procedure_metadata: + procedure_signature = '__procedure_obfuscation_error__' + procedure_content = '__procedure_obfuscation_error__' if self._config.log_unobfuscated_queries: - self.log.warning("Failed to obfuscate stored procedure=[%s] | err=[%s]", repr(row['text']), e) + self.log.warning("Failed to obfuscate query text=[%s] | err=[%s]", repr(row['text']), e) else: self.log.debug( - "Failed to obfuscate stored procedure for query_signature=[%s] | err=[%s]", - query_signature, - e, + "Failed to obfuscate query text for query_signature=[%s] | err=[%s]", query_signature, e ) - self._check.count( - "dd.sqlserver.statements.error", - 1, - **self._check.debug_stats_kwargs(tags=["error:obfuscate-sproc-{}".format(type(e))]), - ) - # If we can't obfuscate the stored procedure, we don't need to give up for this row, - # we just won't have the association with the stored procedure in the metrics payload + if needs_procedure_metadata: + self._check.count( + "dd.sqlserver.statements.error", + 1, + **self._check.debug_stats_kwargs(tags=["error:obfuscate-sproc-{}".format(type(e))]), + ) + # If we can't obfuscate the full text, keep the row using the obfuscated statement text. if procedure_content: row['procedure_text'] = procedure_content diff --git a/sqlserver/tests/test_activity.py b/sqlserver/tests/test_activity.py index a493359eb843d..2347fb9e4daa8 100644 --- a/sqlserver/tests/test_activity.py +++ b/sqlserver/tests/test_activity.py @@ -1060,14 +1060,13 @@ def test_sanitize_activity_row(dbm_instance, row): @pytest.mark.unit def test_sanitize_activity_row_recovers_leading_comment_for_non_proc_statement(dbm_instance, datadog_agent): - # SQL Server's statement_start_offset/statement_end_offset exclude a comment prepended - # to a batch (e.g. sqlcommenter-style /*dddbs=...*/ used for DBM<>APM correlation), so - # `statement_text` never contains it. Before the fix, comments were only recovered from - # the untouched `text` for stored-procedure rows; this asserts recovery for a plain statement. comment = "/*dddbs='orders-service',dde='prod'*/" + statement_text = "SELECT * FROM orders WHERE customer_id = @P1" row = { - 'statement_text': "SELECT * FROM orders WHERE customer_id = @P1", - 'text': f"{comment} SELECT * FROM orders WHERE customer_id = @P1", + # sp_executesql includes the RPC parameter declaration and leading comment in the full + # batch text, but SQL Server's statement offsets exclude both from statement_text. + 'statement_text': statement_text, + 'text': f"(@P1 int){comment} {statement_text}", 'procedure_name': None, 'query_hash': b'\xa4\xffV\x1c\xd4\x14\xbeC', 'query_plan_hash': b'\xfe\xba\xbf\xc6_\x9bo\x83', diff --git a/sqlserver/tests/test_statements.py b/sqlserver/tests/test_statements.py index 4c9715dc6e029..73fbe9276f593 100644 --- a/sqlserver/tests/test_statements.py +++ b/sqlserver/tests/test_statements.py @@ -1236,6 +1236,40 @@ def _obfuscate_sql(sql_query, options=None): assert not result_row.get('procedure_text') +@pytest.mark.unit +def test_normalize_queries_recovers_leading_comment_for_non_proc_statement(instance_docker, datadog_agent): + instance_docker['dbm'] = True + instance_docker['query_metrics'] = {'enabled': True, 'run_sync': True} + check = SQLServer(CHECK_NAME, {}, [instance_docker]) + + comment = "/*dddbs='orders-service',dde='prod'*/" + statement_text = "SELECT * FROM orders WHERE customer_id = @P1" + row = { + 'statement_text': statement_text, + 'text': f"(@P1 int){comment} {statement_text}", + 'procedure_name': None, + 'schema_name': None, + 'sproc_object_id': None, + 'query_hash': b'\x01\x02\x03\x04', + 'query_plan_hash': b'\x05\x06\x07\x08', + 'plan_handle': b'\x09\x0a\x0b\x0c', + } + + def _obfuscate_sql(sql_query, options=None): + comments = [comment] if comment in sql_query else [] + return json.dumps({'query': sql_query, 'metadata': {'comments': comments}}) + + with mock.patch.object(datadog_agent, 'obfuscate_sql', passthrough=True) as mock_agent: + mock_agent.side_effect = _obfuscate_sql + result = check.statement_metrics._normalize_queries([row]) + + assert result[0]['dd_comments'] == [comment] + assert result[0]['text'] == statement_text + assert not result[0]['is_proc'] + assert 'procedure_signature' not in result[0] + assert 'procedure_text' not in result[0] + + @pytest.mark.flaky @pytest.mark.integration @pytest.mark.usefixtures('dd_environment')