diff --git a/sqlserver/CHANGELOG.md b/sqlserver/CHANGELOG.md index 8537b8fb8760d..b6e61087fde98 100644 --- a/sqlserver/CHANGELOG.md +++ b/sqlserver/CHANGELOG.md @@ -5,6 +5,7 @@ ***Fixed***: * Properly decode query_hash when statement_text is None ([#15974](https://github.com/DataDog/integrations-core/pull/15974)) +* Strip sql comments before parsing procedure name ([#16004](https://github.com/DataDog/integrations-core/pull/16004)) ## 15.0.1 / 2023-10-06 diff --git a/sqlserver/datadog_checks/sqlserver/activity.py b/sqlserver/datadog_checks/sqlserver/activity.py index fc67366bde286..c2acfdfef83d0 100644 --- a/sqlserver/datadog_checks/sqlserver/activity.py +++ b/sqlserver/datadog_checks/sqlserver/activity.py @@ -13,7 +13,7 @@ from datadog_checks.base.utils.serialization import json from datadog_checks.base.utils.tracking import tracked_method from datadog_checks.sqlserver.const import STATIC_INFO_ENGINE_EDITION, STATIC_INFO_VERSION -from datadog_checks.sqlserver.utils import PROC_CHAR_LIMIT, extract_sql_comments, is_statement_proc +from datadog_checks.sqlserver.utils import PROC_CHAR_LIMIT, extract_sql_comments_and_procedure_name try: import datadog_agent @@ -275,14 +275,14 @@ def _obfuscate_and_sanitize_row(self, row): statement = obfuscate_sql_with_metadata(row['statement_text'], self.check.obfuscator_options) procedure_statement = None # sqlserver doesn't have a boolean data type so convert integer to boolean - row['is_proc'], procedure_name = is_statement_proc(row['text']) + comments, row['is_proc'], procedure_name = extract_sql_comments_and_procedure_name(row['text']) if row['is_proc'] and 'text' in row: procedure_statement = obfuscate_sql_with_metadata(row['text'], self.check.obfuscator_options) obfuscated_statement = statement['query'] metadata = statement['metadata'] row['dd_commands'] = metadata.get('commands', None) row['dd_tables'] = metadata.get('tables', None) - row['dd_comments'] = extract_sql_comments(row['text']) + row['dd_comments'] = comments row['query_signature'] = compute_sql_signature(obfuscated_statement) # procedure_signature is used to link this activity event with # its related plan events diff --git a/sqlserver/datadog_checks/sqlserver/statements.py b/sqlserver/datadog_checks/sqlserver/statements.py index fce4918978f14..c7f1492a6fa37 100644 --- a/sqlserver/datadog_checks/sqlserver/statements.py +++ b/sqlserver/datadog_checks/sqlserver/statements.py @@ -21,7 +21,7 @@ ) from datadog_checks.base.utils.serialization import json from datadog_checks.base.utils.tracking import tracked_method -from datadog_checks.sqlserver.utils import PROC_CHAR_LIMIT, extract_sql_comments, is_statement_proc +from datadog_checks.sqlserver.utils import PROC_CHAR_LIMIT, extract_sql_comments_and_procedure_name try: import datadog_agent @@ -292,7 +292,7 @@ def _normalize_queries(self, rows): try: statement = obfuscate_sql_with_metadata(row['statement_text'], self.check.obfuscator_options) procedure_statement = None - row['is_proc'], procedure_name = is_statement_proc(row['text']) + comments, row['is_proc'], procedure_name = extract_sql_comments_and_procedure_name(row['text']) if row['is_proc']: procedure_statement = obfuscate_sql_with_metadata(row['text'], self.check.obfuscator_options) except Exception as e: @@ -308,7 +308,6 @@ def _normalize_queries(self, rows): ) continue obfuscated_statement = statement['query'] - comments = extract_sql_comments(row['text']) row['dd_comments'] = comments # update 'text' field to be obfuscated stmt row['text'] = obfuscated_statement diff --git a/sqlserver/datadog_checks/sqlserver/utils.py b/sqlserver/datadog_checks/sqlserver/utils.py index 84f47fdc68af9..4085e888f006d 100644 --- a/sqlserver/datadog_checks/sqlserver/utils.py +++ b/sqlserver/datadog_checks/sqlserver/utils.py @@ -73,14 +73,14 @@ def _get_index_for_keyword(text, keyword): return -1 -def extract_sql_comments(text): +def extract_sql_comments_and_procedure_name(text): if not text: - return [] + return [], False, None in_single_line_comment = False in_multi_line_comment = False comment_start = None result = [] - + stripped_text = "" for i in range(len(text)): if in_multi_line_comment: if i < len(text) - 1 and text[i : i + 2] == '*/': @@ -91,8 +91,7 @@ def extract_sql_comments(text): elif in_single_line_comment: if text[i] == '\n': in_single_line_comment = False - # strip any extra whitespace at the end - # of the single line comment + # strip any extra whitespace at the end of the single line comment result.append(text[comment_start:i].rstrip()) else: if i < len(text) - 1 and text[i : i + 2] == '--': @@ -101,8 +100,10 @@ def extract_sql_comments(text): elif i < len(text) - 1 and text[i : i + 2] == '/*': in_multi_line_comment = True comment_start = i - - return result + else: + stripped_text += text[i] + is_proc, name = is_statement_proc(stripped_text) + return result, is_proc, name def parse_sqlserver_major_version(version): diff --git a/sqlserver/tests/test_activity.py b/sqlserver/tests/test_activity.py index c7bcb1a395a5c..7ac07e02a3323 100644 --- a/sqlserver/tests/test_activity.py +++ b/sqlserver/tests/test_activity.py @@ -21,7 +21,6 @@ from datadog_checks.base.utils.db.utils import DBMAsyncJob, default_json_event_encoding from datadog_checks.sqlserver import SQLServer from datadog_checks.sqlserver.activity import DM_EXEC_REQUESTS_COLS, _hash_to_hex -from datadog_checks.sqlserver.utils import extract_sql_comments, is_statement_proc from .common import CHECK_NAME from .conftest import DEFAULT_TIMEOUT @@ -594,239 +593,6 @@ def test_get_estimated_row_size_bytes(dbm_instance, file): assert abs((actual_size - computed_size) / float(actual_size)) <= 0.10 -@pytest.mark.parametrize( - "query,is_proc,expected_name", - [ - [ - """\ - CREATE PROCEDURE bobProcedure - BEGIN - SELECT name FROM bob - END; - """, - True, - "bobProcedure", - ], - [ - """\ - CREATE PROC bobProcedure - BEGIN - SELECT name FROM bob - END; - """, - True, - "bobProcedure", - ], - [ - """\ - create procedure bobProcedureLowercase - begin - select name from bob - end; - """, - True, - "bobProcedureLowercase", - ], - [ - """\ - /* my sql is very fun */ - CREATE PROCEDURE bobProcedure - BEGIN - SELECT name FROM bob - END; - """, - True, - "bobProcedure", - ], - [ - """\ - CREATE /* this is fun! */ PROC bobProcedure - BEGIN - SELECT name FROM bob - END; - """, - True, - "bobProcedure", - ], - [ - """\ - -- this is a comment! - CREATE - -- additional comment here! - PROCEDURE bobProcedure - BEGIN - SELECT name FROM bob - END; - """, - True, - "bobProcedure", - ], - [ - "CREATE TABLE bob_table", - False, - None, - ], - [ - "Exec procedure", - False, - None, - ], - [ - "CREATEprocedure", - False, - None, - ], - [ - "procedure create", - False, - None, - ], - ], -) -def test_is_statement_procedure(query, is_proc, expected_name): - p, name = is_statement_proc(query) - assert p == is_proc - assert re.match(name, expected_name, re.IGNORECASE) if expected_name else expected_name == name - - -@pytest.mark.parametrize( - "query,expected_comments", - [ - [ - None, - [], - ], - [ - "", - [], - ], - [ - "/*", - [], - ], - [ - "--", - [], - ], - [ - "/*justonecomment*/", - ["/*justonecomment*/"], - ], - [ - """\ - /* a comment */ - -- Single comment - """, - ["/* a comment */", "-- Single comment"], - ], - [ - "/*tag=foo*/ SELECT * FROM foo;", - ["/*tag=foo*/"], - ], - [ - "/*tag=foo*/ SELECT * FROM /*other=tag,incomment=yes*/ foo;", - ["/*tag=foo*/", "/*other=tag,incomment=yes*/"], - ], - [ - "/*tag=foo*/ SELECT * FROM /*other=tag,incomment=yes*/ foo /*lastword=yes*/", - ["/*tag=foo*/", "/*other=tag,incomment=yes*/", "/*lastword=yes*/"], - ], - [ - """\ - -- My Comment - CREATE PROCEDURE bobProcedure - BEGIN - SELECT name FROM bob - END; - """, - ["-- My Comment"], - ], - [ - """\ - -- My Comment - CREATE PROCEDURE bobProcedure - -- In the middle - BEGIN - SELECT name FROM bob - END; - """, - ["-- My Comment", "-- In the middle"], - ], - [ - """\ - -- My Comment - CREATE PROCEDURE bobProcedure - -- In the middle - BEGIN - SELECT name FROM bob - END; - -- And at the end - """, - ["-- My Comment", "-- In the middle", "-- And at the end"], - ], - [ - """\ - -- My Comment - CREATE PROCEDURE bobProcedure - -- In the middle - /*mixed with mult-line foo*/ - BEGIN - SELECT name FROM bob - END; - -- And at the end - """, - ["-- My Comment", "-- In the middle", "/*mixed with mult-line foo*/", "-- And at the end"], - ], - [ - """\ - /* hello - this is a mult-line-comment - tag=foo,blah=tag - */ - /* - second multi-line - comment - */ - CREATE PROCEDURE bobProcedure - BEGIN - SELECT name FROM bob - END; - -- And at the end - """, - [ - "/* hello this is a mult-line-comment tag=foo,blah=tag */", - "/* second multi-line comment */", - "-- And at the end", - ], - ], - [ - """\ - /* hello - this is a mult-line-commet - tag=foo,blah=tag - */ - CREATE PROCEDURE bobProcedure - -- In the middle - /*mixed with mult-line foo*/ - BEGIN - SELECT name FROM bob - END; - -- And at the end - """, - [ - "/* hello this is a mult-line-commet tag=foo,blah=tag */", - "-- In the middle", - "/*mixed with mult-line foo*/", - "-- And at the end", - ], - ], - ], -) -def test_extract_sql_comments(query, expected_comments): - comments = extract_sql_comments(query) - assert comments == expected_comments - - def test_activity_collection_rate_limit(aggregator, dd_run_check, dbm_instance): # test the activity collection loop rate limit collection_interval = 0.1 diff --git a/sqlserver/tests/test_unit.py b/sqlserver/tests/test_unit.py index c361e201bfb4a..8f4a3d21905d5 100644 --- a/sqlserver/tests/test_unit.py +++ b/sqlserver/tests/test_unit.py @@ -3,6 +3,7 @@ # Licensed under a 3-clause BSD style license (see LICENSE) import copy import os +import re from collections import namedtuple import mock @@ -14,7 +15,12 @@ from datadog_checks.sqlserver.connection import split_sqlserver_host_port from datadog_checks.sqlserver.metrics import SqlDbIndexUsageStats, SqlFractionMetric, SqlMasterDatabaseFileStats from datadog_checks.sqlserver.sqlserver import SQLConnectionError -from datadog_checks.sqlserver.utils import Database, parse_sqlserver_major_version, set_default_driver_conf +from datadog_checks.sqlserver.utils import ( + Database, + extract_sql_comments_and_procedure_name, + parse_sqlserver_major_version, + set_default_driver_conf, +) from .common import CHECK_NAME, DOCKER_SERVER, assert_metrics from .utils import windows_ci @@ -486,3 +492,237 @@ def test_database_state(aggregator, dd_run_check, init_config, instance_docker): 'db:{}'.format(instance_docker['database']), ] aggregator.assert_metric('sqlserver.database.state', tags=expected_tags, hostname=sqlserver_check.resolved_hostname) + + +@pytest.mark.parametrize( + "query,expected_comments,is_proc,expected_name", + [ + [ + None, + [], + False, + None, + ], + [ + "", + [], + False, + None, + ], + [ + "/*", + [], + False, + None, + ], + [ + "--", + [], + False, + None, + ], + [ + "/*justonecomment*/", + ["/*justonecomment*/"], + False, + None, + ], + [ + """\ + /* a comment */ + -- Single comment + """, + ["/* a comment */", "-- Single comment"], + False, + None, + ], + [ + "/*tag=foo*/ SELECT * FROM foo;", + ["/*tag=foo*/"], + False, + None, + ], + [ + "/*tag=foo*/ SELECT * FROM /*other=tag,incomment=yes*/ foo;", + ["/*tag=foo*/", "/*other=tag,incomment=yes*/"], + False, + None, + ], + [ + "/*tag=foo*/ SELECT * FROM /*other=tag,incomment=yes*/ foo /*lastword=yes*/", + ["/*tag=foo*/", "/*other=tag,incomment=yes*/", "/*lastword=yes*/"], + False, + None, + ], + [ + """\ + -- My Comment + CREATE PROCEDURE bobProcedure + BEGIN + SELECT name FROM bob + END; + """, + ["-- My Comment"], + True, + "bobProcedure", + ], + [ + """\ + -- My procedure + CREATE PROCEDURE bobProcedure + BEGIN + SELECT name FROM bob + END; + """, + ["-- My procedure"], + True, + "bobProcedure", + ], + [ + """\ + -- My Comment + CREATE PROCEDURE bobProcedure + -- In the middle + BEGIN + SELECT name FROM bob + END; + """, + ["-- My Comment", "-- In the middle"], + True, + "bobProcedure", + ], + [ + """\ + -- My Comment + CREATE PROCEDURE bobProcedure + -- this procedure does foo + BEGIN + SELECT name FROM bob + END; + """, + ["-- My Comment", "-- this procedure does foo"], + True, + "bobProcedure", + ], + [ + """\ + -- My Comment + CREATE PROCEDURE bobProcedure + -- In the middle + BEGIN + SELECT name FROM bob + END; + -- And at the end + """, + ["-- My Comment", "-- In the middle", "-- And at the end"], + True, + "bobProcedure", + ], + [ + """\ + -- My Comment + CREATE PROCEDURE bobProcedure + -- In the middle + /*mixed with mult-line foo*/ + BEGIN + SELECT name FROM bob + END; + -- And at the end + """, + ["-- My Comment", "-- In the middle", "/*mixed with mult-line foo*/", "-- And at the end"], + True, + "bobProcedure", + ], + [ + """\ + -- My procedure + CREATE PROCEDURE bobProcedure + -- In the middle + /*mixed with procedure foo*/ + BEGIN + SELECT name FROM bob + END; + -- And at the end + """, + ["-- My procedure", "-- In the middle", "/*mixed with procedure foo*/", "-- And at the end"], + True, + "bobProcedure", + ], + [ + """\ + /* hello + this is a mult-line-comment + tag=foo,blah=tag + */ + /* + second multi-line + comment + */ + CREATE PROCEDURE bobProcedure + BEGIN + SELECT name FROM bob + END; + -- And at the end + """, + [ + "/* hello this is a mult-line-comment tag=foo,blah=tag */", + "/* second multi-line comment */", + "-- And at the end", + ], + True, + "bobProcedure", + ], + [ + """\ + /* hello + this is a mult-line-comment + tag=foo,blah=tag + */ + /* + second multi-line + for procedure foo + */ + CREATE PROCEDURE bobProcedure + BEGIN + SELECT name FROM bob + END; + -- And at the end + """, + [ + "/* hello this is a mult-line-comment tag=foo,blah=tag */", + "/* second multi-line for procedure foo */", + "-- And at the end", + ], + True, + "bobProcedure", + ], + [ + """\ + /* hello + this is a mult-line-commet + tag=foo,blah=tag + */ + CREATE PROCEDURE bobProcedure + -- In the middle + /*mixed with mult-line foo*/ + BEGIN + SELECT name FROM bob + END; + -- And at the end + """, + [ + "/* hello this is a mult-line-commet tag=foo,blah=tag */", + "-- In the middle", + "/*mixed with mult-line foo*/", + "-- And at the end", + ], + True, + "bobProcedure", + ], + ], +) +def test_extract_sql_comments_and_procedure_name(query, expected_comments, is_proc, expected_name): + comments, p, name = extract_sql_comments_and_procedure_name(query) + assert comments == expected_comments + assert p == is_proc + assert re.match(name, expected_name, re.IGNORECASE) if expected_name else expected_name == name