From c177b64588a7937c44d5fe9f74a21955145ab6f0 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 27 Feb 2026 15:50:36 -0800 Subject: [PATCH 1/2] add SQL parameter values in RequestLogLine so they can be logged/emitted --- .../druid/query/http/ClientSqlParameter.java | 12 +++++ .../query/http/ClientSqlParameterTest.java | 51 +++++++++++++++++++ .../apache/druid/server/RequestLogLine.java | 34 +++++++++++-- .../server/log/DefaultRequestLogEvent.java | 10 +++- .../log/DefaultRequestLogEventTest.java | 11 +++- .../server/log/LoggingRequestLoggerTest.java | 10 +++- .../druid/sql/SqlExecutionReporter.java | 1 + .../org/apache/druid/sql/SqlQueryPlus.java | 16 ++++++ .../sql/avatica/DruidAvaticaHandlerTest.java | 11 ++++ 9 files changed, 148 insertions(+), 8 deletions(-) create mode 100644 processing/src/test/java/org/apache/druid/query/http/ClientSqlParameterTest.java diff --git a/processing/src/main/java/org/apache/druid/query/http/ClientSqlParameter.java b/processing/src/main/java/org/apache/druid/query/http/ClientSqlParameter.java index cc562034e93a..3d42f274af83 100644 --- a/processing/src/main/java/org/apache/druid/query/http/ClientSqlParameter.java +++ b/processing/src/main/java/org/apache/druid/query/http/ClientSqlParameter.java @@ -46,6 +46,18 @@ public ClientSqlParameter( this.value = value; } + @JsonProperty + public String getType() + { + return type; + } + + @JsonProperty + public Object getValue() + { + return value; + } + @Override public boolean equals(Object o) { diff --git a/processing/src/test/java/org/apache/druid/query/http/ClientSqlParameterTest.java b/processing/src/test/java/org/apache/druid/query/http/ClientSqlParameterTest.java new file mode 100644 index 000000000000..3f7af43e9790 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/http/ClientSqlParameterTest.java @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.query.http; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import nl.jqno.equalsverifier.EqualsVerifier; +import org.apache.druid.segment.TestHelper; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +class ClientSqlParameterTest +{ + @Test + public void testSerde() throws JsonProcessingException + { + ObjectMapper mapper = TestHelper.makeJsonMapper(); + ClientSqlParameter sqlParameter = new ClientSqlParameter( + "BIGINT", + 1234 + ); + // serde here suffers normal jackson problems e.g. if 1234 was 1234L this test would fail + Assertions.assertEquals( + sqlParameter, + mapper.readValue(mapper.writeValueAsString(sqlParameter), ClientSqlParameter.class) + ); + } + + @Test + public void testEqualsAndHashcode() + { + EqualsVerifier.forClass(ClientSqlParameter.class).usingGetClass().verify(); + } +} diff --git a/server/src/main/java/org/apache/druid/server/RequestLogLine.java b/server/src/main/java/org/apache/druid/server/RequestLogLine.java index a97098b79825..00d773b48853 100644 --- a/server/src/main/java/org/apache/druid/server/RequestLogLine.java +++ b/server/src/main/java/org/apache/druid/server/RequestLogLine.java @@ -27,11 +27,13 @@ import com.google.common.collect.ImmutableMap; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.query.Query; +import org.apache.druid.query.http.ClientSqlParameter; import org.joda.time.DateTime; import javax.annotation.Nullable; import java.util.Arrays; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Objects; @@ -41,6 +43,7 @@ public class RequestLogLine private final Query query; private final String sql; + private final List sqlParameters; private final Map sqlQueryContext; private final DateTime timestamp; private final String remoteAddr; @@ -49,6 +52,7 @@ public class RequestLogLine private RequestLogLine( @Nullable Query query, @Nullable String sql, + @Nullable List sqlParameters, @Nullable Map sqlQueryContext, DateTime timestamp, @Nullable String remoteAddr, @@ -57,6 +61,7 @@ private RequestLogLine( { this.query = query; this.sql = sql; + this.sqlParameters = sqlParameters; this.sqlQueryContext = sqlQueryContext != null ? sqlQueryContext : ImmutableMap.of(); this.timestamp = Preconditions.checkNotNull(timestamp, "timestamp"); this.remoteAddr = StringUtils.nullToEmptyNonDruidDataString(remoteAddr); @@ -65,7 +70,7 @@ private RequestLogLine( public static RequestLogLine forNative(Query query, DateTime timestamp, String remoteAddr, QueryStats queryStats) { - return new RequestLogLine(query, null, null, timestamp, remoteAddr, queryStats); + return new RequestLogLine(query, null, null, null, timestamp, remoteAddr, queryStats); } public static RequestLogLine forSql( @@ -76,7 +81,19 @@ public static RequestLogLine forSql( QueryStats queryStats ) { - return new RequestLogLine(null, sql, sqlQueryContext, timestamp, remoteAddr, queryStats); + return forSql(sql, null, sqlQueryContext, timestamp, remoteAddr, queryStats); + } + + public static RequestLogLine forSql( + String sql, + List parameters, + Map sqlQueryContext, + DateTime timestamp, + String remoteAddr, + QueryStats queryStats + ) + { + return new RequestLogLine(null, sql, parameters, sqlQueryContext, timestamp, remoteAddr, queryStats); } public String getNativeQueryLine(ObjectMapper objectMapper) throws JsonProcessingException @@ -96,6 +113,9 @@ public String getSqlQueryLine(ObjectMapper objectMapper) throws JsonProcessingEx final Map queryMap = new LinkedHashMap<>(); queryMap.put("context", sqlQueryContext); queryMap.put("query", sql == null ? "" : sql); + if (sqlParameters != null) { + queryMap.put("parameters", sqlParameters); + } return JOINER.join( Arrays.asList( @@ -122,6 +142,13 @@ public String getSql() return sql; } + @Nullable + @JsonProperty("sqlParameters") + public List getSqlParameters() + { + return sqlParameters; + } + @Nullable @JsonProperty public Map getSqlQueryContext() @@ -160,6 +187,7 @@ public boolean equals(Object o) RequestLogLine that = (RequestLogLine) o; return Objects.equals(query, that.query) && Objects.equals(sql, that.sql) && + Objects.equals(sqlParameters, that.sqlParameters) && Objects.equals(sqlQueryContext, that.sqlQueryContext) && Objects.equals(timestamp, that.timestamp) && Objects.equals(remoteAddr, that.remoteAddr) && @@ -169,7 +197,7 @@ public boolean equals(Object o) @Override public int hashCode() { - return Objects.hash(query, sql, sqlQueryContext, timestamp, remoteAddr, queryStats); + return Objects.hash(query, sql, sqlParameters, sqlQueryContext, timestamp, remoteAddr, queryStats); } @Override diff --git a/server/src/main/java/org/apache/druid/server/log/DefaultRequestLogEvent.java b/server/src/main/java/org/apache/druid/server/log/DefaultRequestLogEvent.java index 0487ea54feea..7089049b9aa6 100644 --- a/server/src/main/java/org/apache/druid/server/log/DefaultRequestLogEvent.java +++ b/server/src/main/java/org/apache/druid/server/log/DefaultRequestLogEvent.java @@ -29,6 +29,7 @@ import org.apache.druid.server.RequestLogLine; import org.joda.time.DateTime; +import java.util.List; import java.util.Map; /** @@ -69,7 +70,8 @@ public EventMap toMap() if (getSql() != null) { builder.put("sqlQueryContext", getSqlQueryContext()) - .put("sql", getSql()); + .put("sql", getSql()) + .putNonNull("sqlParameters", getSqlParameters()); } return builder.build(); @@ -112,6 +114,12 @@ public String getSql() return request.getSql(); } + @JsonProperty("sqlParameters") + public List getSqlParameters() + { + return request.getSqlParameters(); + } + @JsonProperty("sqlQueryContext") public Map getSqlQueryContext() { diff --git a/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java b/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java index e9161bb01503..71f1af6cd7e0 100644 --- a/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java +++ b/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java @@ -32,6 +32,7 @@ import org.apache.druid.java.util.emitter.core.EventMap; import org.apache.druid.query.Query; import org.apache.druid.query.TableDataSource; +import org.apache.druid.query.http.ClientSqlParameter; import org.apache.druid.query.spec.MultipleIntervalSegmentSpec; import org.apache.druid.query.timeseries.TimeseriesQuery; import org.apache.druid.segment.VirtualColumns; @@ -43,6 +44,7 @@ import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; public class DefaultRequestLogEventTest @@ -133,7 +135,10 @@ public void testDefaultRequestLogEventToMapSQL() throws JsonProcessingException final DateTime timestamp = DateTimes.of(2019, 12, 12, 3, 1); final String service = "druid-service"; final String host = "127.0.0.1"; - final String sql = "select * from 1337"; + final String sql = "select * from foo where x = ?"; + final List parameters = List.of( + new ClientSqlParameter("BIGINT", 1234L) + ); final QueryStats queryStats = new QueryStats( ImmutableMap.of( "sqlQuery/time", 13L, @@ -146,6 +151,7 @@ public void testDefaultRequestLogEventToMapSQL() throws JsonProcessingException RequestLogLine nativeLine = RequestLogLine.forSql( sql, + parameters, ImmutableMap.of(), timestamp, host, @@ -161,6 +167,7 @@ public void testDefaultRequestLogEventToMapSQL() throws JsonProcessingException expected.put("service", service); expected.put("host", host); expected.put("sql", sql); + expected.put("sqlParameters", parameters); expected.put("sqlQueryContext", ImmutableMap.of()); expected.put("remoteAddr", host); expected.put("queryStats", queryStats); @@ -169,7 +176,7 @@ public void testDefaultRequestLogEventToMapSQL() throws JsonProcessingException Assert.assertEquals(expected, observedEventMap); Assert.assertEquals( StringUtils.format( - "{\"feed\":\"test\",\"timestamp\":\"%s\",\"service\":\"druid-service\",\"host\":\"127.0.0.1\",\"remoteAddr\":\"127.0.0.1\",\"queryStats\":{\"sqlQuery/time\":13,\"sqlQuery/planningTimeMs\":1,\"sqlQuery/bytes\":10,\"success\":true,\"identity\":\"allowAll\"},\"sqlQueryContext\":{},\"sql\":\"select * from 1337\"}", + "{\"feed\":\"test\",\"timestamp\":\"2019-12-12T03:01:00.000Z\",\"service\":\"druid-service\",\"host\":\"127.0.0.1\",\"remoteAddr\":\"127.0.0.1\",\"queryStats\":{\"sqlQuery/time\":13,\"sqlQuery/planningTimeMs\":1,\"sqlQuery/bytes\":10,\"success\":true,\"identity\":\"allowAll\"},\"sqlQueryContext\":{},\"sql\":\"select * from foo where x = ?\",\"sqlParameters\":[{\"type\":\"BIGINT\",\"value\":1234}]}", timestamp ), new DefaultObjectMapper().writeValueAsString(observedEventMap) diff --git a/server/src/test/java/org/apache/druid/server/log/LoggingRequestLoggerTest.java b/server/src/test/java/org/apache/druid/server/log/LoggingRequestLoggerTest.java index a9d9ca81fa46..9e804ff439fe 100644 --- a/server/src/test/java/org/apache/druid/server/log/LoggingRequestLoggerTest.java +++ b/server/src/test/java/org/apache/druid/server/log/LoggingRequestLoggerTest.java @@ -36,6 +36,7 @@ import org.apache.druid.query.TableDataSource; import org.apache.druid.query.UnionDataSource; import org.apache.druid.query.filter.DimFilter; +import org.apache.druid.query.http.ClientSqlParameter; import org.apache.druid.query.spec.QuerySegmentSpec; import org.apache.druid.server.QueryStats; import org.apache.druid.server.RequestLogLine; @@ -212,7 +213,12 @@ public void testSimpleLogging() throws Exception public void testSqlLogging() throws Exception { final RequestLogLine sqlLogLine = RequestLogLine.forSql( - "select * from foo", Map.of("sqlQueryId", "id1"), DateTimes.of("2026-01-01"), null, new QueryStats(Map.of("query/time", 13L)) + "select * from foo WHERE x = ?", + List.of(new ClientSqlParameter("BIGINT", 1234L)), + Map.of("sqlQueryId", "id1"), + DateTimes.of("2026-01-01"), + null, + new QueryStats(Map.of("query/time", 13L)) ); final LoggingRequestLogger requestLogger = new LoggingRequestLogger(MAPPER, true, false); @@ -220,7 +226,7 @@ public void testSqlLogging() throws Exception final String observedLogLine = BAOS.toString(StandardCharsets.UTF_8); Assert.assertEquals( - "2026-01-01T00:00:00.000Z\t\t\t{\"query/time\":13}\t{\"context\":{\"sqlQueryId\":\"id1\"},\"query\":\"select * from foo\"}", + "2026-01-01T00:00:00.000Z\t\t\t{\"query/time\":13}\t{\"context\":{\"sqlQueryId\":\"id1\"},\"query\":\"select * from foo WHERE x = ?\",\"parameters\":[{\"type\":\"BIGINT\",\"value\":1234}]}", MAPPER.readTree(observedLogLine).get("message").asText() ); } diff --git a/sql/src/main/java/org/apache/druid/sql/SqlExecutionReporter.java b/sql/src/main/java/org/apache/druid/sql/SqlExecutionReporter.java index 773a3812dbec..60d0d5948c11 100644 --- a/sql/src/main/java/org/apache/druid/sql/SqlExecutionReporter.java +++ b/sql/src/main/java/org/apache/druid/sql/SqlExecutionReporter.java @@ -164,6 +164,7 @@ public void emit() stmt.sqlToolbox.requestLogger.logSqlQuery( RequestLogLine.forSql( stmt.queryPlus.sql(), + stmt.queryPlus.sqlParameters(), queryContext, DateTimes.utc(startMs), remoteAddress, diff --git a/sql/src/main/java/org/apache/druid/sql/SqlQueryPlus.java b/sql/src/main/java/org/apache/druid/sql/SqlQueryPlus.java index 799c38735dac..77aab6085de7 100644 --- a/sql/src/main/java/org/apache/druid/sql/SqlQueryPlus.java +++ b/sql/src/main/java/org/apache/druid/sql/SqlQueryPlus.java @@ -20,10 +20,12 @@ package org.apache.druid.sql; import com.google.common.base.Preconditions; +import org.apache.calcite.avatica.SqlType; import org.apache.calcite.avatica.remote.TypedValue; import org.apache.calcite.sql.SqlNode; import org.apache.druid.error.DruidException; import org.apache.druid.query.QueryContexts; +import org.apache.druid.query.http.ClientSqlParameter; import org.apache.druid.server.security.AuthenticationResult; import org.apache.druid.sql.calcite.parser.DruidSqlParser; import org.apache.druid.sql.calcite.parser.StatementAndSetContext; @@ -134,6 +136,20 @@ public List parameters() return parameters; } + /** + * Convert parameters list to serde friendly {@link SqlParameter} + */ + @Nullable + public List sqlParameters() + { + if (parameters.isEmpty()) { + return null; + } + return parameters.stream() + .map(p -> new ClientSqlParameter(SqlType.valueOf(p.type.typeId).toString(), p.value)) + .toList(); + } + public AuthenticationResult authResult() { return authResult; diff --git a/sql/src/test/java/org/apache/druid/sql/avatica/DruidAvaticaHandlerTest.java b/sql/src/test/java/org/apache/druid/sql/avatica/DruidAvaticaHandlerTest.java index 22ea225301ca..00d2e771e786 100644 --- a/sql/src/test/java/org/apache/druid/sql/avatica/DruidAvaticaHandlerTest.java +++ b/sql/src/test/java/org/apache/druid/sql/avatica/DruidAvaticaHandlerTest.java @@ -39,6 +39,7 @@ import org.apache.calcite.avatica.Meta; import org.apache.calcite.avatica.MissingResultsException; import org.apache.calcite.avatica.NoSuchStatementException; +import org.apache.calcite.avatica.SqlType; import org.apache.druid.guice.LazySingleton; import org.apache.druid.guice.LifecycleModule; import org.apache.druid.guice.StartupInjectorBuilder; @@ -56,6 +57,7 @@ import org.apache.druid.query.BaseQuery; import org.apache.druid.query.DefaultQueryConfig; import org.apache.druid.query.QueryRunnerFactoryConglomerate; +import org.apache.druid.query.http.ClientSqlParameter; import org.apache.druid.query.policy.NoopPolicyEnforcer; import org.apache.druid.segment.join.JoinableFactoryWrapper; import org.apache.druid.server.DruidNode; @@ -1351,6 +1353,7 @@ public void testSqlRequestLogPrepared() throws SQLException @Test public void testParameterBinding() throws SQLException { + testRequestLogger.clear(); try (PreparedStatement statement = client.prepareStatement( "SELECT COUNT(*) AS cnt FROM druid.foo WHERE dim1 = ? OR dim1 = ?")) { statement.setString(1, "abc"); @@ -1363,6 +1366,14 @@ public void testParameterBinding() throws SQLException ), rows ); + Assert.assertEquals(1, testRequestLogger.getSqlQueryLogs().size()); + Assert.assertEquals( + List.of( + new ClientSqlParameter(SqlType.VARCHAR.toString(), "abc"), + new ClientSqlParameter(SqlType.VARCHAR.toString(), "def") + ), + testRequestLogger.getSqlQueryLogs().get(0).getSqlParameters() + ); } } From 2123593ad8983d03db3bd0d64cd969da64f3e218 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 27 Feb 2026 16:59:06 -0800 Subject: [PATCH 2/2] fixes --- .../server/log/DefaultRequestLogEvent.java | 4 ++ .../log/DefaultRequestLogEventTest.java | 42 ++++++++++--------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/log/DefaultRequestLogEvent.java b/server/src/main/java/org/apache/druid/server/log/DefaultRequestLogEvent.java index 7089049b9aa6..f2274c9047a8 100644 --- a/server/src/main/java/org/apache/druid/server/log/DefaultRequestLogEvent.java +++ b/server/src/main/java/org/apache/druid/server/log/DefaultRequestLogEvent.java @@ -19,6 +19,7 @@ package org.apache.druid.server.log; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonValue; import com.google.common.collect.ImmutableMap; @@ -29,6 +30,7 @@ import org.apache.druid.server.RequestLogLine; import org.joda.time.DateTime; +import javax.annotation.Nullable; import java.util.List; import java.util.Map; @@ -114,7 +116,9 @@ public String getSql() return request.getSql(); } + @Nullable @JsonProperty("sqlParameters") + @JsonInclude(JsonInclude.Include.NON_NULL) public List getSqlParameters() { return request.getSqlParameters(); diff --git a/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java b/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java index 71f1af6cd7e0..9b4807f9ab14 100644 --- a/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java +++ b/server/src/test/java/org/apache/druid/server/log/DefaultRequestLogEventTest.java @@ -55,29 +55,32 @@ public class DefaultRequestLogEventTest public void testDefaultRequestLogEventSerde() throws Exception { RequestLogLine nativeLine = RequestLogLine.forNative( - new TimeseriesQuery( - new TableDataSource("dummy"), - new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2015-01-01/2015-01-02"))), - true, - VirtualColumns.EMPTY, - null, - Granularities.ALL, - ImmutableList.of(), - ImmutableList.of(), - 5, - ImmutableMap.of("key", "value")), - DateTimes.of(2019, 12, 12, 3, 1), - "127.0.0.1", - new QueryStats(ImmutableMap.of("query/time", 13L, "query/bytes", 10L, "success", true, "identity", "allowAll")) + new TimeseriesQuery( + new TableDataSource("dummy"), + new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2015-01-01/2015-01-02"))), + true, + VirtualColumns.EMPTY, + null, + Granularities.ALL, + ImmutableList.of(), + ImmutableList.of(), + 5, + ImmutableMap.of("key", "value") + ), + DateTimes.of(2019, 12, 12, 3, 1), + "127.0.0.1", + new QueryStats(ImmutableMap.of("query/time", 13L, "query/bytes", 10L, "success", true, "identity", "allowAll")) ); DefaultRequestLogEvent defaultRequestLogEvent = new DefaultRequestLogEvent( - ImmutableMap.of("service", "druid-service", "host", "127.0.0.1"), - "feed", - nativeLine); + ImmutableMap.of("service", "druid-service", "host", "127.0.0.1"), + "feed", + nativeLine + ); String logEventJson = objectMapper.writeValueAsString(defaultRequestLogEvent); - String expected = "{\"feed\":\"feed\",\"query\":{\"queryType\":\"timeseries\",\"dataSource\":{\"type\":\"table\",\"name\":\"dummy\"}," + String expected = + "{\"feed\":\"feed\",\"query\":{\"queryType\":\"timeseries\",\"dataSource\":{\"type\":\"table\",\"name\":\"dummy\"}," + "\"intervals\":{\"type\":\"intervals\",\"intervals\":[\"2015-01-01T00:00:00.000Z/2015-01-02T00:00:00.000Z\"]}," + "\"descending\":true,\"granularity\":{\"type\":\"all\"},\"limit\":5," + "\"context\":{\"key\":\"value\"}},\"host\":\"127.0.0.1\",\"timestamp\":\"2019-12-12T03:01:00.000Z\"," @@ -103,7 +106,8 @@ public void testDefaultRequestLogEventToMap() ImmutableList.of(), ImmutableList.of(), 5, - ImmutableMap.of("key", "value")); + ImmutableMap.of("key", "value") + ); final QueryStats queryStats = new QueryStats( ImmutableMap.of("query/time", 13L, "query/bytes", 10L, "success", true, "identity", "allowAll")); RequestLogLine nativeLine = RequestLogLine.forNative(