fix(common-utils): route ClickHouse query debug logging through an injectable logger

.changeset/gate-clickhouse-query-debug-log.md

@@ -0,0 +1,21 @@
+---
+"@hyperdx/common-utils": patch
+"@hyperdx/api": patch
+"@hyperdx/app": patch
+---
+
+fix: route per-query SQL debug logging through an injectable logger (#2416)
+
+`BaseClickhouseClient.logDebugQuery` dumped raw SQL to the console on every
+ClickHouse query, unconditionally and outside the pino logger, flooding API
+logs (and, via the browser SDK's consoleCapture, telemetry) with query spam.
+
+Query logging is now silent by default and routed through an optional
+per-client `customLogger` on `ClickhouseClientOptions` (the `Logger` interface
+from `@clickhouse/client-common`). The app enables it in dev builds and in
+local mode, where queries hit ClickHouse directly and the devtools console is
+the only place to see them; anywhere else, pass a `customLogger` at the client
+you're debugging.
+
+`@hyperdx/api` is bumped without source changes: it bundles common-utils, so
+its released images stop emitting the per-query dump.

packages/app/src/clickhouse.ts

@@ -9,22 +9,29 @@ import {
   chSql,
   ClickhouseClientOptions,
   ColumnMeta,
+  DefaultLogger,
   ResponseJSON,
 } from '@hyperdx/common-utils/dist/clickhouse';
 import { ClickhouseClient } from '@hyperdx/common-utils/dist/clickhouse/browser';
 import { useQuery, UseQueryOptions } from '@tanstack/react-query';
 
-import { IS_LOCAL_MODE } from '@/config';
+import { IS_DEV, IS_LOCAL_MODE } from '@/config';
 import { getLocalConnections } from '@/connection';
 
 import api from './api';
 import { DEFAULT_QUERY_TIMEOUT } from './defaults';
 
 const PROXY_CLICKHOUSE_HOST = '/api/clickhouse-proxy';
 
+const queryLogger = IS_DEV || IS_LOCAL_MODE ? new DefaultLogger() : undefined;
+
 export const getClickhouseClient = (
   options: ClickhouseClientOptions = {},
 ): ClickhouseClient => {
+  options = {
+    customLogger: queryLogger,
+    ...options,
+  };
   if (IS_LOCAL_MODE) {
     const localConnections = getLocalConnections();
     if (localConnections.length === 0) {

packages/cli/src/api/client.ts

@@ -351,9 +351,6 @@ export class ProxyClickhouseClient extends BaseClickhouseClient {
     });
   }
 
-  // Silence the "Sending Query: ..." debug output from BaseClickhouseClient
-  protected override logDebugQuery(): void {}
-
   // This subclass always builds a node client, so narrow the base class's
   // platform-agnostic client type to the node-specific one.
   protected getClient(): NodeClickHouseClient {

packages/common-utils/src/clickhouse/__tests__/index.test.ts

@@ -5,6 +5,7 @@ import {
   isMissingColumnError,
   JSDataType,
 } from '@/clickhouse';
+import { ClickhouseClient } from '@/clickhouse/node';
 
 describe('isMissingColumnError', () => {
   it.each([
@@ -124,3 +125,57 @@ describe('convertCHDataTypeToJSType', () => {
     expect(convertCHDataTypeToJSType('Nullable(Bool)')).toBe(JSDataType.Bool);
   });
 });
+
+describe('BaseClickhouseClient.logQuery', () => {
+  const logQuery = (
+    client: ClickhouseClient,
+    query: string,
+    params?: Record<string, any>,
+  ) => (client as any).logQuery(query, params);
+  const makeLogger = () => ({
+    trace: jest.fn(),
+    debug: jest.fn(),
+    info: jest.fn(),
+    warn: jest.fn(),
+    error: jest.fn(),
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('stays silent when no customLogger is configured', () => {
+    const debugSpy = jest.spyOn(console, 'debug').mockImplementation(() => {});
+    const client = new ClickhouseClient({ host: 'http://localhost' });
+    logQuery(client, 'SELECT 1 FROM system.one');
+    expect(debugSpy).not.toHaveBeenCalled();
+  });
+
+  it('logs through the customLogger passed to the client', () => {
+    const customLogger = makeLogger();
+    const client = new ClickhouseClient({
+      host: 'http://localhost',
+      customLogger,
+    });
+    logQuery(client, 'SELECT 1 FROM system.one');
+    expect(customLogger.trace).toHaveBeenCalledWith({
+      module: 'clickhouse',
+      message: 'Sending query',
+      args: { sql: 'SELECT 1 FROM system.one' },
+    });
+  });
+
+  it('interpolates query_params into the logged SQL', () => {
+    const customLogger = makeLogger();
+    const client = new ClickhouseClient({
+      host: 'http://localhost',
+      customLogger,
+    });
+    logQuery(client, 'SELECT {id:Int32}', { id: 5 });
+    expect(customLogger.trace).toHaveBeenCalledWith({
+      module: 'clickhouse',
+      message: 'Sending query',
+      args: { sql: 'SELECT 5' },
+    });
+  });
+});

packages/common-utils/src/clickhouse/browser.ts

@@ -129,7 +129,7 @@ export class ClickhouseClient extends BaseClickhouseClient {
       this.client = this.buildClient();
     }
 
-    this.logDebugQuery(query, query_params);
+    this.logQuery(query, query_params);
 
     let clickhouseSettings: ClickHouseSettings | undefined;
     // If this is the settings query, we must not process the clickhouse settings, or else we will infinitely recurse

packages/common-utils/src/clickhouse/index.ts

@@ -3,6 +3,7 @@ import type {
   BaseResultSet,
   ClickHouseSettings,
   DataFormat,
+  Logger,
   ResponseHeaders,
   ResponseJSON,
   Row,
@@ -33,9 +34,11 @@ export type {
   BaseResultSet,
   ClickHouseSettings,
   DataFormat,
+  Logger,
   ResponseJSON,
   Row,
 };
+export { DefaultLogger } from '@clickhouse/client-common';
 
 export enum JSDataType {
   Array = 'array',
@@ -631,6 +634,8 @@ export type ClickhouseClientOptions = {
   application?: string;
   /** Defines how long the client will wait for a response from the ClickHouse server before aborting the request, in milliseconds */
   requestTimeout?: number;
+  /** Logger for per-query SQL debug output. When omitted, query logging is silent. */
+  customLogger?: Logger;
 };
 
 export abstract class BaseClickhouseClient {
@@ -647,6 +652,7 @@ export abstract class BaseClickhouseClient {
    */
   protected maxRowReadOnly: boolean;
   protected requestTimeout: number = 3600000;
+  protected readonly customLogger?: Logger;
 
   constructor({
     host,
@@ -655,13 +661,15 @@ export abstract class BaseClickhouseClient {
     queryTimeout,
     application,
     requestTimeout,
+    customLogger,
   }: ClickhouseClientOptions) {
     this.host = host!;
     this.username = username;
     this.password = password;
     this.queryTimeout = queryTimeout;
     this.maxRowReadOnly = false;
     this.application = application;
+    this.customLogger = customLogger;
     if (requestTimeout != null && requestTimeout >= 0) {
       this.requestTimeout = requestTimeout;
     }
@@ -680,22 +688,24 @@ export abstract class BaseClickhouseClient {
     await this.client?.close();
   }
 
-  protected logDebugQuery(
+  protected logQuery(
     query: string,
     query_params: Record<string, any> = {},
   ): void {
+    if (!this.customLogger) return;
+
     let debugSql = '';
     try {
       debugSql = parameterizedQueryToSql({ sql: query, params: query_params });
     } catch (e) {
       debugSql = query;
     }
 
-    console.debug('--------------------------------------------------------');
-
-    console.debug('Sending Query:', debugSql);
-
-    console.debug('--------------------------------------------------------');
+    this.customLogger.trace({
+      module: 'clickhouse',
+      message: 'Sending query',
+      args: { sql: debugSql },
+    });
   }
 
   protected async processClickhouseSettings({

packages/common-utils/src/clickhouse/node.ts

@@ -47,7 +47,7 @@ export class ClickhouseClient extends BaseClickhouseClient {
     queryId,
     shouldSkipApplySettings,
   }: QueryInputs<Format>): Promise<BaseResultSet<ReadableStream, Format>> {
-    this.logDebugQuery(query, query_params);
+    this.logQuery(query, query_params);
 
     let clickhouseSettings: ClickHouseSettings | undefined;
     // If this is the settings query, we must not process the clickhouse settings, or else we will infinitely recurse