Use parameterized log messages instead of string concatenation (#13145)

pinot-broker/src/main/java/org/apache/pinot/broker/broker/BrokerManagedAsyncExecutorProvider.java

@@ -98,7 +98,7 @@ public BrokerThreadPoolRejectExecutionHandler(BrokerMetrics brokerMetrics) {
     @Override
     public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) {
       _brokerMetrics.addMeteredGlobalValue(BrokerMeter.QUERY_REJECTED_EXCEPTIONS, 1L);
-      LOGGER.error("Task " + r + " rejected from " + executor);
+      LOGGER.error("Task {} rejected from {}", r, executor);
 
       throw new ServiceUnavailableException(Response.status(Response.Status.SERVICE_UNAVAILABLE).entity(
           "Pinot Broker thread pool can not accommodate more requests now. " + "Request is rejected from " + executor)

pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotDriver.java

@@ -87,7 +87,7 @@ public Connection connect(String url, Properties info)
       if (!this.acceptsURL(url)) {
         return null;
       }
-      LOGGER.info("Initiating connection to database for url: " + url);
+      LOGGER.info("Initiating connection to database for url: {}", url);
 
       Map<String, String> urlParams = DriverUtils.getURLParams(url);
       info.putAll(urlParams);

pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClient.java

@@ -283,8 +283,8 @@ public SimpleHttpResponse sendRequest(HttpUriRequest request)
       if (response.containsHeader(CommonConstants.Controller.HOST_HTTP_HEADER)) {
         String controllerHost = response.getFirstHeader(CommonConstants.Controller.HOST_HTTP_HEADER).getValue();
         String controllerVersion = response.getFirstHeader(CommonConstants.Controller.VERSION_HTTP_HEADER).getValue();
-        LOGGER.info("Sending request: " + request.getURI() + " to controller: " + controllerHost + ", version: "
-            + controllerVersion);
+        LOGGER.info("Sending request: {} to controller: {}, version: {}", request.getURI(), controllerHost,
+            controllerVersion);
       }
       int statusCode = response.getStatusLine().getStatusCode();
       if (statusCode >= 300) {

pinot-common/src/main/java/org/apache/pinot/serde/SerDe.java

@@ -61,7 +61,7 @@ public byte[] serialize(@SuppressWarnings("rawtypes") TBase obj) {
     try {
       return _serializer.serialize(obj);
     } catch (TException e) {
-      LOGGER.error("Unable to serialize object :" + obj, e);
+      LOGGER.error("Unable to serialize object :{}", obj, e);
       return null;
     }
   }
@@ -70,7 +70,7 @@ public boolean deserialize(@SuppressWarnings("rawtypes") TBase obj, byte[] paylo
     try {
       _deserializer.deserialize(obj, payload);
     } catch (TException e) {
-      LOGGER.error("Unable to deserialize to object :" + obj, e);
+      LOGGER.error("Unable to deserialize to object :{}", obj, e);
       return false;
     }
     return true;

pinot-common/src/test/java/org/apache/pinot/util/TestUtils.java

@@ -50,7 +50,7 @@ public static String getFileFromResourceUrl(@Nonnull URL resourceUrl) {
         String extension = resourceUrlStr.substring(resourceUrlStr.lastIndexOf('.'));
         File tempFile = File.createTempFile("pinot-test-temp", extension);
         String tempFilePath = tempFile.getAbsolutePath();
-        LOGGER.info("Extracting from " + resourceUrlStr + " to " + tempFilePath);
+        LOGGER.info("Extracting from {} to {}", resourceUrlStr, tempFilePath);
         FileUtils.copyURLToFile(resourceUrl, tempFile);
         return tempFilePath;
       } catch (IOException e) {

pinot-compatibility-verifier/src/main/java/org/apache/pinot/compat/StreamOp.java

@@ -282,7 +282,7 @@ private long fetchExistingTotalDocs(String tableName)
       JsonNode errorCode = exceptions.get(ERROR_CODE);
       if (String.valueOf(QueryException.BROKER_INSTANCE_MISSING_ERROR).equals(String.valueOf(errorCode))
           && errorCode != null) {
-        LOGGER.warn(errorMsg + ".Trying again");
+        LOGGER.warn("{}.Trying again", errorMsg);
         return 0;
       }
       LOGGER.error(errorMsg);

pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java

@@ -368,7 +368,7 @@ public void start() {
         setUpHelixController();
         break;
       default:
-        LOGGER.error("Invalid mode: " + _controllerMode);
+        LOGGER.error("Invalid mode: {}", _controllerMode);
         break;
     }
 

pinot-controller/src/main/java/org/apache/pinot/controller/api/exception/ControllerApplicationException.java

@@ -44,7 +44,7 @@ public ControllerApplicationException(Logger logger, String message, int status,
       if (e == null) {
         logger.info(message);
       } else {
-        logger.info(message + " exception: " + e.getMessage());
+        logger.info("{} exception: {}", message, e.getMessage());
       }
     } else {
       if (e == null) {

pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java

@@ -405,7 +405,7 @@ public String sendPostRaw(String urlStr, String requestStr, Map<String, String>
         LOGGER.debug("The request is - " + requestStr);
       }*/
 
-      LOGGER.info("url string passed is : " + urlStr);
+      LOGGER.info("url string passed is : {}", urlStr);
       final URL url = new URL(urlStr);
       conn = (HttpURLConnection) url.openConnection();
       conn.setDoOutput(true);
@@ -484,7 +484,7 @@ public String sendRequestRaw(String url, String query, ObjectNode requestJson, M
       final String pinotResultString = sendPostRaw(url, requestJson.toString(), headers);
 
       final long queryTime = System.currentTimeMillis() - startTime;
-      LOGGER.info("Query: " + query + " Time: " + queryTime);
+      LOGGER.info("Query: {} Time: {}", query, queryTime);
 
       return pinotResultString;
     } catch (final Exception ex) {

pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java

@@ -1069,7 +1069,7 @@ public PinotResourceManagerResponse rebuildBrokerResourceFromHelixTags(String ta
           "Failed to fetch broker tag for table " + tableNameWithType + " due to exception: " + e.getMessage());
     }
     if (tableConfig == null) {
-      LOGGER.warn("Table " + tableNameWithType + " does not exist");
+      LOGGER.warn("Table {} does not exist", tableNameWithType);
       throw new InvalidConfigException(
           "Invalid table configuration for table " + tableNameWithType + ". Table does not exist");
     }

pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java

@@ -430,7 +430,7 @@ private void scheduleJob(String tableWithType, String taskType, String cronExprS
         _controllerMetrics.addValueToTableGauge(getCronJobName(tableWithType, taskType),
             ControllerGauge.CRON_SCHEDULER_JOB_SCHEDULED, 1L);
       } catch (Exception e) {
-        LOGGER.error("Failed to parse Cron expression - " + cronExprStr, e);
+        LOGGER.error("Failed to parse Cron expression - {}", cronExprStr, e);
         throw e;
       }
       Date nextRuntime = trigger.getNextFireTime();

pinot-controller/src/main/java/org/apache/pinot/controller/recommender/data/generator/DataGenerator.java

@@ -80,7 +80,7 @@ public void init(DataGeneratorSpec spec)
         IntRange range = _genSpec.getRangeMap().get(column);
         generator = GeneratorFactory.getGeneratorFor(dataType, range.getMinimumInteger(), range.getMaximumInteger());
       } else {
-        LOGGER.error("cardinality for this column does not exist : " + column);
+        LOGGER.error("cardinality for this column does not exist : {}", column);
         throw new RuntimeException("cardinality for this column does not exist");
       }
       generator.init();

pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilderFactory.java

@@ -43,7 +43,7 @@ public static int getDataTableVersion() {
   }
 
   public static void setDataTableVersion(int version) {
-    LOGGER.info("Setting DataTable version to: " + version);
+    LOGGER.info("Setting DataTable version to: {}", version);
     if (version != DataTableFactory.VERSION_2 && version != DataTableFactory.VERSION_3
         && version != DataTableFactory.VERSION_4) {
       throw new IllegalArgumentException("Unsupported version: " + version);

pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeConsumptionRateManager.java

@@ -206,7 +206,7 @@ interface PartitionCountFetcher {
     try (StreamMetadataProvider streamMetadataProvider = factory.createStreamMetadataProvider(clientId)) {
       return streamMetadataProvider.fetchPartitionCount(/*maxWaitTimeMs*/10_000);
     } catch (Exception e) {
-      LOGGER.warn("Error fetching metadata for topic " + streamConfig.getTopicName(), e);
+      LOGGER.warn("Error fetching metadata for topic {}", streamConfig.getTopicName(), e);
       return null;
     }
   };

pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java

@@ -399,12 +399,12 @@ private void handleTransientStreamErrors(Exception e)
     _serverMetrics.addMeteredTableValue(_tableStreamName, ServerMeter.REALTIME_CONSUMPTION_EXCEPTIONS,
         1L);
     if (_consecutiveErrorCount > MAX_CONSECUTIVE_ERROR_COUNT) {
-      _segmentLogger.warn("Stream transient exception when fetching messages, stopping consumption after "
-          + _consecutiveErrorCount + " attempts", e);
+      _segmentLogger.warn("Stream transient exception when fetching messages, stopping consumption after {} attempts",
+          _consecutiveErrorCount, e);
       throw e;
     } else {
-      _segmentLogger.warn("Stream transient exception when fetching messages, retrying (count="
-          + _consecutiveErrorCount + ")", e);
+      _segmentLogger.warn("Stream transient exception when fetching messages, retrying (count={})",
+          _consecutiveErrorCount, e);
       Uninterruptibles.sleepUninterruptibly(1, TimeUnit.SECONDS);
       recreateStreamConsumer("Too many transient errors");
     }

pinot-core/src/main/java/org/apache/pinot/core/operator/combine/BaseCombineOperator.java

@@ -123,9 +123,9 @@ public void runJob() {
             //       exception into the query response, and the main thread might wait infinitely (until timeout) or
             //       throw unexpected exceptions (such as NPE).
             if (t instanceof Exception) {
-              LOGGER.error("Caught exception while processing query: " + _queryContext, t);
+              LOGGER.error("Caught exception while processing query: {}", _queryContext, t);
             } else {
-              LOGGER.error("Caught serious error while processing query: " + _queryContext, t);
+              LOGGER.error("Caught serious error while processing query: {}", _queryContext, t);
             }
             onProcessSegmentsException(t);
           } finally {

pinot-core/src/main/java/org/apache/pinot/core/periodictask/PeriodicTaskScheduler.java

@@ -149,7 +149,7 @@ public void scheduleNow(String periodicTaskName, Properties periodicTaskProperti
     // level) whether the periodic task exists.
     PeriodicTask periodicTask = getPeriodicTask(periodicTaskName);
     if (periodicTask == null) {
-      LOGGER.error("Unknown Periodic Task " + periodicTaskName);
+      LOGGER.error("Unknown Periodic Task {}", periodicTaskName);
       return;
     }
 

pinot-core/src/test/java/org/apache/pinot/core/accounting/ResourceManagerAccountingTest.java

@@ -410,7 +410,7 @@ public void testThreadMemory()
               Tracing.ThreadAccountantOps.sample();
               if (Thread.interrupted() || thread.isInterrupted()) {
                 Tracing.ThreadAccountantOps.clear();
-                LOGGER.error("KilledWorker " + queryId + " " + finalJ);
+                LOGGER.error("KilledWorker {} {}", queryId, finalJ);
                 return;
               }
               a[i] = new long[200000];
@@ -429,7 +429,7 @@ public void testThreadMemory()
           for (int i = 0; i < 10; i++) {
             futuresThread[i].cancel(true);
           }
-          LOGGER.error("Killed " + queryId);
+          LOGGER.error("Killed {}", queryId);
         }
         Tracing.ThreadAccountantOps.clear();
       });

pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiNodesOfflineClusterIntegrationTest.java

@@ -139,7 +139,7 @@ public void testServerHardFailure()
     testCountStarQuery(3, false);
     assertEquals(getCurrentCountStarResult(), expectedCountStarResult);
 
-    LOGGER.warn("Shutting down server " + _serverStarters.get(NUM_SERVERS - 1).getInstanceId());
+    LOGGER.warn("Shutting down server {}", _serverStarters.get(NUM_SERVERS - 1).getInstanceId());
     // Take a server and shut down its query server to mimic a hard failure
     BaseServerStarter serverStarter = _serverStarters.get(NUM_SERVERS - 1);
     try {

pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/RealtimeConsumptionRateLimiterClusterIntegrationTest.java

@@ -158,8 +158,8 @@ public void testOneTableRateLimit()
         Thread.sleep(1000L);
         long currentCount = getCurrentCountStarResult(tableName);
         double currentRate = (currentCount - startCount) / (double) (System.currentTimeMillis() - startTime) * 1000;
-        LOGGER.info("Second = " + i + ", realtimeRowConsumedMeter = " + realtimeRowConsumedMeter.oneMinuteRate()
-            + ", currentCount = " + currentCount + ", currentRate = " + currentRate);
+        LOGGER.info("Second = {}, realtimeRowConsumedMeter = {}, currentCount = {}, currentRate = {}", i,
+            realtimeRowConsumedMeter.oneMinuteRate(), currentCount, currentRate);
         Assert.assertTrue(realtimeRowConsumedMeter.oneMinuteRate() < SERVER_RATE_LIMIT,
             "Rate should be less than " + SERVER_RATE_LIMIT);
         Assert.assertTrue(currentRate < SERVER_RATE_LIMIT * 1.5, // Put some leeway for the rate calculation
@@ -211,11 +211,11 @@ public void testTwoTableRateLimit()
         double currentRate1 = (currentCount1 - startCount1) / (double) (currentTimeMillis - startTime) * 1000;
         double currentRate2 = (currentCount2 - startCount2) / (double) (currentTimeMillis - startTime) * 1000;
         double currentServerRate = currentRate1 + currentRate2;
-        LOGGER.info("Second = " + i + ", serverRowConsumedMeter = " + serverRowConsumedMeter.oneMinuteRate()
-            + ", currentCount1 = " + currentCount1 + ", currentRate1 = " + currentRate1
-            + ", currentCount2 = " + currentCount2 + ", currentRate2 = " + currentRate2
-            + ", currentServerCount = " + currentServerCount + ", currentServerRate = " + currentServerRate
-        );
+        LOGGER.info(
+            "Second = {}, serverRowConsumedMeter = {}, currentCount1 = {}, currentRate1 = {}, currentCount2 = {}, "
+                + "currentRate2 = {}, currentServerCount = {}, currentServerRate = {}",
+            i, serverRowConsumedMeter.oneMinuteRate(), currentCount1, currentRate1, currentCount2, currentRate2,
+            currentServerCount, currentServerRate);
 
         Assert.assertTrue(serverRowConsumedMeter.oneMinuteRate() < SERVER_RATE_LIMIT,
             "Rate should be less than " + SERVER_RATE_LIMIT + ", serverOneMinuteRate = " + serverRowConsumedMeter

pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/RealtimeKinesisIntegrationTest.java

@@ -166,7 +166,7 @@ public Boolean apply(@Nullable Void aVoid) {
         try {
           return getCurrentCountStarResult() >= _totalRecordsPushedInStream;
         } catch (Exception e) {
-          LOGGER.warn("Could not fetch current number of rows in pinot table " + getTableName(), e);
+          LOGGER.warn("Could not fetch current number of rows in pinot table {}", getTableName(), e);
           return null;
         }
       }

pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/SegmentGenerationMinionClusterIntegrationTest.java

@@ -98,7 +98,7 @@ public void testAdhocSegmentGenerationAndPushTask()
         }
         return getTotalDocs(tableName) == rowCnt;
       } catch (Exception e) {
-        LOGGER.error("Failed to get expected totalDocs: " + rowCnt, e);
+        LOGGER.error("Failed to get expected totalDocs: {}", rowCnt, e);
         return false;
       }
     }, 5000L, 600_000L, "Failed to load " + rowCnt + " documents", true);
@@ -143,7 +143,7 @@ private void testInsertIntoFromFile(String tableName, String taskName, boolean q
         }
         return getTotalDocs(tableName) == rowCnt;
       } catch (Exception e) {
-        LOGGER.error("Failed to get expected totalDocs: " + rowCnt, e);
+        LOGGER.error("Failed to get expected totalDocs: {}", rowCnt, e);
         return false;
       }
     }, 5000L, 600_000L, "Failed to load " + rowCnt + " documents", true);

pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/access/CertBasedTlsChannelAccessControlFactory.java

@@ -61,8 +61,9 @@ public boolean isAuthorizedChannel(ChannelHandlerContext channelHandlerContext)
         Principal principal = x509.getSubjectX500Principal();
         return _aclPrincipalAllowlist.contains(principal.toString());
       } catch (CertificateException | SSLPeerUnverifiedException e) {
-        _logger.error("Access denied - caught exception while validating access to server, with channelHandlerContext:"
-            + channelHandlerContext, e);
+        _logger.error(
+            "Access denied - caught exception while validating access to server, with channelHandlerContext:{}",
+            channelHandlerContext, e);
         return false;
       }
     }

pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java

@@ -332,7 +332,7 @@ public void stop() {
     LOGGER.info("Shutting down admin application");
     _minionAdminApplication.stop();
 
-    LOGGER.info("Stopping Pinot minion: " + _instanceId);
+    LOGGER.info("Stopping Pinot minion: {}", _instanceId);
     _helixManager.disconnect();
     LOGGER.info("Deregistering service status handler");
     ServiceStatus.removeServiceStatusCallback(_instanceId);

pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/segmentgenerationandpush/SegmentGenerationAndPushTaskGenerator.java

@@ -329,7 +329,7 @@ private List<URI> getInputFilesFromDirectory(Map<String, String> batchConfigMap,
       try {
         files = inputDirFS.listFiles(inputDirURI, true);
       } catch (IOException e) {
-        LOGGER.error("Unable to list files under URI: " + inputDirURI, e);
+        LOGGER.error("Unable to list files under URI: {}", inputDirURI, e);
         return Collections.emptyList();
       }
       PathMatcher includeFilePathMatcher = null;

pinot-plugins/pinot-stream-ingestion/pinot-kafka-2.0/src/main/java/org/apache/pinot/plugin/stream/kafka20/KafkaStreamMetadataProvider.java

@@ -108,9 +108,9 @@ public StreamPartitionMsgOffset fetchStreamPartitionOffset(OffsetCriteria offset
         if (offsetAndTimestamp == null) {
           offset = _consumer.endOffsets(Collections.singletonList(_topicPartition), Duration.ofMillis(timeoutMillis))
               .get(_topicPartition);
-          LOGGER.warn("initial offset type is period and its value evaluates "
-              + "to null hence proceeding with offset " + offset + "for topic " + _topicPartition.topic()
-              + " partition " + _topicPartition.partition());
+          LOGGER.warn(
+              "initial offset type is period and its value evaluates to null hence proceeding with offset {} for "
+                  + "topic {} partition {}", offset, _topicPartition.topic(), _topicPartition.partition());
         } else {
           offset = offsetAndTimestamp.offset();
         }
@@ -120,9 +120,9 @@ public StreamPartitionMsgOffset fetchStreamPartitionOffset(OffsetCriteria offset
         if (offsetAndTimestamp == null) {
           offset = _consumer.endOffsets(Collections.singletonList(_topicPartition), Duration.ofMillis(timeoutMillis))
               .get(_topicPartition);
-          LOGGER.warn("initial offset type is timestamp and its value evaluates "
-              + "to null hence proceeding with offset " + offset + "for topic " + _topicPartition.topic()
-              + " partition " + _topicPartition.partition());
+          LOGGER.warn(
+              "initial offset type is timestamp and its value evaluates to null hence proceeding with offset {} for "
+                  + "topic {} partition {}", offset, _topicPartition.topic(), _topicPartition.partition());
         } else {
           offset = offsetAndTimestamp.offset();
         }

pinot-plugins/pinot-stream-ingestion/pinot-kafka-2.0/src/main/java/org/apache/pinot/plugin/stream/kafka20/server/KafkaDataServerStartable.java

@@ -128,9 +128,9 @@ private static void waitForCondition(Function<Void, Boolean> condition, long che
         }
         Thread.sleep(checkIntervalMs);
       } catch (Exception e) {
-        LOGGER.error("Caught exception while checking the condition" + errorMessageSuffix, e);
+        LOGGER.error("Caught exception while checking the condition{}", errorMessageSuffix, e);
       }
     }
-    LOGGER.error("Failed to meet condition in " + timeoutMs + "ms" + errorMessageSuffix);
+    LOGGER.error("Failed to meet condition in {}ms{}", timeoutMs, errorMessageSuffix);
   }
 }

pinot-plugins/pinot-stream-ingestion/pinot-kinesis/src/main/java/org/apache/pinot/plugin/stream/kinesis/KinesisConsumer.java

@@ -170,7 +170,7 @@ private void debugOrLogWarning(String message, Throwable throwable) {
     if (LOGGER.isDebugEnabled()) {
       LOGGER.debug(message, throwable);
     } else {
-      LOGGER.warn(message + ": " + throwable.getMessage());
+      LOGGER.warn("{}: {}", message, throwable.getMessage());
     }
   }
 

pinot-plugins/pinot-stream-ingestion/pinot-kinesis/src/main/java/org/apache/pinot/plugin/stream/kinesis/KinesisStreamMetadataProvider.java

@@ -113,9 +113,9 @@ public List<PartitionGroupMetadata> computePartitionGroupMetadata(String clientI
       if (shard == null) { // Shard has expired
         shardsEnded.add(shardId);
         String lastConsumedSequenceID = kinesisStartCheckpoint.getSequenceNumber();
-        LOGGER.warn("Kinesis shard with id: " + shardId
-            + " has expired. Data has been consumed from the shard till sequence number: " + lastConsumedSequenceID
-            + ". There can be potential data loss.");
+        LOGGER.warn(
+            "Kinesis shard with id: {} has expired. Data has been consumed from the shard till sequence number: {}. "
+                + "There can be potential data loss.", shardId, lastConsumedSequenceID);
         continue;
       }