Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a mechanism to limit default behaviour of capturing all logging #314

Closed
josephw opened this issue Oct 28, 2022 · 4 comments · Fixed by #324
Closed

Provide a mechanism to limit default behaviour of capturing all logging #314

josephw opened this issue Oct 28, 2022 · 4 comments · Fixed by #324
Milestone

Comments

@josephw
Copy link

josephw commented Oct 28, 2022

I'd like some way to prevent existing test cases that generate huge amounts of logging from having performance impact.

I added slf4j-test to a large multi-module build with existing tests, successfully. Later, I noticed one test had become dramatically slower. On investigation, a third-party component was logging a huge volume at DEBUG: hundreds of thousands, repeated across tens of tests. Storing that was impacting the runtime measurably, adding minutes to that module's build. As a workaround, I identified the specific loggers (several) and added code to disable them individually using setEnabledLevelsForAllThreads().

I'd like a more general mechanism to prevent this. If possible, one that's opt-out, rather than opt-in, to avoid needing to change existing tests.

Some possible ways to implement this, with varying degrees of impact on existing tests:

  • default to capturing INFO and above
  • have a default limit to the number of events that will be retained
  • allow setting an enable.level through slf4jtest.properties
  • print a noisy alert of loggers with unexpectedly high volume
  • a more general utility method to disable logging for a whole hierarchy of loggers
  • a single utility method to disable all logging, and require subsequent tests to opt back in

The workaround is easy. Just a case where having this handled would have saved me some time.

@youribonnaffe
Copy link

I have a similar issue where using slf4j-test is making a ArchUnit test execution 10x slower (from ~1s to ~10s). ArchUnit is producing a lot of traces when it is scanning classes.

Having a global setting to decide which levels are captured would solve the issue. Is it something that can be considered? I'll be happy to contribute some code around it.

@valfirst
Copy link
Owner

valfirst commented Dec 6, 2022

Having a global setting to decide which levels are captured would solve the issue. Is it something that can be considered? I'll be happy to contribute some code around it.

yes, I think such option might be useful, system properties or properties file could be used for configuration. And contributions are highly welcome 😄

@josephw
Copy link
Author

josephw commented Dec 6, 2022

making a ArchUnit test execution 10x

I should add that the third-party component causing the problem in my case was also ArchUnit 😄.

youribonnaffe added a commit to youribonnaffe/slf4j-test that referenced this issue Dec 10, 2022
Introduce a new global setting `capture.level` to disable storing (and
printing) logs at a given level (following the level hierarchy).

The implementation is very similar to the `print.level` global setting.

This is useful when tests are generating a lot of logging events that
are not of interest and don't need to be captured. ArchUnit tests are
known to be such tests and can be quite slow when slf4j-test is used.
youribonnaffe added a commit to youribonnaffe/slf4j-test that referenced this issue Dec 10, 2022
Introduce a new global setting `capture.level` to disable storing (and
printing) logs at a given level (following the level hierarchy).

The implementation is very similar to the `print.level` global setting.

This is useful when tests are generating a lot of logging events that
are not of interest and don't need to be captured. ArchUnit tests are
known to be such tests and can be quite slow when slf4j-test is used.
@youribonnaffe
Copy link

Here is an attempt for this: #324
I mostly took inspiration from the existing print level setting.

@valfirst valfirst added this to the 2.7.0 milestone Dec 11, 2022
@valfirst valfirst linked a pull request Dec 11, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants