Morgana future/fix/configuration parsing error mmap log buffer size #2428

MorganaFuture · 2025-05-21T14:06:58Z

What
This PR introduces a JSON-based configuration system for the Tempesta FW logger daemon with:

A dedicated TfwLoggerConfig class for logger settings
JSON configuration file support via Boost's property_tree
Two operation modes: daemon (background) and handle (foreground for debugging)
Support for a logger_config attribute in the access_log directive
Updates to Tempesta scripts for compatibility
Backward compatibility with existing configuration
Add tests

Why
The Tempesta logger configuration is currently scattered across multiple parameters, making it hard to manage. A dedicated JSON file provides a cleaner approach that separates logger settings from the main configuration. This structured format improves error handling and simplifies troubleshooting with the new foreground mode.

Links
#2313

krizhanovsky

A lot of logic was removed with no any comments, just as part of file recreation. Maybe it's better to recreate the PR from #2363 and redo all the changes with small commits with good messages - I affraid we'll just lose some of the logic.

logger/test/data/invalid_config.json

krizhanovsky · 2025-05-23T12:59:49Z

scripts/tempesta.sh

-			mmap_password=$(get_opt_value "$line" "mmap_password")
-
-			[[ -n "$mmap_log" && -n "$mmap_host" ]] ||
-				error "if mmaps enabled in access log, there have to be mmap_host and mmap_log options"
 		fi


Please update the wiki as requested in #2313 (comment)

P.S. I see the new wiki page in https://github.com/tempesta-tech/tempesta/wiki/TFW-Logger and we had a discussion on it

krizhanovsky · 2025-05-23T13:01:01Z

logger/test/Makefile

@@ -0,0 +1,71 @@
+#		Tempesta FW
+#
+# Copyright (C) 2024 Tempesta Technologies, Inc.


Suggested change

# Copyright (C) 2024 Tempesta Technologies, Inc.

# Copyright (C) 2025 Tempesta Technologies, Inc.

krizhanovsky · 2025-05-23T13:23:01Z

logger/Makefile

@@ -21,7 +21,7 @@ CLICKHOUSE_BUILD_DIR := $(CLICKHOUSE_DIR)/build
 CLICKHOUSE_REPO_URL := https://github.com/ClickHouse/clickhouse-cpp.git


Please update the copyright year in all the modified files. Git hub hook should do this automatically, so check your settings

krizhanovsky · 2025-05-23T13:25:58Z

logger/Makefile

+SRCS := tfw_logger.cc mmap_buffer.cc clickhouse.cc tfw_logger_config.cc
 OBJS := $(SRCS:.cc=.o)


Suggested change

SRCS := tfw_logger.cc mmap_buffer.cc clickhouse.cc tfw_logger_config.cc

OBJS := $(SRCS:.cc=.o)

OBJS := $(patsubst %.cc, %.o, $(wildcard *.cc))

krizhanovsky · 2025-05-23T15:12:58Z

logger/tfw_logger.cc

+   	if (fd < 0) {
+   		spdlog::error("Failed to open device");
+   		return 1;
+   	}


Bunch of logic is removed, it seems mostly about daemonization, but I'm not sure that only that logic - it's hard to compare the files line by line. It seems due to my comment #2363 (comment) , which is wrong - in some of our projects we use docker, which doesn't need daemon(), but once we tried to manage with systemctl, we needed the daemonization mode. @althusky can provide more details on this

pid file also removed. By the way, it seems https://github.com/tempesta-tech/escudo/blob/main/manager/main.cc#L73 implements them in a more accurate way, so it could be better to borrow the code from there

krizhanovsky · 2025-05-23T15:22:27Z

logger/tfw_logger_config.hh

+	void override_clickhouse_max_wait(int ms) { clickhouse_.max_wait = std::chrono::milliseconds(ms); }
+
+private:
+	fs::path log_path_{"/var/log/tempesta/tfw_logger.log"}; // Log file path


Probably it's not a good place for the default path, other defaults live in tfw_logger.cc

krizhanovsky · 2025-05-23T15:23:04Z

logger/tfw_logger_config.hh

+	void override_clickhouse_user(const std::string& user) { clickhouse_.user = user; }
+	void override_clickhouse_password(const std::string& password) { clickhouse_.password = password; }
+	void override_clickhouse_max_events(size_t events) { clickhouse_.max_events = events; }
+	void override_clickhouse_max_wait(int ms) { clickhouse_.max_wait = std::chrono::milliseconds(ms); }


