fix(common-utils): route ClickHouse query debug logging through an injectable logger by truehazker · Pull Request #2679 · hyperdxio/hyperdx

truehazker · 2026-07-18T11:30:50Z

Summary

BaseClickhouseClient.logDebugQuery wrote the full SQL of every ClickHouse query to stdout via raw console.debug — unconditionally and outside the pino logger, so HYPERDX_LOG_LEVEL cannot suppress it. In self-hosted deployments with log capture this dominates pod logs and otel_logs retention (#2416).

This PR makes query logging silent by default and injectable, per review feedback: ClickhouseClientOptions accepts an optional customLogger (the Logger interface from @clickhouse/client-common), and the renamed logQuery emits through it — no logger, no output. The gate stays in the shared base-class method that every query path (node and browser clients) already routes through.

Per consumer:

app — injects the re-exported DefaultLogger in dev and local mode, where queries hit ClickHouse directly and the devtools console is the only place to see SQL. Production stays silent so the browser SDK's consoleCapture can't ship query SQL to telemetry.
api — intentionally injects nothing: silent-by-default fixes the flooding. A pino adapter can be passed at any client when actively debugging.
cli — its logDebugQuery-silencing override became dead code and is removed.

Emitting at trace — rather than debug — is deliberate:

Dev and self-hosted .env files already set HYPERDX_LOG_LEVEL=debug, so a future pino adapter emitting at debug would resurrect the exact spam this PR removes.
Per-query SQL is high-volume and can carry user data, so it shouldn't switch on as a side effect of ordinary verbosity; trace keeps it an explicit opt-in even once a logger is wired.

Also included: unit tests for the silent-by-default, injected-logger, and param-interpolation cases (yarn ci:unit in packages/common-utils), a changeset, and removal of the earlier HYPERDX_LOG_QUERIES plumbing from .env and docker-compose.

Maintainer notes:

No new env var, so nothing for the Helm chart or docs.
The changeset bumps @hyperdx/api without source changes: it bundles common-utils, so released API images stop emitting the per-query dump.

How to test on Vercel preview

N/A — backend logging behavior; in dev/local-mode builds, query SQL appears in the devtools console (at Verbose level).

References

Closes ClickHouseClient.logDebugQuery writes per-query SQL via raw console.debug — floods pod logs, no env gate #2416

BaseClickhouseClient.logDebugQuery dumped the full SQL of every ClickHouse query to the console unconditionally, outside the pino logger, so HYPERDX_LOG_LEVEL could not suppress it and API pod logs filled with per-query spam. Query logging is now off by default and enabled only with HYPERDX_LOG_QUERIES=true. The gate is one line in the shared base-class method every query path routes through, reusing the existing isNode helper so browser bundles stay silent. docker-compose passes the variable through to the all-in-one app container, and .env documents the toggle next to HYPERDX_LOG_LEVEL. Closes hyperdxio#2416

vercel · 2026-07-18T11:30:56Z

@truehazker is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

changeset-bot · 2026-07-18T11:30:57Z

🦋 Changeset detected

Latest commit: a550f21

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@hyperdx/common-utils	Patch
@hyperdx/api	Patch
@hyperdx/app	Patch
@hyperdx/otel-collector	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

greptile-apps · 2026-07-18T11:32:24Z

Greptile Summary

This PR makes ClickHouse query logging opt-in through an injectable logger. The main changes are:

Adds customLogger support to shared ClickHouse client options.
Routes node and browser query logging through the shared logQuery method.
Re-exports DefaultLogger for app callers that want query logs.
Enables query logs only in app dev or local mode.
Removes the CLI's stale query-log override.
Adds unit coverage for silent logging, injected logging, and parameter interpolation.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
packages/common-utils/src/clickhouse/index.ts	Adds the logger option, exports the logger types, and makes query logging silent unless a logger is passed.
packages/common-utils/src/clickhouse/node.ts	Updates the node query path to call the renamed query logging method.
packages/common-utils/src/clickhouse/browser.ts	Updates the browser query path to call the renamed query logging method.
packages/app/src/clickhouse.ts	Injects the default query logger for app dev and local-mode clients while preserving caller overrides.
packages/cli/src/api/client.ts	Removes the old CLI query-log override that no longer applies.
packages/common-utils/src/clickhouse/tests/index.test.ts	Adds unit tests for the new ClickHouse query logging behavior.
.changeset/gate-clickhouse-query-debug-log.md	Adds patch changesets for the packages affected by the logging change.

_{Reviews (4): Last reviewed commit: "Merge branch 'main' into claude/gate-cli..." | Re-trigger Greptile}

github-actions · 2026-07-18T11:36:45Z

Deep Review

Scope: PR #2679 — gate ClickHouse per-query SQL debug logging behind HYPERDX_LOG_QUERIES. Reviewed the PR-specific commits (3d02a56a..HEAD): 5 files, +56 lines. Note: the wrapper-supplied base 4b1aefb is ~40 commits stale and would fold in many already-merged PRs; the review was scoped to the actual PR changes to keep findings attributable to this diff.

Intent: BaseClickhouseClient.logDebugQuery previously dumped raw SQL to console.debug on every query, outside pino and unsuppressable by HYPERDX_LOG_LEVEL. The diff adds one guard — if (!isNode || process.env.HYPERDX_LOG_QUERIES !== 'true') return; — plus tests, a docker-compose passthrough, an .env example, and a changeset.

✅ No critical issues found. The guard sits on a void, side-effect-only method that both the node (node.ts:49) and browser (browser.ts:131) clients route through, so the early return cannot affect query execution or state. The browser stays silent by construction (!isNode), and the strict === 'true' check correctly rejects '', '1', 'TRUE', and unset — matching the added tests. Default-off is the safe and intended behavior, and the change strictly reduces data exposure.

🔵 P3 nitpicks (2)

packages/common-utils/src/clickhouse/index.ts:687 — HYPERDX_LOG_QUERIES is read via raw process.env inline in the shared base class rather than through a resolved-once config value.
- Fix: Read the flag through the codebase's existing env/config resolution path if one exists, or hoist it to a module-level constant, so the toggle is discoverable and consistently parsed.
packages/common-utils/src/clickhouse/__tests__/index.test.ts:141 — the new suite covers only the Node gate; the !isNode browser branch (silent even when the flag is 'true') has no direct test.
- Fix: Add a browser-client case asserting logDebugQuery stays silent when isNode is false so the cross-runtime guarantee is regression-protected.

Reviewers (2): ce-correctness-reviewer, orchestrator-analysis.

Testing gaps:

No test exercises the browser (@/clickhouse/browser) path to confirm it stays silent when HYPERDX_LOG_QUERIES=true; browser silence currently rests on the !isNode guard alone.
No test asserts a query still executes when logging is gated off (only the console.debug side-effect is asserted), and the query_params rendering branch is not exercised.

The passthrough referenced ${HYPERDX_LOG_QUERIES} while .env only ships the variable commented out, so every `docker compose up` printed a "variable is not set" warning. Use the :- default-empty form; the gate still requires an explicit 'true', so behavior is unchanged.

The disabled-state test asserted five logDebugQuery calls behind a single expect, so a failure would not name the offending value. Each non-'true' value ('', 'false', '1', 'TRUE') and the unset case now get their own test, matching the it.each idiom already used in this file.

wrn14897 · 2026-07-20T15:38:12Z

    query: string,
    query_params: Record<string, any> = {},
  ): void {
+    if (!isNode || process.env.HYPERDX_LOG_QUERIES !== 'true') return;


The SQL debugging logs are useful on the app side. What do you think about passing a custom logger (e.g. Pino for the API) through the client instead? We could add a customLogger option here:

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

Lines 624 to 633 in 7a4ad98

export type ClickhouseClientOptions = {

host?: string;

username?: string;

password?: string;

queryTimeout?: number;

/** Application name, used as the client's HTTP user-agent header */

application?: string;

/** Defines how long the client will wait for a response from the ClickHouse server before aborting the request, in milliseconds */

requestTimeout?: number;

};

ref: https://clickhouse.com/docs/integrations/javascript#logging-nodejs-only

I agree, I just didn't want to deal with nextjs env vars for logging purposes, but this way they're not the issue anymore. Let me dig into your proposal first, I'll update my PR once I check all the consumers (app, api, cli) and I'll leave a note in this thread.

Turned out nicely, updates are pushed, PR description updated. I've changed the logging level to trace by default to keep it silent and loud once tuning the log level on purpose. Let me know if renaming/level changing was extra and not needed. But I guess it makes things cleaner

…le customLogger Rework of the query-log gate per review feedback on hyperdxio#2679: instead of a dedicated env var, `ClickhouseClientOptions` now accepts an optional `customLogger` (the `Logger` interface from `@clickhouse/client-common`), and `logQuery` (was `logDebugQuery`) emits through it at `trace` — silent whenever no logger is injected. - The app injects the re-exported `DefaultLogger` in dev and local mode, where the devtools console is the only place to see query SQL; prod stays silent so consoleCapture can't ship SQL to telemetry. - The API intentionally injects nothing: silent by default fixes the log flooding, and a pino adapter can be passed at any client later (trace-level emit keeps HYPERDX_LOG_LEVEL=debug deployments quiet). - The CLI's silencing override is now dead code and removed. - HYPERDX_LOG_QUERIES is gone from .env and docker-compose.

…ub.com/truehazker/hyperdx into claude/gate-clickhouse-query-debug-log # Conflicts: # docker-compose.yml # packages/common-utils/src/clickhouse/__tests__/index.test.ts

github-actions Bot added the external label Jul 18, 2026

greptile-apps Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread packages/common-utils/src/clickhouse/index.ts Outdated

truehazker added 2 commits July 18, 2026 15:58

wrn14897 reviewed Jul 20, 2026

View reviewed changes

truehazker added 2 commits July 21, 2026 01:22

Merge branch 'claude/gate-clickhouse-query-debug-log' of https://gith…

de59dda

…ub.com/truehazker/hyperdx into claude/gate-clickhouse-query-debug-log # Conflicts: # docker-compose.yml # packages/common-utils/src/clickhouse/__tests__/index.test.ts

truehazker changed the title ~~fix(common-utils): make ClickHouse query debug logging opt-in~~ fix(common-utils): route ClickHouse query debug logging through an injectable logger Jul 20, 2026

Merge branch 'main' into claude/gate-clickhouse-query-debug-log

a550f21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

fix(common-utils): route ClickHouse query debug logging through an injectable logger#2679
truehazker wants to merge 6 commits into
hyperdxio:mainfrom
truehazker:claude/gate-clickhouse-query-debug-log

truehazker commented Jul 18, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Jul 18, 2026

Uh oh!

changeset-bot Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jul 18, 2026 •

edited

Loading

Important Files Changed

Uh oh!

Uh oh!

github-actions Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

wrn14897 Jul 20, 2026

Uh oh!

truehazker Jul 20, 2026 •

edited

Loading

Uh oh!

truehazker Jul 20, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	export type ClickhouseClientOptions = {
	host?: string;
	username?: string;
	password?: string;
	queryTimeout?: number;
	/** Application name, used as the client's HTTP user-agent header */
	application?: string;
	/** Defines how long the client will wait for a response from the ClickHouse server before aborting the request, in milliseconds */
	requestTimeout?: number;
	};

Uh oh!

Conversation

truehazker commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How to test on Vercel preview

References

Uh oh!

vercel Bot commented Jul 18, 2026

Uh oh!

changeset-bot Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

greptile-apps Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

github-actions Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deep Review

Uh oh!

wrn14897 Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

truehazker Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

truehazker Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

truehazker commented Jul 18, 2026 •

edited

Loading

changeset-bot Bot commented Jul 18, 2026 •

edited

Loading

greptile-apps Bot commented Jul 18, 2026 •

edited

Loading

github-actions Bot commented Jul 18, 2026 •

edited

Loading

truehazker Jul 20, 2026 •

edited

Loading

truehazker Jul 20, 2026 •

edited

Loading