1884215 - pytest_runtest_setup configures markus but gets overridden by libmarkus.setup_metrics

Status: NEW

(Socorro :: Antenna, defect, P2)

Description

[Daniel Thorn [:relud]]

The call to markus.configure in pytest_runtest_setup doesn't set antenna.libmarkus._IS_MARKUS_SETUP, so it is overridden when antenna.libmarkus.setup_metrics is called the first time:

https://github.com/mozilla-services/antenna/blob/0421a3b9d8dac6fe30d545fd54adfca4eb4be6a2/tests/unittest/conftest.py#L28-L31

def pytest_runtest_setup():
    # Make sure we set up logging and metrics to sane default values.
    setup_logging(logging_level="DEBUG", debug=True, host_id="", processname="antenna")
    markus.configure([{"class": "markus.backends.logging.LoggingMetrics"}])

https://github.com/mozilla-services/antenna/blob/0421a3b9d8dac6fe30d545fd54adfca4eb4be6a2/antenna/libmarkus.py#L12-L50

_IS_MARKUS_SETUP = False
    [...]


def setup_metrics(statsd_host, statsd_port, statsd_namespace, debug=False):
    [...]
    global _IS_MARKUS_SETUP
    if _IS_MARKUS_SETUP:
        return
    [...]
    markus.configure(markus_backends)

This has the unwanted side-effect that markus is configured by setup_metrics to write to statsd during tests.

Under certain circumstances the socket markus is using for statsd isn't properly closed, resulting in PytestUnraisableExceptionWarning in random tests, as seen in https://github.com/mozilla-services/antenna/pull/977#discussion_r1514802826

Comment 1

[Daniel Thorn [:relud]]

(In reply to Daniel Thorn [:relud] from comment #0)

Under certain circumstances the socket markus is using for statsd isn't properly closed, resulting in PytestUnraisableExceptionWarning in random tests, as seen in https://github.com/mozilla-services/antenna/pull/977#discussion_r1514802826

if I start from a clean main and run the following commands, the error shows up:

mkdir tests/external
mv tests/unittest/test_fs_crashstorage.py tests/external/
mv tests/unittest/conftest.py tests/
# correct the number of parents to the repo root
sed '20s/.parent//' -i tests/conftest.py
# change testpaths to all of tests/
sed '24s|tests/unittest/|tests/|' -i setup.cfg
pytest -x

Comment 2

[Will Kahn-Greene [:willkg] ET needinfo? me]

Making this a P2 because this is a wart in the Socorro tests which should get fixed so we can continue to evolve the tests.