This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 6f68898b74b Mark the "query received" log record so it can be routed
independently (#19454)
6f68898b74b is described below
commit 6f68898b74b4d986d32915abbc64da23d685db77
Author: shivam-startree <[email protected]>
AuthorDate: Thu Sep 17 15:18:35 2026 -0700
Mark the "query received" log record so it can be routed independently
(#19454)
---
.../apache/pinot/broker/querylog/QueryLogger.java | 16 ++++++++++-
.../pinot/broker/querylog/QueryLoggerTest.java | 31 ++++++++++++++++++++++
2 files changed, 46 insertions(+), 1 deletion(-)
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java
index e4be84010fa..1d79a6f9916 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java
@@ -33,6 +33,8 @@ import org.apache.pinot.spi.trace.RequestContext;
import org.apache.pinot.spi.utils.CommonConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.slf4j.Marker;
+import org.slf4j.MarkerFactory;
import static org.apache.pinot.spi.utils.CommonConstants.Broker;
@@ -49,6 +51,17 @@ public class QueryLogger {
private static final String FINGERPRINT_FAILED_QUERY_REDACTED =
"FINGERPRINT_FAILED_QUERY_REDACTED";
private static final String FULLY_REDACTED = "REDACTED";
+ /// Marks the pre-processing "query received" record so a deployment can
route or suppress it
+ /// independently of the completion record, which shares this logger. The
two cannot be told apart
+ /// by logger name, and telling them apart by message text is brittle -- it
breaks the moment the
+ /// wording changes. Follows the existing marker convention in this module
(MSE_STATS_MARKER,
+ /// QUERY_RESPONSE_EXCEPTION), so a log4j2 MarkerFilter is all that is
needed:
+ ///
+ /// <MarkerFilter marker="QUERY_RECEIVED" onMatch="DENY"
onMismatch="NEUTRAL"/>
+ ///
+ /// Kept enabled by default: this record is the only trace of a query that
never completes.
+ private static final Marker QUERY_RECEIVED_MARKER =
MarkerFactory.getMarker("QUERY_RECEIVED");
+
public enum SqlRedactionMode {
// Log the full SQL query text with backslashes and line endings escaped.
// e.g. "SELECT name FROM users WHERE id = 42 AND status = 'active'"
@@ -123,7 +136,8 @@ public class QueryLogger {
}
if (_logBeforeProcessing) {
- _logger.info("SQL query for request {}: {}", requestId,
redactQuery(query, queryFingerprint));
+ _logger.info(QUERY_RECEIVED_MARKER, "SQL query for request {}: {}",
requestId,
+ redactQuery(query, queryFingerprint));
}
tryLogDropped();
diff --git
a/pinot-broker/src/test/java/org/apache/pinot/broker/querylog/QueryLoggerTest.java
b/pinot-broker/src/test/java/org/apache/pinot/broker/querylog/QueryLoggerTest.java
index eb2f32d62de..15a9bf7d382 100644
---
a/pinot-broker/src/test/java/org/apache/pinot/broker/querylog/QueryLoggerTest.java
+++
b/pinot-broker/src/test/java/org/apache/pinot/broker/querylog/QueryLoggerTest.java
@@ -35,9 +35,11 @@ import org.apache.pinot.spi.exception.QueryErrorCode;
import org.apache.pinot.spi.trace.DefaultRequestContext;
import org.apache.pinot.spi.trace.QueryFingerprint;
import org.apache.pinot.spi.trace.RequestContext;
+import org.mockito.ArgumentMatchers;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.slf4j.Logger;
+import org.slf4j.Marker;
import org.testng.Assert;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
@@ -88,6 +90,16 @@ public class QueryLoggerTest {
return null;
}).when(_logger).info(Mockito.anyString(), Mockito.anyLong(),
Mockito.anyString());
+ // logQueryReceived tags its record with QUERY_RECEIVED, which is a
different overload
+ Mockito.doAnswer(invocationOnMock -> {
+ String format = invocationOnMock.getArgument(1);
+ Object arg1 = invocationOnMock.getArgument(2);
+ Object arg2 = invocationOnMock.getArgument(3);
+ _infoLog.add(String.format(format.replace("{}", "%s"), arg1, arg2));
+ return null;
+ }).when(_logger)
+ .info(Mockito.any(Marker.class), Mockito.anyString(),
Mockito.anyLong(), Mockito.anyString());
+
Mockito.doAnswer(inv -> {
_numDropped.add(inv.getArgument(1));
return null;
@@ -231,6 +243,25 @@ public class QueryLoggerTest {
Assert.assertTrue(_infoLog.get(0).contains("SQL query for request 123"));
}
+ @Test
+ public void shouldMarkOnlyTheQueryReceivedRecord() {
+ // Given:
+ Mockito.when(_logRateLimiter.tryAcquire()).thenReturn(true);
+ QueryLogger queryLogger = new QueryLogger(_logRateLimiter, 100, true, true,
+ SqlRedactionMode.NONE, _logger, _droppedRateLimiter);
+
+ // When: a query is received and then completes
+ queryLogger.logQueryReceived(123L, "SELECT * FROM foo", null);
+ queryLogger.logQueryCompleted(generateParams(false, false, 0, 456, null),
true);
+
+ // Then: the received record carries the marker so it can be routed
independently...
+ Mockito.verify(_logger).info(
+ ArgumentMatchers.argThat(marker -> marker != null &&
"QUERY_RECEIVED".equals(marker.getName())),
+ Mockito.anyString(), Mockito.anyLong(), Mockito.anyString());
+ // ...and the completion record, which shares this logger, does not
+ Mockito.verify(_logger).info(Mockito.anyString());
+ }
+
@Test
public void shouldLogMultiLineQueryOnASingleLine() {
// Given: a client that submits pretty-printed SQL, as JDBC/BI tools
routinely do
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
