shivam-startree opened a new pull request, #19453: URL: https://github.com/apache/pinot/pull/19453
## Summary Collapses CR/LF runs in a query to a single space before it is logged, so one logged query occupies one physical log line. ## Motivation Clients routinely submit pretty-printed SQL — JDBC and BI tools do so by default — and the broker logs the query verbatim. A single log *event* then renders as one physical *line per line of SQL* as soon as anything downstream splits on newlines: a container runtime, a log shipper, a file tailer. Measured on one production broker, 60s window, 3,995 queries: | | lines | per query | |---|---|---| | `SQL query for request <id>:` headers | 3,995 | 1 | | continuation lines after them | 30,975 | 7.8 | | completion stats lines | 3,994 | 1 | | continuation lines after those | 30,969 | 7.8 | | **total continuation lines** | **61,944** | **79.4% of that broker's entire log volume** | Collapsing newlines removes those 61,944 lines while keeping both log records and every field they carry — 2 lines per query instead of 15.5. The continuation lines are also harmful beyond their volume: they carry no timestamp, level or logger name, so nothing downstream can filter, sample or attribute them, and a multiline log collector will attach them to whichever record happened to precede them — silently corrupting an unrelated log entry as well as the query. We observed exactly that: SQL fragments glued onto the tail of unrelated broker log lines. ## Implementation Applied inside `redactQuery`, which every query-logging call site in the broker already routes through — roughly 45 of them across `QueryLogger`, `BaseSingleStageBrokerRequestHandler` and `MultiStageBrokerRequestHandler`. Every caller is a log statement, so normalising there is consistent with the method's purpose, and the error/cancellation paths get the same treatment rather than only the two `QueryLogger` sites. ## Backward compatibility No config changes, no new options, no API changes. Deliberately conservative: **only line breaks are touched.** Indentation and spacing within a line are left alone, so the logged text stays as close to what the client submitted as a single-line rendering allows and remains copy-pasteable. `SqlRedactionMode.FULL` and `LITERAL_VALUES` are unaffected — neither returns a multi-line value. The one behavioural edge: a newline inside a string literal becomes a space in the log output. That is already true of any single-line log rendering, and collapsing *all* whitespace (the alternative) would additionally mangle intra-literal spacing — which is why this only touches CR/LF. ## Tests Two tests, one per log site (`logQueryReceived` and `logQueryCompleted`), plus a `generateParams` overload so a test can supply the query text. ## Note for reviewers I was unable to build locally — `master` now requires JDK 25 and only 21 was available on this machine, and the `-Djdk.version=21` fallback fails on dependency class-file versions. The regex output was verified character-for-character against the assertion strings, but I am relying on CI for compile and test. Opened as a draft for that reason; happy to mark ready once CI is green. ## Related Companion draft adds a `QUERY_RECEIVED` marker so the two per-query records become separately routable. Independent of this change — that one affects addressability, this one affects rendering. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
