Skip to content

Commit e97d81a

Browse files
asottile-sentryandrewshie-sentry
authored andcommitted
ref: fix usage_accountant-related test pollution (#88113)
previously failing with: ```console $ pytest tests/sentry/ingest/ingest_consumer/test_ingest_consumer_processing.py::test_accountant_transaction tests/sentry/usage_accountant/test_accountant.py::test_accountant ============================= test session starts ============================== platform darwin -- Python 3.13.1, pytest-8.1.2, pluggy-1.5.0 django: version: 5.1.7 rootdir: /Users/asottile/workspace/sentry configfile: pyproject.toml plugins: fail-slow-0.3.0, time-machine-2.16.0, json-report-1.5.0, metadata-3.1.1, xdist-3.0.2, django-4.9.0, pytest_sentry-0.3.0, anyio-3.7.1, rerunfailures-15.0, cov-4.0.0 collected 2 items tests/sentry/ingest/ingest_consumer/test_ingest_consumer_processing.py . [ 50%] [ 50%] tests/sentry/usage_accountant/test_accountant.py F [100%] =================================== FAILURES =================================== _______________________________ test_accountant ________________________________ tests/sentry/usage_accountant/test_accountant.py:47: in test_accountant accountant.init_backend(producer) src/sentry/usage_accountant/accountant.py:33: in init_backend assert _accountant_backend is None, "Accountant already initialized once." E AssertionError: Accountant already initialized once. =========================== short test summary info ============================ FAILED tests/sentry/usage_accountant/test_accountant.py::test_accountant - AssertionError: Accountant already initialized once. ========================= 1 failed, 1 passed in 5.12s ========================== ``` <!-- Describe your PR here. -->
1 parent ca893b8 commit e97d81a

File tree

6 files changed

+84
-86
lines changed

6 files changed

+84
-86
lines changed

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,7 @@ module = [
406406
"sentry.tempest.endpoints.*",
407407
"sentry.tempest.migrations.*",
408408
"sentry.testutils.helpers.task_runner",
409+
"sentry.testutils.helpers.usage_accountant",
409410
"sentry.testutils.pytest.json_report_reruns",
410411
"sentry.testutils.pytest.show_flaky_failures",
411412
"sentry.testutils.skips",
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
from __future__ import annotations
2+
3+
import contextlib
4+
from collections.abc import Generator
5+
6+
from arroyo.backends.abstract import Producer
7+
from arroyo.backends.kafka import KafkaPayload
8+
from usageaccountant import UsageAccumulator
9+
10+
from sentry.usage_accountant import accountant
11+
12+
13+
@contextlib.contextmanager
14+
def usage_accountant_backend(producer: Producer[KafkaPayload]) -> Generator[None]:
15+
assert accountant._accountant_backend is None, accountant._accountant_backend
16+
accountant._accountant_backend = UsageAccumulator(producer=producer)
17+
try:
18+
yield
19+
finally:
20+
accountant._shutdown()
21+
accountant._accountant_backend = None

src/sentry/usage_accountant/accountant.py

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@
1010
import atexit
1111
import logging
1212

13-
from arroyo.backends.abstract import Producer
14-
from arroyo.backends.kafka import KafkaPayload, KafkaProducer, build_kafka_configuration
13+
from arroyo.backends.kafka import KafkaProducer, build_kafka_configuration
1514
from usageaccountant import UsageAccumulator, UsageUnit
1615

1716
from sentry.conf.types.kafka_definition import Topic
@@ -23,29 +22,7 @@
2322
_accountant_backend: UsageAccumulator | None = None
2423

2524

26-
def init_backend(producer: Producer[KafkaPayload]) -> None:
27-
"""
28-
This method should be used externally only in tests to fit a
29-
mock producer.
30-
"""
31-
global _accountant_backend
32-
33-
assert _accountant_backend is None, "Accountant already initialized once."
34-
_accountant_backend = UsageAccumulator(producer=producer)
35-
atexit.register(_shutdown)
36-
37-
38-
def reset_backend() -> None:
39-
"""
40-
This method should be used externally only in tests to reset
41-
the accountant backend.
42-
"""
43-
global _accountant_backend
44-
_accountant_backend = None
45-
46-
4725
def _shutdown() -> None:
48-
global _accountant_backend
4926
if _accountant_backend is not None:
5027
_accountant_backend.flush()
5128
_accountant_backend.close()

tests/sentry/event_manager/test_event_manager.py

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@
6565
from sentry.models.release import Release
6666
from sentry.models.releasecommit import ReleaseCommit
6767
from sentry.models.releaseprojectenvironment import ReleaseProjectEnvironment
68-
from sentry.options import set
6968
from sentry.projectoptions.defaults import DEFAULT_GROUPING_CONFIG
7069
from sentry.spans.grouping.utils import hash_values
7170
from sentry.testutils.asserts import assert_mock_called_once_with_partial
@@ -77,13 +76,13 @@
7776
)
7877
from sentry.testutils.helpers import apply_feature_flag_on_cls, override_options
7978
from sentry.testutils.helpers.datetime import before_now, freeze_time
79+
from sentry.testutils.helpers.usage_accountant import usage_accountant_backend
8080
from sentry.testutils.performance_issues.event_generators import get_event
8181
from sentry.testutils.pytest.fixtures import django_db_all
8282
from sentry.testutils.skips import requires_snuba
8383
from sentry.tsdb.base import TSDBModel
8484
from sentry.types.activity import ActivityType
8585
from sentry.types.group import PriorityLevel
86-
from sentry.usage_accountant import accountant
8786
from sentry.utils import json
8887
from sentry.utils.cache import cache_key_for_event
8988
from sentry.utils.eventuser import EventUser
@@ -3517,21 +3516,21 @@ def test_cogs_event_manager(
35173516
broker.create_topic(topic, 1)
35183517
producer = broker.get_producer()
35193518

3520-
set("shared_resources_accounting_enabled", [settings.COGS_EVENT_STORE_LABEL])
3521-
3522-
accountant.init_backend(producer)
3523-
3524-
raw_event_params = make_event(**event_data)
3519+
with (
3520+
override_options(
3521+
{"shared_resources_accounting_enabled": [settings.COGS_EVENT_STORE_LABEL]}
3522+
),
3523+
usage_accountant_backend(producer),
3524+
):
3525+
raw_event_params = make_event(**event_data)
35253526

3526-
manager = EventManager(raw_event_params)
3527-
manager.normalize()
3528-
normalized_data = dict(manager.get_data())
3529-
_ = manager.save(default_project)
3527+
manager = EventManager(raw_event_params)
3528+
manager.normalize()
3529+
normalized_data = dict(manager.get_data())
3530+
_ = manager.save(default_project)
35303531

3531-
expected_len = len(json.dumps(normalized_data))
3532+
expected_len = len(json.dumps(normalized_data))
35323533

3533-
accountant._shutdown()
3534-
accountant.reset_backend()
35353534
msg1 = broker.consume(Partition(topic, 0), 0)
35363535
assert msg1 is not None
35373536
payload = msg1.payload

tests/sentry/ingest/ingest_consumer/test_ingest_consumer_processing.py

Lines changed: 38 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,11 @@
2929
from sentry.models.debugfile import create_files_from_dif_zip
3030
from sentry.models.eventattachment import EventAttachment
3131
from sentry.models.userreport import UserReport
32-
from sentry.options import set
3332
from sentry.testutils.helpers.features import Feature
33+
from sentry.testutils.helpers.options import override_options
34+
from sentry.testutils.helpers.usage_accountant import usage_accountant_backend
3435
from sentry.testutils.pytest.fixtures import django_db_all
3536
from sentry.testutils.skips import requires_snuba, requires_symbolicator
36-
from sentry.usage_accountant import accountant
3737
from sentry.utils.eventuser import EventUser
3838
from sentry.utils.json import loads
3939

@@ -169,42 +169,43 @@ def test_accountant_transaction(default_project):
169169
broker.create_topic(topic, 1)
170170
producer = broker.get_producer()
171171

172-
set("shared_resources_accounting_enabled", [settings.EVENT_PROCESSING_STORE])
173-
174-
accountant.init_backend(producer)
175-
176-
now = datetime.datetime.now()
177-
event = {
178-
"type": "transaction",
179-
"timestamp": now.isoformat(),
180-
"start_timestamp": now.isoformat(),
181-
"spans": [],
182-
"contexts": {
183-
"trace": {
184-
"parent_span_id": "8988cec7cc0779c1",
185-
"type": "trace",
186-
"op": "foobar",
187-
"trace_id": "a7d67cf796774551a95be6543cacd459",
188-
"span_id": "babaae0d4b7512d9",
189-
"status": "ok",
190-
}
191-
},
192-
}
193-
payload = get_normalized_event(event, default_project)
194-
serialized = orjson.dumps(payload).decode()
195-
process_event(
196-
ConsumerType.Events,
197-
{
198-
"payload": serialized,
199-
"start_time": time.time() - 3600,
200-
"event_id": payload["event_id"],
201-
"project_id": default_project.id,
202-
"remote_addr": "127.0.0.1",
203-
},
204-
project=default_project,
205-
)
172+
with (
173+
override_options(
174+
{"shared_resources_accounting_enabled": [settings.EVENT_PROCESSING_STORE]}
175+
),
176+
usage_accountant_backend(producer),
177+
):
178+
now = datetime.datetime.now()
179+
event = {
180+
"type": "transaction",
181+
"timestamp": now.isoformat(),
182+
"start_timestamp": now.isoformat(),
183+
"spans": [],
184+
"contexts": {
185+
"trace": {
186+
"parent_span_id": "8988cec7cc0779c1",
187+
"type": "trace",
188+
"op": "foobar",
189+
"trace_id": "a7d67cf796774551a95be6543cacd459",
190+
"span_id": "babaae0d4b7512d9",
191+
"status": "ok",
192+
}
193+
},
194+
}
195+
payload = get_normalized_event(event, default_project)
196+
serialized = orjson.dumps(payload).decode()
197+
process_event(
198+
ConsumerType.Events,
199+
{
200+
"payload": serialized,
201+
"start_time": time.time() - 3600,
202+
"event_id": payload["event_id"],
203+
"project_id": default_project.id,
204+
"remote_addr": "127.0.0.1",
205+
},
206+
project=default_project,
207+
)
206208

207-
accountant._shutdown()
208209
msg1 = broker.consume(Partition(topic, 0), 0)
209210
assert msg1 is not None
210211
payload = msg1.payload

tests/sentry/usage_accountant/test_accountant.py

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@
66
from arroyo.types import BrokerValue, Partition, Topic
77
from usageaccountant import UsageUnit
88

9-
from sentry.options import set
9+
from sentry.testutils.helpers.options import override_options
10+
from sentry.testutils.helpers.usage_accountant import usage_accountant_backend
1011
from sentry.testutils.pytest.fixtures import django_db_all
11-
from sentry.usage_accountant import accountant, record
12+
from sentry.usage_accountant import record
1213
from sentry.utils.json import loads
1314

1415

@@ -42,15 +43,13 @@ def test_accountant(mock_time: mock.Mock) -> None:
4243
broker.create_topic(topic, 1)
4344
producer = broker.get_producer()
4445

45-
set("shared_resources_accounting_enabled", ["resource_1"])
46-
47-
accountant.init_backend(producer)
48-
49-
mock_time.return_value = 1594839910.1
50-
record("resource_1", "feature_1", 100, UsageUnit.BYTES)
51-
record("resource_1", "feature_2", 100, UsageUnit.BYTES)
52-
53-
accountant._shutdown()
46+
with (
47+
override_options({"shared_resources_accounting_enabled": ["resource_1"]}),
48+
usage_accountant_backend(producer),
49+
):
50+
mock_time.return_value = 1594839910.1
51+
record("resource_1", "feature_1", 100, UsageUnit.BYTES)
52+
record("resource_1", "feature_2", 100, UsageUnit.BYTES)
5453

5554
msg1 = broker.consume(Partition(topic, 0), 0)
5655
assert_msg(

0 commit comments

Comments
 (0)