Skip to content

QVAC-20891 feat[bc]: silence SDK and native logs by default, opt in to surface them#2653

Merged
arun-mani-j merged 3 commits into
tetherto:mainfrom
arun-mani-j:silenceLogs
Jun 18, 2026
Merged

QVAC-20891 feat[bc]: silence SDK and native logs by default, opt in to surface them#2653
arun-mani-j merged 3 commits into
tetherto:mainfrom
arun-mani-j:silenceLogs

Conversation

@arun-mani-j

@arun-mani-j arun-mani-j commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🎯 What problem does this PR solve?

  • The SDK was verbose by default — client, server, and native addon logs printed to the console out of the box, and silencing them required configuration.
  • Logs should be opt-in: by default the SDK emits nothing to the console, and users enable exactly what they want.

📝 How does it solve it?

  • The SDK's client and server loggers (and per-model / RAG stream loggers) default to console-off. Level stays info, so loggingStream / transports still receive everything; a consumer's own getLogger("my-app") is unaffected.
  • Console output is opt-in via loggerConsoleOutput: true in qvac.config.*; loggerLevel also accepts "off" to silence capture entirely (console, streams, and transports). The default lives in the logger factories rather than a schema default, because a missing config file is never parsed.
  • Native addon logs go through the same loggers: structured logs from each addon's JS callback reach console-off stream loggers at their real level, so loggingStream / transports still receive them. The worker's raw stderr (ggml backends that write directly) is captured client-side and re-emitted at debug under the sdk:server logger, surfacing through console output when enabled and the crash-diagnostic stderr tail. Both honor loggerConsoleOutput / loggerLevel, so native output needs no separate switch.
    • Why this route: the loud native output is ggml backend init writing straight to stderr, which bypasses the addon log callback and the per-model verbosity gate (and verbosity exists for only some engines). Routing the worker stderr through the logger silences every engine uniformly with no native-addon changes, and removes the last place the SDK wrote to the console directly. The captured stderr tail still backs crash diagnostics.
  • The runtime library routes all diagnostics through the logger — no direct console/stdout/stderr writes remain, except the logging system's own recursion-safe fallbacks.

🧪 How was it tested?

  • New test/unit/logging-defaults.test.ts: SDK loggers are console-off by default, transports still receive info logs, enableConsole opts back in, and logLevelSchema accepts "off".
  • worker-startup-error.test.ts passes — crash diagnostics use the captured stderr tail.
  • bun run build (lint + typecheck + compile) passes.

💥 Breaking Changes

BEFORE:

// SDK and native (llama.cpp / ggml) logs print to the console by default.
await loadModel({ modelSrc: LLAMA_3_2_1B_INST_Q4_0 });
// → console fills with SDK + native output

AFTER:

// Silent by default — no console output.
await loadModel({ modelSrc: LLAMA_3_2_1B_INST_Q4_0 });

// Opt in:
//   qvac.config.json → { "loggerConsoleOutput": true }                          // SDK logs
//   qvac.config.json → { "loggerConsoleOutput": true, "loggerLevel": "debug" }  // + native backend output
//   loggingStream({ id: SDK_LOG_ID })                                           // capture SDK logs programmatically

@arun-mani-j arun-mani-j requested review from a team as code owners June 17, 2026 08:36
@arun-mani-j arun-mani-j added test-e2e-smoke Triggers smoke e2e test suite [Currently SDK-only] verified Authorize secrets / label-gate in PR workflows labels Jun 17, 2026
@arun-mani-j arun-mani-j changed the title feat[bc|notask]: silence SDK and native logs by default, opt in to surface them QVAC-20891 feat[bc]: silence SDK and native logs by default, opt in to surface them Jun 17, 2026

@simon-iribarren simon-iribarren left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

Blocking: loggerLevel: "off" still emits to streams and transports

packages/sdk/schemas/logging-stream.ts now accepts "off" as a public config/logging level, and the PR body says this should silence capture entirely. The SDK wrapper gate still compares priorities directly in packages/sdk/logging/utils.ts:

return messagePriority <= currentPriority;

Because @qvac/logging defines off with priority 4, every normal level still passes that comparison (error=0, debug=3, off=4). createBaseLogger() may skip console output because the wrapped QvacLogger handles off, but it still calls stream callbacks and custom transports. Consumers who set loggerLevel: "off" will still receive SDK logs through loggingStream() / transports, which contradicts the documented behavior.

