From 593b3147228ed060fd61cf608d2c269daa9a59a7 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Wed, 2 Sep 2026 17:23:02 +0300 Subject: [PATCH 01/13] feat: add metric_stability test for changes to already-measured values Regular anomaly detection compares one bucket against neighbouring buckets, which cannot see a value being rewritten for a period that was already measured. A restatement spanning many historical buckets moves the training baseline along with the data, so the score barely changes, and normal period-to-period variation is usually far wider than the change being looked for. Tests stay green while the numbers underneath them change. metric_stability compares a bucket against its own earlier measurements instead. The version history it needs is already collected: metric ids hash the table, column, metric name and bucket_end while excluding updated_at and metric_value, and rows are appended by the on-run-end hook, so re-measuring a bucket leaves the earlier measurements in place. It is a threshold test rather than an anomaly test by design. For settled data the expected change is zero, so the series has no variance to learn from: with the value excluded from its own training set the stddev is zero and the score is forced to zero, and with it included the score reduces to n/sqrt(n+1), independent of magnitude. A relative threshold also transfers across metrics, where an absolute one has to be retuned per metric. backfill_days is derived from min_bucket_age, because a bucket can only be compared while it is still being re-measured. The default of 2 would freeze every older bucket before it became eligible, and an explicit value too small to produce a comparison now raises rather than passing silently. Co-Authored-By: Claude Opus 5 --- .../monitors_query/metric_stability_query.sql | 150 +++++++++++ macros/edr/tests/test_metric_stability.sql | 247 ++++++++++++++++++ macros/utils/cross_db_utils/first_value.sql | 14 + 3 files changed, 411 insertions(+) create mode 100644 macros/edr/data_monitoring/monitors_query/metric_stability_query.sql create mode 100644 macros/edr/tests/test_metric_stability.sql create mode 100644 macros/utils/cross_db_utils/first_value.sql diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql new file mode 100644 index 000000000..7a6384827 --- /dev/null +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -0,0 +1,150 @@ +{# + Detects metrics whose value for an already-observed time bucket has changed + since a previous run. + + Standard anomaly detection compares different buckets at one point in time. + This compares one bucket against its own earlier measurements, which is a + different axis and a far lower noise floor: for settled data the expected + change is zero. + + The version history this reads is already collected. `data_monitoring_metrics` + is append-only (rows are inserted by the on-run-end hook), and a metric `id` + hashes the table, column, metric name and bucket_end while deliberately + excluding `updated_at` and `metric_value`. So re-measuring a bucket appends a + new row, and the earlier measurements remain. +#} +{% macro metric_stability_query( + test_metrics_table_relation, + full_table_name, + metric_names, + metric_properties, + detection_end, + min_bucket_age=none, + max_change_percent=0, + change_since=["last_check"], + column_name=none, + data_monitoring_metrics_table=none +) %} + {%- if not data_monitoring_metrics_table %} + {%- set data_monitoring_metrics_table = elementary.get_elementary_relation( + "data_monitoring_metrics" + ) %} + {%- endif %} + + {#- Only evaluate buckets old enough to be considered settled. Recent data is + expected to keep moving (late arrivals, unsettled records), so comparing it + produces noise rather than signal. -#} + {%- if min_bucket_age %} + {%- set age_kwargs = {min_bucket_age.period ~ "s": min_bucket_age.count} %} + {%- set max_bucket_end = detection_end - modules.datetime.timedelta( + **age_kwargs + ) %} + {%- else %} {%- set max_bucket_end = detection_end %} + {%- endif %} + {%- set max_bucket_end_expr = elementary.edr_cast_as_timestamp( + elementary.edr_datetime_to_sql(max_bucket_end) + ) %} + + {#- A move away from exactly zero is always a change: the relative form is + undefined there, so it is handled explicitly rather than dividing by zero. -#} + {%- set exceeds_conditions = [] %} + {%- if "last_check" in change_since %} + {%- do exceeds_conditions.append( + "(previous_value is not null and case" + ~ " when previous_value = 0 then metric_value != 0" + ~ " else abs(metric_value - previous_value) / abs(previous_value) * 100.0 > " + ~ max_change_percent + ~ " end)" + ) %} + {%- endif %} + {%- if "first_check" in change_since %} + {%- do exceeds_conditions.append( + "(initial_value is not null and case" + ~ " when initial_value = 0 then metric_value != 0" + ~ " else abs(metric_value - initial_value) / abs(initial_value) * 100.0 > " + ~ max_change_percent + ~ " end)" + ) %} + {%- endif %} + {%- if not exceeds_conditions %} + {%- do exceptions.raise_compiler_error( + "`change_since` must contain at least one of 'last_check', 'first_check'." + ) %} + {%- endif %} + + {%- set metric_stability_query %} + with metrics_history as ( + + select id, full_table_name, column_name, metric_name, metric_type, + bucket_start, bucket_end, bucket_duration_hours, + metric_value, updated_at, dimension, dimension_value + from {{ data_monitoring_metrics_table }} + where + upper(full_table_name) = upper('{{ full_table_name }}') + and metric_name in {{ elementary.strings_list_to_tuple(metric_names) }} + and metric_properties = {{ elementary.dict_to_quoted_json(metric_properties) }} + and bucket_end <= {{ max_bucket_end_expr }} + {%- if column_name %} + and upper(column_name) = upper('{{ column_name }}') + {%- endif %} + + union all + + select id, full_table_name, column_name, metric_name, metric_type, + bucket_start, bucket_end, bucket_duration_hours, + metric_value, updated_at, dimension, dimension_value + from {{ test_metrics_table_relation }} + where bucket_end <= {{ max_bucket_end_expr }} + + ), + + versioned_metrics as ( + + select + id, full_table_name, column_name, metric_name, metric_type, + bucket_start, bucket_end, bucket_duration_hours, + metric_value, updated_at, dimension, dimension_value, + {{ elementary.lag("metric_value") }} over ( + partition by id order by updated_at + ) as previous_value, + {{ elementary.first_value("metric_value") }} over ( + partition by id order by updated_at + rows between unbounded preceding and current row + ) as initial_value, + row_number() over ( + partition by id order by updated_at desc + ) as recency + from metrics_history + + ), + + latest_measurement as ( + + {#- One row per bucket: its newest measurement, carrying the values it + is being compared against. -#} + select * from versioned_metrics where recency = 1 + + ) + + select + id as metric_id, + full_table_name, + column_name, + metric_name, + metric_type, + bucket_start, + bucket_end, + bucket_duration_hours, + dimension, + dimension_value, + updated_at as measured_at, + metric_value, + previous_value, + initial_value, + metric_value - previous_value as change_since_last_check, + metric_value - initial_value as change_since_first_check + from latest_measurement + where {{ exceeds_conditions | join(" or ") }} + {%- endset %} + {%- do return(metric_stability_query) %} +{% endmacro %} diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql new file mode 100644 index 000000000..302263fa8 --- /dev/null +++ b/macros/edr/tests/test_metric_stability.sql @@ -0,0 +1,247 @@ +{# + elementary.metric_stability + + Fails when a metric's value for an already-settled time bucket has changed + since a previous run. + + Regular anomaly detection compares one bucket against neighbouring buckets, so + it cannot see this: a restatement that shifts many historical buckets together + moves the training baseline along with the data, and normal period-to-period + variation is usually far wider than the change being looked for. + + This is a threshold test rather than an anomaly test by design. For settled + data the expected change is zero, so the metric series has no variance to + learn from. A relative threshold also transfers across metrics, where an + absolute one has to be retuned for every metric. + + Arguments: + columns - columns to monitor. + metrics - metric types to monitor per column (e.g. [sum]). + timestamp_column - column that buckets the data into periods. + change_since - baselines to compare against: 'last_check' (the previous + measurement), 'first_check' (the earliest measurement), + or both. 'last_check' catches a sudden correction; + 'first_check' catches slow drift where no single step is + large enough to trip the threshold. + min_bucket_age - only check buckets at least this old, e.g. + {count: 4, period: week}. Recent data is expected to + keep changing, so comparing it produces noise. + max_change_percent - permitted relative change before failing. Defaults to 0, + meaning any change to settled data fails. +#} +{% test metric_stability( + model, + columns, + metrics, + timestamp_column, + change_since=["last_check"], + min_bucket_age=none, + max_change_percent=0, + time_bucket=none, + where_expression=none, + days_back=none, + backfill_days=none, + detection_delay=none, + dimensions=none +) %} + {{ config(tags=["elementary-tests"]) }} + + {%- if execute and elementary.is_test_command() and elementary.is_elementary_enabled() %} + + {%- if max_change_percent < 0 %} + {{ + exceptions.raise_compiler_error( + "max_change_percent must be non-negative." + ) + }} + {%- endif %} + + {%- if not columns %} + {{ + exceptions.raise_compiler_error( + "metric_stability requires at least one column in `columns`." + ) + }} + {%- endif %} + + {%- if not metrics %} + {{ + exceptions.raise_compiler_error( + "metric_stability requires at least one metric type in `metrics`." + ) + }} + {%- endif %} + + {%- for baseline in change_since %} + {%- if baseline not in ["last_check", "first_check"] %} + {{ + exceptions.raise_compiler_error( + "Unsupported `change_since` value '" + ~ baseline + ~ "'. Supported values are 'last_check' and 'first_check'." + ) + }} + {%- endif %} + {%- endfor %} + + {% set model_relation = elementary.get_model_relation_for_test( + model, elementary.get_test_model() + ) %} + {%- if not model_relation %} + {{ exceptions.raise_compiler_error("Unsupported model: " ~ model) }} + {%- endif %} + + {%- if elementary.is_ephemeral_model(model_relation) %} + {{ + exceptions.raise_compiler_error( + "Test not supported for ephemeral models: " + ~ model_relation.identifier + ) + }} + {%- endif %} + + {% set timestamp_column_data_type = ( + elementary.find_normalized_data_type_for_column( + model_relation, timestamp_column + ) + ) %} + {%- if not elementary.is_column_timestamp( + model_relation, timestamp_column, timestamp_column_data_type + ) %} + {{ + exceptions.raise_compiler_error( + "Column '" + ~ timestamp_column + ~ "' is not a timestamp type. metric_stability buckets data over time and requires a timestamp column." + ) + }} + {%- endif %} + + {%- if not dimensions %} {% set dimensions = [] %} {%- endif %} + + {#- The comparison needs each bucket measured more than once, so buckets + must keep being re-measured for as long as they are being checked. + backfill_days controls that window, and its default of 2 would leave + nothing to compare for any older bucket. Derive it from min_bucket_age + so the test cannot silently find nothing. -#} + {%- set required_backfill_days = ( + elementary.get_metric_stability_backfill_days( + min_bucket_age, backfill_days + ) + ) %} + + {% set column_metrics = [] %} + {% set metric_names = [] %} + {%- for metric_type in metrics %} + {% do column_metrics.append({"name": metric_type, "type": metric_type}) %} + {% do metric_names.append(metric_type) %} + {%- endfor %} + + {#- Collect this run's metrics. Shared infrastructure handles bucket + selection, computation, temp table creation and cache storage, and the + on-run-end hook persists them, which is what builds the history this + test reads on later runs. -#} + {%- for column_name in columns %} + {% do elementary.collect_column_metrics( + column_metrics=column_metrics, + model_expr=model, + model_relation=model_relation, + column_name=column_name, + timestamp_column=timestamp_column, + time_bucket=time_bucket, + days_back=days_back, + backfill_days=required_backfill_days, + where_expression=where_expression, + dimensions=dimensions, + collected_by="metric_stability", + ) %} + {%- endfor %} + + {% set model_graph_node = elementary.get_model_graph_node(model_relation) %} + {% set metric_properties = elementary.get_metric_properties( + model_graph_node, + timestamp_column, + where_expression, + time_bucket, + dimensions, + collected_by="metric_stability", + ) %} + + {% set test_metrics_table = elementary.get_elementary_test_table( + elementary.get_elementary_test_table_name(), "metrics" + ) %} + {% set full_table_name = elementary.relation_to_full_name(model_relation) %} + {% set detection_end = elementary.get_detection_end(detection_delay) %} + + {% set metric_stability_query = elementary.metric_stability_query( + test_metrics_table_relation=test_metrics_table, + full_table_name=full_table_name, + metric_names=metric_names, + metric_properties=metric_properties, + detection_end=detection_end, + min_bucket_age=min_bucket_age, + max_change_percent=max_change_percent, + change_since=change_since, + ) %} + {{ + elementary.debug_log( + "metric_stability_query - \n" ~ metric_stability_query + ) + }} + + {{ metric_stability_query }} + + {%- else %} + + {#- test must run an sql query -#} + {{ elementary.no_results_query() }} + + {%- endif %} +{% endtest %} + + +{# + backfill_days sets how far back buckets are re-measured on each run, and a + bucket can only be checked while it is still being re-measured. Eligibility + starts at min_bucket_age, so the window has to reach meaningfully past that + age or a bucket freezes before it can ever be compared. + + The default keeps watching a bucket for as long again as it took to settle, + which gives real coverage rather than a single-day overlap, and matters more + for 'first_check': catching slow drift needs a bucket observed over a stretch, + not once. Cost scales with this window, since that many days of the model are + re-scanned each run. + + An explicit backfill_days that cannot produce a comparison is a configuration + error rather than a silent pass. +#} +{% macro get_metric_stability_backfill_days(min_bucket_age, backfill_days) %} + {%- if not min_bucket_age %} {%- do return(backfill_days) %} {%- endif %} + + {%- set age_kwargs = {min_bucket_age.period ~ "s": min_bucket_age.count} %} + {%- set min_age_days = ( + (modules.datetime.timedelta(**age_kwargs).total_seconds() / 86400) + | round(0, "ceil") + | int + ) %} + {%- set derived = min_age_days * 2 %} + {#- Absolute floor: at least one day of overlap past the age cutoff. -#} + {%- set minimum_viable = min_age_days + 1 %} + + {%- if backfill_days is none %} {%- do return(derived) %} {%- endif %} + + {%- if backfill_days < minimum_viable %} + {%- do exceptions.raise_compiler_error( + "backfill_days is " + ~ backfill_days + ~ ", which is too small to detect changes in buckets at least " + ~ min_age_days + ~ " days old: those buckets stop being re-measured before they become eligible to check, so the test would never find a change. Use at least " + ~ minimum_viable + ~ " (ideally " + ~ derived + ~ "), or remove backfill_days to have it derived automatically." + ) %} + {%- endif %} + {%- do return(backfill_days) %} +{% endmacro %} diff --git a/macros/utils/cross_db_utils/first_value.sql b/macros/utils/cross_db_utils/first_value.sql new file mode 100644 index 000000000..a5123b7a7 --- /dev/null +++ b/macros/utils/cross_db_utils/first_value.sql @@ -0,0 +1,14 @@ +{% macro first_value(column) %} + {{ return(adapter.dispatch("first_value", "elementary")(column)) }} +{% endmacro %} + +{% macro default__first_value(column) %} first_value({{ column }}) {% endmacro %} + +{# + ClickHouse's plain first_value ignores the window frame, so the frame-aware + variant is required to read the earliest value within the frame. + Mirrors the lagInFrame handling in lag.sql. +#} +{% macro clickhouse__first_value(column) %} + first_valueinframe({{ column }}) +{% endmacro %} From 0beca4038c50b9ea88273dab783d87767299cea3 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Wed, 2 Sep 2026 17:27:04 +0300 Subject: [PATCH 02/13] test: cover metric_stability across warehouses Three cases, all driven through the shared harness so they run on every supported adapter: - a restatement of a settled bucket is caught, after two runs establish that the bucket had been measured and was stable - a change inside min_bucket_age is ignored, since recent data is expected to keep moving as late records arrive - max_change_percent tolerates a change below the threshold and still fails one above it, which is what makes a single relative threshold usable across metrics with very different magnitudes Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 integration_tests/tests/test_metric_stability.py diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py new file mode 100644 index 000000000..c7032e7e6 --- /dev/null +++ b/integration_tests/tests/test_metric_stability.py @@ -0,0 +1,97 @@ +from datetime import datetime, time, timedelta +from typing import Any, Dict, List + +from data_generator import DATE_FORMAT +from dbt_project import DbtProject + +TIMESTAMP_COLUMN = "updated_at" +VALUE_COLUMN = "amount" +DBT_TEST_NAME = "elementary.metric_stability" + +BASE_AMOUNT = 100 +DAYS_OF_HISTORY = 6 + +# min_bucket_age of one day means buckets older than a day are checked, and the +# derived backfill window (twice the age) keeps them being re-measured, so a +# bucket two days old is both settled and still under observation. +SETTLED_DAYS_AGO = 2 +UNSETTLED_DAYS_AGO = 1 + +DBT_TEST_ARGS: Dict[str, Any] = { + "columns": [VALUE_COLUMN], + "metrics": ["sum"], + "timestamp_column": TIMESTAMP_COLUMN, + "time_bucket": {"period": "day", "count": 1}, + "days_back": 7, + "change_since": ["last_check", "first_check"], + "min_bucket_age": {"count": 1, "period": "day"}, +} + + +def _rows(restatements: Dict[int, int]) -> List[Dict[str, Any]]: + """One row per day, midday so it lands unambiguously inside a daily bucket.""" + utc_today = datetime.utcnow().date() + rows = [] + for days_ago in range(1, DAYS_OF_HISTORY + 1): + timestamp = datetime.combine(utc_today - timedelta(days=days_ago), time(12, 0)) + rows.append( + { + TIMESTAMP_COLUMN: timestamp.strftime(DATE_FORMAT), + VALUE_COLUMN: restatements.get(days_ago, BASE_AMOUNT), + } + ) + return rows + + +def _run(dbt_project: DbtProject, test_id: str, data, **overrides) -> str: + args = {**DBT_TEST_ARGS, **overrides} + result = dbt_project.test(test_id, DBT_TEST_NAME, args, data=data) + return result["status"] + + +def test_metric_stability_detects_restated_settled_value( + test_id: str, dbt_project: DbtProject +): + baseline = _rows({}) + + # First run only establishes an initial measurement, so there is nothing to + # compare against yet. + assert _run(dbt_project, test_id, baseline) == "pass" + + # Second run measures the same buckets again and the values are unchanged. + assert _run(dbt_project, test_id, baseline) == "pass" + + # Rewriting the value of an already-settled bucket is what the test exists + # to catch, even though the value itself is unremarkable next to other days. + restated = _rows({SETTLED_DAYS_AGO: BASE_AMOUNT * 2}) + assert _run(dbt_project, test_id, restated) == "fail" + + +def test_metric_stability_ignores_unsettled_buckets( + test_id: str, dbt_project: DbtProject +): + baseline = _rows({}) + assert _run(dbt_project, test_id, baseline) == "pass" + assert _run(dbt_project, test_id, baseline) == "pass" + + # Recent data is expected to keep moving as late records arrive, so a change + # inside min_bucket_age must not be reported. + restated = _rows({UNSETTLED_DAYS_AGO: BASE_AMOUNT * 2}) + assert _run(dbt_project, test_id, restated) == "pass" + + +def test_metric_stability_tolerates_change_within_threshold( + test_id: str, dbt_project: DbtProject +): + baseline = _rows({}) + assert _run(dbt_project, test_id, baseline, max_change_percent=25) == "pass" + assert _run(dbt_project, test_id, baseline, max_change_percent=25) == "pass" + + # A 10% restatement sits under the 25% tolerance and should be allowed, + # which is what makes one relative threshold usable across metrics. + within = _rows({SETTLED_DAYS_AGO: int(BASE_AMOUNT * 1.1)}) + assert _run(dbt_project, test_id, within, max_change_percent=25) == "pass" + + # The same bucket moving well past the tolerance must still fail. + beyond = _rows({SETTLED_DAYS_AGO: BASE_AMOUNT * 2}) + assert _run(dbt_project, test_id, beyond, max_change_percent=25) == "fail" From 100a1bf10a6a0b7373b28144f1624ddcb15a1d11 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Thu, 3 Sep 2026 13:10:29 +0300 Subject: [PATCH 03/13] fix: address first review round on metric_stability Two compile errors on adapters Postgres does not exercise: - clickhouse__first_value called first_valueInFrame, which does not exist. The premise was wrong too: ClickHouse needs lagInFrame because it has no lag at all, not because of framing, and its first_value does respect an ordered frame. The override and its dispatch macro are removed. - A CASE returning a boolean is invalid T-SQL, which has no first-class boolean value, so the parser failed on the "!". Conditions now keep booleans in boolean position. Two correctness bugs: - With more than one column, collect_column_metrics created a table per column and left the cache pointing at the last one, so every other column was compared against the previous run rather than this one and a restatement surfaced a run late. Columns now share one temp table, built the way all_columns_anomalies does it. - The read had no lower bound on bucket_end, so a bucket that stopped being re-measured kept satisfying the predicate on every subsequent run: one restatement failed the test permanently, with no way to clear it. Every run also scanned the table's whole metric history. days_back now bounds the read, making the eligible set a band and giving partition pruning. min_bucket_age becomes required, since defaulting it meant the out-of-the-box configuration compared buckets still inside the backfill window at zero tolerance, which is the noise the design exists to avoid. It is also shape validated, as is metrics, so a bad value gives a compiler error rather than a raw traceback or a query rendered against None. The window guard now checks the parameter that actually governs. backfill_days only widens the measurement window on the incremental branch of get_metric_buckets_min_and_max; a plain table model takes the regular branch, where days_back alone decides. Guarding backfill_days there bought nothing while reporting everything as fine. detection_delay is dropped rather than left half-wired, since it shifted the read cutoff but not the measurement window and min_bucket_age already covers the same ground. Tests now isolate the two baselines, so swapping them can no longer pass, and assert the compared values rather than only the pass/fail status. Multi-column coverage is added, which is how the per-column bug got through. Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 172 ++++++++-- .../monitors_query/metric_stability_query.sql | 109 +++--- macros/edr/tests/test_metric_stability.sql | 319 +++++++++++++----- macros/utils/cross_db_utils/first_value.sql | 14 - 4 files changed, 446 insertions(+), 168 deletions(-) delete mode 100644 macros/utils/cross_db_utils/first_value.sql diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index c7032e7e6..42eb80bba 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -1,35 +1,40 @@ from datetime import datetime, time, timedelta -from typing import Any, Dict, List +from typing import Any, Dict, List, Optional from data_generator import DATE_FORMAT from dbt_project import DbtProject TIMESTAMP_COLUMN = "updated_at" VALUE_COLUMN = "amount" +OTHER_VALUE_COLUMN = "other_amount" DBT_TEST_NAME = "elementary.metric_stability" BASE_AMOUNT = 100 +OTHER_BASE_AMOUNT = 500 DAYS_OF_HISTORY = 6 -# min_bucket_age of one day means buckets older than a day are checked, and the -# derived backfill window (twice the age) keeps them being re-measured, so a +# A min_bucket_age of one day means buckets older than a day are checked, and the +# derived observation window (twice the age) keeps them being measured, so a # bucket two days old is both settled and still under observation. SETTLED_DAYS_AGO = 2 UNSETTLED_DAYS_AGO = 1 -DBT_TEST_ARGS: Dict[str, Any] = { +BASE_ARGS: Dict[str, Any] = { "columns": [VALUE_COLUMN], "metrics": ["sum"], "timestamp_column": TIMESTAMP_COLUMN, "time_bucket": {"period": "day", "count": 1}, - "days_back": 7, - "change_since": ["last_check", "first_check"], "min_bucket_age": {"count": 1, "period": "day"}, } -def _rows(restatements: Dict[int, int]) -> List[Dict[str, Any]]: +def _rows( + restatements: Optional[Dict[int, int]] = None, + other_restatements: Optional[Dict[int, int]] = None, +) -> List[Dict[str, Any]]: """One row per day, midday so it lands unambiguously inside a daily bucket.""" + restatements = restatements or {} + other_restatements = other_restatements or {} utc_today = datetime.utcnow().date() rows = [] for days_ago in range(1, DAYS_OF_HISTORY + 1): @@ -38,39 +43,125 @@ def _rows(restatements: Dict[int, int]) -> List[Dict[str, Any]]: { TIMESTAMP_COLUMN: timestamp.strftime(DATE_FORMAT), VALUE_COLUMN: restatements.get(days_ago, BASE_AMOUNT), + OTHER_VALUE_COLUMN: other_restatements.get(days_ago, OTHER_BASE_AMOUNT), } ) return rows def _run(dbt_project: DbtProject, test_id: str, data, **overrides) -> str: - args = {**DBT_TEST_ARGS, **overrides} - result = dbt_project.test(test_id, DBT_TEST_NAME, args, data=data) + result = dbt_project.test( + test_id, DBT_TEST_NAME, {**BASE_ARGS, **overrides}, data=data + ) return result["status"] +def _bucket_values( + dbt_project: DbtProject, test_id: str, column_name: str = VALUE_COLUMN +) -> Dict[Any, List[float]]: + """Measured values per bucket, oldest measurement first. + + Asserting on these rather than only on pass/fail means a wrong baseline or a + wrong sign in the comparison cannot slip through. + """ + metrics = dbt_project.read_table( + "data_monitoring_metrics", + where=( + f"full_table_name LIKE '%{test_id.upper()}' " + f"and metric_name = 'sum' " + f"and lower(column_name) = '{column_name}'" + ), + ) + by_bucket: Dict[Any, List[Any]] = {} + for metric in metrics: + by_bucket.setdefault(str(metric["bucket_end"]), []).append( + (str(metric["updated_at"]), float(metric["metric_value"])) + ) + return { + bucket: [value for _, value in sorted(measurements)] + for bucket, measurements in by_bucket.items() + } + + +def _restated_bucket(values: Dict[Any, List[float]]) -> List[float]: + """The one bucket whose measurements are not all identical.""" + moved = [ + measurements for measurements in values.values() if len(set(measurements)) > 1 + ] + assert len(moved) == 1, f"expected exactly one bucket to move, got {moved}" + return moved[0] + + def test_metric_stability_detects_restated_settled_value( test_id: str, dbt_project: DbtProject ): - baseline = _rows({}) + baseline = _rows() + args = {"change_since": ["last_check"]} - # First run only establishes an initial measurement, so there is nothing to - # compare against yet. - assert _run(dbt_project, test_id, baseline) == "pass" + # The first run only establishes an initial measurement, so there is nothing + # to compare against yet. + assert _run(dbt_project, test_id, baseline, **args) == "pass" - # Second run measures the same buckets again and the values are unchanged. - assert _run(dbt_project, test_id, baseline) == "pass" + # The second measures the same buckets again and the values are unchanged. + assert _run(dbt_project, test_id, baseline, **args) == "pass" - # Rewriting the value of an already-settled bucket is what the test exists - # to catch, even though the value itself is unremarkable next to other days. + # Rewriting the value of an already-settled bucket is what the test exists to + # catch, even though the value itself is unremarkable next to other days. restated = _rows({SETTLED_DAYS_AGO: BASE_AMOUNT * 2}) - assert _run(dbt_project, test_id, restated) == "fail" + assert _run(dbt_project, test_id, restated, **args) == "fail" + + # The bucket the test flagged must be the one that actually moved, and by + # the amount restated, so a wrong baseline cannot pass unnoticed. + measurements = _restated_bucket(_bucket_values(dbt_project, test_id)) + assert measurements[0] == BASE_AMOUNT + assert measurements[-1] == BASE_AMOUNT * 2 + + +def test_metric_stability_first_check_catches_gradual_drift( + test_id: str, dbt_project: DbtProject +): + """Drift too small to trip the threshold on any single step, but not overall. + + This is the case that justifies having 'first_check' at all: comparing only + against the previous measurement never sees it. + """ + args = {"change_since": ["first_check"], "max_change_percent": 15} + assert _run(dbt_project, test_id, _rows(), **args) == "pass" + + # +10% against the original value: under the threshold either way. + assert _run(dbt_project, test_id, _rows({SETTLED_DAYS_AGO: 110}), **args) == "pass" + + # A further +9% step, still under the threshold on its own, but now 20% away + # from where the bucket started. + assert _run(dbt_project, test_id, _rows({SETTLED_DAYS_AGO: 120}), **args) == "fail" + + # Each step is under the threshold; only the distance from the first + # measurement crosses it. + measurements = _restated_bucket(_bucket_values(dbt_project, test_id)) + assert measurements[0] == BASE_AMOUNT + assert measurements[-1] == 120 + steps = [ + later - earlier + for earlier, later in zip(measurements, measurements[1:]) + if later != earlier + ] + assert all(step / BASE_AMOUNT * 100 < 15 for step in steps), steps + + +def test_metric_stability_last_check_ignores_gradual_drift( + test_id: str, dbt_project: DbtProject +): + """The same drift, compared only against the previous run, stays invisible.""" + args = {"change_since": ["last_check"], "max_change_percent": 15} + assert _run(dbt_project, test_id, _rows(), **args) == "pass" + assert _run(dbt_project, test_id, _rows({SETTLED_DAYS_AGO: 110}), **args) == "pass" + assert _run(dbt_project, test_id, _rows({SETTLED_DAYS_AGO: 120}), **args) == "pass" def test_metric_stability_ignores_unsettled_buckets( test_id: str, dbt_project: DbtProject ): - baseline = _rows({}) + baseline = _rows() assert _run(dbt_project, test_id, baseline) == "pass" assert _run(dbt_project, test_id, baseline) == "pass" @@ -83,15 +174,42 @@ def test_metric_stability_ignores_unsettled_buckets( def test_metric_stability_tolerates_change_within_threshold( test_id: str, dbt_project: DbtProject ): - baseline = _rows({}) - assert _run(dbt_project, test_id, baseline, max_change_percent=25) == "pass" - assert _run(dbt_project, test_id, baseline, max_change_percent=25) == "pass" + args = {"max_change_percent": 25} + assert _run(dbt_project, test_id, _rows(), **args) == "pass" + assert _run(dbt_project, test_id, _rows(), **args) == "pass" - # A 10% restatement sits under the 25% tolerance and should be allowed, - # which is what makes one relative threshold usable across metrics. + # A 10% restatement sits under the 25% tolerance and should be allowed, which + # is what makes one relative threshold usable across metrics of very + # different magnitudes. within = _rows({SETTLED_DAYS_AGO: int(BASE_AMOUNT * 1.1)}) - assert _run(dbt_project, test_id, within, max_change_percent=25) == "pass" + assert _run(dbt_project, test_id, within, **args) == "pass" - # The same bucket moving well past the tolerance must still fail. beyond = _rows({SETTLED_DAYS_AGO: BASE_AMOUNT * 2}) - assert _run(dbt_project, test_id, beyond, max_change_percent=25) == "fail" + assert _run(dbt_project, test_id, beyond, **args) == "fail" + + +def test_metric_stability_detects_restatement_in_any_column( + test_id: str, dbt_project: DbtProject +): + """Every monitored column is compared against this run's own measurements. + + Collecting per column into separate tables would leave all but the last + column comparing against the previous run, so a restatement in an earlier + column would surface a run late. + """ + args = {"columns": [VALUE_COLUMN, OTHER_VALUE_COLUMN]} + baseline = _rows() + assert _run(dbt_project, test_id, baseline, **args) == "pass" + assert _run(dbt_project, test_id, baseline, **args) == "pass" + + # Restate the first of the two columns, which is the one a per-column table + # would have left stale. + restated = _rows({SETTLED_DAYS_AGO: BASE_AMOUNT * 2}) + assert _run(dbt_project, test_id, restated, **args) == "fail" + + # The restated column moved, and the other one did not. + measurements = _restated_bucket(_bucket_values(dbt_project, test_id)) + assert measurements[0] == BASE_AMOUNT + assert measurements[-1] == BASE_AMOUNT * 2 + other = _bucket_values(dbt_project, test_id, OTHER_VALUE_COLUMN) + assert all(len(set(m)) == 1 for m in other.values()), other diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 7a6384827..77d4aa50e 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -1,5 +1,5 @@ {# - Detects metrics whose value for an already-observed time bucket has changed + Detects metrics whose value for an already-settled time bucket has changed since a previous run. Standard anomaly detection compares different buckets at one point in time. @@ -7,6 +7,14 @@ different axis and a far lower noise floor: for settled data the expected change is zero. + This is deliberately a threshold test rather than an anomaly test. A settled + series has no variance to learn from, and the scoring degenerates in both + directions. With the value excluded from its own training set the stddev is + zero and the score is forced to zero, so it never fires. With the value + included, n unchanged observations followed by one value v give mean v/(n+1) + and stddev v/sqrt(n+1), so the score is n/sqrt(n+1): the v cancels and the + score reflects how long the history is rather than how large the change was. + The version history this reads is already collected. `data_monitoring_metrics` is append-only (rows are inserted by the on-run-end hook), and a metric `id` hashes the table, column, metric name and bucket_end while deliberately @@ -19,10 +27,10 @@ metric_names, metric_properties, detection_end, - min_bucket_age=none, + days_back, + min_bucket_age, max_change_percent=0, change_since=["last_check"], - column_name=none, data_monitoring_metrics_table=none ) %} {%- if not data_monitoring_metrics_table %} @@ -31,46 +39,62 @@ ) %} {%- endif %} - {#- Only evaluate buckets old enough to be considered settled. Recent data is - expected to keep moving (late arrivals, unsettled records), so comparing it - produces noise rather than signal. -#} - {%- if min_bucket_age %} - {%- set age_kwargs = {min_bucket_age.period ~ "s": min_bucket_age.count} %} - {%- set max_bucket_end = detection_end - modules.datetime.timedelta( - **age_kwargs - ) %} - {%- else %} {%- set max_bucket_end = detection_end %} - {%- endif %} - {%- set max_bucket_end_expr = elementary.edr_cast_as_timestamp( - elementary.edr_datetime_to_sql(max_bucket_end) + {%- set bucket_period = metric_properties.time_bucket.period %} + + {#- Eligible buckets form a band. The upper edge keeps recent data out: + it is expected to keep moving as late records arrive, so comparing it + produces noise. The lower edge bounds the read, which both prunes the + scan (and enables partition pruning) and lets a reported change age out + of the window instead of failing the test forever. -#} + {%- set age_kwargs = {min_bucket_age.period ~ "s": min_bucket_age.count} %} + {%- set max_bucket_end = detection_end - modules.datetime.timedelta(**age_kwargs) %} + {%- set min_bucket_end = detection_end - modules.datetime.timedelta( + days=days_back | int + ) %} + {%- set max_bucket_end_expr = elementary.edr_date_trunc( + bucket_period, + elementary.edr_cast_as_timestamp( + elementary.edr_datetime_to_sql(max_bucket_end) + ), + ) %} + {%- set min_bucket_end_expr = elementary.edr_date_trunc( + bucket_period, + elementary.edr_cast_as_timestamp( + elementary.edr_datetime_to_sql(min_bucket_end) + ), + ) %} + {%- set bucket_window = ( + "bucket_end > " + ~ min_bucket_end_expr + ~ " and bucket_end <= " + ~ max_bucket_end_expr ) %} - {#- A move away from exactly zero is always a change: the relative form is - undefined there, so it is handled explicitly rather than dividing by zero. -#} + {#- Conditions keep booleans in boolean position rather than returning one + from a CASE, which T-SQL has no first-class value for. A move away from + exactly zero is handled separately, since the relative form is undefined + there. -#} {%- set exceeds_conditions = [] %} - {%- if "last_check" in change_since %} - {%- do exceeds_conditions.append( - "(previous_value is not null and case" - ~ " when previous_value = 0 then metric_value != 0" - ~ " else abs(metric_value - previous_value) / abs(previous_value) * 100.0 > " - ~ max_change_percent - ~ " end)" + {%- for baseline in change_since %} + {%- set baseline_column = ( + "previous_value" if baseline == "last_check" else "initial_value" ) %} - {%- endif %} - {%- if "first_check" in change_since %} {%- do exceeds_conditions.append( - "(initial_value is not null and case" - ~ " when initial_value = 0 then metric_value != 0" - ~ " else abs(metric_value - initial_value) / abs(initial_value) * 100.0 > " + "(" + ~ baseline_column + ~ " is not null and ((" + ~ baseline_column + ~ " = 0 and metric_value != 0) or (" + ~ baseline_column + ~ " != 0 and abs(metric_value - " + ~ baseline_column + ~ ") / abs(" + ~ baseline_column + ~ ") * 100.0 > " ~ max_change_percent - ~ " end)" + ~ ")))" ) %} - {%- endif %} - {%- if not exceeds_conditions %} - {%- do exceptions.raise_compiler_error( - "`change_since` must contain at least one of 'last_check', 'first_check'." - ) %} - {%- endif %} + {%- endfor %} {%- set metric_stability_query %} with metrics_history as ( @@ -83,10 +107,7 @@ upper(full_table_name) = upper('{{ full_table_name }}') and metric_name in {{ elementary.strings_list_to_tuple(metric_names) }} and metric_properties = {{ elementary.dict_to_quoted_json(metric_properties) }} - and bucket_end <= {{ max_bucket_end_expr }} - {%- if column_name %} - and upper(column_name) = upper('{{ column_name }}') - {%- endif %} + and {{ bucket_window }} union all @@ -94,7 +115,7 @@ bucket_start, bucket_end, bucket_duration_hours, metric_value, updated_at, dimension, dimension_value from {{ test_metrics_table_relation }} - where bucket_end <= {{ max_bucket_end_expr }} + where {{ bucket_window }} ), @@ -107,7 +128,7 @@ {{ elementary.lag("metric_value") }} over ( partition by id order by updated_at ) as previous_value, - {{ elementary.first_value("metric_value") }} over ( + first_value(metric_value) over ( partition by id order by updated_at rows between unbounded preceding and current row ) as initial_value, @@ -120,8 +141,8 @@ latest_measurement as ( - {#- One row per bucket: its newest measurement, carrying the values it - is being compared against. -#} + {#- One row per bucket: its newest measurement, carrying the values + it is being compared against. -#} select * from versioned_metrics where recency = 1 ) diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index 302263fa8..7442c1840 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -9,39 +9,40 @@ moves the training baseline along with the data, and normal period-to-period variation is usually far wider than the change being looked for. - This is a threshold test rather than an anomaly test by design. For settled - data the expected change is zero, so the metric series has no variance to - learn from. A relative threshold also transfers across metrics, where an - absolute one has to be retuned for every metric. - Arguments: columns - columns to monitor. metrics - metric types to monitor per column (e.g. [sum]). timestamp_column - column that buckets the data into periods. + min_bucket_age - required. Only check buckets at least this old, e.g. + {count: 4, period: week}. Recent data is expected to + keep changing as late records arrive, so comparing it + reports noise rather than restatements. change_since - baselines to compare against: 'last_check' (the previous measurement), 'first_check' (the earliest measurement), or both. 'last_check' catches a sudden correction; - 'first_check' catches slow drift where no single step is - large enough to trip the threshold. - min_bucket_age - only check buckets at least this old, e.g. - {count: 4, period: week}. Recent data is expected to - keep changing, so comparing it produces noise. - max_change_percent - permitted relative change before failing. Defaults to 0, - meaning any change to settled data fails. + 'first_check' catches slow drift, where each step is too + small to trip the threshold but the total movement is + not. + max_change_percent - permitted change in percentage points before failing + (25 means 25%). Defaults to 0, so any change to settled + data fails. + days_back - how far back buckets are measured and compared. This is + the observation window, and it must extend past + min_bucket_age or no bucket is ever both settled and + still being measured. #} {% test metric_stability( model, columns, metrics, timestamp_column, + min_bucket_age, change_since=["last_check"], - min_bucket_age=none, max_change_percent=0, time_bucket=none, where_expression=none, days_back=none, backfill_days=none, - detection_delay=none, dimensions=none ) %} {{ config(tags=["elementary-tests"]) }} @@ -72,6 +73,23 @@ }} {%- endif %} + {%- set available_column_monitors = ( + elementary.get_available_column_monitors() + ) %} + {%- for metric_type in metrics %} + {%- if metric_type not in available_column_monitors %} + {{ + exceptions.raise_compiler_error( + "Unsupported column metric: '" + ~ metric_type + ~ "'. Supported metrics are: " + ~ available_column_monitors + | join(", ") ~ "." + ) + }} + {%- endif %} + {%- endfor %} + {%- for baseline in change_since %} {%- if baseline not in ["last_check", "first_check"] %} {{ @@ -84,6 +102,8 @@ {%- endif %} {%- endfor %} + {% do elementary.validate_min_bucket_age(min_bucket_age) %} + {% set model_relation = elementary.get_model_relation_for_test( model, elementary.get_test_model() ) %} @@ -119,16 +139,19 @@ {%- if not dimensions %} {% set dimensions = [] %} {%- endif %} - {#- The comparison needs each bucket measured more than once, so buckets - must keep being re-measured for as long as they are being checked. - backfill_days controls that window, and its default of 2 would leave - nothing to compare for any older bucket. Derive it from min_bucket_age - so the test cannot silently find nothing. -#} - {%- set required_backfill_days = ( - elementary.get_metric_stability_backfill_days( - min_bucket_age, backfill_days - ) + {% set model_graph_node = elementary.get_model_graph_node(model_relation) %} + {#- The measurement window has to extend past min_bucket_age, so it is + derived from it when not set explicitly rather than falling back to + defaults that would leave nothing to compare. -#} + {% set resolved_window = elementary.resolve_metric_stability_window( + model_relation, + model_graph_node, + min_bucket_age, + days_back, + backfill_days, ) %} + {% set days_back = resolved_window["days_back"] %} + {% set backfill_days = resolved_window["backfill_days"] %} {% set column_metrics = [] %} {% set metric_names = [] %} @@ -137,27 +160,6 @@ {% do metric_names.append(metric_type) %} {%- endfor %} - {#- Collect this run's metrics. Shared infrastructure handles bucket - selection, computation, temp table creation and cache storage, and the - on-run-end hook persists them, which is what builds the history this - test reads on later runs. -#} - {%- for column_name in columns %} - {% do elementary.collect_column_metrics( - column_metrics=column_metrics, - model_expr=model, - model_relation=model_relation, - column_name=column_name, - timestamp_column=timestamp_column, - time_bucket=time_bucket, - days_back=days_back, - backfill_days=required_backfill_days, - where_expression=where_expression, - dimensions=dimensions, - collected_by="metric_stability", - ) %} - {%- endfor %} - - {% set model_graph_node = elementary.get_model_graph_node(model_relation) %} {% set metric_properties = elementary.get_metric_properties( model_graph_node, timestamp_column, @@ -167,18 +169,99 @@ collected_by="metric_stability", ) %} - {% set test_metrics_table = elementary.get_elementary_test_table( - elementary.get_elementary_test_table_name(), "metrics" + {% set test_table_name = elementary.get_elementary_test_table_name() %} + {% set ( + database_name, + schema_name, + ) = elementary.get_package_database_and_schema("elementary") %} + {% set tests_schema_name = elementary.get_elementary_tests_schema( + database_name, schema_name ) %} {% set full_table_name = elementary.relation_to_full_name(model_relation) %} - {% set detection_end = elementary.get_detection_end(detection_delay) %} + {#- One shared metrics table for every column. collect_column_metrics + would create a table per column and leave the cache pointing at the + last one, so all but the final column would be compared against + stale measurements. -#} + {% set temp_table_relation = elementary.create_elementary_test_table( + database_name, + tests_schema_name, + test_table_name, + "metrics", + elementary.empty_data_monitoring_metrics(with_created_at=false), + ) %} + + {%- for column_name in columns %} + {%- set column_obj_and_monitors = ( + elementary.get_column_obj_and_monitors( + model_relation, column_name, metrics + ) + ) -%} + {%- if not column_obj_and_monitors %} + {{ + exceptions.raise_compiler_error( + "Unable to find column `" + ~ column_name + ~ "` in `" + ~ full_table_name + ~ "`." + ) + }} + {%- endif %} + {%- set column_monitors = column_obj_and_monitors["monitors"] %} + {%- if not column_monitors %} + {{ + exceptions.raise_compiler_error( + "None of the metrics " ~ metrics + | join(", ") + ~ " apply to column `" + ~ column_name + ~ "` given its data type." + ) + }} + {%- endif %} + + {%- set ( + min_bucket_start, + max_bucket_end, + ) = elementary.get_metric_buckets_min_and_max( + model_relation=model_relation, + backfill_days=backfill_days, + days_back=days_back, + metric_names=column_monitors, + column_name=column_name, + metric_properties=metric_properties, + ) %} + {%- set column_monitoring_query = elementary.column_monitoring_query( + model, + model_relation, + min_bucket_start, + max_bucket_end, + days_back, + column_obj_and_monitors["column"], + column_metrics, + metric_properties, + dimensions, + ) %} + {%- do elementary.run_query( + elementary.insert_as_select( + temp_table_relation, column_monitoring_query + ) + ) -%} + {%- endfor %} + + {#- Persist this run's measurements, which is what builds the history + the next run compares against. -#} + {% do elementary.store_metrics_table_in_cache() %} + + {% set detection_end = elementary.get_detection_end(none) %} {% set metric_stability_query = elementary.metric_stability_query( - test_metrics_table_relation=test_metrics_table, + test_metrics_table_relation=temp_table_relation, full_table_name=full_table_name, metric_names=metric_names, metric_properties=metric_properties, detection_end=detection_end, + days_back=days_back, min_bucket_age=min_bucket_age, max_change_percent=max_change_percent, change_since=change_since, @@ -200,48 +283,118 @@ {% endtest %} +{% macro validate_min_bucket_age(min_bucket_age) %} + {%- set valid_periods = ["day", "week", "hour", "minute", "second"] %} + {%- if not min_bucket_age or min_bucket_age is not mapping %} + {# fmt: off #} + {% do exceptions.raise_compiler_error( + "min_bucket_age is required and must be a mapping. Expected format: min_bucket_age: count: int period: string" + ) %} + {# fmt: on #} + {%- endif %} + {%- for key in min_bucket_age %} + {%- if key not in ["count", "period"] %} + {% do exceptions.raise_compiler_error( + "Found invalid key in min_bucket_age: '" + ~ key + ~ "'. Supported keys: count, period." + ) %} + {%- endif %} + {%- endfor %} + {%- if min_bucket_age.period not in valid_periods %} + {% do exceptions.raise_compiler_error( + "Unsupported min_bucket_age period '" + ~ min_bucket_age.period + ~ "'. Supported periods: " + ~ valid_periods + | join(", ") ~ "." + ) %} + {%- endif %} + {%- if min_bucket_age.count is not integer or min_bucket_age.count < 1 %} + {% do exceptions.raise_compiler_error( + "min_bucket_age count must be a positive integer, got '" + ~ min_bucket_age.count + ~ "'." + ) %} + {%- endif %} +{% endmacro %} + + {# - backfill_days sets how far back buckets are re-measured on each run, and a - bucket can only be checked while it is still being re-measured. Eligibility - starts at min_bucket_age, so the window has to reach meaningfully past that - age or a bucket freezes before it can ever be compared. - - The default keeps watching a bucket for as long again as it took to settle, - which gives real coverage rather than a single-day overlap, and matters more - for 'first_check': catching slow drift needs a bucket observed over a stretch, - not once. Cost scales with this window, since that many days of the model are - re-scanned each run. - - An explicit backfill_days that cannot produce a comparison is a configuration - error rather than a silent pass. -#} -{% macro get_metric_stability_backfill_days(min_bucket_age, backfill_days) %} - {%- if not min_bucket_age %} {%- do return(backfill_days) %} {%- endif %} + A bucket can only be compared while it is still being re-measured, so the + measurement window has to extend past min_bucket_age or nothing is ever both + settled and still under observation. + + Which parameter governs that window depends on the materialization. + get_metric_buckets_min_and_max takes its incremental branch for sources and + incremental models, where backfill_days sets the window; every other model + takes the regular branch, which re-measures the whole days_back window and + ignores backfill_days entirely. days_back additionally bounds the comparison + itself, so it always matters. + Unset parameters are derived from min_bucket_age, because the package defaults + (days_back 14, backfill_days 2) are unrelated to how long a bucket needs + watching and would silently leave nothing to compare. An explicit value too + small to ever produce a comparison raises instead. +#} +{% macro resolve_metric_stability_window( + model_relation, + model_graph_node, + min_bucket_age, + days_back, + backfill_days +) %} {%- set age_kwargs = {min_bucket_age.period ~ "s": min_bucket_age.count} %} {%- set min_age_days = ( (modules.datetime.timedelta(**age_kwargs).total_seconds() / 86400) | round(0, "ceil") | int ) %} - {%- set derived = min_age_days * 2 %} - {#- Absolute floor: at least one day of overlap past the age cutoff. -#} - {%- set minimum_viable = min_age_days + 1 %} - - {%- if backfill_days is none %} {%- do return(derived) %} {%- endif %} - - {%- if backfill_days < minimum_viable %} - {%- do exceptions.raise_compiler_error( - "backfill_days is " - ~ backfill_days - ~ ", which is too small to detect changes in buckets at least " - ~ min_age_days - ~ " days old: those buckets stop being re-measured before they become eligible to check, so the test would never find a change. Use at least " - ~ minimum_viable - ~ " (ideally " - ~ derived - ~ "), or remove backfill_days to have it derived automatically." - ) %} + {#- Twice the age, so a bucket is observed over a stretch rather than for a + single run, which is what lets 'first_check' see drift accumulate. -#} + {%- set derived = [min_age_days * 2, min_age_days + 1] | max %} + + {%- set uses_backfill_window = elementary.is_incremental_model( + model_graph_node, source_included=true + ) %} + + {%- if days_back is none %} {%- set resolved_days_back = derived %} + {%- else %} + {%- set resolved_days_back = days_back %} + {%- if resolved_days_back <= min_age_days %} + {% do exceptions.raise_compiler_error( + "days_back is " + ~ resolved_days_back + ~ ", which does not extend past a min_bucket_age of " + ~ min_age_days + ~ " days, so no bucket is ever both settled and still measured and the test can never report a change. Use at least " + ~ derived + ~ ", or omit days_back to have it derived." + ) %} + {%- endif %} {%- endif %} - {%- do return(backfill_days) %} + + {%- if backfill_days is none %} + {%- set resolved_backfill_days = resolved_days_back %} + {%- else %} + {%- set resolved_backfill_days = backfill_days %} + {%- if uses_backfill_window and resolved_backfill_days <= min_age_days %} + {% do exceptions.raise_compiler_error( + "backfill_days is " + ~ resolved_backfill_days + ~ ", which does not extend past a min_bucket_age of " + ~ min_age_days + ~ " days. On incremental models and sources backfill_days sets how far back buckets are re-measured, so those buckets freeze before they become eligible to check. Use at least " + ~ derived + ~ ", or omit backfill_days to have it derived." + ) %} + {%- endif %} + {%- endif %} + + {%- do return( + { + "days_back": resolved_days_back, + "backfill_days": resolved_backfill_days, + } + ) %} {% endmacro %} diff --git a/macros/utils/cross_db_utils/first_value.sql b/macros/utils/cross_db_utils/first_value.sql deleted file mode 100644 index a5123b7a7..000000000 --- a/macros/utils/cross_db_utils/first_value.sql +++ /dev/null @@ -1,14 +0,0 @@ -{% macro first_value(column) %} - {{ return(adapter.dispatch("first_value", "elementary")(column)) }} -{% endmacro %} - -{% macro default__first_value(column) %} first_value({{ column }}) {% endmacro %} - -{# - ClickHouse's plain first_value ignores the window frame, so the frame-aware - variant is required to read the earliest value within the frame. - Mirrors the lagInFrame handling in lag.sql. -#} -{% macro clickhouse__first_value(column) %} - first_valueinframe({{ column }}) -{% endmacro %} From a37b93b07d3aaa6fbf25f1b2014cedd8048e54fd Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Thu, 3 Sep 2026 13:25:31 +0300 Subject: [PATCH 04/13] fix: address second review round on metric_stability Cross-test contamination: the history read filtered by table, metric name and metric_properties, but not by column. metric_properties does not carry the column, so two metric_stability tests on the same model would each load the other's history, and a change in a column this test never configured could surface as its failure. The read is now scoped to the monitored columns. Invalid metrics per column: the per-column loop resolved the monitors that apply to a column's data type and used them for bucket selection, but still handed the unfiltered list to column_monitoring_query. Monitoring a mixed set across numeric and string columns would generate sum() and fail on the warehouse. Each column now gets only its applicable monitors. Sub-day min_bucket_age: the age was ceiled to whole days before being compared against days_back, so an age of one hour was treated as a day and days_back of 1 was rejected, even though it covers 23 settled hourly buckets. The comparison now uses a fraction of a day, ceiling only when deriving the default, and the error reports the units the user wrote. Argument shapes: yaml allows a single value as a scalar, and iterating a string in jinja walks it character by character, so `change_since: last_check` failed with "Unsupported change_since value 'l'". Scalars are normalised to lists, columns are deduplicated, and an empty change_since now raises instead of rendering a WHERE with no predicate. timestamp_column is commonly set once in a model's elementary config rather than repeated per test. It is now resolved through get_test_argument before the column type is validated, instead of failing with "Column 'None' is not a timestamp type". Failing rows also carry the relative change, which is what the threshold is applied to, so a failure is interpretable without recomputing it by hand. Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 5 +- .../monitors_query/metric_stability_query.sql | 17 ++++- macros/edr/tests/test_metric_stability.sql | 74 +++++++++++++++---- 3 files changed, 79 insertions(+), 17 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 42eb80bba..56a634068 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -1,4 +1,5 @@ from datetime import datetime, time, timedelta +from itertools import pairwise from typing import Any, Dict, List, Optional from data_generator import DATE_FORMAT @@ -141,9 +142,7 @@ def test_metric_stability_first_check_catches_gradual_drift( assert measurements[0] == BASE_AMOUNT assert measurements[-1] == 120 steps = [ - later - earlier - for earlier, later in zip(measurements, measurements[1:]) - if later != earlier + later - earlier for earlier, later in pairwise(measurements) if later != earlier ] assert all(step / BASE_AMOUNT * 100 < 15 for step in steps), steps diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 77d4aa50e..97b88e6e5 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -31,6 +31,7 @@ min_bucket_age, max_change_percent=0, change_since=["last_check"], + column_names=none, data_monitoring_metrics_table=none ) %} {%- if not data_monitoring_metrics_table %} @@ -105,6 +106,12 @@ from {{ data_monitoring_metrics_table }} where upper(full_table_name) = upper('{{ full_table_name }}') + {%- if column_names %} + {#- metric_properties does not carry the column, so without + this a test picks up history for every other column + monitored on the same table with the same properties. -#} + and upper(column_name) in {{ elementary.strings_list_to_tuple(column_names | map("upper") | list) }} + {%- endif %} and metric_name in {{ elementary.strings_list_to_tuple(metric_names) }} and metric_properties = {{ elementary.dict_to_quoted_json(metric_properties) }} and {{ bucket_window }} @@ -163,7 +170,15 @@ previous_value, initial_value, metric_value - previous_value as change_since_last_check, - metric_value - initial_value as change_since_first_check + metric_value - initial_value as change_since_first_check, + case + when previous_value is not null and previous_value != 0 + then abs(metric_value - previous_value) / abs(previous_value) * 100.0 + end as change_percent_since_last_check, + case + when initial_value is not null and initial_value != 0 + then abs(metric_value - initial_value) / abs(initial_value) * 100.0 + end as change_percent_since_first_check from latest_measurement where {{ exceeds_conditions | join(" or ") }} {%- endset %} diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index 7442c1840..e38c8c1da 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -49,6 +49,23 @@ {%- if execute and elementary.is_test_command() and elementary.is_elementary_enabled() %} + {#- yaml lets a single value be written as a scalar, and iterating a + string in jinja walks it character by character. -#} + {%- if columns is string %} {% set columns = [columns] %} {%- endif %} + {%- if metrics is string %} {% set metrics = [metrics] %} {%- endif %} + {%- if change_since is string %} + {% set change_since = [change_since] %} + {%- endif %} + {%- set columns = columns | unique | list if columns else columns %} + + {%- if not change_since %} + {{ + exceptions.raise_compiler_error( + "metric_stability requires at least one baseline in `change_since`: 'last_check', 'first_check', or both." + ) + }} + {%- endif %} + {%- if max_change_percent < 0 %} {{ exceptions.raise_compiler_error( @@ -120,6 +137,20 @@ }} {%- endif %} + {% set model_graph_node = elementary.get_model_graph_node(model_relation) %} + {#- timestamp_column is commonly set once in the model's elementary + config rather than repeated on every test. -#} + {% set timestamp_column = elementary.get_test_argument( + "timestamp_column", timestamp_column, model_graph_node + ) %} + {%- if not timestamp_column %} + {{ + exceptions.raise_compiler_error( + "metric_stability requires a `timestamp_column`, either on the test or in the model's elementary config." + ) + }} + {%- endif %} + {% set timestamp_column_data_type = ( elementary.find_normalized_data_type_for_column( model_relation, timestamp_column @@ -139,7 +170,6 @@ {%- if not dimensions %} {% set dimensions = [] %} {%- endif %} - {% set model_graph_node = elementary.get_model_graph_node(model_relation) %} {#- The measurement window has to extend past min_bucket_age, so it is derived from it when not set explicitly rather than falling back to defaults that would leave nothing to compare. -#} @@ -232,6 +262,12 @@ column_name=column_name, metric_properties=metric_properties, ) %} + {#- Only the monitors that apply to this column's data type. + Passing the full list would generate e.g. sum(). -#} + {%- set this_column_metrics = [] %} + {%- for monitor in column_monitors %} + {%- do this_column_metrics.append({"name": monitor, "type": monitor}) %} + {%- endfor %} {%- set column_monitoring_query = elementary.column_monitoring_query( model, model_relation, @@ -239,7 +275,7 @@ max_bucket_end, days_back, column_obj_and_monitors["column"], - column_metrics, + this_column_metrics, metric_properties, dimensions, ) %} @@ -265,6 +301,7 @@ min_bucket_age=min_bucket_age, max_change_percent=max_change_percent, change_since=change_since, + column_names=columns, ) %} {{ elementary.debug_log( @@ -345,14 +382,25 @@ backfill_days ) %} {%- set age_kwargs = {min_bucket_age.period ~ "s": min_bucket_age.count} %} - {%- set min_age_days = ( - (modules.datetime.timedelta(**age_kwargs).total_seconds() / 86400) - | round(0, "ceil") - | int + {#- Kept as a fraction of a day. Ceiling it first would turn a sub-day age + into a whole day and reject a days_back that in fact covers many + settled buckets. -#} + {%- set age_days = ( + modules.datetime.timedelta(**age_kwargs).total_seconds() / 86400.0 ) %} {#- Twice the age, so a bucket is observed over a stretch rather than for a single run, which is what lets 'first_check' see drift accumulate. -#} - {%- set derived = [min_age_days * 2, min_age_days + 1] | max %} + {%- set derived = [ + (age_days * 2) | round(0, "ceil") | int, + (age_days + 1) | round(0, "ceil") | int, + 1, + ] | max %} + {%- set age_description = ( + min_bucket_age.count + ~ " " + ~ min_bucket_age.period + ~ ("s" if min_bucket_age.count > 1 else "") + ) %} {%- set uses_backfill_window = elementary.is_incremental_model( model_graph_node, source_included=true @@ -361,13 +409,13 @@ {%- if days_back is none %} {%- set resolved_days_back = derived %} {%- else %} {%- set resolved_days_back = days_back %} - {%- if resolved_days_back <= min_age_days %} + {%- if resolved_days_back <= age_days %} {% do exceptions.raise_compiler_error( "days_back is " ~ resolved_days_back ~ ", which does not extend past a min_bucket_age of " - ~ min_age_days - ~ " days, so no bucket is ever both settled and still measured and the test can never report a change. Use at least " + ~ age_description + ~ ", so no bucket is ever both settled and still measured and the test can never report a change. Use at least " ~ derived ~ ", or omit days_back to have it derived." ) %} @@ -378,13 +426,13 @@ {%- set resolved_backfill_days = resolved_days_back %} {%- else %} {%- set resolved_backfill_days = backfill_days %} - {%- if uses_backfill_window and resolved_backfill_days <= min_age_days %} + {%- if uses_backfill_window and resolved_backfill_days <= age_days %} {% do exceptions.raise_compiler_error( "backfill_days is " ~ resolved_backfill_days ~ ", which does not extend past a min_bucket_age of " - ~ min_age_days - ~ " days. On incremental models and sources backfill_days sets how far back buckets are re-measured, so those buckets freeze before they become eligible to check. Use at least " + ~ age_description + ~ ". On incremental models and sources backfill_days sets how far back buckets are re-measured, so those buckets freeze before they become eligible to check. Use at least " ~ derived ~ ", or omit backfill_days to have it derived." ) %} From 7e0683fca9d84edf7078f5373ed1f9e78440611e Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Sun, 6 Sep 2026 07:54:48 +0300 Subject: [PATCH 05/13] fix: make metric_stability actually fire Two silent failures, both reproduced on DuckDB. The metrics table was created empty and filled with INSERT statements. On adapters where dbt rolls back the test's transaction those rows are gone before the on-run-end flush, so data_monitoring_metrics never receives any history and the comparison has nothing to compare. Four of the six tests failed this way on DuckDB, Vertica and Redshift. Each column now gets its own table created directly from its select, and they are unioned at read time. The observation window was derived purely in days and never looked at time_bucket, and the bucket grid was anchored on a value that moves by a day between runs. For any period longer than a day that gave every measurement a fresh surrogate id, so no bucket was ever measured twice. The window now accounts for the bucket length, the grid anchor is snapped to the bucket period, and a time_bucket count above 1 raises instead of passing forever. Also: dedupe columns case-insensitively so a duplicate spelling cannot make 'last_check' compare a run against itself; type-check the numeric arguments before comparing them; skip the backfill_days validation when force_metrics_backfill makes it irrelevant; share one change-percent expression between the predicate and the reported columns; drop an unused local and an unread macro parameter. Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 85 ++++++++ .../monitors_query/metric_stability_query.sql | 56 +++-- macros/edr/tests/test_metric_stability.sql | 193 +++++++++++++----- 3 files changed, 268 insertions(+), 66 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 56a634068..0702206aa 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -157,6 +157,91 @@ def test_metric_stability_last_check_ignores_gradual_drift( assert _run(dbt_project, test_id, _rows({SETTLED_DAYS_AGO: 120}), **args) == "pass" +def test_metric_stability_records_history_across_runs( + test_id: str, dbt_project: DbtProject +): + """Every run must add a measurement, not replace the previous one. + + The comparison has nothing to compare unless earlier measurements survive in + data_monitoring_metrics. On adapters that roll back the test transaction an + INSERT-populated metrics table is discarded before the on-run-end flush, and + the test then passes forever without ever recording anything. Asserting on + the accumulating row counts catches that directly, where a pass/fail + assertion cannot tell "nothing changed" from "nothing was measured". + """ + baseline = _rows() + counts = [] + for _ in range(3): + assert _run(dbt_project, test_id, baseline) == "pass" + values = _bucket_values(dbt_project, test_id) + assert values, "no metrics were recorded at all" + counts.append(sum(len(m) for m in values.values())) + + assert counts[0] > 0, counts + assert counts[1] > counts[0], counts + assert counts[2] > counts[1], counts + + settled = [m for m in values.values() if len(m) == 3] + assert settled, f"no bucket was measured on all three runs: {values}" + + +def test_metric_stability_detects_restatement_with_weekly_buckets( + test_id: str, dbt_project: DbtProject +): + """A bucket longer than a day must still get a stable identity across runs. + + The bucket grid is anchored on a value that moves by a day between runs, so + without snapping the anchor to the bucket period every measurement lands on + a fresh surrogate id, no bucket is ever measured twice and the test silently + never fires. + """ + args = { + "time_bucket": {"period": "week", "count": 1}, + "min_bucket_age": {"count": 1, "period": "week"}, + } + restate_days_ago = 16 + + def weekly_rows(restated=None): + utc_today = datetime.utcnow().date() + rows = [] + for days_ago in range(1, 36): + timestamp = datetime.combine( + utc_today - timedelta(days=days_ago), time(12, 0) + ) + rows.append( + { + TIMESTAMP_COLUMN: timestamp.strftime(DATE_FORMAT), + VALUE_COLUMN: ( + restated + if restated and days_ago == restate_days_ago + else BASE_AMOUNT + ), + OTHER_VALUE_COLUMN: OTHER_BASE_AMOUNT, + } + ) + return rows + + assert _run(dbt_project, test_id, weekly_rows(), **args) == "pass" + assert _run(dbt_project, test_id, weekly_rows(), **args) == "pass" + assert ( + _run(dbt_project, test_id, weekly_rows(restated=BASE_AMOUNT * 2), **args) + == "fail" + ) + + +def test_metric_stability_rejects_multi_step_buckets( + test_id: str, dbt_project: DbtProject +): + """A count > 1 bucket cannot be given a stable identity, so it must raise.""" + result = _run( + dbt_project, + test_id, + _rows(), + time_bucket={"period": "day", "count": 3}, + ) + assert result == "error" + + def test_metric_stability_ignores_unsettled_buckets( test_id: str, dbt_project: DbtProject ): diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 97b88e6e5..f2aae7f85 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -22,7 +22,7 @@ new row, and the earlier measurements remain. #} {% macro metric_stability_query( - test_metrics_table_relation, + test_metrics_table_relations, full_table_name, metric_names, metric_properties, @@ -76,25 +76,27 @@ exactly zero is handled separately, since the relative form is undefined there. -#} {%- set exceeds_conditions = [] %} + {%- set baseline_columns = [] %} {%- for baseline in change_since %} {%- set baseline_column = ( "previous_value" if baseline == "last_check" else "initial_value" ) %} - {%- do exceeds_conditions.append( - "(" - ~ baseline_column - ~ " is not null and ((" - ~ baseline_column - ~ " = 0 and metric_value != 0) or (" - ~ baseline_column - ~ " != 0 and abs(metric_value - " - ~ baseline_column - ~ ") / abs(" - ~ baseline_column - ~ ") * 100.0 > " - ~ max_change_percent - ~ ")))" - ) %} + {%- if baseline_column not in baseline_columns %} + {%- do baseline_columns.append(baseline_column) %} + {%- do exceeds_conditions.append( + "(" + ~ baseline_column + ~ " is not null and ((" + ~ baseline_column + ~ " = 0 and metric_value != 0) or (" + ~ baseline_column + ~ " != 0 and " + ~ elementary.metric_stability_change_percent(baseline_column) + ~ " > " + ~ max_change_percent + ~ ")))" + ) %} + {%- endif %} {%- endfor %} {%- set metric_stability_query %} @@ -116,6 +118,8 @@ and metric_properties = {{ elementary.dict_to_quoted_json(metric_properties) }} and {{ bucket_window }} + {%- for test_metrics_table_relation in test_metrics_table_relations %} + union all select id, full_table_name, column_name, metric_name, metric_type, @@ -123,6 +127,7 @@ metric_value, updated_at, dimension, dimension_value from {{ test_metrics_table_relation }} where {{ bucket_window }} + {%- endfor %} ), @@ -173,14 +178,29 @@ metric_value - initial_value as change_since_first_check, case when previous_value is not null and previous_value != 0 - then abs(metric_value - previous_value) / abs(previous_value) * 100.0 + then {{ elementary.metric_stability_change_percent("previous_value") }} end as change_percent_since_last_check, case when initial_value is not null and initial_value != 0 - then abs(metric_value - initial_value) / abs(initial_value) * 100.0 + then {{ elementary.metric_stability_change_percent("initial_value") }} end as change_percent_since_first_check from latest_measurement where {{ exceeds_conditions | join(" or ") }} {%- endset %} {%- do return(metric_stability_query) %} {% endmacro %} + + +{# + Relative change from a baseline column, in percentage points. Shared by the + WHERE predicate and the reported columns so the two cannot drift apart. +#} +{% macro metric_stability_change_percent(baseline_column) %} + {%- do return( + "abs(metric_value - " + ~ baseline_column + ~ ") / abs(" + ~ baseline_column + ~ ") * 100.0" + ) %} +{% endmacro %} diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index e38c8c1da..a8ae42f4f 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -56,7 +56,21 @@ {%- if change_since is string %} {% set change_since = [change_since] %} {%- endif %} - {%- set columns = columns | unique | list if columns else columns %} + {#- Column lookup is case-insensitive, so a duplicate spelling would + otherwise be collected twice under one metric id and make the + 'last_check' baseline this run's own second measurement. -#} + {%- if columns %} + {%- set seen_columns = [] %} + {%- set deduped_columns = [] %} + {%- for column_name in columns %} + {%- set key = column_name | trim('"') | lower %} + {%- if key not in seen_columns %} + {%- do seen_columns.append(key) %} + {%- do deduped_columns.append(column_name) %} + {%- endif %} + {%- endfor %} + {%- set columns = deduped_columns %} + {%- endif %} {%- if not change_since %} {{ @@ -66,6 +80,17 @@ }} {%- endif %} + {#- Comparing a string against 0 raises a bare Python TypeError, which + surfaces as an unreadable stack trace rather than a config error. -#} + {%- if max_change_percent is not number %} + {{ + exceptions.raise_compiler_error( + "max_change_percent must be a number, got '" + ~ max_change_percent + ~ "'. Write it unquoted, e.g. max_change_percent: 25." + ) + }} + {%- endif %} {%- if max_change_percent < 0 %} {{ exceptions.raise_compiler_error( @@ -73,6 +98,23 @@ ) }} {%- endif %} + {%- for arg_name, arg_value in [ + ("days_back", days_back), + ("backfill_days", backfill_days), + ] %} + {%- if arg_value is not none and arg_value is not number %} + {{ + exceptions.raise_compiler_error( + arg_name + ~ " must be a number, got '" + ~ arg_value + ~ "'. Write it unquoted, e.g. " + ~ arg_name + ~ ": 30." + ) + }} + {%- endif %} + {%- endfor %} {%- if not columns %} {{ @@ -170,26 +212,6 @@ {%- if not dimensions %} {% set dimensions = [] %} {%- endif %} - {#- The measurement window has to extend past min_bucket_age, so it is - derived from it when not set explicitly rather than falling back to - defaults that would leave nothing to compare. -#} - {% set resolved_window = elementary.resolve_metric_stability_window( - model_relation, - model_graph_node, - min_bucket_age, - days_back, - backfill_days, - ) %} - {% set days_back = resolved_window["days_back"] %} - {% set backfill_days = resolved_window["backfill_days"] %} - - {% set column_metrics = [] %} - {% set metric_names = [] %} - {%- for metric_type in metrics %} - {% do column_metrics.append({"name": metric_type, "type": metric_type}) %} - {% do metric_names.append(metric_type) %} - {%- endfor %} - {% set metric_properties = elementary.get_metric_properties( model_graph_node, timestamp_column, @@ -198,6 +220,21 @@ dimensions, collected_by="metric_stability", ) %} + {% set metric_names = metrics %} + + {#- The measurement window has to extend past min_bucket_age and has to + be wide enough to hold whole buckets, so it is derived from both + rather than falling back to defaults that would leave nothing to + compare. -#} + {% set resolved_window = elementary.resolve_metric_stability_window( + model_graph_node, + min_bucket_age, + metric_properties.time_bucket, + days_back, + backfill_days, + ) %} + {% set days_back = resolved_window["days_back"] %} + {% set backfill_days = resolved_window["backfill_days"] %} {% set test_table_name = elementary.get_elementary_test_table_name() %} {% set ( @@ -212,14 +249,15 @@ {#- One shared metrics table for every column. collect_column_metrics would create a table per column and leave the cache pointing at the last one, so all but the final column would be compared against - stale measurements. -#} - {% set temp_table_relation = elementary.create_elementary_test_table( - database_name, - tests_schema_name, - test_table_name, - "metrics", - elementary.empty_data_monitoring_metrics(with_created_at=false), - ) %} + stale measurements. + + Each column gets its own table, created directly from its select. + Creating one table empty and filling it with INSERT statements + loses the rows on adapters where dbt rolls back the test's + transaction, which leaves data_monitoring_metrics with no history + at all and makes this test a silent permanent pass. The tables are + unioned at read time instead. -#} + {% set temp_table_relations = [] %} {%- for column_name in columns %} {%- set column_obj_and_monitors = ( @@ -252,7 +290,7 @@ {%- endif %} {%- set ( - min_bucket_start, + raw_min_bucket_start, max_bucket_end, ) = elementary.get_metric_buckets_min_and_max( model_relation=model_relation, @@ -262,6 +300,16 @@ column_name=column_name, metric_properties=metric_properties, ) %} + {#- get_metric_buckets_min_and_max can return a plain midnight + (backfill_bucket_start), which the bucket grid is then anchored + on. For any period longer than a day that midnight moves with + the run, so the grid drifts, every bucket_end lands on a new + surrogate id and no bucket is ever measured twice. Snapping the + anchor to the bucket period keeps ids stable across runs. -#} + {%- set min_bucket_start = elementary.edr_date_trunc( + metric_properties.time_bucket.period, + elementary.edr_cast_as_timestamp(raw_min_bucket_start), + ) %} {#- Only the monitors that apply to this column's data type. Passing the full list would generate e.g. sum(). -#} {%- set this_column_metrics = [] %} @@ -279,20 +327,31 @@ metric_properties, dimensions, ) %} - {%- do elementary.run_query( - elementary.insert_as_select( - temp_table_relation, column_monitoring_query + {%- do temp_table_relations.append( + elementary.create_elementary_test_table( + database_name, + tests_schema_name, + test_table_name, + "metrics_" ~ loop.index0, + column_monitoring_query, ) - ) -%} + ) %} {%- endfor %} {#- Persist this run's measurements, which is what builds the history - the next run compares against. -#} - {% do elementary.store_metrics_table_in_cache() %} + the next run compares against. store_metrics_table_in_cache only + knows about a single "metrics" table, so the per-column relations + are registered directly. -#} + {% set metrics_tables_cache = ( + elementary.get_cache("tables").get("metrics").get("relations") + ) %} + {%- for temp_table_relation in temp_table_relations %} + {% do metrics_tables_cache.append(temp_table_relation) %} + {%- endfor %} {% set detection_end = elementary.get_detection_end(none) %} {% set metric_stability_query = elementary.metric_stability_query( - test_metrics_table_relation=temp_table_relation, + test_metrics_table_relations=temp_table_relations, full_table_name=full_table_name, metric_names=metric_names, metric_properties=metric_properties, @@ -344,7 +403,8 @@ ~ min_bucket_age.period ~ "'. Supported periods: " ~ valid_periods - | join(", ") ~ "." + | join(", ") + ~ ". time_bucket also accepts month, quarter and year; express an age over those in days, e.g. {count: 60, period: day}." ) %} {%- endif %} {%- if min_bucket_age.count is not integer or min_bucket_age.count < 1 %} @@ -375,11 +435,7 @@ small to ever produce a comparison raises instead. #} {% macro resolve_metric_stability_window( - model_relation, - model_graph_node, - min_bucket_age, - days_back, - backfill_days + model_graph_node, min_bucket_age, time_bucket, days_back, backfill_days ) %} {%- set age_kwargs = {min_bucket_age.period ~ "s": min_bucket_age.count} %} {#- Kept as a fraction of a day. Ceiling it first would turn a sub-day age @@ -388,10 +444,47 @@ {%- set age_days = ( modules.datetime.timedelta(**age_kwargs).total_seconds() / 86400.0 ) %} + + {#- The grid anchor moves by a day between runs, so a bucket spanning more + than one period step cannot be given a stable identity and the test + would silently never fire. Refuse rather than pass forever. -#} + {%- if time_bucket.count | int != 1 %} + {% do exceptions.raise_compiler_error( + "metric_stability requires a time_bucket count of 1, got " + ~ time_bucket.count + ~ ". A multi-step bucket cannot be measured on a stable grid across runs, so the test would never report a change." + ) %} + {%- endif %} + + {#- Bucket length in days, used to make sure the eligible band can actually + hold whole buckets. month/quarter/year are nominal: they only have to be + good enough to size the window. -#} + {%- set period_days = { + "second": 1.0 / 86400.0, + "minute": 1.0 / 1440.0, + "hour": 1.0 / 24.0, + "day": 1.0, + "week": 7.0, + "month": 30.0, + "quarter": 91.0, + "year": 365.0, + } %} + {%- set bucket_days = period_days.get(time_bucket.period | lower) %} + {%- if not bucket_days %} + {% do exceptions.raise_compiler_error( + "Unsupported time_bucket period for metric_stability: '" + ~ time_bucket.period + ~ "'." + ) %} + {%- endif %} + {#- Twice the age, so a bucket is observed over a stretch rather than for a - single run, which is what lets 'first_check' see drift accumulate. -#} + single run, which is what lets 'first_check' see drift accumulate; and + at least two whole buckets past the age, or the settled band is narrower + than one bucket and nothing is ever both settled and still measured. -#} {%- set derived = [ (age_days * 2) | round(0, "ceil") | int, + (age_days + 2 * bucket_days) | round(0, "ceil") | int, (age_days + 1) | round(0, "ceil") | int, 1, ] | max %} @@ -402,18 +495,22 @@ ~ ("s" if min_bucket_age.count > 1 else "") ) %} + {#- get_metric_buckets_min_and_max only takes its backfill branch when + force_metrics_backfill is off; with it on every model re-measures the + whole days_back window and backfill_days is ignored, so validating it + would abort the run over a value that has no effect. -#} {%- set uses_backfill_window = elementary.is_incremental_model( model_graph_node, source_included=true - ) %} + ) and not elementary.get_config_var("force_metrics_backfill") %} {%- if days_back is none %} {%- set resolved_days_back = derived %} {%- else %} {%- set resolved_days_back = days_back %} - {%- if resolved_days_back <= age_days %} + {%- if resolved_days_back < derived %} {% do exceptions.raise_compiler_error( "days_back is " ~ resolved_days_back - ~ ", which does not extend past a min_bucket_age of " + ~ ", which does not leave room for whole buckets past a min_bucket_age of " ~ age_description ~ ", so no bucket is ever both settled and still measured and the test can never report a change. Use at least " ~ derived @@ -426,7 +523,7 @@ {%- set resolved_backfill_days = resolved_days_back %} {%- else %} {%- set resolved_backfill_days = backfill_days %} - {%- if uses_backfill_window and resolved_backfill_days <= age_days %} + {%- if uses_backfill_window and resolved_backfill_days < derived %} {% do exceptions.raise_compiler_error( "backfill_days is " ~ resolved_backfill_days From 2ff2da7ab85de737c0a64bd3613a55d5014e23f2 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Sun, 6 Sep 2026 10:14:58 +0300 Subject: [PATCH 06/13] fix: keep settling and float noise out of metric_stability A bucket's first measurements are taken while late records are still arriving, which is the period min_bucket_age exists to exclude. They were eligible as the 'first_check' baseline, so every comparison carried the settling as a permanent offset and the slow drift 'first_check' exists to find sat underneath it. Measurements are now bounded by the same age as the buckets. The current run's own measurement always qualifies, since a bucket is only eligible once bucket_end + min_bucket_age has passed. Repeating a float aggregate can differ in the last bits when the scan is partitioned differently between runs, and a strict comparison against the default max_change_percent of 0 reported that as a failure on data nobody touched. A noise floor well above float error and well below any real movement now sits under the threshold, leaving the zero-crossing rule alone. Also documents two things that are not being changed: 'first_check' needs several measurements per bucket to differ from 'last_check', so min_bucket_age should be a multiple of the run interval; and a bucket that loses all of its rows produces no measurement rather than a zero, so total deletion is not reported while partial deletion still is. Co-Authored-By: Claude Opus 5 --- .../tests/test_metric_stability.py | 28 ++++++++++++++++++ .../monitors_query/metric_stability_query.sql | 29 +++++++++++++++++-- macros/edr/tests/test_metric_stability.sql | 18 ++++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 0702206aa..4ca5480d1 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -242,6 +242,34 @@ def test_metric_stability_rejects_multi_step_buckets( assert result == "error" +def test_metric_stability_ignores_measurements_taken_while_settling( + test_id: str, dbt_project: DbtProject +): + """A bucket's own settling must not become the 'first_check' baseline. + + The first measurements of a bucket are taken while late records are still + arriving, which is the period min_bucket_age exists to exclude. If they are + used as the baseline, every later comparison carries that settling as a + permanent offset and real drift is buried under it. + """ + args = {"change_since": ["first_check"], "max_change_percent": 15} + assert _run(dbt_project, test_id, _rows(), **args) == "pass" + + # Backdate this run's measurements into the settling window and move their + # values far away. Were they still eligible as a baseline, the next run + # would compare 100 against 10 and report a 900% change. + dbt_project.run_query( + f""" + UPDATE {{{{ ref('data_monitoring_metrics') }}}} + SET metric_value = 10, updated_at = bucket_end + WHERE full_table_name LIKE '%{test_id.upper()}' + AND metric_name = 'sum' + """ + ) + + assert _run(dbt_project, test_id, _rows(), **args) == "pass" + + def test_metric_stability_ignores_unsettled_buckets( test_id: str, dbt_project: DbtProject ): diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index f2aae7f85..bd2cb6133 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -71,10 +71,33 @@ ~ max_bucket_end_expr ) %} + {#- A bucket's first measurements are taken while it is still settling, and + min_bucket_age exists precisely to keep that period out of scope. Left + in, they become the 'first_check' baseline, so every comparison carries + the settling as a permanent offset and the drift 'first_check' exists to + find is buried under it. Measurements are therefore bounded by the same + age as the buckets. The current run's own measurement always qualifies: + a bucket is only eligible once bucket_end + min_bucket_age has passed. -#} + {%- set settled_measurement_window = "updated_at >= " ~ elementary.edr_timeadd( + min_bucket_age.period, min_bucket_age.count, "bucket_end" + ) %} + {%- set history_window = bucket_window ~ " and " ~ settled_measurement_window %} + {#- Conditions keep booleans in boolean position rather than returning one from a CASE, which T-SQL has no first-class value for. A move away from exactly zero is handled separately, since the relative form is undefined there. -#} + {#- Repeating a float aggregate can differ in the last bits when the scan is + partitioned differently between runs, since floating point addition is + not associative. That is a relative change around 1e-14, which a strict + comparison against the default of 0 reports as a failure on data nobody + touched. The floor sits far above that and far below any real movement, + and leaves the zero-crossing rule below untouched. -#} + {%- set change_percent_noise_floor = 0.000000001 %} + {%- set change_threshold = "%.10f" | format( + [max_change_percent, change_percent_noise_floor] | max + ) %} + {%- set exceeds_conditions = [] %} {%- set baseline_columns = [] %} {%- for baseline in change_since %} @@ -93,7 +116,7 @@ ~ " != 0 and " ~ elementary.metric_stability_change_percent(baseline_column) ~ " > " - ~ max_change_percent + ~ change_threshold ~ ")))" ) %} {%- endif %} @@ -116,7 +139,7 @@ {%- endif %} and metric_name in {{ elementary.strings_list_to_tuple(metric_names) }} and metric_properties = {{ elementary.dict_to_quoted_json(metric_properties) }} - and {{ bucket_window }} + and {{ history_window }} {%- for test_metrics_table_relation in test_metrics_table_relations %} @@ -126,7 +149,7 @@ bucket_start, bucket_end, bucket_duration_hours, metric_value, updated_at, dimension, dimension_value from {{ test_metrics_table_relation }} - where {{ bucket_window }} + where {{ history_window }} {%- endfor %} ), diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index a8ae42f4f..e911d2001 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -30,6 +30,24 @@ the observation window, and it must extend past min_bucket_age or no bucket is ever both settled and still being measured. + + Choosing min_bucket_age: + + A bucket is compared against its own earlier measurements, so it needs + several of them before 'first_check' says anything 'last_check' does not. + The count is roughly (days_back - min_bucket_age) / run interval, and + days_back is derived from min_bucket_age, so an age close to the run + interval leaves only two measurements and the two baselines collapse into + the same comparison. Set min_bucket_age to a multiple of how often the + project runs, not to the smallest age that looks settled. + + Limitations: + + A bucket that loses all of its rows produces no new measurement at all, + rather than a measurement of zero, so the newest value stays whatever it + was and the change is not reported. Partial deletion is caught normally, + since the metric moves. Pair this with a volume test if whole periods can + disappear. #} {% test metric_stability( model, From 976080d1b0711c23860372c7e1c484887885b44b Mon Sep 17 00:00:00 2001 From: Itamar Hartstein Date: Sun, 6 Sep 2026 21:02:44 +0300 Subject: [PATCH 07/13] fix: make metric_stability run on BigQuery and ClickHouse (#1053) - Cast the settled-measurement lower bound to a timestamp: on BigQuery edr_timeadd returns a DATE for week/month/quarter/year parts, so comparing it against the TIMESTAMP updated_at column failed. - ClickHouse has no plain UPDATE; the settling test now issues an ALTER TABLE ... UPDATE mutation (synchronously) on that target. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- integration_tests/tests/test_metric_stability.py | 11 +++++++++-- .../monitors_query/metric_stability_query.sql | 9 +++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 4ca5480d1..24717a3e6 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -258,12 +258,19 @@ def test_metric_stability_ignores_measurements_taken_while_settling( # Backdate this run's measurements into the settling window and move their # values far away. Were they still eligible as a baseline, the next run # would compare 100 against 10 and report a 900% change. + if dbt_project.target == "clickhouse": + # ClickHouse only supports updates as (asynchronous) mutations. + update_clause = "ALTER TABLE {{ ref('data_monitoring_metrics') }} UPDATE" + update_suffix = "SETTINGS mutations_sync = 1" + else: + update_clause = "UPDATE {{ ref('data_monitoring_metrics') }} SET" + update_suffix = "" dbt_project.run_query( f""" - UPDATE {{{{ ref('data_monitoring_metrics') }}}} - SET metric_value = 10, updated_at = bucket_end + {update_clause} metric_value = 10, updated_at = bucket_end WHERE full_table_name LIKE '%{test_id.upper()}' AND metric_name = 'sum' + {update_suffix} """ ) diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index bd2cb6133..25139e402 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -78,8 +78,13 @@ find is buried under it. Measurements are therefore bounded by the same age as the buckets. The current run's own measurement always qualifies: a bucket is only eligible once bucket_end + min_bucket_age has passed. -#} - {%- set settled_measurement_window = "updated_at >= " ~ elementary.edr_timeadd( - min_bucket_age.period, min_bucket_age.count, "bucket_end" + {%- set settled_measurement_window = ( + "updated_at >= " + ~ elementary.edr_cast_as_timestamp( + elementary.edr_timeadd( + min_bucket_age.period, min_bucket_age.count, "bucket_end" + ) + ) ) %} {%- set history_window = bucket_window ~ " and " ~ settled_measurement_window %} From 3091bf5f68aeca12f8a08b137ad3c54bf6eb94d7 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Mon, 7 Sep 2026 11:28:25 +0300 Subject: [PATCH 08/13] fix: make metric stability failures complete and actionable --- README.md | 2 + docs/metric_stability.md | 85 ++++++++++++++++ .../tests/test_metric_stability.py | 98 ++++++++++++++++++- .../monitors_query/metric_stability_query.sql | 51 +++++++--- macros/edr/tests/test_metric_stability.sql | 51 ++++++++-- 5 files changed, 261 insertions(+), 26 deletions(-) create mode 100644 docs/metric_stability.md diff --git a/README.md b/README.md index 3be912b5f..9a800a100 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,8 @@ These tables are the backbone of any observability setup — enabling alerts, re **2. Elementary Tests** A suite of anomaly detection and data quality tests that run like native dbt tests — no separate tooling. Covers volume, freshness, column distributions, schema changes, and AI-powered validation for structured and unstructured data. → [See all tests](https://docs.elementary-data.com/data-tests/introduction) +[Metric stability](docs/metric_stability.md) detects restatements of settled historical aggregates within a configured observation window. + --- ## Quickstart diff --git a/docs/metric_stability.md b/docs/metric_stability.md new file mode 100644 index 000000000..7e2569e05 --- /dev/null +++ b/docs/metric_stability.md @@ -0,0 +1,85 @@ +# Metric stability + +`elementary.metric_stability` detects changes to a time bucket's own previously +measured aggregates after the bucket has settled. Use it for historical revenue, +costs, or other measures that should stop changing after late data has arrived. +Ordinary anomaly detection compares different periods; it may notice some effects +of a restatement, but does not directly enforce this expectation. + +```yaml +models: + - name: orders + tests: + - elementary.metric_stability: + columns: [cost_amount, revenue_amount] + metrics: [sum] + timestamp_column: order_ts + time_bucket: {count: 1, period: day} + min_bucket_age: {count: 4, period: week} + days_back: 90 + change_since: [first_check] + max_change_percent: 1 +``` + +Choose a business/event timestamp whose historical periods you want to protect. +A row's ingestion or last-modified timestamp can move it between buckets when it +is updated, which answers a different question. + +## Coverage and cost + +`min_bucket_age` measures time since the bucket ended. Measurements taken before +that age are excluded from both baselines. The first eligible measurement only +establishes a baseline; a pass at that point does not verify historical stability. + +`days_back` bounds the observation window. The example protects daily buckets +roughly 28 to 90 days old, not all historical data. Corrections outside that window +are not detected. Incremental models and sources also use `backfill_days` to +control remeasurement; it defaults to `days_back`. An explicitly shorter backfill +window reduces coverage to the buckets actually scanned on that run. + +Without an explicit window, the test derives one from the settling age (roughly +twice the age, with room for whole buckets). This is a convenience default, not a +business retention policy. Set the window to cover the corrections you care about +and run frequently enough to measure each eligible bucket more than once. +Longer windows increase rescanning and metric-history storage costs. + +## Baselines and legitimate corrections + +- `last_check` compares against the previous eligible measurement. With zero + tolerance, 100 -> 120 fails; a subsequent 120 passes. New measurements become + the baseline automatically, including measurements from failing runs. +- `first_check` compares against the earliest retained eligible measurement. + It catches cumulative drift: 100 -> 110 -> 120 exceeds a 15% threshold overall, + although each step is smaller. A corrected 120 continues failing against 100 + until it returns within tolerance or the bucket leaves coverage. +- Selecting both fails if either comparison exceeds the threshold. + +There is no explicit accept/reset-baseline operation in this version. Choose +`last_check` when changes should be reported once and then automatically accepted. +Choose `first_check` when continued deviation should remain a failure. Switching +to `last_check` changes the policy; it does not reset `first_check`. Account for +history retention and cleanup: the baseline is the earliest *retained* +measurement, not an immutable approved snapshot. Do not use this test as a +substitute for an auditable financial close or an immutable snapshot. + +## Failure details + +With Elementary’s test materialization enabled, stored samples include the bucket, column, metric, dimensions, current and baseline +values, measurement timestamps, and absolute and percentage deltas. Normal +Elementary sample limits and privacy controls apply. + +`change_type: value_changed` reports numeric movement above the configured +percentage threshold. `max_change_percent: 1` means 1%, not 100%; the default is +zero with a tiny relative floor to suppress floating-point aggregation noise. +Movement away from a zero baseline always fails because relative change is +undefined there. + +`change_type: missing_bucket` means a previously measured bucket or dimension has +no current metric in a window the test actually rescanned. It fails regardless of +the percentage tolerance, reports a NULL current value and the last observed +value, and remains a failure while missing and within coverage. It does not invent +a zero for aggregates such as average or minimum. + +Stable aggregates do not guarantee unchanged source rows. Offsetting changes can +cancel in a sum; pair this test with row-level checks when record immutability is +the requirement. diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 24717a3e6..080816b0a 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -1,7 +1,9 @@ +import json from datetime import datetime, time, timedelta from itertools import pairwise from typing import Any, Dict, List, Optional +import pytest from data_generator import DATE_FORMAT from dbt_project import DbtProject @@ -52,7 +54,11 @@ def _rows( def _run(dbt_project: DbtProject, test_id: str, data, **overrides) -> str: result = dbt_project.test( - test_id, DBT_TEST_NAME, {**BASE_ARGS, **overrides}, data=data + test_id, + DBT_TEST_NAME, + {**BASE_ARGS, **overrides}, + data=data, + test_vars={"enable_elementary_test_materialization": True}, ) return result["status"] @@ -265,14 +271,12 @@ def test_metric_stability_ignores_measurements_taken_while_settling( else: update_clause = "UPDATE {{ ref('data_monitoring_metrics') }} SET" update_suffix = "" - dbt_project.run_query( - f""" + dbt_project.run_query(f""" {update_clause} metric_value = 10, updated_at = bucket_end WHERE full_table_name LIKE '%{test_id.upper()}' AND metric_name = 'sum' {update_suffix} - """ - ) + """) assert _run(dbt_project, test_id, _rows(), **args) == "pass" @@ -332,3 +336,87 @@ def test_metric_stability_detects_restatement_in_any_column( assert measurements[-1] == BASE_AMOUNT * 2 other = _bucket_values(dbt_project, test_id, OTHER_VALUE_COLUMN) assert all(len(set(m)) == 1 for m in other.values()), other + + +def _samples(dbt_project: DbtProject, test_id: str): + test_id = test_id.replace("[", "_").replace("]", "_") + return [ + {key.lower(): value for key, value in json.loads(row["result_row"]).items()} + for row in dbt_project.run_query(dbt_project.samples_query(test_id)) + ] + + +@pytest.mark.parametrize("baseline", ["last_check", "first_check"]) +def test_metric_stability_quoted_columns_and_failure_details( + test_id: str, dbt_project: DbtProject, baseline: str +): + args = {"columns": ['"amount"'], "change_since": [baseline]} + assert _run(dbt_project, test_id, _rows(), **args) == "pass" + assert _run(dbt_project, test_id, _rows({SETTLED_DAYS_AGO: 200}), **args) == "fail" + samples = _samples(dbt_project, test_id) + assert len(samples) == 1 + sample = samples[0] + assert sample["change_type"] == "value_changed" + assert float(sample["metric_value"]) == 200 + assert float(sample["previous_value"]) == 100 + assert float(sample["initial_value"]) == 100 + assert sample["bucket_end"] + assert sample["previous_measured_at"] + assert sample["initial_measured_at"] + # A repeated corrected value is accepted only by the moving baseline. + expected = "pass" if baseline == "last_check" else "fail" + assert ( + _run(dbt_project, test_id, _rows({SETTLED_DAYS_AGO: 200}), **args) == expected + ) + + +@pytest.mark.parametrize("dimensions", [[], ["other_amount"]]) +def test_metric_stability_reports_disappearing_bucket( + test_id: str, dbt_project: DbtProject, dimensions +): + assert _run(dbt_project, test_id, _rows(), dimensions=dimensions) == "pass" + rows = _rows() + del rows[SETTLED_DAYS_AGO - 1] + assert _run(dbt_project, test_id, rows, dimensions=dimensions) == "fail" + samples = _samples(dbt_project, test_id) + assert len(samples) == 1 + assert samples[0]["change_type"] == "missing_bucket" + assert samples[0]["metric_value"] is None + assert float(samples[0]["previous_value"]) == BASE_AMOUNT + # Absence is not accepted as a new numeric baseline on the next run. + assert _run(dbt_project, test_id, rows, dimensions=dimensions) == "fail" + assert _run(dbt_project, test_id, _rows(), dimensions=dimensions) == "pass" + + +def test_metric_stability_ignores_disappearance_outside_observation_window( + test_id: str, dbt_project: DbtProject +): + assert _run(dbt_project, test_id, _rows(), days_back=10) == "pass" + # Keep only the recent rows; historical measurements still exist but the + # default window no longer covers the deleted older buckets. + assert _run(dbt_project, test_id, _rows()[:3]) == "pass" + + +def test_metric_stability_reports_disappearing_dimension( + test_id: str, dbt_project: DbtProject +): + rows = _rows() + rows.append({**rows[SETTLED_DAYS_AGO - 1], OTHER_VALUE_COLUMN: 999}) + args = {"dimensions": [OTHER_VALUE_COLUMN]} + assert _run(dbt_project, test_id, rows, **args) == "pass" + assert _run(dbt_project, test_id, _rows(), **args) == "fail" + samples = _samples(dbt_project, test_id) + assert len(samples) == 1 + assert samples[0]["change_type"] == "missing_bucket" + assert "999" in str(samples[0]["dimension_value"]) + + +def test_metric_stability_does_not_report_unscanned_buckets_as_missing( + test_id: str, dbt_project: DbtProject +): + assert _run(dbt_project, test_id, _rows(), days_back=6) == "pass" + # Sources use the incremental backfill window. Older buckets have history + # and remain in days_back, but are not rescanned with backfill_days=3. + assert ( + _run(dbt_project, test_id, _rows()[:3], days_back=6, backfill_days=3) == "pass" + ) diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 25139e402..d38b5d439 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -32,7 +32,8 @@ max_change_percent=0, change_since=["last_check"], column_names=none, - data_monitoring_metrics_table=none + data_monitoring_metrics_table=none, + measurement_windows=none ) %} {%- if not data_monitoring_metrics_table %} {%- set data_monitoring_metrics_table = elementary.get_elementary_relation( @@ -132,7 +133,8 @@ select id, full_table_name, column_name, metric_name, metric_type, bucket_start, bucket_end, bucket_duration_hours, - metric_value, updated_at, dimension, dimension_value + metric_value, updated_at, dimension, dimension_value, + 0 as is_current from {{ data_monitoring_metrics_table }} where upper(full_table_name) = upper('{{ full_table_name }}') @@ -145,6 +147,18 @@ and metric_name in {{ elementary.strings_list_to_tuple(metric_names) }} and metric_properties = {{ elementary.dict_to_quoted_json(metric_properties) }} and {{ history_window }} + {# History outside a column's actual rescan cannot establish + absence. Restrict it before selecting the newest version. #} + {%- if measurement_windows %} + and ( + {%- for window in measurement_windows %} + (upper(column_name) = upper({{ elementary.edr_quote(window.column_name) }}) + and bucket_start >= {{ window.min_bucket_start }} + and bucket_end <= {{ window.max_bucket_end }}) + {% if not loop.last %} or {% endif %} + {%- endfor %} + ) + {%- endif %} {%- for test_metrics_table_relation in test_metrics_table_relations %} @@ -152,7 +166,8 @@ select id, full_table_name, column_name, metric_name, metric_type, bucket_start, bucket_end, bucket_duration_hours, - metric_value, updated_at, dimension, dimension_value + metric_value, updated_at, dimension, dimension_value, + 1 as is_current from {{ test_metrics_table_relation }} where {{ history_window }} {%- endfor %} @@ -164,10 +179,17 @@ select id, full_table_name, column_name, metric_name, metric_type, bucket_start, bucket_end, bucket_duration_hours, - metric_value, updated_at, dimension, dimension_value, + metric_value, updated_at, dimension, dimension_value, is_current, {{ elementary.lag("metric_value") }} over ( partition by id order by updated_at ) as previous_value, + {{ elementary.lag("updated_at") }} over ( + partition by id order by updated_at + ) as previous_measured_at, + first_value(updated_at) over ( + partition by id order by updated_at + rows between unbounded preceding and current row + ) as initial_measured_at, first_value(metric_value) over ( partition by id order by updated_at rows between unbounded preceding and current row @@ -198,22 +220,27 @@ bucket_duration_hours, dimension, dimension_value, - updated_at as measured_at, - metric_value, - previous_value, + case when is_current = 0 + then {{ elementary.edr_cast_as_timestamp(elementary.edr_quote(elementary.run_started_at_as_string())) }} + else updated_at end as measured_at, + case when is_current = 0 then 'missing_bucket' else 'value_changed' end as change_type, + case when is_current = 1 then metric_value end as metric_value, + case when is_current = 0 then metric_value else previous_value end as previous_value, + case when is_current = 0 then updated_at else previous_measured_at end as previous_measured_at, + initial_measured_at, initial_value, - metric_value - previous_value as change_since_last_check, - metric_value - initial_value as change_since_first_check, + case when is_current = 1 then metric_value - previous_value end as change_since_last_check, + case when is_current = 1 then metric_value - initial_value end as change_since_first_check, case - when previous_value is not null and previous_value != 0 + when is_current = 1 and previous_value is not null and previous_value != 0 then {{ elementary.metric_stability_change_percent("previous_value") }} end as change_percent_since_last_check, case - when initial_value is not null and initial_value != 0 + when is_current = 1 and initial_value is not null and initial_value != 0 then {{ elementary.metric_stability_change_percent("initial_value") }} end as change_percent_since_first_check from latest_measurement - where {{ exceeds_conditions | join(" or ") }} + where is_current = 0 or {{ exceeds_conditions | join(" or ") }} {%- endset %} {%- do return(metric_stability_query) %} {% endmacro %} diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index e911d2001..557a16599 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -41,13 +41,21 @@ the same comparison. Set min_bucket_age to a multiple of how often the project runs, not to the smallest age that looks settled. - Limitations: + Coverage: - A bucket that loses all of its rows produces no new measurement at all, - rather than a measurement of zero, so the newest value stays whatever it - was and the change is not reported. Partial deletion is caught normally, - since the metric moves. Pair this with a volume test if whole periods can - disappear. + Only buckets within the current measurement window are protected. A + previously measured bucket or dimension that disappears fails with + change_type = 'missing_bucket'; its current value is NULL, not an invented + zero. Missing buckets keep failing while under observation. + + last_check accepts each new measurement automatically: 100 -> 120 fails, + then another 120 passes. first_check uses the earliest retained measurement + taken after settling, and keeps failing until the values return within + tolerance or the bucket leaves coverage. There is no explicit baseline + reset in this test. See docs/metric_stability.md for operational guidance. + + Stable aggregates do not guarantee unchanged rows: offsetting changes can + cancel. Initial runs without eligible history establish a baseline. #} {% test metric_stability( model, @@ -276,6 +284,8 @@ at all and makes this test a silent permanent pass. The tables are unioned at read time instead. -#} {% set temp_table_relations = [] %} + {% set measurement_windows = [] %} + {% set resolved_columns = [] %} {%- for column_name in columns %} {%- set column_obj_and_monitors = ( @@ -294,6 +304,8 @@ ) }} {%- endif %} + {%- set resolved_column = column_obj_and_monitors["column"].name %} + {%- do resolved_columns.append(resolved_column) %} {%- set column_monitors = column_obj_and_monitors["monitors"] %} {%- if not column_monitors %} {{ @@ -315,7 +327,7 @@ backfill_days=backfill_days, days_back=days_back, metric_names=column_monitors, - column_name=column_name, + column_name=resolved_column, metric_properties=metric_properties, ) %} {#- get_metric_buckets_min_and_max can return a plain midnight @@ -328,6 +340,15 @@ metric_properties.time_bucket.period, elementary.edr_cast_as_timestamp(raw_min_bucket_start), ) %} + {%- do measurement_windows.append( + { + "column_name": resolved_column, + "min_bucket_start": min_bucket_start, + "max_bucket_end": elementary.edr_cast_as_timestamp( + max_bucket_end + ), + } + ) %} {#- Only the monitors that apply to this column's data type. Passing the full list would generate e.g. sum(). -#} {%- set this_column_metrics = [] %} @@ -378,7 +399,8 @@ min_bucket_age=min_bucket_age, max_change_percent=max_change_percent, change_since=change_since, - column_names=columns, + column_names=resolved_columns, + measurement_windows=measurement_windows, ) %} {{ elementary.debug_log( @@ -386,7 +408,18 @@ ) }} - {{ metric_stability_query }} + {# Freeze failures before dbt executes the test and samples its results. + Use the normal sampling path so sample limits and privacy settings + still apply. This relation is cleaned up with the metrics tables. #} + {% set result_relation = elementary.create_elementary_test_table( + database_name, + tests_schema_name, + test_table_name, + "stability_results", + metric_stability_query, + ) %} + select * + from {{ result_relation }} {%- else %} From 4c82b03a47df359f7fa36586dcf5fa3bbd9c50c2 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Mon, 7 Sep 2026 22:17:17 +0300 Subject: [PATCH 09/13] fix: address review round on metric_stability, unbreak postgres and clickhouse Review feedback: - Move argument validation into `_validate_metric_stability_arguments` and metric collection into `_collect_metric_stability_metrics`, both below the test, cutting the test body from ~440 lines to ~100. - Return early when the test is not applicable instead of indenting the whole body inside an `if`. - Cut the long explanatory comment blocks in both files, rewrite the ones that were unclear or named a macro they did not call, and point at docs/metric_stability.md for the details. - Build the change predicates and the settled-history window with `{% set %}...{% endset %}` blocks rather than string concatenation. - Name the three window floors (`twice_the_age`, `two_buckets_past_age`, `one_day_past_age`) and say what each is for. The third is what keeps the floor at 2 or more for every age, so a `days_back` the query would truncate to 0 or 1 days is rejected instead of yielding an empty comparison window. Covered by a new test. - Document dimension support: each bucket/dimension combination is a separate metric with its own history and baseline. Cross-adapter fixes: - Postgres caps relation names at 63 characters and the seed table is named after the test, so four over-long test names failed to seed. Shorten them. - ClickHouse resolves a select alias anywhere in the same select list, so the output `metric_value` alias shadowed the source column for its sibling expressions and a missing bucket reported a NULL baseline. Carry the compared values as `measured_value`, `last_check_value`, `first_check_value`, `last_check_at` and `first_check_at`, so no output alias repeats a source column name. Output columns are unchanged. - Apply black to the integration test and prettier to the docs. 19 integration cases pass on DuckDB and on Postgres. Co-Authored-By: Claude Opus 5 --- docs/metric_stability.md | 12 +- .../tests/test_metric_stability.py | 31 +- .../monitors_query/metric_stability_query.sql | 152 +--- macros/edr/tests/test_metric_stability.sql | 856 +++++++++--------- 4 files changed, 495 insertions(+), 556 deletions(-) diff --git a/docs/metric_stability.md b/docs/metric_stability.md index 7e2569e05..16c26d7b1 100644 --- a/docs/metric_stability.md +++ b/docs/metric_stability.md @@ -14,8 +14,8 @@ models: columns: [cost_amount, revenue_amount] metrics: [sum] timestamp_column: order_ts - time_bucket: {count: 1, period: day} - min_bucket_age: {count: 4, period: week} + time_bucket: { count: 1, period: day } + min_bucket_age: { count: 4, period: week } days_back: 90 change_since: [first_check] max_change_percent: 1 @@ -25,6 +25,12 @@ Choose a business/event timestamp whose historical periods you want to protect. A row's ingestion or last-modified timestamp can move it between buckets when it is updated, which answers a different question. +`dimensions` and `where_expression` behave as they do in the other metric-based +tests. Each bucket/dimension combination is a separate metric with its own +history and baseline, so a restatement confined to one dimension value is still +reported, and a dimension value that stops appearing is reported as a missing +bucket. Every extra combination is another measurement to store and compare. + ## Coverage and cost `min_bucket_age` measures time since the bucket ended. Measurements taken before @@ -58,7 +64,7 @@ There is no explicit accept/reset-baseline operation in this version. Choose `last_check` when changes should be reported once and then automatically accepted. Choose `first_check` when continued deviation should remain a failure. Switching to `last_check` changes the policy; it does not reset `first_check`. Account for -history retention and cleanup: the baseline is the earliest *retained* +history retention and cleanup: the baseline is the earliest _retained_ measurement, not an immutable approved snapshot. Do not use this test as a substitute for an auditable financial close or an immutable snapshot. diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index 080816b0a..fc390d1a5 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -248,6 +248,25 @@ def test_metric_stability_rejects_multi_step_buckets( assert result == "error" +@pytest.mark.parametrize("days_back", [1, 0.5]) +def test_metric_stability_rejects_too_short_window( + test_id: str, dbt_project: DbtProject, days_back +): + """A window too short to outlast settling must raise, not pass vacuously. + + The query truncates days_back to whole days, so a fractional value has to be + rejected on its effective size rather than on the number as written. + """ + result = _run( + dbt_project, + test_id, + _rows(), + min_bucket_age={"count": 1, "period": "hour"}, + days_back=days_back, + ) + assert result == "error" + + def test_metric_stability_ignores_measurements_taken_while_settling( test_id: str, dbt_project: DbtProject ): @@ -271,12 +290,14 @@ def test_metric_stability_ignores_measurements_taken_while_settling( else: update_clause = "UPDATE {{ ref('data_monitoring_metrics') }} SET" update_suffix = "" - dbt_project.run_query(f""" + dbt_project.run_query( + f""" {update_clause} metric_value = 10, updated_at = bucket_end WHERE full_table_name LIKE '%{test_id.upper()}' AND metric_name = 'sum' {update_suffix} - """) + """ + ) assert _run(dbt_project, test_id, _rows(), **args) == "pass" @@ -347,7 +368,7 @@ def _samples(dbt_project: DbtProject, test_id: str): @pytest.mark.parametrize("baseline", ["last_check", "first_check"]) -def test_metric_stability_quoted_columns_and_failure_details( +def test_metric_stability_quoted_column_details( test_id: str, dbt_project: DbtProject, baseline: str ): args = {"columns": ['"amount"'], "change_since": [baseline]} @@ -388,7 +409,7 @@ def test_metric_stability_reports_disappearing_bucket( assert _run(dbt_project, test_id, _rows(), dimensions=dimensions) == "pass" -def test_metric_stability_ignores_disappearance_outside_observation_window( +def test_metric_stability_ignores_disappearance_off_window( test_id: str, dbt_project: DbtProject ): assert _run(dbt_project, test_id, _rows(), days_back=10) == "pass" @@ -411,7 +432,7 @@ def test_metric_stability_reports_disappearing_dimension( assert "999" in str(samples[0]["dimension_value"]) -def test_metric_stability_does_not_report_unscanned_buckets_as_missing( +def test_metric_stability_skips_unscanned_buckets( test_id: str, dbt_project: DbtProject ): assert _run(dbt_project, test_id, _rows(), days_back=6) == "pass" diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index d38b5d439..8775dbc74 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -1,26 +1,5 @@ -{# - Detects metrics whose value for an already-settled time bucket has changed - since a previous run. - - Standard anomaly detection compares different buckets at one point in time. - This compares one bucket against its own earlier measurements, which is a - different axis and a far lower noise floor: for settled data the expected - change is zero. - - This is deliberately a threshold test rather than an anomaly test. A settled - series has no variance to learn from, and the scoring degenerates in both - directions. With the value excluded from its own training set the stddev is - zero and the score is forced to zero, so it never fires. With the value - included, n unchanged observations followed by one value v give mean v/(n+1) - and stddev v/sqrt(n+1), so the score is n/sqrt(n+1): the v cancels and the - score reflects how long the history is rather than how large the change was. - - The version history this reads is already collected. `data_monitoring_metrics` - is append-only (rows are inserted by the on-run-end hook), and a metric `id` - hashes the table, column, metric name and bucket_end while deliberately - excluding `updated_at` and `metric_value`. So re-measuring a bucket appends a - new row, and the earlier measurements remain. -#} +{# Compare each settled bucket's current value with its previous/first retained + measurement. Missing current measurements fail only within the rescan window. #} {% macro metric_stability_query( test_metrics_table_relations, full_table_name, @@ -43,11 +22,7 @@ {%- set bucket_period = metric_properties.time_bucket.period %} - {#- Eligible buckets form a band. The upper edge keeps recent data out: - it is expected to keep moving as late records arrive, so comparing it - produces noise. The lower edge bounds the read, which both prunes the - scan (and enables partition pruning) and lets a reported change age out - of the window instead of failing the test forever. -#} + {# Only compare settled buckets within days_back. #} {%- set age_kwargs = {min_bucket_age.period ~ "s": min_bucket_age.count} %} {%- set max_bucket_end = detection_end - modules.datetime.timedelta(**age_kwargs) %} {%- set min_bucket_end = detection_end - modules.datetime.timedelta( @@ -65,40 +40,19 @@ elementary.edr_datetime_to_sql(min_bucket_end) ), ) %} - {%- set bucket_window = ( - "bucket_end > " - ~ min_bucket_end_expr - ~ " and bucket_end <= " - ~ max_bucket_end_expr - ) %} - - {#- A bucket's first measurements are taken while it is still settling, and - min_bucket_age exists precisely to keep that period out of scope. Left - in, they become the 'first_check' baseline, so every comparison carries - the settling as a permanent offset and the drift 'first_check' exists to - find is buried under it. Measurements are therefore bounded by the same - age as the buckets. The current run's own measurement always qualifies: - a bucket is only eligible once bucket_end + min_bucket_age has passed. -#} - {%- set settled_measurement_window = ( - "updated_at >= " - ~ elementary.edr_cast_as_timestamp( - elementary.edr_timeadd( - min_bucket_age.period, min_bucket_age.count, "bucket_end" - ) + {# Baselines must also have been measured after the bucket settled. #} + {%- set settled_at_expr = elementary.edr_cast_as_timestamp( + elementary.edr_timeadd( + min_bucket_age.period, min_bucket_age.count, "bucket_end" ) ) %} - {%- set history_window = bucket_window ~ " and " ~ settled_measurement_window %} + {% set history_window %} + bucket_end > {{ min_bucket_end_expr }} + and bucket_end <= {{ max_bucket_end_expr }} + and updated_at >= {{ settled_at_expr }} + {% endset %} - {#- Conditions keep booleans in boolean position rather than returning one - from a CASE, which T-SQL has no first-class value for. A move away from - exactly zero is handled separately, since the relative form is undefined - there. -#} - {#- Repeating a float aggregate can differ in the last bits when the scan is - partitioned differently between runs, since floating point addition is - not associative. That is a relative change around 1e-14, which a strict - comparison against the default of 0 reports as a failure on data nobody - touched. The floor sits far above that and far below any real movement, - and leaves the zero-crossing rule below untouched. -#} + {# Suppress floating-point aggregation noise; zero baselines are handled separately. #} {%- set change_percent_noise_floor = 0.000000001 %} {%- set change_threshold = "%.10f" | format( [max_change_percent, change_percent_noise_floor] | max @@ -107,24 +61,17 @@ {%- set exceeds_conditions = [] %} {%- set baseline_columns = [] %} {%- for baseline in change_since %} - {%- set baseline_column = ( - "previous_value" if baseline == "last_check" else "initial_value" - ) %} + {%- set baseline_column = baseline ~ "_value" %} {%- if baseline_column not in baseline_columns %} {%- do baseline_columns.append(baseline_column) %} - {%- do exceeds_conditions.append( - "(" - ~ baseline_column - ~ " is not null and ((" - ~ baseline_column - ~ " = 0 and metric_value != 0) or (" - ~ baseline_column - ~ " != 0 and " - ~ elementary.metric_stability_change_percent(baseline_column) - ~ " > " - ~ change_threshold - ~ ")))" - ) %} + {% set exceeds_condition %} + ({{ baseline_column }} is not null and ( + ({{ baseline_column }} = 0 and measured_value != 0) + or ({{ baseline_column }} != 0 and + {{ elementary.metric_stability_change_percent(baseline_column) }} > {{ change_threshold }}) + )) + {% endset %} + {% do exceeds_conditions.append(exceeds_condition) %} {%- endif %} {%- endfor %} @@ -176,24 +123,31 @@ versioned_metrics as ( + {#- Each measurement carries the values it is compared against, + named after the `change_since` baseline it serves. None of these + names is reused as an output alias of the final select: + ClickHouse resolves a select alias anywhere in the same select + list, so an output `metric_value` shadows the source column for + its sibling expressions and reports NULL baselines. -#} select id, full_table_name, column_name, metric_name, metric_type, bucket_start, bucket_end, bucket_duration_hours, - metric_value, updated_at, dimension, dimension_value, is_current, + updated_at, dimension, dimension_value, is_current, + metric_value as measured_value, {{ elementary.lag("metric_value") }} over ( partition by id order by updated_at - ) as previous_value, + ) as last_check_value, {{ elementary.lag("updated_at") }} over ( partition by id order by updated_at - ) as previous_measured_at, - first_value(updated_at) over ( + ) as last_check_at, + first_value(metric_value) over ( partition by id order by updated_at rows between unbounded preceding and current row - ) as initial_measured_at, - first_value(metric_value) over ( + ) as first_check_value, + first_value(updated_at) over ( partition by id order by updated_at rows between unbounded preceding and current row - ) as initial_value, + ) as first_check_at, row_number() over ( partition by id order by updated_at desc ) as recency @@ -224,20 +178,20 @@ then {{ elementary.edr_cast_as_timestamp(elementary.edr_quote(elementary.run_started_at_as_string())) }} else updated_at end as measured_at, case when is_current = 0 then 'missing_bucket' else 'value_changed' end as change_type, - case when is_current = 1 then metric_value end as metric_value, - case when is_current = 0 then metric_value else previous_value end as previous_value, - case when is_current = 0 then updated_at else previous_measured_at end as previous_measured_at, - initial_measured_at, - initial_value, - case when is_current = 1 then metric_value - previous_value end as change_since_last_check, - case when is_current = 1 then metric_value - initial_value end as change_since_first_check, + case when is_current = 1 then measured_value end as metric_value, + case when is_current = 0 then measured_value else last_check_value end as previous_value, + case when is_current = 0 then updated_at else last_check_at end as previous_measured_at, + first_check_at as initial_measured_at, + first_check_value as initial_value, + case when is_current = 1 then measured_value - last_check_value end as change_since_last_check, + case when is_current = 1 then measured_value - first_check_value end as change_since_first_check, case - when is_current = 1 and previous_value is not null and previous_value != 0 - then {{ elementary.metric_stability_change_percent("previous_value") }} + when is_current = 1 and last_check_value is not null and last_check_value != 0 + then {{ elementary.metric_stability_change_percent("last_check_value") }} end as change_percent_since_last_check, case - when is_current = 1 and initial_value is not null and initial_value != 0 - then {{ elementary.metric_stability_change_percent("initial_value") }} + when is_current = 1 and first_check_value is not null and first_check_value != 0 + then {{ elementary.metric_stability_change_percent("first_check_value") }} end as change_percent_since_first_check from latest_measurement where is_current = 0 or {{ exceeds_conditions | join(" or ") }} @@ -250,12 +204,6 @@ Relative change from a baseline column, in percentage points. Shared by the WHERE predicate and the reported columns so the two cannot drift apart. #} -{% macro metric_stability_change_percent(baseline_column) %} - {%- do return( - "abs(metric_value - " - ~ baseline_column - ~ ") / abs(" - ~ baseline_column - ~ ") * 100.0" - ) %} -{% endmacro %} +{% macro metric_stability_change_percent(baseline_column) -%} + abs(measured_value - {{ baseline_column }}) / abs({{ baseline_column }}) * 100.0 +{%- endmacro %} diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index 557a16599..a51dfde19 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -1,62 +1,5 @@ -{# - elementary.metric_stability - - Fails when a metric's value for an already-settled time bucket has changed - since a previous run. - - Regular anomaly detection compares one bucket against neighbouring buckets, so - it cannot see this: a restatement that shifts many historical buckets together - moves the training baseline along with the data, and normal period-to-period - variation is usually far wider than the change being looked for. - - Arguments: - columns - columns to monitor. - metrics - metric types to monitor per column (e.g. [sum]). - timestamp_column - column that buckets the data into periods. - min_bucket_age - required. Only check buckets at least this old, e.g. - {count: 4, period: week}. Recent data is expected to - keep changing as late records arrive, so comparing it - reports noise rather than restatements. - change_since - baselines to compare against: 'last_check' (the previous - measurement), 'first_check' (the earliest measurement), - or both. 'last_check' catches a sudden correction; - 'first_check' catches slow drift, where each step is too - small to trip the threshold but the total movement is - not. - max_change_percent - permitted change in percentage points before failing - (25 means 25%). Defaults to 0, so any change to settled - data fails. - days_back - how far back buckets are measured and compared. This is - the observation window, and it must extend past - min_bucket_age or no bucket is ever both settled and - still being measured. - - Choosing min_bucket_age: - - A bucket is compared against its own earlier measurements, so it needs - several of them before 'first_check' says anything 'last_check' does not. - The count is roughly (days_back - min_bucket_age) / run interval, and - days_back is derived from min_bucket_age, so an age close to the run - interval leaves only two measurements and the two baselines collapse into - the same comparison. Set min_bucket_age to a multiple of how often the - project runs, not to the smallest age that looks settled. - - Coverage: - - Only buckets within the current measurement window are protected. A - previously measured bucket or dimension that disappears fails with - change_type = 'missing_bucket'; its current value is NULL, not an invented - zero. Missing buckets keep failing while under observation. - - last_check accepts each new measurement automatically: 100 -> 120 fails, - then another 120 passes. first_check uses the earliest retained measurement - taken after settling, and keeps failing until the values return within - tolerance or the bucket leaves coverage. There is no explicit baseline - reset in this test. See docs/metric_stability.md for operational guidance. - - Stable aggregates do not guarantee unchanged rows: offsetting changes can - cancel. Initial runs without eligible history establish a baseline. -#} +{# Detect restatements of settled bucket metrics within the observation window. + Configuration, baseline behavior, and limits: docs/metric_stability.md. #} {% test metric_stability( model, columns, @@ -73,371 +16,203 @@ ) %} {{ config(tags=["elementary-tests"]) }} - {%- if execute and elementary.is_test_command() and elementary.is_elementary_enabled() %} - - {#- yaml lets a single value be written as a scalar, and iterating a - string in jinja walks it character by character. -#} - {%- if columns is string %} {% set columns = [columns] %} {%- endif %} - {%- if metrics is string %} {% set metrics = [metrics] %} {%- endif %} - {%- if change_since is string %} - {% set change_since = [change_since] %} - {%- endif %} - {#- Column lookup is case-insensitive, so a duplicate spelling would - otherwise be collected twice under one metric id and make the - 'last_check' baseline this run's own second measurement. -#} - {%- if columns %} - {%- set seen_columns = [] %} - {%- set deduped_columns = [] %} - {%- for column_name in columns %} - {%- set key = column_name | trim('"') | lower %} - {%- if key not in seen_columns %} - {%- do seen_columns.append(key) %} - {%- do deduped_columns.append(column_name) %} - {%- endif %} - {%- endfor %} - {%- set columns = deduped_columns %} - {%- endif %} - - {%- if not change_since %} - {{ - exceptions.raise_compiler_error( - "metric_stability requires at least one baseline in `change_since`: 'last_check', 'first_check', or both." - ) - }} - {%- endif %} - - {#- Comparing a string against 0 raises a bare Python TypeError, which - surfaces as an unreadable stack trace rather than a config error. -#} - {%- if max_change_percent is not number %} - {{ - exceptions.raise_compiler_error( - "max_change_percent must be a number, got '" - ~ max_change_percent - ~ "'. Write it unquoted, e.g. max_change_percent: 25." - ) - }} - {%- endif %} - {%- if max_change_percent < 0 %} - {{ - exceptions.raise_compiler_error( - "max_change_percent must be non-negative." - ) - }} - {%- endif %} - {%- for arg_name, arg_value in [ - ("days_back", days_back), - ("backfill_days", backfill_days), - ] %} - {%- if arg_value is not none and arg_value is not number %} - {{ - exceptions.raise_compiler_error( - arg_name - ~ " must be a number, got '" - ~ arg_value - ~ "'. Write it unquoted, e.g. " - ~ arg_name - ~ ": 30." - ) - }} - {%- endif %} - {%- endfor %} - - {%- if not columns %} - {{ - exceptions.raise_compiler_error( - "metric_stability requires at least one column in `columns`." - ) - }} - {%- endif %} - - {%- if not metrics %} - {{ - exceptions.raise_compiler_error( - "metric_stability requires at least one metric type in `metrics`." - ) - }} - {%- endif %} - - {%- set available_column_monitors = ( - elementary.get_available_column_monitors() - ) %} - {%- for metric_type in metrics %} - {%- if metric_type not in available_column_monitors %} - {{ - exceptions.raise_compiler_error( - "Unsupported column metric: '" - ~ metric_type - ~ "'. Supported metrics are: " - ~ available_column_monitors - | join(", ") ~ "." - ) - }} - {%- endif %} - {%- endfor %} - - {%- for baseline in change_since %} - {%- if baseline not in ["last_check", "first_check"] %} - {{ - exceptions.raise_compiler_error( - "Unsupported `change_since` value '" - ~ baseline - ~ "'. Supported values are 'last_check' and 'first_check'." - ) - }} - {%- endif %} - {%- endfor %} - - {% do elementary.validate_min_bucket_age(min_bucket_age) %} - - {% set model_relation = elementary.get_model_relation_for_test( - model, elementary.get_test_model() - ) %} - {%- if not model_relation %} - {{ exceptions.raise_compiler_error("Unsupported model: " ~ model) }} - {%- endif %} - - {%- if elementary.is_ephemeral_model(model_relation) %} - {{ - exceptions.raise_compiler_error( - "Test not supported for ephemeral models: " - ~ model_relation.identifier - ) - }} - {%- endif %} - - {% set model_graph_node = elementary.get_model_graph_node(model_relation) %} - {#- timestamp_column is commonly set once in the model's elementary - config rather than repeated on every test. -#} - {% set timestamp_column = elementary.get_test_argument( - "timestamp_column", timestamp_column, model_graph_node - ) %} - {%- if not timestamp_column %} - {{ - exceptions.raise_compiler_error( - "metric_stability requires a `timestamp_column`, either on the test or in the model's elementary config." - ) - }} - {%- endif %} - - {% set timestamp_column_data_type = ( - elementary.find_normalized_data_type_for_column( - model_relation, timestamp_column - ) - ) %} - {%- if not elementary.is_column_timestamp( - model_relation, timestamp_column, timestamp_column_data_type - ) %} - {{ - exceptions.raise_compiler_error( - "Column '" - ~ timestamp_column - ~ "' is not a timestamp type. metric_stability buckets data over time and requires a timestamp column." - ) - }} - {%- endif %} + {% if not ( + execute + and elementary.is_test_command() + and elementary.is_elementary_enabled() + ) %} + {% do return(elementary.no_results_query()) %} + {% endif %} + + {% set arguments = elementary._validate_metric_stability_arguments( + model, + columns, + metrics, + timestamp_column, + min_bucket_age, + change_since, + max_change_percent, + days_back, + backfill_days, + ) %} + {% set model_relation = arguments.model_relation %} + {% set model_graph_node = arguments.model_graph_node %} + {% set timestamp_column = arguments.timestamp_column %} + {% set metrics = arguments.metrics %} + {% set change_since = arguments.change_since %} + {%- if not dimensions %} {% set dimensions = [] %} {%- endif %} + + {% set metric_properties = elementary.get_metric_properties( + model_graph_node, + timestamp_column, + where_expression, + time_bucket, + dimensions, + collected_by="metric_stability", + ) %} + {% set metric_names = metrics %} + + {# Size the scan from the settling age and bucket duration. #} + {% set resolved_window = elementary.resolve_metric_stability_window( + model_graph_node, + min_bucket_age, + metric_properties.time_bucket, + days_back, + backfill_days, + ) %} + {% set days_back = resolved_window["days_back"] %} + {% set backfill_days = resolved_window["backfill_days"] %} + + {% set test_table_name = elementary.get_elementary_test_table_name() %} + {% set ( + database_name, + schema_name, + ) = elementary.get_package_database_and_schema("elementary") %} + {% set tests_schema_name = elementary.get_elementary_tests_schema( + database_name, schema_name + ) %} + {% set full_table_name = elementary.relation_to_full_name(model_relation) %} + + {% set collected = elementary._collect_metric_stability_metrics( + model, + model_relation, + arguments.columns, + metric_properties, + days_back, + backfill_days, + dimensions, + database_name, + tests_schema_name, + test_table_name, + ) %} + {% set detection_end = elementary.get_detection_end(none) %} + {% set metric_stability_query = elementary.metric_stability_query( + test_metrics_table_relations=collected.relations, + full_table_name=full_table_name, + metric_names=metric_names, + metric_properties=metric_properties, + detection_end=detection_end, + days_back=days_back, + min_bucket_age=min_bucket_age, + max_change_percent=max_change_percent, + change_since=change_since, + column_names=collected.columns, + measurement_windows=collected.windows, + ) %} + {{ elementary.debug_log("metric_stability_query - \n" ~ metric_stability_query) }} + + {# Freeze failures before dbt executes the test and samples its results. + Use the normal sampling path so sample limits and privacy settings + still apply. This relation is cleaned up with the metrics tables. #} + {% set result_relation = elementary.create_elementary_test_table( + database_name, + tests_schema_name, + test_table_name, + "stability_results", + metric_stability_query, + ) %} + select * + from {{ result_relation }} - {%- if not dimensions %} {% set dimensions = [] %} {%- endif %} +{% endtest %} - {% set metric_properties = elementary.get_metric_properties( - model_graph_node, - timestamp_column, - where_expression, - time_bucket, - dimensions, - collected_by="metric_stability", - ) %} - {% set metric_names = metrics %} - - {#- The measurement window has to extend past min_bucket_age and has to - be wide enough to hold whole buckets, so it is derived from both - rather than falling back to defaults that would leave nothing to - compare. -#} - {% set resolved_window = elementary.resolve_metric_stability_window( - model_graph_node, - min_bucket_age, - metric_properties.time_bucket, - days_back, - backfill_days, +{# Collect one persistent CTAS table per column and return relations, scan + windows, and resolved names. Tables are unioned when comparing history. #} +{% macro _collect_metric_stability_metrics( + model, + model_relation, + column_definitions, + metric_properties, + days_back, + backfill_days, + dimensions, + database_name, + tests_schema_name, + test_table_name +) %} + {% set temp_table_relations = [] %} + {% set measurement_windows = [] %} + {% set resolved_columns = [] %} + + {%- for column_obj_and_monitors in column_definitions %} + {% set resolved_column = column_obj_and_monitors["column"].name %} + {% do resolved_columns.append(resolved_column) %} + {% set column_monitors = column_obj_and_monitors["monitors"] %} + + {%- set ( + raw_min_bucket_start, + max_bucket_end, + ) = elementary.get_metric_buckets_min_and_max( + model_relation=model_relation, + backfill_days=backfill_days, + days_back=days_back, + metric_names=column_monitors, + column_name=resolved_column, + metric_properties=metric_properties, ) %} - {% set days_back = resolved_window["days_back"] %} - {% set backfill_days = resolved_window["backfill_days"] %} - - {% set test_table_name = elementary.get_elementary_test_table_name() %} - {% set ( - database_name, - schema_name, - ) = elementary.get_package_database_and_schema("elementary") %} - {% set tests_schema_name = elementary.get_elementary_tests_schema( - database_name, schema_name + {# Keep bucket identities stable across runs, including weekly buckets. #} + {%- set min_bucket_start = elementary.edr_date_trunc( + metric_properties.time_bucket.period, + elementary.edr_cast_as_timestamp(raw_min_bucket_start), ) %} - {% set full_table_name = elementary.relation_to_full_name(model_relation) %} - - {#- One shared metrics table for every column. collect_column_metrics - would create a table per column and leave the cache pointing at the - last one, so all but the final column would be compared against - stale measurements. - - Each column gets its own table, created directly from its select. - Creating one table empty and filling it with INSERT statements - loses the rows on adapters where dbt rolls back the test's - transaction, which leaves data_monitoring_metrics with no history - at all and makes this test a silent permanent pass. The tables are - unioned at read time instead. -#} - {% set temp_table_relations = [] %} - {% set measurement_windows = [] %} - {% set resolved_columns = [] %} - - {%- for column_name in columns %} - {%- set column_obj_and_monitors = ( - elementary.get_column_obj_and_monitors( - model_relation, column_name, metrics - ) - ) -%} - {%- if not column_obj_and_monitors %} - {{ - exceptions.raise_compiler_error( - "Unable to find column `" - ~ column_name - ~ "` in `" - ~ full_table_name - ~ "`." - ) - }} - {%- endif %} - {%- set resolved_column = column_obj_and_monitors["column"].name %} - {%- do resolved_columns.append(resolved_column) %} - {%- set column_monitors = column_obj_and_monitors["monitors"] %} - {%- if not column_monitors %} - {{ - exceptions.raise_compiler_error( - "None of the metrics " ~ metrics - | join(", ") - ~ " apply to column `" - ~ column_name - ~ "` given its data type." - ) - }} - {%- endif %} - - {%- set ( - raw_min_bucket_start, - max_bucket_end, - ) = elementary.get_metric_buckets_min_and_max( - model_relation=model_relation, - backfill_days=backfill_days, - days_back=days_back, - metric_names=column_monitors, - column_name=resolved_column, - metric_properties=metric_properties, - ) %} - {#- get_metric_buckets_min_and_max can return a plain midnight - (backfill_bucket_start), which the bucket grid is then anchored - on. For any period longer than a day that midnight moves with - the run, so the grid drifts, every bucket_end lands on a new - surrogate id and no bucket is ever measured twice. Snapping the - anchor to the bucket period keeps ids stable across runs. -#} - {%- set min_bucket_start = elementary.edr_date_trunc( - metric_properties.time_bucket.period, - elementary.edr_cast_as_timestamp(raw_min_bucket_start), - ) %} - {%- do measurement_windows.append( - { - "column_name": resolved_column, - "min_bucket_start": min_bucket_start, - "max_bucket_end": elementary.edr_cast_as_timestamp( - max_bucket_end - ), - } - ) %} - {#- Only the monitors that apply to this column's data type. - Passing the full list would generate e.g. sum(). -#} - {%- set this_column_metrics = [] %} - {%- for monitor in column_monitors %} - {%- do this_column_metrics.append({"name": monitor, "type": monitor}) %} - {%- endfor %} - {%- set column_monitoring_query = elementary.column_monitoring_query( - model, - model_relation, - min_bucket_start, - max_bucket_end, - days_back, - column_obj_and_monitors["column"], - this_column_metrics, - metric_properties, - dimensions, - ) %} - {%- do temp_table_relations.append( - elementary.create_elementary_test_table( - database_name, - tests_schema_name, - test_table_name, - "metrics_" ~ loop.index0, - column_monitoring_query, - ) - ) %} - {%- endfor %} - - {#- Persist this run's measurements, which is what builds the history - the next run compares against. store_metrics_table_in_cache only - knows about a single "metrics" table, so the per-column relations - are registered directly. -#} - {% set metrics_tables_cache = ( - elementary.get_cache("tables").get("metrics").get("relations") + {%- do measurement_windows.append( + { + "column_name": resolved_column, + "min_bucket_start": min_bucket_start, + "max_bucket_end": elementary.edr_cast_as_timestamp( + max_bucket_end + ), + } ) %} - {%- for temp_table_relation in temp_table_relations %} - {% do metrics_tables_cache.append(temp_table_relation) %} + {%- set this_column_metrics = [] %} + {%- for monitor in column_monitors %} + {%- do this_column_metrics.append({"name": monitor, "type": monitor}) %} {%- endfor %} - - {% set detection_end = elementary.get_detection_end(none) %} - {% set metric_stability_query = elementary.metric_stability_query( - test_metrics_table_relations=temp_table_relations, - full_table_name=full_table_name, - metric_names=metric_names, - metric_properties=metric_properties, - detection_end=detection_end, - days_back=days_back, - min_bucket_age=min_bucket_age, - max_change_percent=max_change_percent, - change_since=change_since, - column_names=resolved_columns, - measurement_windows=measurement_windows, + {%- set column_monitoring_query = elementary.column_monitoring_query( + model, + model_relation, + min_bucket_start, + max_bucket_end, + days_back, + column_obj_and_monitors["column"], + this_column_metrics, + metric_properties, + dimensions, ) %} - {{ - elementary.debug_log( - "metric_stability_query - \n" ~ metric_stability_query + {%- do temp_table_relations.append( + elementary.create_elementary_test_table( + database_name, + tests_schema_name, + test_table_name, + "metrics_" ~ loop.index0, + column_monitoring_query, ) - }} - - {# Freeze failures before dbt executes the test and samples its results. - Use the normal sampling path so sample limits and privacy settings - still apply. This relation is cleaned up with the metrics tables. #} - {% set result_relation = elementary.create_elementary_test_table( - database_name, - tests_schema_name, - test_table_name, - "stability_results", - metric_stability_query, ) %} - select * - from {{ result_relation }} - - {%- else %} - - {#- test must run an sql query -#} - {{ elementary.no_results_query() }} + {%- endfor %} - {%- endif %} -{% endtest %} + {# Register every column table for history persistence at on-run-end. #} + {% set metrics_tables_cache = ( + elementary.get_cache("tables").get("metrics").get("relations") + ) %} + {%- for temp_table_relation in temp_table_relations %} + {% do metrics_tables_cache.append(temp_table_relation) %} + {%- endfor %} + {% do return( + { + "relations": temp_table_relations, + "windows": measurement_windows, + "columns": resolved_columns, + } + ) %} +{% endmacro %} {% macro validate_min_bucket_age(min_bucket_age) %} {%- set valid_periods = ["day", "week", "hour", "minute", "second"] %} {%- if not min_bucket_age or min_bucket_age is not mapping %} - {# fmt: off #} {% do exceptions.raise_compiler_error( "min_bucket_age is required and must be a mapping. Expected format: min_bucket_age: count: int period: string" ) %} - {# fmt: on #} {%- endif %} {%- for key in min_bucket_age %} {%- if key not in ["count", "period"] %} @@ -468,37 +243,16 @@ {% endmacro %} -{# - A bucket can only be compared while it is still being re-measured, so the - measurement window has to extend past min_bucket_age or nothing is ever both - settled and still under observation. - - Which parameter governs that window depends on the materialization. - get_metric_buckets_min_and_max takes its incremental branch for sources and - incremental models, where backfill_days sets the window; every other model - takes the regular branch, which re-measures the whole days_back window and - ignores backfill_days entirely. days_back additionally bounds the comparison - itself, so it always matters. - - Unset parameters are derived from min_bucket_age, because the package defaults - (days_back 14, backfill_days 2) are unrelated to how long a bucket needs - watching and would silently leave nothing to compare. An explicit value too - small to ever produce a comparison raises instead. -#} +{# Return days_back/backfill_days sized for settled buckets; reject windows + below the required minimum. Backfill applies to sources/incremental models. #} {% macro resolve_metric_stability_window( model_graph_node, min_bucket_age, time_bucket, days_back, backfill_days ) %} {%- set age_kwargs = {min_bucket_age.period ~ "s": min_bucket_age.count} %} - {#- Kept as a fraction of a day. Ceiling it first would turn a sub-day age - into a whole day and reject a days_back that in fact covers many - settled buckets. -#} {%- set age_days = ( modules.datetime.timedelta(**age_kwargs).total_seconds() / 86400.0 ) %} - {#- The grid anchor moves by a day between runs, so a bucket spanning more - than one period step cannot be given a stable identity and the test - would silently never fire. Refuse rather than pass forever. -#} {%- if time_bucket.count | int != 1 %} {% do exceptions.raise_compiler_error( "metric_stability requires a time_bucket count of 1, got " @@ -507,9 +261,7 @@ ) %} {%- endif %} - {#- Bucket length in days, used to make sure the eligible band can actually - hold whole buckets. month/quarter/year are nominal: they only have to be - good enough to size the window. -#} + {# Calendar periods use nominal day lengths for sizing only. #} {%- set period_days = { "second": 1.0 / 86400.0, "minute": 1.0 / 1440.0, @@ -529,16 +281,24 @@ ) %} {%- endif %} - {#- Twice the age, so a bucket is observed over a stretch rather than for a - single run, which is what lets 'first_check' see drift accumulate; and - at least two whole buckets past the age, or the settled band is narrower - than one bucket and nothing is ever both settled and still measured. -#} - {%- set derived = [ - (age_days * 2) | round(0, "ceil") | int, - (age_days + 2 * bucket_days) | round(0, "ceil") | int, - (age_days + 1) | round(0, "ceil") | int, - 1, - ] | max %} + {# The window has to outlast settling, or nothing is ever both settled and + still measured. Take the widest of three floors: + - twice the age, so a bucket is observed over a stretch of runs rather + than measured once and never compared; + - two whole buckets past the age, or the settled band is narrower than + a single bucket; + - one whole day past the age. days_back is counted in whole days and + the metrics scan starts at midnight, so this keeps a sub-day age from + collapsing the window to a sliver of a day, and it is what makes the + floor at least 2 for every age, so a days_back the query would + truncate to 0 or 1 days is rejected below instead of silently + producing an empty comparison window. #} + {%- set twice_the_age = (age_days * 2) | round(0, "ceil") | int %} + {%- set two_buckets_past_age = ( + (age_days + 2 * bucket_days) | round(0, "ceil") | int + ) %} + {%- set one_day_past_age = (age_days + 1) | round(0, "ceil") | int %} + {%- set derived = [twice_the_age, two_buckets_past_age, one_day_past_age] | max %} {%- set age_description = ( min_bucket_age.count ~ " " @@ -546,10 +306,6 @@ ~ ("s" if min_bucket_age.count > 1 else "") ) %} - {#- get_metric_buckets_min_and_max only takes its backfill branch when - force_metrics_backfill is off; with it on every model re-measures the - whole days_back window and backfill_days is ignored, so validating it - would abort the run over a value that has no effect. -#} {%- set uses_backfill_window = elementary.is_incremental_model( model_graph_node, source_included=true ) and not elementary.get_config_var("force_metrics_backfill") %} @@ -594,3 +350,211 @@ } ) %} {% endmacro %} + + +{# Validate and normalize arguments; return resolved model, timestamp, and columns. #} +{% macro _validate_metric_stability_arguments( + model, + columns, + metrics, + timestamp_column, + min_bucket_age, + change_since, + max_change_percent, + days_back, + backfill_days +) %} + {%- if columns is string %} {% set columns = [columns] %} {%- endif %} + {%- if metrics is string %} {% set metrics = [metrics] %} {%- endif %} + {%- if change_since is string %} + {% set change_since = [change_since] %} + {%- endif %} + {%- if columns %} + {%- set seen_columns = [] %} + {%- set deduped_columns = [] %} + {%- for column_name in columns %} + {%- set key = column_name | trim('"') | lower %} + {%- if key not in seen_columns %} + {%- do seen_columns.append(key) %} + {%- do deduped_columns.append(column_name) %} + {%- endif %} + {%- endfor %} + {%- set columns = deduped_columns %} + {%- endif %} + + {%- if not change_since %} + {{ + exceptions.raise_compiler_error( + "metric_stability requires at least one baseline in `change_since`: 'last_check', 'first_check', or both." + ) + }} + {%- endif %} + + {%- if max_change_percent is not number %} + {{ + exceptions.raise_compiler_error( + "max_change_percent must be a number, got '" + ~ max_change_percent + ~ "'. Write it unquoted, e.g. max_change_percent: 25." + ) + }} + {%- endif %} + {%- if max_change_percent < 0 %} + {{ + exceptions.raise_compiler_error( + "max_change_percent must be non-negative." + ) + }} + {%- endif %} + {%- for arg_name, arg_value in [ + ("days_back", days_back), + ("backfill_days", backfill_days), + ] %} + {%- if arg_value is not none and arg_value is not number %} + {{ + exceptions.raise_compiler_error( + arg_name + ~ " must be a number, got '" + ~ arg_value + ~ "'. Write it unquoted, e.g. " + ~ arg_name + ~ ": 30." + ) + }} + {%- endif %} + {%- endfor %} + + {%- if not columns %} + {{ + exceptions.raise_compiler_error( + "metric_stability requires at least one column in `columns`." + ) + }} + {%- endif %} + + {%- if not metrics %} + {{ + exceptions.raise_compiler_error( + "metric_stability requires at least one metric type in `metrics`." + ) + }} + {%- endif %} + + {%- set available_column_monitors = elementary.get_available_column_monitors() %} + {%- for metric_type in metrics %} + {%- if metric_type not in available_column_monitors %} + {{ + exceptions.raise_compiler_error( + "Unsupported column metric: '" + ~ metric_type + ~ "'. Supported metrics are: " + ~ available_column_monitors + | join(", ") ~ "." + ) + }} + {%- endif %} + {%- endfor %} + + {%- for baseline in change_since %} + {%- if baseline not in ["last_check", "first_check"] %} + {{ + exceptions.raise_compiler_error( + "Unsupported `change_since` value '" + ~ baseline + ~ "'. Supported values are 'last_check' and 'first_check'." + ) + }} + {%- endif %} + {%- endfor %} + + {% do elementary.validate_min_bucket_age(min_bucket_age) %} + + {% set model_relation = elementary.get_model_relation_for_test( + model, elementary.get_test_model() + ) %} + {%- if not model_relation %} + {{ exceptions.raise_compiler_error("Unsupported model: " ~ model) }} + {%- endif %} + + {%- if elementary.is_ephemeral_model(model_relation) %} + {{ + exceptions.raise_compiler_error( + "Test not supported for ephemeral models: " + ~ model_relation.identifier + ) + }} + {%- endif %} + + {% set model_graph_node = elementary.get_model_graph_node(model_relation) %} + {% set timestamp_column = elementary.get_test_argument( + "timestamp_column", timestamp_column, model_graph_node + ) %} + {%- if not timestamp_column %} + {{ + exceptions.raise_compiler_error( + "metric_stability requires a `timestamp_column`, either on the test or in the model's elementary config." + ) + }} + {%- endif %} + + {% set timestamp_column_data_type = ( + elementary.find_normalized_data_type_for_column( + model_relation, timestamp_column + ) + ) %} + {%- if not elementary.is_column_timestamp( + model_relation, timestamp_column, timestamp_column_data_type + ) %} + {{ + exceptions.raise_compiler_error( + "Column '" + ~ timestamp_column + ~ "' is not a timestamp type. metric_stability buckets data over time and requires a timestamp column." + ) + }} + {%- endif %} + + {% set full_table_name = elementary.relation_to_full_name(model_relation) %} + {% set column_definitions = [] %} + {% for column_name in columns %} + {%- set column_obj_and_monitors = elementary.get_column_obj_and_monitors( + model_relation, column_name, metrics + ) -%} + {%- if not column_obj_and_monitors %} + {{ + exceptions.raise_compiler_error( + "Unable to find column `" + ~ column_name + ~ "` in `" + ~ full_table_name + ~ "`." + ) + }} + {%- endif %} + {%- set resolved_column = column_obj_and_monitors["column"].name %} + {%- set column_monitors = column_obj_and_monitors["monitors"] %} + {%- if not column_monitors %} + {{ + exceptions.raise_compiler_error( + "None of the metrics " ~ metrics + | join(", ") + ~ " apply to column `" + ~ column_name + ~ "` given its data type." + ) + }} + {%- endif %} + + {% do column_definitions.append(column_obj_and_monitors) %} + {% endfor %} + {% do return( + { + "columns": column_definitions, + "metrics": metrics, + "change_since": change_since, + "model_relation": model_relation, + "model_graph_node": model_graph_node, + "timestamp_column": timestamp_column, + } + ) %} +{% endmacro %} From 7cf1194b1012d222c3c11de9b421befe72696ab2 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Mon, 7 Sep 2026 22:43:26 +0300 Subject: [PATCH 10/13] fix: keep metric_stability seed names free of dots The parametrized `days_back` value lands in the seed relation name, and the Hive metastore behind Trino rejects a "." in one, so the fractional case failed to seed. Give both cases explicit ids. Co-Authored-By: Claude Opus 5 --- integration_tests/tests/test_metric_stability.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/integration_tests/tests/test_metric_stability.py b/integration_tests/tests/test_metric_stability.py index fc390d1a5..1fe162313 100644 --- a/integration_tests/tests/test_metric_stability.py +++ b/integration_tests/tests/test_metric_stability.py @@ -248,7 +248,11 @@ def test_metric_stability_rejects_multi_step_buckets( assert result == "error" -@pytest.mark.parametrize("days_back", [1, 0.5]) +# Explicit ids: the value ends up in the seed relation name, and a "." in one +# is rejected by the Hive metastore behind Trino. +@pytest.mark.parametrize( + "days_back", [pytest.param(1, id="whole"), pytest.param(0.5, id="fractional")] +) def test_metric_stability_rejects_too_short_window( test_id: str, dbt_project: DbtProject, days_back ): From cd8e7e92cec07d9cdef37ef7829fe1975b35c028 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Tue, 8 Sep 2026 09:21:04 +0300 Subject: [PATCH 11/13] fix: write metric_stability results at the table's timestamp precision The results select is frozen into a table before dbt samples it, and on Athena that CTAS reads timestamp(6) out of the metric history while the destination columns are millisecond precision, so every metric_stability test errored with "Incorrect timestamp precision for timestamp(6) ... column name: bucket_start". No other test creates a table from a select over data_monitoring_metrics, which is why this is specific to this test. Cast each timestamp the select carries out of the history, so the frozen table gets the precision it accepts. Co-Authored-By: Claude Opus 5 --- .../monitors_query/metric_stability_query.sql | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql index 8775dbc74..6b31c6049 100644 --- a/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql +++ b/macros/edr/data_monitoring/monitors_query/metric_stability_query.sql @@ -163,25 +163,34 @@ ) + {#- This select is frozen into a table, so every timestamp it carries out + of the metric history is cast to the precision that table accepts. + Athena reads timestamp(6) from the history and writes millisecond + columns, and rejects the CTAS otherwise. -#} select id as metric_id, full_table_name, column_name, metric_name, metric_type, - bucket_start, - bucket_end, + {{ elementary.edr_cast_as_timestamp("bucket_start") }} as bucket_start, + {{ elementary.edr_cast_as_timestamp("bucket_end") }} as bucket_end, bucket_duration_hours, dimension, dimension_value, - case when is_current = 0 - then {{ elementary.edr_cast_as_timestamp(elementary.edr_quote(elementary.run_started_at_as_string())) }} - else updated_at end as measured_at, + {{ elementary.edr_cast_as_timestamp( + "case when is_current = 0" + ~ " then " ~ elementary.edr_cast_as_timestamp(elementary.edr_quote(elementary.run_started_at_as_string())) + ~ " else updated_at end" + ) }} as measured_at, case when is_current = 0 then 'missing_bucket' else 'value_changed' end as change_type, case when is_current = 1 then measured_value end as metric_value, case when is_current = 0 then measured_value else last_check_value end as previous_value, - case when is_current = 0 then updated_at else last_check_at end as previous_measured_at, - first_check_at as initial_measured_at, + {{ elementary.edr_cast_as_timestamp( + "case when is_current = 0 then updated_at else last_check_at end" + ) }} as previous_measured_at, + {{ elementary.edr_cast_as_timestamp("first_check_at") }} + as initial_measured_at, first_check_value as initial_value, case when is_current = 1 then measured_value - last_check_value end as change_since_last_check, case when is_current = 1 then measured_value - first_check_value end as change_since_first_check, From dfe779f8146215f2478296ddca6a188d7a17d7be Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 09:52:52 +0000 Subject: [PATCH 12/13] ci: raise warehouse test job timeout to 90 minutes Co-Authored-By: Itamar Hartstein --- .github/workflows/test-warehouse.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-warehouse.yml b/.github/workflows/test-warehouse.yml index 675d65cc2..c167516bc 100644 --- a/.github/workflows/test-warehouse.yml +++ b/.github/workflows/test-warehouse.yml @@ -60,7 +60,7 @@ env: jobs: test: runs-on: ubuntu-latest - timeout-minutes: 60 + timeout-minutes: 90 permissions: contents: read # Mint an OIDC token to assume the shared elementary-oss AWS role. From 216ccd68645a9efdd649252cb7ec48f75ab0afe3 Mon Sep 17 00:00:00 2001 From: Joost Boonzajer Flaes Date: Tue, 8 Sep 2026 17:22:55 +0300 Subject: [PATCH 13/13] refactor: address review, move metric_stability docs to the docs site - Rename `_validate_metric_stability_arguments` to `_parse_and_validate_metric_stability_arguments`, since it also normalizes the arguments (scalar to list, column dedupe) rather than only checking them. - Drop docs/metric_stability.md. That directory holds the mintlify docs, so a standalone markdown file there was out of place. The content now lives at elementary-data/elementary#2343 against the docs branch, and the README and the test's header comment point at the published page. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- docs/metric_stability.md | 91 ---------------------- macros/edr/tests/test_metric_stability.sql | 7 +- 3 files changed, 5 insertions(+), 95 deletions(-) delete mode 100644 docs/metric_stability.md diff --git a/README.md b/README.md index 9a800a100..8aef2480e 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ These tables are the backbone of any observability setup — enabling alerts, re **2. Elementary Tests** A suite of anomaly detection and data quality tests that run like native dbt tests — no separate tooling. Covers volume, freshness, column distributions, schema changes, and AI-powered validation for structured and unstructured data. → [See all tests](https://docs.elementary-data.com/data-tests/introduction) -[Metric stability](docs/metric_stability.md) detects restatements of settled historical aggregates within a configured observation window. +[Metric stability](https://docs.elementary-data.com/data-tests/metric-stability) detects restatements of settled historical aggregates within a configured observation window. --- diff --git a/docs/metric_stability.md b/docs/metric_stability.md deleted file mode 100644 index 16c26d7b1..000000000 --- a/docs/metric_stability.md +++ /dev/null @@ -1,91 +0,0 @@ -# Metric stability - -`elementary.metric_stability` detects changes to a time bucket's own previously -measured aggregates after the bucket has settled. Use it for historical revenue, -costs, or other measures that should stop changing after late data has arrived. -Ordinary anomaly detection compares different periods; it may notice some effects -of a restatement, but does not directly enforce this expectation. - -```yaml -models: - - name: orders - tests: - - elementary.metric_stability: - columns: [cost_amount, revenue_amount] - metrics: [sum] - timestamp_column: order_ts - time_bucket: { count: 1, period: day } - min_bucket_age: { count: 4, period: week } - days_back: 90 - change_since: [first_check] - max_change_percent: 1 -``` - -Choose a business/event timestamp whose historical periods you want to protect. -A row's ingestion or last-modified timestamp can move it between buckets when it -is updated, which answers a different question. - -`dimensions` and `where_expression` behave as they do in the other metric-based -tests. Each bucket/dimension combination is a separate metric with its own -history and baseline, so a restatement confined to one dimension value is still -reported, and a dimension value that stops appearing is reported as a missing -bucket. Every extra combination is another measurement to store and compare. - -## Coverage and cost - -`min_bucket_age` measures time since the bucket ended. Measurements taken before -that age are excluded from both baselines. The first eligible measurement only -establishes a baseline; a pass at that point does not verify historical stability. - -`days_back` bounds the observation window. The example protects daily buckets -roughly 28 to 90 days old, not all historical data. Corrections outside that window -are not detected. Incremental models and sources also use `backfill_days` to -control remeasurement; it defaults to `days_back`. An explicitly shorter backfill -window reduces coverage to the buckets actually scanned on that run. - -Without an explicit window, the test derives one from the settling age (roughly -twice the age, with room for whole buckets). This is a convenience default, not a -business retention policy. Set the window to cover the corrections you care about -and run frequently enough to measure each eligible bucket more than once. -Longer windows increase rescanning and metric-history storage costs. - -## Baselines and legitimate corrections - -- `last_check` compares against the previous eligible measurement. With zero - tolerance, 100 -> 120 fails; a subsequent 120 passes. New measurements become - the baseline automatically, including measurements from failing runs. -- `first_check` compares against the earliest retained eligible measurement. - It catches cumulative drift: 100 -> 110 -> 120 exceeds a 15% threshold overall, - although each step is smaller. A corrected 120 continues failing against 100 - until it returns within tolerance or the bucket leaves coverage. -- Selecting both fails if either comparison exceeds the threshold. - -There is no explicit accept/reset-baseline operation in this version. Choose -`last_check` when changes should be reported once and then automatically accepted. -Choose `first_check` when continued deviation should remain a failure. Switching -to `last_check` changes the policy; it does not reset `first_check`. Account for -history retention and cleanup: the baseline is the earliest _retained_ -measurement, not an immutable approved snapshot. Do not use this test as a -substitute for an auditable financial close or an immutable snapshot. - -## Failure details - -With Elementary’s test materialization enabled, stored samples include the bucket, column, metric, dimensions, current and baseline -values, measurement timestamps, and absolute and percentage deltas. Normal -Elementary sample limits and privacy controls apply. - -`change_type: value_changed` reports numeric movement above the configured -percentage threshold. `max_change_percent: 1` means 1%, not 100%; the default is -zero with a tiny relative floor to suppress floating-point aggregation noise. -Movement away from a zero baseline always fails because relative change is -undefined there. - -`change_type: missing_bucket` means a previously measured bucket or dimension has -no current metric in a window the test actually rescanned. It fails regardless of -the percentage tolerance, reports a NULL current value and the last observed -value, and remains a failure while missing and within coverage. It does not invent -a zero for aggregates such as average or minimum. - -Stable aggregates do not guarantee unchanged source rows. Offsetting changes can -cancel in a sum; pair this test with row-level checks when record immutability is -the requirement. diff --git a/macros/edr/tests/test_metric_stability.sql b/macros/edr/tests/test_metric_stability.sql index a51dfde19..5eab81833 100644 --- a/macros/edr/tests/test_metric_stability.sql +++ b/macros/edr/tests/test_metric_stability.sql @@ -1,5 +1,6 @@ {# Detect restatements of settled bucket metrics within the observation window. - Configuration, baseline behavior, and limits: docs/metric_stability.md. #} + Configuration, baseline behavior, and limits: + https://docs.elementary-data.com/data-tests/metric-stability #} {% test metric_stability( model, columns, @@ -24,7 +25,7 @@ {% do return(elementary.no_results_query()) %} {% endif %} - {% set arguments = elementary._validate_metric_stability_arguments( + {% set arguments = elementary._parse_and_validate_metric_stability_arguments( model, columns, metrics, @@ -353,7 +354,7 @@ {# Validate and normalize arguments; return resolved model, timestamp, and columns. #} -{% macro _validate_metric_stability_arguments( +{% macro _parse_and_validate_metric_stability_arguments( model, columns, metrics,