Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions jetstream/analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ def publish_view(
extra={
"experiment": self.config.experiment.normandy_slug,
"analysis_basis": analysis_basis,
"analysis_period": window_period.value,
},
)
return
Expand Down Expand Up @@ -384,6 +385,7 @@ def publish_view(
extra={
"experiment": self.config.experiment.normandy_slug,
"analysis_basis": analysis_basis,
"analysis_period": window_period.value,
},
)
raise
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 ""
Expand Down Expand Up @@ -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([])
Expand Down Expand Up @@ -707,6 +712,7 @@ def subset_metric_table(
"metric": summary.metric.name,
"analysis_basis": analysis_basis,
"segment": segment,
"analysis_period": period.value,
},
)
return None
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -1387,6 +1394,7 @@ def run(
extra={
"experiment": self.config.experiment.normandy_slug,
"analysis_basis": analysis_basis.value,
"analysis_period": period.value,
},
)
continue
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions jetstream/logging/bigquery_log_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions jetstream/statistics.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ def apply(
"statistic": self.name(),
"analysis_basis": analysis_basis.value,
"segment": segment,
"analysis_period": self.period.value if self.period else None,
},
)

Expand Down
2 changes: 2 additions & 0 deletions jetstream/tests/integration/test_analysis_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -1186,13 +1186,15 @@ 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("analysis_period") == "week"

assert error_logs[5].get("log_level") == "ERROR"
assert error_logs[5].get("experiment") == "test-experiment"
assert error_logs[5].get("metric") == "active_hours"
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("analysis_period") == "week"

# wait for profiling results to land in BigQuery
# todo: improve this test as it might lead to flakiness
Expand Down
3 changes: 3 additions & 0 deletions jetstream/tests/integration/test_logging_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
36 changes: 36 additions & 0 deletions jetstream/tests/test_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
EnrollmentNotCompleteException,
ExplicitSkipException,
HighPopulationException,
StatisticComputationException,
UnsupportedApplicationException,
)
from jetstream.metric import Metric
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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.value
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
Expand Down
Loading