Please update the SDK gate so current level "off" disables all emissions, and add coverage that verifies console, loggingStream(), and transports receive no messages at "off".

Major: raw worker stderr is not routed to loggingStream()

packages/sdk/client/rpc/node-rpc-client.ts creates workerLogger with getLogger(SDK_SERVER_NAMESPACE, { enableConsole: false }) and logs stderr chunks with workerLogger.debug(...). That logger is registered for global level/console toggles, but it is not a stream logger. Only createStreamLogger() wires onLog to sendLogToStreams().

Impact: loggerConsoleOutput: true plus loggerLevel: "debug" can surface raw worker stderr to the console after config applies, but loggingStream({ id: SDK_LOG_ID }) will not receive those raw native stderr lines. The PR description says native stderr is re-emitted under the existing sdk:server logger and that stream/transports still receive logs; the implementation does not satisfy that for this path.

Please either route this through a stream logger path, or explicitly document that raw worker stderr can only be surfaced through console output and crash diagnostics, not loggingStream().

CI Status

Checked with gh pr checks 2653 --repo tetherto/qvac: all relevant PR checks are passing, including SDK Pod Checks, Docs Website Build, SDK build, bare-sdk build, and PR validation. Publish/release jobs are skipped as expected for a PR.

API Surface & Tagging

This changes public SDK logging behavior and config semantics, including the default console behavior and the accepted loggerLevel values. The [bc] title tag is appropriate, and the PR body includes BEFORE/AFTER migration evidence plus docs updates.

Recommendation

Request changes until the "off" implementation and worker stderr streaming semantics are corrected or documented accurately.

@arun-mani-j

Copy link
Copy Markdown
Contributor Author

@simon-iribarren thanks for the review.

Blocking — loggerLevel: "off" still emits to streams and transports — fixed.
isLevelEnabled in logging/utils.ts now short-circuits to false when the current level is "off", so the base logger's single gate stops console, the stream callback, and transports together. Added coverage in test/unit/logging-defaults.test.ts (level off: silences console, stream, and transports together) that drives a logger with console enabled, a transport, and a stream onLog, then asserts zero emissions at "off" across all three.

raw worker stderr is not routed to loggingStream()
Previously, the server (worker)'s stderr was captured raw and written to client's stderr as it is. Now we write them to debug logs. We can not redirect them to loggingStream again as they are in client scope now (and not in the server's scope where loggingStream is). The PR description was a little confusing on this part, I have updated it now.

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Tier-based Approval Status

**PR Tier:** TIER1

**Current Status:** ✅ APPROVED

**Requirements:**
- 1 Team Member approval ✅ (1/1)
- 1 Team Lead OR Management approval ✅ (1/1)



---
*This comment is automatically updated when reviews change.*

@arun-mani-j arun-mani-j added test-e2e-smoke Triggers smoke e2e test suite [Currently SDK-only] and removed test-e2e-smoke Triggers smoke e2e test suite [Currently SDK-only] labels Jun 18, 2026
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — ios — ✅ all tests passed (84/98, 1184s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts: reports · Device Farm logs

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — android — ✅ all tests passed (86/98, 2778s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts: reports · Device Farm logs

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — windows — ✅ all tests passed (98/98, 394s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts: reports

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — linux⚠️ no results

Config: suite=smoke · filter=(none) · exclude=(none)
View run

The test job did not produce a results artifact. Check the run for job-level failures.

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

QVAC E2E — macos — ✅ all tests passed (98/98, 279s)

Config: suite=smoke · filter=(none) · exclude=(none)
View run · Artifacts: reports

Comment thread packages/sdk/client/rpc/node-rpc-client.ts
@arun-mani-j

Copy link
Copy Markdown
Contributor Author

review

Signed-off-by: Arun Mani J <j.arunmani@proton.me>
Signed-off-by: Arun Mani J <j.arunmani@proton.me>
…ilent by default

Signed-off-by: Arun Mani J <j.arunmani@proton.me>
@arun-mani-j

Copy link
Copy Markdown
Contributor Author

review

@arun-mani-j arun-mani-j merged commit feb738d into tetherto:main Jun 18, 2026
16 checks passed
@arun-mani-j arun-mani-j deleted the silenceLogs branch June 23, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test-e2e-smoke Triggers smoke e2e test suite [Currently SDK-only] verified Authorize secrets / label-gate in PR workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants