-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Reduce log verbosity in tests #27540
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
base: master
Are you sure you want to change the base?
Conversation
| logging.setLevel("io.trino.connector.CatalogStoreManager", Level.ERROR); | ||
| logging.setLevel("io.trino.server.PluginManager", Level.ERROR); | ||
| logging.setLevel("io.trino.memory.RemoteNodeMemory", Level.ERROR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory we could put ERROR as the default level, but that may make reproducing errors harder. Also, it opens a possibility for the code to work in tests, but not work under defaults settings (yes, i know no code semantics should depend on log levels being enabled).
So for every such special measure we should understand -- and document -- why we're doing that. Please add a line of comment for each of these perhaps exemplifying what's too verbose (at the moment of writing).
BTW sometimes we can fix the problem at source. For instance, airlift configuration logs all configured values, but afaict it's disabled in tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, why not INFO -> WARN, when INFO is too verbose for tests?
| static { | ||
| Logging logging = Logging.initialize(); | ||
| logging.setLevel("org.hibernate.validator.internal.util.Version", Level.ERROR); | ||
| logging.setLevel("Bootstrap", Level.ERROR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is Bootstrap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default logger used for unnamed io.airlift.bootstrap.Bootstrap instances
...n/src/test/java/io/trino/server/protocol/spooling/TestPreferredQueryDataEncoderSelector.java
Show resolved
Hide resolved
| { | ||
| static { | ||
| Logging logging = Logging.initialize(); | ||
| logging.setLevel("io.trino.bootstrap.exchange", Level.ERROR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| logging.setLevel("io.trino.bootstrap.exchange", Level.ERROR); | |
| // Airlift configuration logs all the properties | |
| logging.setLevel("io.trino.bootstrap.exchange.filesystem", Level.WARN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be much more robust to have io.airlift.bootstrap.Bootstrap#quiet initialize from env.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.