From bd65d479bdd0e3d61b80c701293aa18dc1fec342 Mon Sep 17 00:00:00 2001 From: Mike Williams Date: Mon, 18 May 2026 17:02:44 -0400 Subject: [PATCH 1/5] feat: add analysis period logging --- jetstream/analysis.py | 9 +++++++++ jetstream/logging/bigquery_log_handler.py | 3 +++ jetstream/tests/integration/test_analysis_integration.py | 2 ++ jetstream/tests/integration/test_logging_integration.py | 3 +++ 4 files changed, 17 insertions(+) diff --git a/jetstream/analysis.py b/jetstream/analysis.py index da888daf8..4aa6bd5a3 100644 --- a/jetstream/analysis.py +++ b/jetstream/analysis.py @@ -269,6 +269,7 @@ def publish_view( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis, + "analysis_period": window_period.value, }, ) return @@ -384,6 +385,7 @@ def publish_view( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis, + "analysis_period": window_period.value, }, ) raise @@ -478,6 +480,7 @@ def calculate_metrics( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis, + "analysis_period": f"{period.value}_{window}", }, ) raise @@ -576,6 +579,7 @@ def calculate_metric_for_ds( "experiment": self.config.experiment.normandy_slug, "metric": metric.name, "analysis_basis": analysis_basis, + "analysis_period": f"{period.value}_{window}", }, ) return "" @@ -618,6 +622,7 @@ def calculate_statistics( "statistic": metric.statistic.name(), "analysis_basis": analysis_basis, "segment": segment, + "analysis_period": period.value, }, ) return StatisticResultCollection.model_validate([]) @@ -707,6 +712,7 @@ def subset_metric_table( "metric": summary.metric.name, "analysis_basis": analysis_basis, "segment": segment, + "analysis_period": period.value, }, ) return None @@ -858,6 +864,7 @@ def _create_subset_metric_table_query_covariate( "metric": metric.name, "analysis_basis": analysis_basis.value, "segment": segment, + "analysis_period": period.value, }, ) return self._create_subset_metric_table_query_univariate( @@ -1387,6 +1394,7 @@ def run( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis.value, + "analysis_period": period.value, }, ) continue @@ -1524,6 +1532,7 @@ def run( "experiment": self.config.experiment.normandy_slug, "metric": metric.name, "analysis_basis": analysis_basis.value, + "analysis_period": period.value, }, ) continue diff --git a/jetstream/logging/bigquery_log_handler.py b/jetstream/logging/bigquery_log_handler.py index 43c46ad88..dce06a593 100644 --- a/jetstream/logging/bigquery_log_handler.py +++ b/jetstream/logging/bigquery_log_handler.py @@ -41,6 +41,9 @@ def _buffer_to_json(self, buffer): None if not hasattr(record, "analysis_basis") else record.analysis_basis ), "segment": None if not hasattr(record, "segment") else record.segment, + "analysis_period": ( + None if not hasattr(record, "analysis_period") else record.analysis_period + ), "message": record.getMessage(), "log_level": record.levelname, "exception": str(record.exc_info), diff --git a/jetstream/tests/integration/test_analysis_integration.py b/jetstream/tests/integration/test_analysis_integration.py index dd0d3d4c6..9f95f1530 100644 --- a/jetstream/tests/integration/test_analysis_integration.py +++ b/jetstream/tests/integration/test_analysis_integration.py @@ -1186,6 +1186,7 @@ def test_logging( assert error_logs[4].get("statistic") == "bootstrap_mean" assert error_logs[4].get("analysis_basis") == "enrollments" assert error_logs[4].get("segment") == "regular_user_v3" + assert error_logs[4].get("segment") == "week" assert error_logs[5].get("log_level") == "ERROR" assert error_logs[5].get("experiment") == "test-experiment" @@ -1193,6 +1194,7 @@ def test_logging( assert error_logs[5].get("statistic") == "bootstrap_mean" assert error_logs[5].get("analysis_basis") == "exposures" assert error_logs[5].get("segment") == "regular_user_v3" + assert error_logs[5].get("segment") == "week" # wait for profiling results to land in BigQuery # todo: improve this test as it might lead to flakiness diff --git a/jetstream/tests/integration/test_logging_integration.py b/jetstream/tests/integration/test_logging_integration.py index d25dd20c3..6ff78d35f 100644 --- a/jetstream/tests/integration/test_logging_integration.py +++ b/jetstream/tests/integration/test_logging_integration.py @@ -16,6 +16,7 @@ def _logging_table_setup(self, client, temporary_dataset, project_id): bigquery.SchemaField("statistic", "STRING"), bigquery.SchemaField("analysis_basis", "STRING"), bigquery.SchemaField("segment", "STRING"), + bigquery.SchemaField("analysis_period", "STRING"), bigquery.SchemaField("message", "STRING"), bigquery.SchemaField("log_level", "STRING"), bigquery.SchemaField("exception", "STRING"), @@ -54,6 +55,7 @@ def test_logging_to_bigquery(self, client, temporary_dataset, project_id): "statistic": "test_statistic", "segment": "all", "analysis_basis": "enrollments", + "analysis_period": "week_2", }, ) logger.exception( @@ -85,6 +87,7 @@ def test_logging_to_bigquery(self, client, temporary_dataset, project_id): and r.segment == "all" and r.analysis_basis == "enrollments" and r.source == "jetstream" + and r.analysis_period == "week_2" for r in result ) assert any( From 8949e37c7a82ad840903f686dc91319c479e8974 Mon Sep 17 00:00:00 2001 From: Mike Williams Date: Tue, 19 May 2026 09:43:02 -0400 Subject: [PATCH 2/5] fix test --- jetstream/tests/integration/test_analysis_integration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetstream/tests/integration/test_analysis_integration.py b/jetstream/tests/integration/test_analysis_integration.py index 9f95f1530..e39db848b 100644 --- a/jetstream/tests/integration/test_analysis_integration.py +++ b/jetstream/tests/integration/test_analysis_integration.py @@ -1186,7 +1186,7 @@ def test_logging( assert error_logs[4].get("statistic") == "bootstrap_mean" assert error_logs[4].get("analysis_basis") == "enrollments" assert error_logs[4].get("segment") == "regular_user_v3" - assert error_logs[4].get("segment") == "week" + assert error_logs[4].get("analysis_period") == "week" assert error_logs[5].get("log_level") == "ERROR" assert error_logs[5].get("experiment") == "test-experiment" @@ -1194,7 +1194,7 @@ def test_logging( assert error_logs[5].get("statistic") == "bootstrap_mean" assert error_logs[5].get("analysis_basis") == "exposures" assert error_logs[5].get("segment") == "regular_user_v3" - assert error_logs[5].get("segment") == "week" + assert error_logs[5].get("analysis_period") == "week" # wait for profiling results to land in BigQuery # todo: improve this test as it might lead to flakiness From daac07619cc898221d7f29cc71c93cdb9fdf03e6 Mon Sep 17 00:00:00 2001 From: Mike Williams Date: Tue, 19 May 2026 10:44:58 -0400 Subject: [PATCH 3/5] refactor and update tests --- jetstream/analysis.py | 14 ++++++------- jetstream/statistics.py | 1 + jetstream/tests/test_analysis.py | 36 ++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/jetstream/analysis.py b/jetstream/analysis.py index 4aa6bd5a3..315a6cb45 100644 --- a/jetstream/analysis.py +++ b/jetstream/analysis.py @@ -269,7 +269,7 @@ def publish_view( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis, - "analysis_period": window_period.value, + "analysis_period": window_period, }, ) return @@ -385,7 +385,7 @@ def publish_view( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis, - "analysis_period": window_period.value, + "analysis_period": window_period, }, ) raise @@ -622,7 +622,7 @@ def calculate_statistics( "statistic": metric.statistic.name(), "analysis_basis": analysis_basis, "segment": segment, - "analysis_period": period.value, + "analysis_period": period, }, ) return StatisticResultCollection.model_validate([]) @@ -712,7 +712,7 @@ def subset_metric_table( "metric": summary.metric.name, "analysis_basis": analysis_basis, "segment": segment, - "analysis_period": period.value, + "analysis_period": period, }, ) return None @@ -864,7 +864,7 @@ def _create_subset_metric_table_query_covariate( "metric": metric.name, "analysis_basis": analysis_basis.value, "segment": segment, - "analysis_period": period.value, + "analysis_period": period, }, ) return self._create_subset_metric_table_query_univariate( @@ -1394,7 +1394,7 @@ def run( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis.value, - "analysis_period": period.value, + "analysis_period": period, }, ) continue @@ -1532,7 +1532,7 @@ def run( "experiment": self.config.experiment.normandy_slug, "metric": metric.name, "analysis_basis": analysis_basis.value, - "analysis_period": period.value, + "analysis_period": period, }, ) continue diff --git a/jetstream/statistics.py b/jetstream/statistics.py index f5756cf84..91abbbff5 100644 --- a/jetstream/statistics.py +++ b/jetstream/statistics.py @@ -274,6 +274,7 @@ def apply( "statistic": self.name(), "analysis_basis": analysis_basis.value, "segment": segment, + "analysis_period": self.period, }, ) diff --git a/jetstream/tests/test_analysis.py b/jetstream/tests/test_analysis.py index dcf34a661..9ffbc3c31 100644 --- a/jetstream/tests/test_analysis.py +++ b/jetstream/tests/test_analysis.py @@ -25,6 +25,7 @@ EnrollmentNotCompleteException, ExplicitSkipException, HighPopulationException, + StatisticComputationException, UnsupportedApplicationException, ) from jetstream.metric import Metric @@ -1292,6 +1293,13 @@ def test_calculate_metric_for_ds_returns_empty_on_specific_errors( assert result == "" assert "simulated error" in caplog.text + # assert logged extras are as expected + assert len(caplog.records) == 1 + log = caplog.records[0] + assert log.experiment == experiments[0].normandy_slug + assert log.analysis_basis == AnalysisBasis.ENROLLMENTS + assert log.analysis_period == f"{AnalysisPeriod.WEEK.value}_1" + assert log.metric == mock_metric.name def test_calculate_metric_for_ds_raises_for_other_errors(experiments, monkeypatch): @@ -1401,6 +1409,34 @@ def test_calculate_statistics_returns_empty_for_none_segment_data(experiments): assert result.root == [] +def test_calculate_statistics_logging_extras(experiments, monkeypatch, caplog): + """Ensure that the expected extras are logged.""" + summary = MagicMock() + segment_data = MagicMock() + mocked_from_config = Mock(side_effect=StatisticComputationException("Statistic failed!")) + monkeypatch.setattr("jetstream.analysis.Summary.from_config", mocked_from_config) + + with caplog.at_level(logging.ERROR): + result = ( + _empty_analysis(experiments) + .calculate_statistics( + summary, segment_data, "all", AnalysisBasis.ENROLLMENTS, 7, AnalysisPeriod.WEEK + ) + .compute(scheduler="synchronous") + ) + + assert result.root == [] + assert "Statistic failed!" in caplog.text + assert len(caplog.records) == 1 + log = caplog.records[0] + assert log.experiment == "normandy-test-slug" + assert log.segment == "all" + assert log.analysis_basis == AnalysisBasis.ENROLLMENTS + assert log.analysis_period == AnalysisPeriod.WEEK + assert log.metric == summary.metric.name + assert log.statistic == summary.statistic.name() + + def test_run_continues_after_google_api_error(experiments, monkeypatch, caplog): """Ensure that a GoogleAPICallError from one data source does not prevent save_statistics from being called, and no downstream BQ calls are made with From c1c0230f2ad675288131a81e6546e17a89d83a91 Mon Sep 17 00:00:00 2001 From: Mike Williams Date: Tue, 19 May 2026 10:57:54 -0400 Subject: [PATCH 4/5] need to log period.value --- jetstream/analysis.py | 14 +++++++------- jetstream/statistics.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/jetstream/analysis.py b/jetstream/analysis.py index 315a6cb45..4aa6bd5a3 100644 --- a/jetstream/analysis.py +++ b/jetstream/analysis.py @@ -269,7 +269,7 @@ def publish_view( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis, - "analysis_period": window_period, + "analysis_period": window_period.value, }, ) return @@ -385,7 +385,7 @@ def publish_view( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis, - "analysis_period": window_period, + "analysis_period": window_period.value, }, ) raise @@ -622,7 +622,7 @@ def calculate_statistics( "statistic": metric.statistic.name(), "analysis_basis": analysis_basis, "segment": segment, - "analysis_period": period, + "analysis_period": period.value, }, ) return StatisticResultCollection.model_validate([]) @@ -712,7 +712,7 @@ def subset_metric_table( "metric": summary.metric.name, "analysis_basis": analysis_basis, "segment": segment, - "analysis_period": period, + "analysis_period": period.value, }, ) return None @@ -864,7 +864,7 @@ def _create_subset_metric_table_query_covariate( "metric": metric.name, "analysis_basis": analysis_basis.value, "segment": segment, - "analysis_period": period, + "analysis_period": period.value, }, ) return self._create_subset_metric_table_query_univariate( @@ -1394,7 +1394,7 @@ def run( extra={ "experiment": self.config.experiment.normandy_slug, "analysis_basis": analysis_basis.value, - "analysis_period": period, + "analysis_period": period.value, }, ) continue @@ -1532,7 +1532,7 @@ def run( "experiment": self.config.experiment.normandy_slug, "metric": metric.name, "analysis_basis": analysis_basis.value, - "analysis_period": period, + "analysis_period": period.value, }, ) continue diff --git a/jetstream/statistics.py b/jetstream/statistics.py index 91abbbff5..dcdf0f239 100644 --- a/jetstream/statistics.py +++ b/jetstream/statistics.py @@ -274,7 +274,7 @@ def apply( "statistic": self.name(), "analysis_basis": analysis_basis.value, "segment": segment, - "analysis_period": self.period, + "analysis_period": self.period.value if self.period else None, }, ) From 794919a196c4e1201886b2471e495d868827a68f Mon Sep 17 00:00:00 2001 From: Mike Williams Date: Tue, 19 May 2026 11:18:40 -0400 Subject: [PATCH 5/5] fix log test --- jetstream/tests/test_analysis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetstream/tests/test_analysis.py b/jetstream/tests/test_analysis.py index 9ffbc3c31..26872e847 100644 --- a/jetstream/tests/test_analysis.py +++ b/jetstream/tests/test_analysis.py @@ -1432,7 +1432,7 @@ def test_calculate_statistics_logging_extras(experiments, monkeypatch, caplog): assert log.experiment == "normandy-test-slug" assert log.segment == "all" assert log.analysis_basis == AnalysisBasis.ENROLLMENTS - assert log.analysis_period == AnalysisPeriod.WEEK + assert log.analysis_period == AnalysisPeriod.WEEK.value assert log.metric == summary.metric.name assert log.statistic == summary.statistic.name()