Add remote address tracking for JDBC/Avatica SQL queries#19231
Open
dprmfl wants to merge 1 commit intoapache:masterfrom
Open
Add remote address tracking for JDBC/Avatica SQL queries#19231dprmfl wants to merge 1 commit intoapache:masterfrom
dprmfl wants to merge 1 commit intoapache:masterfrom
Conversation
17dd93d to
5aac04a
Compare
test: verify remote address capture in Avatica SQL queries test: Replace forbidden API calls in DruidAvaticaHandlerTest test: Update test expectations to include remoteAddress in query context & Update Quidem test files for remote address tracking test: Fix checkstyle violations
1563f92 to
fc1d96f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #19230.
Description
remoteAddress is always
nullfor JDBC/Avatica queries, even though it's a documented dimension forsqlQuery/time,sqlQuery/bytes, and sqlQuery/planningTimeMs. This PR fixes that.Problem
See #19230. In short, remoteAddress was hardcoded as
nullinPreparedStatement.javaandSqlStatementFactory.javafor the Avatica path, whileSqlResource.java(the HTTP path) correctly passeshttpRequest.getRemoteAddr().
Solution
remoteAddrfrom the HTTP request and stores it in aThreadLocal. Cleans up infinallyto prevent memory leaks.ThreadLocalinopenConnection()and injectsremoteAddressinto the connection context map.remoteAddressfrom context and passes it toDirectStatement.remoteAddressas a dimension in metrics and includes it in request logs.No changes to
DruidConnectionor any existing data structures —remoteAddresspiggybacks on the context map that's already there.Testing
Added three test methods to DruidAvaticaHandlerTest:
Release note
sqlQuery/time,sqlQuery/bytes, and sqlQuery/planningTimeMs metrics now includeremoteAddressfor JDBC/Avatica queries, consistent with the HTTP SQL API.Key changed/added classes in this PR
DruidAvaticaJsonHandler— ThreadLocal-based remote address captureDruidMeta— connection context enrichment with remote addressSqlStatementFactory— remote address extraction and propagationPreparedStatement— remote address handling for prepared statementsSqlExecutionReporter— remote address emission in metrics and logsThis PR has: