Skip to content

Commit be13318

Browse files
authored
[dbm] add optional execution_indicator to rule out false positive query metrics move (#20037)
* add optional execution_indicator to rule out false positive query metrics move * add changelog * update comment * update changelog * remove oracle and db2 from comments
1 parent 640f0f9 commit be13318

File tree

3 files changed

+145
-3
lines changed

3 files changed

+145
-3
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Added execution indicators to StatementMetrics to filter out false positives from normalized queries being evicted and re-inserted with same call count and slight duration change.

datadog_checks_base/datadog_checks/base/utils/db/statement_metrics.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,7 @@ class StatementMetrics:
1313
1414
- Postgres: pg_stat_statements
1515
- MySQL: performance_schema.events_statements_summary_by_digest
16-
- Oracle: V$SQLAREA
1716
- SQL Server: sys.dm_exec_query_stats
18-
- DB2: mon_db_summary
1917
2018
These tables are monotonically increasing, so the metrics are computed from the difference
2119
in values between check runs.
@@ -24,7 +22,7 @@ class StatementMetrics:
2422
def __init__(self):
2523
self._previous_statements = {}
2624

27-
def compute_derivative_rows(self, rows, metrics, key):
25+
def compute_derivative_rows(self, rows, metrics, key, execution_indicators=None):
2826
"""
2927
Compute the first derivative of column-based metrics for a given set of rows. This function
3028
takes the difference of the previous check run's values and the current check run's values
@@ -41,10 +39,20 @@ def compute_derivative_rows(self, rows, metrics, key):
4139
:params rows (_List[dict]_): rows from current check run
4240
:params metrics (_List[str]_): the metrics to compute for each row
4341
:params key (_callable_): function for an ID which uniquely identifies a row across runs
42+
:params execution_indicators (_List[str]_): list of metrics that must change to consider a query as executed.
43+
These are typically metrics that increment only when a query actually executes, such as:
44+
- PostgreSQL: 'calls' from pg_stat_statements
45+
- MySQL: 'exec_count' from performance_schema.events_statements_summary_by_digest
46+
- SQL Server: 'execution_count' from sys.dm_exec_query_stats
47+
This helps filter out cases where a normalized query was evicted then re-inserted with same call count
48+
(usually 1) and slight duration change. In this case, the new normalized query entry should be treated
49+
as the baseline for future diffs.
4450
:return (_List[dict]_): a list of rows with the first derivative of the metrics
4551
"""
4652
result = []
4753
metrics = set(metrics)
54+
if execution_indicators:
55+
execution_indicators = set(execution_indicators)
4856

4957
merged_rows, dropped_metrics = _merge_duplicate_rows(rows, metrics, key)
5058
if dropped_metrics:
@@ -69,6 +77,12 @@ def compute_derivative_rows(self, rows, metrics, key):
6977
# 2. No changes since the previous run: There is no need to store metrics of 0, since that is implied by
7078
# the absence of metrics. On any given check run, most rows will have no difference so this optimization
7179
# avoids having to send a lot of unnecessary metrics.
80+
#
81+
# 3. Execution indicators: If execution_indicators is specified, only consider a query as changed if at
82+
# least one of the execution indicator metrics has changed. This helps filter out cases where an old or
83+
# less frequently executed normalized query was evicted due to the stats table being full, and then
84+
# re-inserted to the stats table with a small call count and slight duration change. In this case,
85+
# the new normalized query entry should be treated as the baseline for future diffs.
7286

7387
diffed_row = {k: row[k] - prev[k] if k in metric_columns else row[k] for k in row.keys()}
7488

@@ -79,6 +93,12 @@ def compute_derivative_rows(self, rows, metrics, key):
7993
# of potentially including truncated rows that exceed previous run counts.
8094
continue
8195

96+
# If execution_indicators is specified, check if any of the execution indicator metrics have changed
97+
if execution_indicators:
98+
indicator_columns = execution_indicators & metric_columns
99+
if not any(diffed_row[k] > 0 for k in indicator_columns):
100+
continue
101+
82102
# No changes to the query; no metric needed
83103
if all(diffed_row[k] == 0 for k in metric_columns):
84104
continue

datadog_checks_base/tests/base/utils/db/test_db_statements.py

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,3 +303,124 @@ def test_compute_derivative_rows_mem_usage(self):
303303
def test_compute_derivative_rows_benchmark(self, benchmark):
304304
sm = StatementMetrics()
305305
benchmark(self.__run_compute_derivative_rows, sm)
306+
307+
def test_compute_derivative_rows_with_execution_indicators(self):
308+
sm = StatementMetrics()
309+
310+
def key(row):
311+
return (row['query'], row['db'], row['user'])
312+
313+
metrics = ['calls', 'total_time', 'rows']
314+
execution_indicators = ['calls']
315+
316+
# Initial state
317+
rows1 = [
318+
{'calls': 10, 'total_time': 1000, 'rows': 50, 'query': 'SELECT 1', 'db': 'test', 'user': 'user1'},
319+
{'calls': 5, 'total_time': 500, 'rows': 25, 'query': 'SELECT 2', 'db': 'test', 'user': 'user1'},
320+
]
321+
322+
sm.compute_derivative_rows(rows1, metrics, key=key, execution_indicators=execution_indicators)
323+
324+
# Second run - only duration changes (should be ignored)
325+
rows2 = [
326+
{'calls': 10, 'total_time': 1001, 'rows': 50, 'query': 'SELECT 1', 'db': 'test', 'user': 'user1'},
327+
{'calls': 5, 'total_time': 501, 'rows': 25, 'query': 'SELECT 2', 'db': 'test', 'user': 'user1'},
328+
]
329+
assert [] == sm.compute_derivative_rows(rows2, metrics, key=key, execution_indicators=execution_indicators)
330+
331+
# Third run - calls change (should be included)
332+
rows3 = [
333+
{'calls': 11, 'total_time': 1002, 'rows': 51, 'query': 'SELECT 1', 'db': 'test', 'user': 'user1'},
334+
{'calls': 5, 'total_time': 502, 'rows': 25, 'query': 'SELECT 2', 'db': 'test', 'user': 'user1'},
335+
]
336+
result = sm.compute_derivative_rows(rows3, metrics, key=key, execution_indicators=execution_indicators)
337+
assert len(result) == 1
338+
assert result[0]['calls'] == 1
339+
assert result[0]['total_time'] == 1
340+
assert result[0]['rows'] == 1
341+
342+
def test_compute_derivative_rows_with_multiple_execution_indicators(self):
343+
sm = StatementMetrics()
344+
345+
def key(row):
346+
return (row['query'], row['db'], row['user'])
347+
348+
metrics = ['calls', 'executions', 'total_time', 'rows']
349+
execution_indicators = ['calls', 'executions']
350+
351+
# Initial state
352+
rows1 = [
353+
{
354+
'calls': 10,
355+
'executions': 10,
356+
'total_time': 1000,
357+
'rows': 50,
358+
'query': 'SELECT 1',
359+
'db': 'test',
360+
'user': 'user1',
361+
},
362+
]
363+
364+
sm.compute_derivative_rows(rows1, metrics, key=key, execution_indicators=execution_indicators)
365+
366+
# Second run - only one execution indicator changes
367+
rows2 = [
368+
{
369+
'calls': 11,
370+
'executions': 10,
371+
'total_time': 1001,
372+
'rows': 50,
373+
'query': 'SELECT 1',
374+
'db': 'test',
375+
'user': 'user1',
376+
},
377+
]
378+
result = sm.compute_derivative_rows(rows2, metrics, key=key, execution_indicators=execution_indicators)
379+
assert len(result) == 1
380+
assert result[0]['calls'] == 1
381+
assert result[0]['executions'] == 0
382+
assert result[0]['total_time'] == 1
383+
assert result[0]['rows'] == 0
384+
385+
# Third run - both execution indicators change
386+
rows3 = [
387+
{
388+
'calls': 12,
389+
'executions': 11,
390+
'total_time': 1002,
391+
'rows': 51,
392+
'query': 'SELECT 1',
393+
'db': 'test',
394+
'user': 'user1',
395+
},
396+
]
397+
result = sm.compute_derivative_rows(rows3, metrics, key=key, execution_indicators=execution_indicators)
398+
assert len(result) == 1
399+
assert result[0]['calls'] == 1
400+
assert result[0]['executions'] == 1
401+
assert result[0]['total_time'] == 1
402+
assert result[0]['rows'] == 1
403+
404+
def test_compute_derivative_rows_with_invalid_execution_indicators(self):
405+
sm = StatementMetrics()
406+
407+
def key(row):
408+
return (row['query'], row['db'], row['user'])
409+
410+
metrics = ['calls', 'total_time', 'rows']
411+
412+
# Test with empty execution indicators
413+
rows1 = [
414+
{'calls': 10, 'total_time': 1000, 'rows': 50, 'query': 'SELECT 1', 'db': 'test', 'user': 'user1'},
415+
]
416+
rows2 = [
417+
{'calls': 11, 'total_time': 1001, 'rows': 51, 'query': 'SELECT 1', 'db': 'test', 'user': 'user1'},
418+
]
419+
420+
# Empty execution indicators should behave like no execution indicators specified
421+
_ = sm.compute_derivative_rows(rows1, metrics, key=key, execution_indicators=[])
422+
result = sm.compute_derivative_rows(rows2, metrics, key=key, execution_indicators=[])
423+
assert len(result) == 1
424+
assert result[0]['calls'] == 1
425+
assert result[0]['total_time'] == 1
426+
assert result[0]['rows'] == 1

0 commit comments

Comments
 (0)