Skip to content

Commit fead687

Browse files
abnabn
committed
fix(logging): scope startup validation to logging settings and clarify size bounds
1 parent d674c05 commit fead687

7 files changed

Lines changed: 421 additions & 89 deletions

File tree

‎CMakeLists.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2940,6 +2940,13 @@ if(BUILD_TESTING AND EXISTS "${_CONFIG_DEFAULT_SOURCE_TEST_SRC}")
29402940
add_cpp_ci_test(ConfigDefaultSourceTest CI ON COMMAND test_config_default_source)
29412941
endif()
29422942

2943+
set(_LOG_ROTATION_TEST_SRC "${CMAKE_CURRENT_SOURCE_DIR}/test/cpp/test_log_rotation.cpp")
2944+
if(BUILD_TESTING AND EXISTS "${_LOG_ROTATION_TEST_SRC}")
2945+
add_executable(test_log_rotation test/cpp/test_log_rotation.cpp)
2946+
target_link_libraries(test_log_rotation PRIVATE lemonade-server-core)
2947+
add_cpp_ci_test(LogRotationTest CI ON COMMAND test_log_rotation)
2948+
endif()
2949+
29432950
# ROCm root resolution (ROCM_PATH / rocm-sdk / /opt/rocm priority): covers the
29442951
# external-ROCm detection that lets Lemonade skip the bundled TheRock download.
29452952
set(_ROCM_ROOT_TEST_SRC "${CMAKE_CURRENT_SOURCE_DIR}/test/cpp/test_rocm_root_resolution.cpp")

‎docs/guide/configuration/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,8 @@ Values set in the user's `config.json` always take precedence over these seeded
181181
| `host` | string | "localhost" | Address to bind for connections |
182182
| `log_level` | string | "info" | Logging level (trace, debug, info, warning, error, fatal, none) |
183183
| `log_file` | string | "auto" | File logging mode: "auto" (console-only for direct server runs, lemonade-server.log for embedded tray app), "disabled", "enabled", or custom target file path |
184-
| `log_max_file_size_mb` | int | 10 | Max active log file size in MB before triggering log rotation |
185-
| `log_max_files` | int | 5 | Max number of rotated log backup files to retain (.1 through .N) |
184+
| `log_max_file_size_mb` | int | 10 | Max active log file size in MB before triggering rotation (steady-state footprint bounded to ~`log_max_file_size_mb * (log_max_files + 1)`) |
185+
| `log_max_files` | int | 5 | Max number of rotated log backup files to retain (.1 through .N); legacy oversized files are rotated into .1 and pruned over cycles |
186186
| `global_timeout` | int | 600 | Timeout in seconds for HTTP, inference, and readiness checks |
187187
| `max_loaded_models` | int | 1 | Max models per type slot. Use -1 for unlimited |
188188
| `broadcast` | bool | true | Enable or disable UDP broadcasting for server discovery |

‎src/cpp/include/lemon/logging_config.h‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
#pragma once
22

3+
#include "lemon/utils/aixlog.hpp"
4+
5+
#include <fstream>
6+
#include <mutex>
37
#include <optional>
48
#include <string>
59

@@ -24,6 +28,30 @@ struct LoggingTargets {
2428
LogRotationConfig rotation;
2529
};
2630

31+
class RotatingFileSink : public AixLog::SinkFormat {
32+
public:
33+
RotatingFileSink(const AixLog::Filter& filter,
34+
const std::string& filename,
35+
const std::string& format,
36+
size_t max_file_size_mb,
37+
size_t max_files);
38+
~RotatingFileSink() override;
39+
40+
void log(const AixLog::Metadata& metadata, const std::string& message) override;
41+
42+
size_t current_size() const;
43+
44+
private:
45+
void rotate_if_needed_nolock();
46+
47+
std::string filename_;
48+
size_t max_file_size_bytes_;
49+
size_t max_files_;
50+
size_t current_size_{0};
51+
std::ofstream file_;
52+
mutable std::mutex mutex_;
53+
};
54+
2755
LoggingTargets resolve_logging_targets(LoggingMode mode, const LogRotationConfig& rotation = {});
2856
void configure_application_logging(const std::string& log_level, LoggingMode mode, const LogRotationConfig& rotation = {});
2957
void reconfigure_application_logging(const std::string& log_level, const LogRotationConfig& rotation = {});

‎src/cpp/server/logging_config.cpp‎

Lines changed: 94 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -38,119 +38,126 @@ class HubPublishingSink : public AixLog::SinkFormat {
3838
}
3939
};
4040

