diff --git a/conftest.py b/conftest.py index 4bce98bd8..abfd10a65 100644 --- a/conftest.py +++ b/conftest.py @@ -298,3 +298,29 @@ def mock_submit_fn(metric, start, end): "submit_subflow", mock_submit, ) + + +@pytest.fixture(autouse=True) +def mock_metric_context(mocker, request): + if request.node.get_closest_marker("real_metric_context"): + return + + from helpers.telemetry import MetricContext + + def populate(self): + self.populated = True + + return mocker.patch.object(MetricContext, "populate", populate) + + +@pytest.fixture(autouse=True) +def mock_feature(mocker, request): + if request.node.get_closest_marker("real_feature"): + return + + from shared.rollouts import Feature + + def check_value(self, *, owner_id=None, repo_id=None, default=False): + return default + + return mocker.patch.object(Feature, "check_value", check_value) diff --git a/helpers/telemetry.py b/helpers/telemetry.py index 11fe338b7..5a4e0f276 100644 --- a/helpers/telemetry.py +++ b/helpers/telemetry.py @@ -142,7 +142,7 @@ def log_simple_metric(self, name: str, value: float): @fire_and_forget async def attempt_log_simple_metric(self, name: str, value: float): - self.log_simple_metric(name, value) + await self.log_simple_metric_async(name, value) class TimeseriesTimer: diff --git a/helpers/tests/unit/test_telemetry.py b/helpers/tests/unit/test_telemetry.py index 755309510..1a86e47ff 100644 --- a/helpers/tests/unit/test_telemetry.py +++ b/helpers/tests/unit/test_telemetry.py @@ -47,6 +47,7 @@ def make_metric_context(dbsession): @pytest.mark.asyncio +@pytest.mark.real_metric_context async def test_fire_and_forget(): """ @fire_and_forget wraps an async function in a sync function that schedules @@ -84,6 +85,7 @@ async def fn(): class TestMetricContext: + @pytest.mark.real_metric_context def test_populate_complete(self, dbsession, mocker): mc = make_metric_context(dbsession) assert mc.repo_id is not None @@ -106,6 +108,7 @@ def test_populate_complete(self, dbsession, mocker): ) assert mc.commit_id is not None + @pytest.mark.real_metric_context def test_populate_no_repo(self, dbsession, mocker): mc = make_metric_context(dbsession) mc.repo_id = None @@ -119,6 +122,7 @@ def test_populate_no_repo(self, dbsession, mocker): assert mc.commit_id is None @pytest.mark.django_db(databases={"default", "timeseries"}) + @pytest.mark.real_metric_context def test_log_simple_metric(self, dbsession, mocker): mc = make_metric_context(dbsession) @@ -148,6 +152,7 @@ def test_log_simple_metric(self, dbsession, mocker): """ @pytest.mark.asyncio + @pytest.mark.real_metric_context async def test_attempt_log_simple_metric(self, dbsession, mocker): mc = make_metric_context(dbsession) mock_fn = mocker.patch.object(mc, "log_simple_metric") @@ -161,6 +166,7 @@ async def test_attempt_log_simple_metric(self, dbsession, mocker): class TestTimeseriesTimer: + @pytest.mark.real_metric_context def test_sync(self, dbsession, mocker): mc = Mock() time1 = datetime.fromisoformat("2023-11-21").replace(tzinfo=timezone.utc) @@ -177,6 +183,7 @@ def test_sync(self, dbsession, mocker): assert ("test_sync_timer", expected_value) in mc.log_simple_metric.call_args @pytest.mark.asyncio + @pytest.mark.real_metric_context async def test_async(self, dbsession, mocker): mc = Mock() time1 = datetime.fromisoformat("2023-11-21").replace(tzinfo=timezone.utc) diff --git a/pytest.ini b/pytest.ini index 26fb5f73d..750027976 100644 --- a/pytest.ini +++ b/pytest.ini @@ -4,3 +4,5 @@ addopts = --sqlalchemy-connect-url="postgresql://postgres@postgres:5432/backgrou markers= integration: integration tests (includes tests with vcrs) real_checkpoint_logger: prevents use of stubbed CheckpointLogger + real_metric_context: prevents use of stubbed MetricContext + real_feature: prevents use of stubbed Feature diff --git a/services/report/__init__.py b/services/report/__init__.py index c9908a14d..80eb0ac94 100644 --- a/services/report/__init__.py +++ b/services/report/__init__.py @@ -767,7 +767,7 @@ async def create_new_report_for_commit(self, commit: Commit) -> Report: "Could not find parent for possible carryforward", extra=dict(commit=commit.commitid, repoid=commit.repoid), ) - metric_context.attempt_log_simple_metric( + await metric_context.log_simple_metric_async( "worker_service_report_carryforward_base_not_found", 1 ) return Report() @@ -823,7 +823,7 @@ async def create_new_report_for_commit(self, commit: Commit) -> Report: await self._possibly_shift_carryforward_report( carryforward_report, parent_commit, commit ) - metric_context.attempt_log_simple_metric( + await metric_context.log_simple_metric_async( "worker_service_report_carryforward_success", 1 ) return carryforward_report diff --git a/services/tests/test_report.py b/services/tests/test_report.py index 0115047a1..2e6848930 100644 --- a/services/tests/test_report.py +++ b/services/tests/test_report.py @@ -4980,6 +4980,7 @@ def fake_get_compare(base, head): } @pytest.mark.asyncio + @pytest.mark.django_db(databases={"default", "timeseries"}) async def test_create_new_report_for_commit_and_shift( self, dbsession, sample_report, mocker, mock_repo_provider, mock_storage ):