fix startup_console_log_level removal

[garrettthomaskth]

Closes: #103472 (auto-closes the issue when this PR is merged into the default branch)
Related: #95919

Fixed logger.startup_console_log_level not being reverted after startup: the console log level was captured from the wrong config key, so the console stayed at the elevated startup level for the lifetime of the server. Introduced in #95919. Fixes #103472.

Fix
Capture the actual logger.console_log_level before overriding it.
On restore, since an unset console_log_level dynamically follows logger.level (see Loggers::updateLevels):
if console_log_level was explicitly configured, restore that value;
if it was unset, remove the temporary startup override so it falls back to logger.level again (this also avoids restoring an empty string, which would break parseLevel).

Changelog category (leave one):

• Not for changelog (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

...

Version info

• Merged into: 26.7.1.743 (included in 26.7 and later)
• Backported to: 26.6.2.63, 26.5.6.35, 26.4.5.124, 26.3.17.40

[clickhouse-gh]

Workflow [PR], commit [9fb6857]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR fixes the startup restore path for logger.console_log_level by saving the real pre-startup value and removing the temporary override when the setting was originally unset. The server-side change looks correct, but the new integration coverage does not actually turn on console logging for these stay_alive instances, so it does not verify the user-visible behavior the PR claims to fix.

PR Metadata

• Changelog category should be Bug Fix, not Not for changelog. This is user-visible misbehavior in an official stable release, which matches the template's Bug Fix category.
• Changelog entry is required for Bug Fix, and the current ... placeholder is not sufficient.
• Suggested replacement: Fixes \logger.startup_console_log_level` so the console log level is restored after startup instead of remaining at the temporary startup level.`

Findings

⚠️ Majors

• [tests/integration/test_server_startup_and_shutdown_logs/test.py:40-53] The new regression test never enables console logging. stay_alive=True instances are started via --daemon, and neither new config sets logger.console, so Loggers::updateLevels disables the console channel entirely in these cases. The assertions therefore only validate the file-log message emitted by Server.cpp; they do not prove that console output actually falls back from the startup level after initialization, which is the contract of this fix. Suggested fix: enable logger.console in the new configs and assert on stderr.log, container logs, or another console-only observable after startup.

Final Verdict

Changes requested. The runtime fix looks right, but the regression test should exercise a real console channel before this is merged, and the PR metadata should be updated to use the Bug Fix changelog category with a real entry.

[clickhouse-gh]

These nodes are stay_alive=True, so the integration harness launches them via --daemon (tests/integration/helpers/cluster.py:4942-4950, tests/integration/helpers/cluster.py:6376-6377). Since neither new config sets logger.console, Loggers::updateLevels takes the split->setLevel("Console", 0) branch in that mode (src/Loggers/Loggers.cpp:355-366).

That means the assertions here only prove that Server.cpp wrote the expected message into clickhouse-server.log; they do not exercise the actual console channel or the user-visible misbehavior from #103472. Please enable logger.console in these cases and assert on stderr.log / container logs (or another console-only observable) so the regression test proves that console verbosity really drops back after startup.

[bharatnc]

I think we might need to enable <console>true</console> in the logging configs.

[bharatnc]

I think a slightly better test would be to enable the console and then assert on the stderr.log like the bot suggests. But I think that's unnecessary since the test already catches the regression correctly. I think we can add this test later if needed.

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	79.50%	85.80%	+6.30%
Functions	91.80%	92.70%	+0.90%
Branches	71.60%	77.90%	+6.30%

Changed lines: Changed C/C++ lines covered: 10/13 (76.92%) · Uncovered code

Full report · Diff report

[bharatnc]

Looks good to me.

[bharatnc]

Also absolutely no flaky tests - all tests are green!