metrics-rs / metrics

A metrics ecosystem for Rust.
MIT License
1.1k stars 151 forks source link

what's the recommended way to run concurrent tests? #368

Closed horacimacias closed 1 year ago

horacimacias commented 1 year ago

I'm developing web application that uses metrics, and I have several tests where requests are made to various endpoints.

I've separated the tests into various functions and I'm running into trouble unless I run only 1 test at a time and use something like this in my test:

        unsafe {
            metrics::clear_recorder();
        }

while this works, it is a bit of a pain as I also need to make sure test-threads is set to 1 which I do by using the following .cargo/config:

[env]
RUST_TEST_THREADS = "1"

Is there any better way of doing this? For the test themselves I don't rely/use the metrics but if possible I'd like to avoid using something like #[cfg(test)] to not setup the metrics. I want to make sure the code I'm testing is the closest possible to the real application.

tobz commented 1 year ago

👋🏻

For the overwhelming majority of applications, initializing telemetry subsystems (metrics, logging, etc) is a one-time thing done at startup. Providing methods in metrics that silently ignores the fact that a global recorder is already initialized/installed would be a footgun, and would make metrics less misuse-resistant.

If you want to test the metrics subsystem in a more non-test/production fashion, do that with an integration test. Otherwise, don't initialize your non-test resources during tests. If you're dead set on initializing the metrics subsystem in the same way whether you're under test or not, then you're stuck with something like #[cfg(test)] to either not run any of the initialization logic, or to ignore any error returned by metrics::set_boxed_recorder.

horacimacias commented 1 year ago

thanks. I didn't realise what I'm trying to do is more suited for integration test, not unit test. Using integration tests instead, I don't need to clear the recorder or pass any special flag to cargo test. Thanks!