editoast: ensure spans are created in tests regardless of log level #10526
Conversation
Signed-off-by: hamz2a <[email protected]>
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #10526 +/- ##
==========================================
+ Coverage 81.81% 81.82% +0.01%
==========================================
Files 1073 1073
Lines 106718 106840 +122
Branches 730 730
==========================================
+ Hits 87306 87423 +117
- Misses 19373 19378 +5
Partials 39 39
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
To give more precisions, we evaluated another solution with forcing the directive The rational for choosing this solution was: this is not an application problem, this is a "test" problem since we do not control enough the test environment (configuring |
| @@ -46,7 +46,7 @@ pub struct RunserverArgs { | |||
| /// If this option is set, logging for the STDCM will be enabled. | |||
| /// When enabled, relevant logs will be captured to aid in debugging and monitoring. | |||
| #[clap(long, env = "ENABLE_STDCM_LOGGING", default_value_t = true)] | |||
| enable_stdcm_logging: bool, | |||
| pub(crate) enable_stdcm_logging: bool, | |||
There was a problem hiding this comment.
Why do you need this pub?
There was a problem hiding this comment.
Probably a left over from previous modifications when we experimented different solutions, we don't need that anymore I believe.
| .with_default_directive(log_level.into()); | ||
|
|
||
| let env_filter_layer = if ignore_rust_log { | ||
| env_filter_builder.parse_lossy("") |
There was a problem hiding this comment.
After discussing with @woshilapin, Opening a non-mandatory conversation after comparing RUST_LOG=warn,otel::tracing=info cargo test on dev with RUST_LOG=warn cargo test on this branch: both are working as for tests.
The "current branch case" still logs a lot more on affected tests.
So here are a few ideas (non-blocking, can be ignored because "not worth the time"):
- just add
otel::tracing=infoinstead of ignoring RUST_LOG for tests? - maybe really always force
otel::tracing=info(except explicit, which is probably a pain to parse) - use INFO log level for effected tests (instead of current DEBUG)
There was a problem hiding this comment.
One solution we explored was to always have otel::tracing=info enabled. But this makes it not possible to deactivate through configuration. The only place where we want it activated is for tests where we want to control our test environment. This is something we didn't explore with @hamz2a and I made #10531 as an alternative proposition.
Problem:
When running tests with
RUST_LOG=warnorRUST_LOG=error, theaxum-tracing-opentelemetrycrate doesn't create spans if the log level is belowinfo. This causes tests ofviews::stdcm_logsto fail because there is notrace_id(span) generated.Solution:
To fix this, we modified the
create_tracing_subscriberfunction to ignore theRUST_LOGenvironment variable during tests. This ensures that spans (trace_id) are always created, even when the log level is set towarnorerror, allowing the tests to run correctly.