Skip to content

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.

Sign up for GitHub

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

Open
martint wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Reduce log verbosity in tests #27540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions core/trino-main/src/main/java/io/trino/server/testing/TestingTrinoServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,12 @@ public class TestingTrinoServer
{
static {
Logging logging = Logging.initialize();
logging.setLevel("io.trino.connector.CatalogStoreManager", Level.ERROR);
logging.setLevel("io.trino.server.PluginManager", Level.ERROR);
logging.setLevel("io.trino.memory.RemoteNodeMemory", Level.ERROR);
Comment on lines +161 to +163
Copy link
Member

@findepi findepi Dec 4, 2025

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.

Copy link
Member

@findepi findepi Dec 4, 2025

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?

logging.setLevel("io.trino.event.QueryMonitor", Level.ERROR);
logging.setLevel("org.eclipse.jetty", Level.ERROR);
logging.setLevel("io.airlift.http.server", Level.ERROR);
logging.setLevel("io.airlift.concurrent.BoundedExecutor", Level.OFF);

// Trino server behavior does not depend on locale settings.
Expand Down
8 changes: 8 additions & 0 deletions core/trino-main/src/main/java/io/trino/testing/PlanTester.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.io.Closer;
import io.airlift.configuration.secrets.SecretsResolver;
import io.airlift.log.Level;
import io.airlift.log.Logging;
import io.airlift.node.NodeInfo;
import io.airlift.units.Duration;
import io.opentelemetry.api.trace.Span;
Expand Down Expand Up @@ -276,6 +278,12 @@ public class PlanTester
{
public static final BlockEncodingManager TESTING_BLOCK_ENCODING_MANAGER = new BlockEncodingManager(new BlockEncodingSimdSupport(true));

static {
Logging logging = Logging.initialize();
logging.setLevel("io.trino.bootstrap", Level.WARN);
logging.setLevel("org.hibernate.validator.internal.util.Version", Level.WARN);
}

private final Session defaultSession;
private final ExecutorService notificationExecutor;
private final ScheduledExecutorService yieldExecutor;
Expand Down
13 changes: 12 additions & 1 deletion ...n/src/test/java/io/trino/execution/scheduler/faulttolerant/TestTaskDescriptorStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import io.airlift.jaxrs.JsonMapper;
import io.airlift.json.JsonCodec;
import io.airlift.json.JsonModule;
import io.airlift.log.Level;
import io.airlift.log.Logging;
import io.airlift.units.DataSize;
import io.trino.connector.CatalogHandle;
import io.trino.exchange.SpoolingExchangeInput;
Expand Down Expand Up @@ -53,6 +55,12 @@

public class TestTaskDescriptorStorage
{
static {
Logging logging = Logging.initialize();
logging.setLevel("org.hibernate.validator.internal.util.Version", Level.ERROR);
logging.setLevel("Bootstrap", Level.ERROR);
Copy link
Member

@findepi findepi Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is Bootstrap?

Copy link
Contributor

@wendigo wendigo Dec 4, 2025

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

}

private static final QueryId QUERY_1 = new QueryId("query1");
private static final QueryId QUERY_2 = new QueryId("query2");

Expand Down Expand Up @@ -363,7 +371,10 @@ private static TaskDescriptorStorage createTaskDescriptorStorage(DataSize maxMem
jsonCodecBinder(binder).bindJsonCodec(Split.class);
});

Injector injector = app.initialize();
Injector injector = app
.doNotInitializeLogging()
.quiet()
.initialize();
JsonCodec<TaskDescriptor> taskDescriptorJsonCodec = injector.getInstance(Key.get(new TypeLiteral<>() { }));
JsonCodec<Split> splitJsonCodec = injector.getInstance(Key.get(new TypeLiteral<>() { }));

Expand Down
7 changes: 7 additions & 0 deletions ...rc/test/java/io/trino/server/protocol/spooling/TestPreferredQueryDataEncoderSelector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
package io.trino.server.protocol.spooling;

import io.airlift.log.Level;
import io.airlift.log.Logging;
import io.trino.server.protocol.spooling.QueryDataEncoder.EncoderSelector;
import io.trino.server.protocol.spooling.encoding.JsonQueryDataEncoder;
import org.junit.jupiter.api.Test;
Expand All @@ -24,6 +26,11 @@

class TestPreferredQueryDataEncoderSelector
{
static {
Logging logging = Logging.initialize();
logging.setLevel("io.trino.server.protocol.spooling", Level.ERROR);
}

@Test
public void testNoEncoderWhenNoneIsMatching()
{
Expand Down
7 changes: 7 additions & 0 deletions ...ava/io/trino/plugin/exchange/filesystem/alluxio/TestAlluxioFileSystemExchangeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
package io.trino.plugin.exchange.filesystem.alluxio;

import io.airlift.log.Level;
import io.airlift.log.Logging;
import io.trino.plugin.exchange.filesystem.AbstractTestExchangeManager;
import io.trino.plugin.exchange.filesystem.FileSystemExchangeManagerFactory;
import io.trino.plugin.exchange.filesystem.TestExchangeManagerContext;
Expand All @@ -25,6 +27,11 @@
public class TestAlluxioFileSystemExchangeManager
extends AbstractTestExchangeManager
{
static {
Logging logging = Logging.initialize();
logging.setLevel("io.trino.bootstrap.exchange", Level.ERROR);
}

private Alluxio alluxio;

@Override
Expand Down
7 changes: 7 additions & 0 deletions ...st/java/io/trino/plugin/exchange/filesystem/local/TestLocalFileSystemExchangeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package io.trino.plugin.exchange.filesystem.local;

import com.google.common.collect.ImmutableMap;
import io.airlift.log.Level;
import io.airlift.log.Logging;
import io.trino.plugin.exchange.filesystem.AbstractTestExchangeManager;
import io.trino.plugin.exchange.filesystem.FileSystemExchangeManagerFactory;
import io.trino.plugin.exchange.filesystem.TestExchangeManagerContext;
Expand All @@ -22,6 +24,11 @@
public class TestLocalFileSystemExchangeManager
extends AbstractTestExchangeManager
{
static {
Logging logging = Logging.initialize();
logging.setLevel("io.trino.bootstrap.exchange", Level.ERROR);
}

@Override
protected ExchangeManager createExchangeManager()
{
Expand Down
7 changes: 7 additions & 0 deletions ...src/test/java/io/trino/plugin/exchange/filesystem/s3/TestS3FileSystemExchangeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
package io.trino.plugin.exchange.filesystem.s3;

import io.airlift.log.Level;
import io.airlift.log.Logging;
import io.trino.plugin.exchange.filesystem.AbstractTestExchangeManager;
import io.trino.plugin.exchange.filesystem.FileSystemExchangeManagerFactory;
import io.trino.plugin.exchange.filesystem.TestExchangeManagerContext;
Expand All @@ -26,6 +28,11 @@
public class TestS3FileSystemExchangeManager
extends AbstractTestExchangeManager
{
static {
Logging logging = Logging.initialize();
logging.setLevel("io.trino.bootstrap.exchange", Level.ERROR);
Copy link
Member

@findepi findepi Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logging.setLevel("io.trino.bootstrap.exchange", Level.ERROR);
// Airlift configuration logs all the properties
logging.setLevel("io.trino.bootstrap.exchange.filesystem", Level.WARN);

Copy link
Member

@findepi findepi Dec 4, 2025

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.

Copy link
Member

@findepi findepi Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see

}

private MinioStorage minioStorage;

@Override
Expand Down
7 changes: 7 additions & 0 deletions ...st/java/io/trino/plugin/exchange/filesystem/s3/TestS3FileSystemExchangeManagerSseKms.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
package io.trino.plugin.exchange.filesystem.s3;

import io.airlift.log.Level;
import io.airlift.log.Logging;
import io.trino.plugin.exchange.filesystem.AbstractTestExchangeManager;
import io.trino.plugin.exchange.filesystem.FileSystemExchangeManagerFactory;
import io.trino.plugin.exchange.filesystem.TestExchangeManagerContext;
Expand All @@ -27,6 +29,11 @@
public class TestS3FileSystemExchangeManagerSseKms
extends AbstractTestExchangeManager
{
static {
Logging logging = Logging.initialize();
logging.setLevel("io.trino.bootstrap.exchange", Level.ERROR);
}

private MinioStorage minioStorage;

@Override
Expand Down
7 changes: 7 additions & 0 deletions ...est/java/io/trino/plugin/exchange/filesystem/s3/TestS3FileSystemExchangeManagerSseS3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
package io.trino.plugin.exchange.filesystem.s3;

import io.airlift.log.Level;
import io.airlift.log.Logging;
import io.trino.plugin.exchange.filesystem.AbstractTestExchangeManager;
import io.trino.plugin.exchange.filesystem.FileSystemExchangeManagerFactory;
import io.trino.plugin.exchange.filesystem.TestExchangeManagerContext;
Expand All @@ -27,6 +29,11 @@
public class TestS3FileSystemExchangeManagerSseS3
extends AbstractTestExchangeManager
{
static {
Logging logging = Logging.initialize();
logging.setLevel("io.trino.bootstrap.exchange", Level.ERROR);
}

private MinioStorage minioStorage;

@Override
Expand Down
Loading