Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

system_config: add path attribute in system/log section #4604

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lib/fluent/supervisor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,6 @@ def initialize(cl_opt)
@inline_config = opt[:inline_config]
@use_v1_config = opt[:use_v1_config]
@conf_encoding = opt[:conf_encoding]
@log_path = opt[:log_path]
@show_plugin_config = opt[:show_plugin_config]
@libs = opt[:libs]
@plugin_dirs = opt[:plugin_dirs]
Expand All @@ -527,13 +526,15 @@ def initialize(cl_opt)
@chumask = opt[:chumask]
@signame = opt[:signame]

# TODO: `@log_rotate_age` and `@log_rotate_size` should be removed
# TODO: `@log_path`, `@log_rotate_age` and `@log_rotate_size` should be removed
# since it should be merged with SystemConfig in `build_system_config()`.
# We should always use `system_config.log.rotate_age` and `system_config.log.rotate_size`.
# We should always use `system_config.log.path`, `system_config.log.rotate_age`
# and `system_config.log.rotate_size`.
# However, currently, there is a bug that `system_config.log` parameters
# are not in `Fluent::SystemConfig::SYSTEM_CONFIG_PARAMETERS`, and these
# parameters are not merged in `build_system_config()`.
# Until we fix the bug of `Fluent::SystemConfig`, we need to use these instance variables.
@log_path = opt[:log_path]
@log_rotate_age = opt[:log_rotate_age]
@log_rotate_size = opt[:log_rotate_size]

Expand Down Expand Up @@ -690,6 +691,7 @@ def setup_global_logger(supervisor: false)

# TODO: we should remove this logic. This merging process should be done
# in `build_system_config()`.
@log_path ||= system_config.log.path
@log_rotate_age ||= system_config.log.rotate_age
@log_rotate_size ||= system_config.log.rotate_size

Expand Down
1 change: 1 addition & 0 deletions lib/fluent/system_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class SystemConfig
v.to_i(8)
end
config_section :log, required: false, init: true, multi: false do
config_param :path, :string, default: nil
config_param :format, :enum, list: [:text, :json], default: :text
config_param :time_format, :string, default: '%Y-%m-%d %H:%M:%S %z'
config_param :rotate_age, default: nil do |v|
Expand Down
3 changes: 3 additions & 0 deletions test/config/test_system_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def parse_text(text)
assert_nil(sc.enable_size_metrics)
assert_nil(sc.enable_msgpack_time_support)
assert(!sc.enable_jit)
assert_nil(sc.log.path)
assert_equal(:text, sc.log.format)
assert_equal('%Y-%m-%d %H:%M:%S %z', sc.log.time_format)
end
Expand Down Expand Up @@ -117,6 +118,7 @@ def parse_text(text)
conf = parse_text(<<-EOS)
<system>
<log>
path /tmp/fluentd.log
format json
time_format %Y
</log>
Expand All @@ -125,6 +127,7 @@ def parse_text(text)
s = FakeSupervisor.new
sc = Fluent::SystemConfig.new(conf)
sc.overwrite_variables(**s.for_system_config)
assert_equal('/tmp/fluentd.log', sc.log.path)
assert_equal(:json, sc.log.format)
assert_equal('%Y', sc.log.time_format)
end
Expand Down
5 changes: 5 additions & 0 deletions test/test_supervisor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ def test_system_config
log_level info
root_dir #{@tmp_root_dir}
<log>
path /tmp/fluentd.log
format json
time_format %Y
</log>
Expand Down Expand Up @@ -94,6 +95,7 @@ def test_system_config
assert_equal "process_name", sys_conf.process_name
assert_equal 2, sys_conf.log_level
assert_equal @tmp_root_dir, sys_conf.root_dir
assert_equal "/tmp/fluentd.log", sys_conf.log.path
assert_equal :json, sys_conf.log.format
assert_equal '%Y', sys_conf.log.time_format
counter_server = sys_conf.counter_server
Expand Down Expand Up @@ -134,6 +136,7 @@ def test_system_config
log_level: info
root_dir: !fluent/s "#{@tmp_root_dir}"
log:
path: /tmp/fluentd.log
format: json
time_format: "%Y"
counter_server:
Expand Down Expand Up @@ -161,6 +164,7 @@ def test_system_config
"process_name",
2,
@tmp_root_dir,
"/tmp/fluentd.log",
:json,
'%Y',
'127.0.0.1',
Expand All @@ -180,6 +184,7 @@ def test_system_config
sys_conf.process_name,
sys_conf.log_level,
sys_conf.root_dir,
sys_conf.log.path,
sys_conf.log.format,
sys_conf.log.time_format,
counter_server.bind,
Expand Down