41-
class RotatingFileSink : public AixLog::SinkFormat {
42-
public:
43-
RotatingFileSink(const AixLog::Filter& filter,
44-
const std::string& filename,
45-
const std::string& format,
46-
size_t max_file_size_mb,
47-
size_t max_files)
48-
: AixLog::SinkFormat(filter, format),
49-
filename_(filename),
50-
max_file_size_bytes_(std::clamp(max_file_size_mb, static_cast<size_t>(1), static_cast<size_t>(2048)) * 1024 * 1024),
51-
max_files_(std::min(max_files, static_cast<size_t>(100))) {
52-
std::error_code ec;
53-
fs::path p = utils::path_from_utf8(filename_);
54-
if (fs::exists(p, ec)) {
55-
current_size_ = static_cast<size_t>(fs::file_size(p, ec));
56-
}
41+
} // namespace
42+
43+
RotatingFileSink::RotatingFileSink(const AixLog::Filter& filter,
44+
const std::string& filename,
45+
const std::string& format,
46+
size_t max_file_size_mb,
47+
size_t max_files)
48+
: AixLog::SinkFormat(filter, format),
49+
filename_(filename),
50+
max_file_size_bytes_(max_file_size_mb * 1024 * 1024),
51+
max_files_(max_files) {
52+
if (max_file_size_mb < 1 || max_file_size_mb > 2048) {
53+
throw std::invalid_argument("'log_max_file_size_mb' must be between 1 and 2048");
54+
}
55+
if (max_files > 100) {
56+
throw std::invalid_argument("'log_max_files' must be between 0 and 100");
57+
}
5758

59+
std::error_code ec;
60+
fs::path p = utils::path_from_utf8(filename_);
61+
if (p.has_parent_path()) {
62+
fs::create_directories(p.parent_path(), ec);
63+
}
64+
if (fs::exists(p, ec)) {
65+
current_size_ = static_cast<size_t>(fs::file_size(p, ec));
66+
}
67+
68+
if (max_file_size_bytes_ > 0 && current_size_ >= max_file_size_bytes_) {
69+
rotate_if_needed_nolock();
70+
} else {
5871
file_.open(p, std::ofstream::out | std::ofstream::app | std::ofstream::binary);
72+
}
73+
}
5974

60-
if (max_file_size_bytes_ > 0 && current_size_ >= max_file_size_bytes_) {
61-
rotate_if_needed_nolock();
62-
}
75+
RotatingFileSink::~RotatingFileSink() {
76+
std::lock_guard<std::mutex> lock(mutex_);
77+
if (file_.is_open()) {
78+
file_.flush();
79+
file_.close();
6380
}
81+
}
6482

65-
~RotatingFileSink() override {
66-
std::lock_guard<std::mutex> lock(mutex_);
67-
if (file_.is_open()) {
68-
file_.flush();
69-
file_.close();
70-
}
83+
void RotatingFileSink::log(const AixLog::Metadata& metadata, const std::string& message) {
84+
std::ostringstream stream;
85+
do_log(stream, metadata, message);
86+
87+
std::string formatted = stream.str();
88+
if (!formatted.empty() && formatted.back() == '\n') {
89+
formatted.pop_back();
7190
}
7291

73-
void log(const AixLog::Metadata& metadata, const std::string& message) override {
74-
std::ostringstream stream;
75-
do_log(stream, metadata, message);
92+
size_t line_len = formatted.size() + 1; // +1 for newline
7693

77-
std::string formatted = stream.str();
78-
if (!formatted.empty() && formatted.back() == '\n') {
79-
formatted.pop_back();
80-
}
94+
std::lock_guard<std::mutex> lock(mutex_);
95+
if (!file_.is_open()) {
96+
return;
97+
}
8198

82-
size_t line_len = formatted.size() + 1; // +1 for newline
99+
if (max_file_size_bytes_ > 0 && current_size_ > 0 && (current_size_ + line_len) >= max_file_size_bytes_) {
100+
rotate_if_needed_nolock();
101+
}
83102

84-
std::lock_guard<std::mutex> lock(mutex_);
85-
if (!file_.is_open()) {
86-
return;
87-
}
103+
file_ << formatted << '\n';
104+
file_.flush();
105+
current_size_ += line_len;
106+
}
88107

89-
if (max_file_size_bytes_ > 0 && current_size_ > 0 && (current_size_ + line_len) >= max_file_size_bytes_) {
90-
rotate_if_needed_nolock();
91-
}
108+
size_t RotatingFileSink::current_size() const {
109+
std::lock_guard<std::mutex> lock(mutex_);
110+
return current_size_;
111+
}
92112

93-
file_ << formatted << '\n';
113+
void RotatingFileSink::rotate_if_needed_nolock() {
114+
if (file_.is_open()) {
94115
file_.flush();
95-
current_size_ += line_len;
116+
file_.close();
96117
}
97118

98-
private:
99-
void rotate_if_needed_nolock() {
100-
if (file_.is_open()) {
101-
file_.flush();
102-
file_.close();
103-
}
119+
std::error_code ec;
120+
fs::path active_log = utils::path_from_utf8(filename_);
104121

105-
if (max_files_ > 0) {
106-
std::error_code ec;
107-
fs::path max_backup = utils::path_from_utf8(filename_ + "." + std::to_string(max_files_));
108-
if (fs::exists(max_backup, ec)) {
109-
fs::remove(max_backup, ec);
110-
}
122+
if (max_files_ > 0) {
123+
fs::path max_backup = utils::path_from_utf8(filename_ + "." + std::to_string(max_files_));
124+
if (fs::exists(max_backup, ec)) {
125+
fs::remove(max_backup, ec);
126+
}
111127

112-
for (size_t i = max_files_; i > 1; --i) {
113-
fs::path dst = utils::path_from_utf8(filename_ + "." + std::to_string(i));
114-
fs::path src = utils::path_from_utf8(filename_ + "." + std::to_string(i - 1));
115-
if (fs::exists(src, ec)) {
116-
if (fs::exists(dst, ec)) {
117-
fs::remove(dst, ec);
118-
}
119-
fs::rename(src, dst, ec);
128+
for (size_t i = max_files_; i > 1; --i) {
129+
fs::path dst = utils::path_from_utf8(filename_ + "." + std::to_string(i));
130+
fs::path src = utils::path_from_utf8(filename_ + "." + std::to_string(i - 1));
131+
if (fs::exists(src, ec)) {
132+
if (fs::exists(dst, ec)) {
133+
fs::remove(dst, ec);
120134
}
135+
fs::rename(src, dst, ec);
121136
}
137+
}
122138

123-
fs::path active_log = utils::path_from_utf8(filename_);
124-
fs::path backup_1 = utils::path_from_utf8(filename_ + ".1");
125-
if (fs::exists(active_log, ec)) {
126-
if (fs::exists(backup_1, ec)) {
127-
fs::remove(backup_1, ec);
128-
}
129-
fs::rename(active_log, backup_1, ec);
139+
fs::path backup_1 = utils::path_from_utf8(filename_ + ".1");
140+
if (fs::exists(active_log, ec)) {
141+
if (fs::exists(backup_1, ec)) {
142+
fs::remove(backup_1, ec);
130143
}
144+
fs::rename(active_log, backup_1, ec);
131145
}
146+
}
132147

133-
fs::path p = utils::path_from_utf8(filename_);
134-
file_.open(p, std::ofstream::out | std::ofstream::trunc | std::ofstream::binary);
135-
if (!file_.is_open()) {
136-
file_.open(p, std::ofstream::out | std::ofstream::app | std::ofstream::binary);
137-
}
148+
file_.open(active_log, std::ofstream::out | std::ofstream::trunc | std::ofstream::binary);
149+
if (!file_.is_open()) {
150+
file_.open(active_log, std::ofstream::out | std::ofstream::app | std::ofstream::binary);
151+
}
138152

139-
std::error_code ec;
140-
if (fs::exists(p, ec)) {
141-
current_size_ = static_cast<size_t>(fs::file_size(p, ec));
142-
} else {
143-
current_size_ = 0;
144-
}
153+
if (fs::exists(active_log, ec)) {
154+
current_size_ = static_cast<size_t>(fs::file_size(active_log, ec));
155+
} else {
156+
current_size_ = 0;
145157
}
158+
}
146159

147-
std::string filename_;
148-
size_t max_file_size_bytes_;
149-
size_t max_files_;
150-
size_t current_size_{0};
151-
std::ofstream file_;
152-
std::mutex mutex_;
153-
};
160+
namespace {
154161

155162
std::vector<std::shared_ptr<AixLog::Sink>> build_logging_sinks(
156163
const std::string& log_level,

‎src/cpp/server/runtime_config.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,20 @@ RuntimeConfig::RuntimeConfig(const json& config)
273273
config_.erase("no_broadcast");
274274
}
275275

276+
// Validate logging settings on startup
277+
if (config_.contains("log_max_file_size_mb")) {
278+
validate("log_max_file_size_mb", config_["log_max_file_size_mb"]);
279+
}
280+
if (config_.contains("log_max_files")) {
281+
validate("log_max_files", config_["log_max_files"]);
282+
}
283+
if (config_.contains("log_file")) {
284+
validate("log_file", config_["log_file"]);
285+
}
286+
if (config_.contains("log_level")) {
287+
validate("log_level", config_["log_level"]);
288+
}
289+
276290
// In CI mode, override log level to debug for easier diagnostics
277291
const char* ci_mode = std::getenv("LEMONADE_CI_MODE");
278292
if (ci_mode && (std::string(ci_mode) == "1" || std::string(ci_mode) == "true" ||

0 commit comments

Comments
 (0)