diff --git a/core/trino-main/pom.xml b/core/trino-main/pom.xml index 5cda66286072..03191c775313 100644 --- a/core/trino-main/pom.xml +++ b/core/trino-main/pom.xml @@ -212,6 +212,11 @@ jsr305 + + com.google.errorprone + error_prone_annotations + + com.google.guava guava diff --git a/core/trino-main/src/main/java/io/trino/operator/Driver.java b/core/trino-main/src/main/java/io/trino/operator/Driver.java index 155c9792a7d3..44c6fee62960 100644 --- a/core/trino-main/src/main/java/io/trino/operator/Driver.java +++ b/core/trino-main/src/main/java/io/trino/operator/Driver.java @@ -20,6 +20,7 @@ import com.google.common.collect.Sets; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.log.Logger; import io.airlift.units.Duration; import io.trino.execution.ScheduledSplit; @@ -609,6 +610,7 @@ private Optional> getBlockedFuture(Operator operator) return Optional.empty(); } + @FormatMethod private static Throwable addSuppressedException(Throwable inFlightException, Throwable newException, String message, Object... args) { if (newException instanceof Error) { diff --git a/core/trino-main/src/main/java/io/trino/operator/WorkProcessorPipelineSourceOperator.java b/core/trino-main/src/main/java/io/trino/operator/WorkProcessorPipelineSourceOperator.java index e5e8db34e2f5..98522e50deb9 100644 --- a/core/trino-main/src/main/java/io/trino/operator/WorkProcessorPipelineSourceOperator.java +++ b/core/trino-main/src/main/java/io/trino/operator/WorkProcessorPipelineSourceOperator.java @@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.log.Logger; import io.airlift.units.DataSize; import io.airlift.units.Duration; @@ -555,6 +556,7 @@ private void closeOperators(int lastOperatorIndex) } } + @FormatMethod private static Throwable handleOperatorCloseError(Throwable inFlightException, Throwable newException, String message, Object... args) { if (newException instanceof Error) { diff --git a/core/trino-main/src/main/java/io/trino/server/remotetask/RequestErrorTracker.java b/core/trino-main/src/main/java/io/trino/server/remotetask/RequestErrorTracker.java index c74cd3872494..5f49981c4e61 100644 --- a/core/trino-main/src/main/java/io/trino/server/remotetask/RequestErrorTracker.java +++ b/core/trino-main/src/main/java/io/trino/server/remotetask/RequestErrorTracker.java @@ -16,6 +16,7 @@ import com.google.common.collect.ObjectArrays; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.event.client.ServiceUnavailableException; import io.airlift.log.Logger; import io.airlift.units.Duration; @@ -110,10 +111,10 @@ public void requestFailed(Throwable reason) // log failure message if (isExpectedError(reason)) { // don't print a stack for a known errors - log.warn("Error " + jobDescription + " %s: %s: %s", taskId, reason.getMessage(), taskUri); + log.warn("Error %s %s: %s: %s", jobDescription, taskId, reason.getMessage(), taskUri); } else { - log.warn(reason, "Error " + jobDescription + " %s: %s", taskId, taskUri); + log.warn(reason, "Error %s %s: %s", jobDescription, taskId, taskUri); } // remember the first 10 errors @@ -138,6 +139,8 @@ public void requestFailed(Throwable reason) } } + @FormatMethod + @SuppressWarnings("FormatStringAnnotation") // we manipulate the format string and there's no way to make Error Prone accept the result static void logError(Throwable t, String format, Object... args) { if (isExpectedError(t)) { diff --git a/core/trino-main/src/main/java/io/trino/server/ui/OAuth2WebUiAuthenticationFilter.java b/core/trino-main/src/main/java/io/trino/server/ui/OAuth2WebUiAuthenticationFilter.java index bfa6692652a0..e259e4db9642 100644 --- a/core/trino-main/src/main/java/io/trino/server/ui/OAuth2WebUiAuthenticationFilter.java +++ b/core/trino-main/src/main/java/io/trino/server/ui/OAuth2WebUiAuthenticationFilter.java @@ -119,7 +119,7 @@ private Optional> getAccessToken(ContainerRequestContext req return service.convertTokenToClaims(accessToken.get()); } catch (JwtException | IllegalArgumentException e) { - LOG.debug("Unable to parse JWT token: " + e.getMessage(), e); + LOG.debug(e, "Unable to parse JWT token"); } } return Optional.empty(); diff --git a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloClient.java b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloClient.java index 097bab051aee..5de8d5aa7410 100644 --- a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloClient.java +++ b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloClient.java @@ -364,7 +364,7 @@ private void setLocalityGroups(Map tableProperties, AccumuloTabl } Map> localityGroups = localityGroupsBuilder.build(); - LOG.debug("Setting locality groups: {}", localityGroups); + LOG.debug("Setting locality groups: %s", localityGroups); tableManager.setLocalityGroups(table.getFullTableName(), localityGroups); } @@ -851,7 +851,7 @@ private Optional getTabletLocation(String table, Key key) // Swallow this exception so the query does not fail due to being unable // to locate the tablet server for the provided Key. // This is purely an optimization, but we will want to log the error. - LOG.error("Failed to get tablet location, returning dummy location", e); + LOG.error(e, "Failed to get tablet location, returning dummy location"); return Optional.empty(); } } @@ -882,7 +882,7 @@ private Optional getDefaultTabletLocation(String fulltable) catch (Exception e) { // Swallow this exception so the query does not fail due to being unable to locate the tablet server for the default tablet. // This is purely an optimization, but we will want to log the error. - LOG.error("Failed to get tablet location, returning dummy location", e); + LOG.error(e, "Failed to get tablet location, returning dummy location"); return Optional.empty(); } } diff --git a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/index/IndexLookup.java b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/index/IndexLookup.java index 75e0b271d384..3dc25663a53c 100644 --- a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/index/IndexLookup.java +++ b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/index/IndexLookup.java @@ -270,7 +270,7 @@ private boolean getRangesWithMetrics( // of rows long numEntries = indexRanges.size(); double ratio = (double) numEntries / (double) numRows; - LOG.debug("Use of index would scan %d of %d rows, ratio %s. Threshold %2f, Using for table? %b", numEntries, numRows, ratio, threshold, ratio < threshold, table); + LOG.debug("Use of index would scan %d of %d rows, ratio %s. Threshold %2f, Using for table %s? %b", numEntries, numRows, ratio, threshold, table, ratio < threshold); // If the percentage of scanned rows, the ratio, less than the configured threshold if (ratio < threshold) { diff --git a/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/AccumuloQueryRunner.java b/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/AccumuloQueryRunner.java index 4e30a63e6abd..5876a3f94646 100644 --- a/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/AccumuloQueryRunner.java +++ b/plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/AccumuloQueryRunner.java @@ -128,8 +128,7 @@ private static void copyTable( break; } - LOG.info("Running import for %s", target, sql); - LOG.info("%s", sql); + LOG.info("Running import for %s%n%s", target, sql); long start = System.nanoTime(); long rows = queryRunner.execute(session, sql).getUpdateCount().getAsLong(); LOG.info("Imported %s rows for %s in %s", rows, target, nanosSince(start)); diff --git a/plugin/trino-elasticsearch/src/main/java/io/trino/plugin/elasticsearch/ScanQueryPageSource.java b/plugin/trino-elasticsearch/src/main/java/io/trino/plugin/elasticsearch/ScanQueryPageSource.java index 7e2db7d66d76..94701c48ecd0 100644 --- a/plugin/trino-elasticsearch/src/main/java/io/trino/plugin/elasticsearch/ScanQueryPageSource.java +++ b/plugin/trino-elasticsearch/src/main/java/io/trino/plugin/elasticsearch/ScanQueryPageSource.java @@ -316,7 +316,7 @@ public void close() } catch (Exception e) { // ignore - LOG.debug("Error clearing scroll", e); + LOG.debug(e, "Error clearing scroll"); } } } diff --git a/plugin/trino-hive/pom.xml b/plugin/trino-hive/pom.xml index 3effb846fdd7..6bd2a03fa33d 100644 --- a/plugin/trino-hive/pom.xml +++ b/plugin/trino-hive/pom.xml @@ -169,6 +169,11 @@ true + + com.google.errorprone + error_prone_annotations + + com.google.guava guava diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java index e4e1d695095d..c57ea719b7ff 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/SemiTransactionalHiveMetastore.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import com.google.errorprone.annotations.FormatMethod; import io.airlift.log.Logger; import io.airlift.units.Duration; import io.trino.plugin.hive.HdfsEnvironment; @@ -2308,6 +2309,7 @@ private static boolean isSameOrParent(Path parent, Path child) return parent.equals(child); } + @FormatMethod private void logCleanupFailure(String format, Object... args) { if (throwOnCleanupFailure) { @@ -2316,6 +2318,7 @@ private void logCleanupFailure(String format, Object... args) log.warn(format, args); } + @FormatMethod private void logCleanupFailure(Throwable t, String format, Object... args) { if (throwOnCleanupFailure) { diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/parquet/write/TestDataWritableWriter.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/parquet/write/TestDataWritableWriter.java index e9781702d216..0cc21d07192e 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/parquet/write/TestDataWritableWriter.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/parquet/write/TestDataWritableWriter.java @@ -84,7 +84,7 @@ public void write(ParquetHiveRecord record) } catch (RuntimeException e) { String errorMessage = "Parquet record is malformed: " + e.getMessage(); - log.error(errorMessage, e); + log.error(e, errorMessage); throw new RuntimeException(errorMessage, e); } recordConsumer.endMessage(); diff --git a/plugin/trino-kafka/src/test/java/io/trino/plugin/kafka/KafkaQueryRunner.java b/plugin/trino-kafka/src/test/java/io/trino/plugin/kafka/KafkaQueryRunner.java index 7f6de17dc065..b77fce96b783 100644 --- a/plugin/trino-kafka/src/test/java/io/trino/plugin/kafka/KafkaQueryRunner.java +++ b/plugin/trino-kafka/src/test/java/io/trino/plugin/kafka/KafkaQueryRunner.java @@ -145,7 +145,7 @@ public void postInit(DistributedQueryRunner queryRunner) long start = System.nanoTime(); log.info("Running import for %s", table.getTableName()); queryRunner.execute(format("INSERT INTO %1$s SELECT * FROM tpch.tiny.%1$s", table.getTableName())); - log.info("Imported %s in %s", 0, table.getTableName(), nanosSince(start).convertToMostSuccinctTimeUnit()); + log.info("Imported %s in %s", table.getTableName(), nanosSince(start).convertToMostSuccinctTimeUnit()); } log.info("Loading complete in %s", nanosSince(startTime).toString(SECONDS)); } diff --git a/plugin/trino-kinesis/src/main/java/io/trino/plugin/kinesis/s3config/S3TableConfigClient.java b/plugin/trino-kinesis/src/main/java/io/trino/plugin/kinesis/s3config/S3TableConfigClient.java index f2ae559e0938..7e832b3f5359 100644 --- a/plugin/trino-kinesis/src/main/java/io/trino/plugin/kinesis/s3config/S3TableConfigClient.java +++ b/plugin/trino-kinesis/src/main/java/io/trino/plugin/kinesis/s3config/S3TableConfigClient.java @@ -198,7 +198,7 @@ private void updateTablesFromS3() log.info("Put table description into the map from %s", summary.getKey()); } catch (IOException iox) { - log.error("Problem reading input stream from object.", iox); + log.error(iox, "Problem reading input stream from object."); throw new RuntimeException(iox); } } diff --git a/plugin/trino-kudu/src/main/java/io/trino/plugin/kudu/KuduClientSession.java b/plugin/trino-kudu/src/main/java/io/trino/plugin/kudu/KuduClientSession.java index 56d38c737483..3c25f72feea4 100644 --- a/plugin/trino-kudu/src/main/java/io/trino/plugin/kudu/KuduClientSession.java +++ b/plugin/trino-kudu/src/main/java/io/trino/plugin/kudu/KuduClientSession.java @@ -231,7 +231,7 @@ public KuduTable openTable(SchemaTableName schemaTableName) return client.openTable(rawName); } catch (KuduException e) { - log.debug("Error on doOpenTable: " + e, e); + log.debug(e, "Error on doOpenTable"); if (!listSchemaNames().contains(schemaTableName.getSchemaName())) { throw new SchemaNotFoundException(schemaTableName.getSchemaName()); } diff --git a/pom.xml b/pom.xml index 1a2db181f632..9b35f394d161 100644 --- a/pom.xml +++ b/pom.xml @@ -1721,6 +1721,7 @@ -Xep:EqualsIncompatibleType:ERROR -Xep:FallThrough:ERROR -Xep:FormatString:ERROR + -Xep:FormatStringAnnotation:ERROR -Xep:GetClassOnAnnotation:ERROR -Xep:GetClassOnClass:ERROR -Xep:IdentityBinaryExpression:ERROR diff --git a/testing/trino-testing/src/main/java/io/trino/testing/datatype/DataTypeTest.java b/testing/trino-testing/src/main/java/io/trino/testing/datatype/DataTypeTest.java index e847b543ea7f..8ef9e0f62a47 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/datatype/DataTypeTest.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/datatype/DataTypeTest.java @@ -104,7 +104,7 @@ private void queryWithWhere(QueryRunner trinoExecutor, Session session, List