Too wide lines and other coding style violations, e.g. everything in single line

krizhanovsky · 2025-05-23T15:25:40Z

logger/tfw_logger.cc

+	if (!default_config.save_to_file(config_path)) {
+		throw Except("Failed to create default configuration file");
+	}
+}


Why do we need this and TfwLoggerConfig::save_to_file? We can jsut create a default file and keep in the repo - it will be easier for users to learn from the default file, probably with comments, and we don't need extra C++ programming. At least we do this for Tempesta FW itself

krizhanovsky · 2025-05-23T15:26:48Z

logger/tfw_logger_config.cc

+	if (clickhouse_.max_events == 0) {
+		throw std::runtime_error("max_events must be greater than 0");
+	}
+}


even for C++ we usually organize the code to read it from bottom to top, i.e. the calee parse_from_ptree must be above the caller TfwLoggerConfig::load_from_file

- Move all logger-related files to dedicated logger/ directory - Improve project structure organization - No functional changes in this commit

MorganaFuture force-pushed the morganaFuture/fix/configuration_parsing_error_mmap_log_buffer_size branch 6 times, most recently from dd475f2 to ed8f747 Compare May 21, 2025 14:30

krizhanovsky requested review from krizhanovsky and const-t May 22, 2025 12:27

MorganaFuture force-pushed the morganaFuture/fix/configuration_parsing_error_mmap_log_buffer_size branch 3 times, most recently from 6665ef6 to 11122b7 Compare May 22, 2025 13:29

MorganaFuture marked this pull request as ready for review May 23, 2025 09:15

MorganaFuture mentioned this pull request May 23, 2025

fix: configuration parsing error mmap_log_buffer_size #2363

Closed

krizhanovsky requested changes May 23, 2025

View reviewed changes

MorganaFuture force-pushed the morganaFuture/fix/configuration_parsing_error_mmap_log_buffer_size branch 5 times, most recently from c399c84 to 548ae7b Compare May 26, 2025 10:11

refactor: move logger component from utils/ to logger/

c94a9de

- Move all logger-related files to dedicated logger/ directory - Improve project structure organization - No functional changes in this commit

MorganaFuture force-pushed the morganaFuture/fix/configuration_parsing_error_mmap_log_buffer_size branch from 548ae7b to cb84d27 Compare May 26, 2025 13:15

refactor: move logger component from utils/ to logger/

262c2df

- Move all logger-related files to dedicated logger/ directory - Improve project structure organization - No functional changes in this commit

MorganaFuture force-pushed the morganaFuture/fix/configuration_parsing_error_mmap_log_buffer_size branch from cb84d27 to 262c2df Compare May 26, 2025 13:17

MorganaFuture marked this pull request as draft May 26, 2025 13:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Morgana future/fix/configuration parsing error mmap log buffer size #2428

Morgana future/fix/configuration parsing error mmap log buffer size #2428

MorganaFuture commented May 21, 2025 •

edited

Loading

Uh oh!

krizhanovsky left a comment

Uh oh!

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

krizhanovsky May 23, 2025

Uh oh!

Uh oh!

	# Copyright (C) 2024 Tempesta Technologies, Inc.
	# Copyright (C) 2025 Tempesta Technologies, Inc.

		@@ -21,7 +21,7 @@ CLICKHOUSE_BUILD_DIR := $(CLICKHOUSE_DIR)/build
		CLICKHOUSE_REPO_URL := https://github.com/ClickHouse/clickhouse-cpp.git

		SRCS := tfw_logger.cc mmap_buffer.cc clickhouse.cc tfw_logger_config.cc
		OBJS := $(SRCS:.cc=.o)

	SRCS := tfw_logger.cc mmap_buffer.cc clickhouse.cc tfw_logger_config.cc
	OBJS := $(SRCS:.cc=.o)
	OBJS := $(patsubst %.cc, %.o, $(wildcard *.cc))

Morgana future/fix/configuration parsing error mmap log buffer size #2428

Are you sure you want to change the base?

Morgana future/fix/configuration parsing error mmap log buffer size #2428

Conversation

MorganaFuture commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krizhanovsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MorganaFuture commented May 21, 2025 •

edited

Loading