Skip to content

Commit 30fbbd7

Browse files
committed
in_monitor_agent: change default visibility of config, retry, and debug info
**Which issue(s) this PR fixes**: Fixes # **What this PR does / why we need it**: This PR updates the default behavior of the `monitor_agent` API to provide a more minimal and predictable response footprint. ### Before The HTTP API (`/api/plugins.json`, etc.) included `config`, `retry` state, and `instance_variables` of plugins by default (or when dynamically queried with parameters like `debug=1`, `with_config`, `with_retry`, or `with_ivars`). This resulted in unnecessarily verbose payloads that exposed deep internal plugin states not typically required for standard health monitoring. ### After Inclusion of these detailed internal states is now disabled by default to reduce payload size and enforce explicit configuration. * Changed * `include_config` default is changed from `true` to `false`. * `include_retry` default is changed from `true` to `false`. * The API no longer allows URL query parameters to override these settings. * Added * `include_debug_info` parameter to explicitly control the visibility of internal instance variables (default `false`). * Debug information (via queries such as `debug=1` or `with_ivars=xxx`) is now only included when this parameter is explicitly enabled in the configuration block. **Docs Changes**: **Release Note**: * in_monitor_agent: change default visibility of config, retry, and debug info Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
1 parent 45c87a8 commit 30fbbd7

2 files changed

Lines changed: 114 additions & 27 deletions

File tree

lib/fluent/plugin/in_monitor_agent.rb

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@ class MonitorAgentInput < Input
3737
desc 'Determine the rate to emit internal metrics as events.'
3838
config_param :emit_interval, :time, default: 60
3939
desc 'Determine whether to include the config information.'
40-
config_param :include_config, :bool, default: true
40+
config_param :include_config, :bool, default: false
4141
desc 'Determine whether to include the retry information.'
42-
config_param :include_retry, :bool, default: true
42+
config_param :include_retry, :bool, default: false
43+
desc 'Determine whether to include the debug information.'
44+
config_param :include_debug_info, :bool, default: false
4345

4446
class APIHandler
4547
def initialize(agent)
@@ -151,28 +153,23 @@ def build_option(req)
151153
# parse ?=query string
152154
qs.merge!(req.query || {})
153155

154-
# if ?debug=1 is set, set :with_debug_info for get_monitor_info
155-
# and :pretty_json for render_json_error
156-
opts = { query: qs }
157-
if qs['debug'.freeze].first
158-
opts[:with_debug_info] = true
159-
opts[:pretty_json] = true
160-
end
161-
162-
if ivars = qs['with_ivars'.freeze].first
163-
opts[:ivars] = ivars.split(',')
164-
end
156+
opts = {
157+
query: qs,
158+
with_config: @agent.include_config,
159+
with_retry: @agent.include_retry
160+
}
165161

166-
if with_config = qs['with_config'.freeze].first
167-
opts[:with_config] = Fluent::Config.bool_value(with_config)
168-
else
169-
opts[:with_config] = @agent.include_config
170-
end
162+
if @agent.include_debug_info
163+
# if ?debug=1 is set, set :with_debug_info for get_monitor_info
164+
# and :pretty_json for render_json_error
165+
if qs['debug'.freeze].first
166+
opts[:with_debug_info] = true
167+
opts[:pretty_json] = true
168+
end
171169

172-
if with_retry = qs['with_retry'.freeze].first
173-
opts[:with_retry] = Fluent::Config.bool_value(with_retry)
174-
else
175-
opts[:with_retry] = @agent.include_retry
170+
if ivars = qs['with_ivars'.freeze].first
171+
opts[:ivars] = ivars.split(',')
172+
end
176173
end
177174

178175
opts

test/plugin/test_in_monitor_agent.rb

Lines changed: 95 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ def test_configure
3535
assert_equal(24220, d.instance.port)
3636
assert_equal(nil, d.instance.tag)
3737
assert_equal(60, d.instance.emit_interval)
38-
assert_true d.instance.include_config
38+
assert_false d.instance.include_config
39+
assert_false d.instance.include_retry
40+
assert_false d.instance.include_debug_info
3941
end
4042

4143
sub_test_case "collect in_monitor_agent plugin statistics" do
@@ -614,15 +616,17 @@ def get(uri, header = {})
614616
d.instance_shutdown
615617
end
616618

617-
data(:with_config_and_retry_yes => [true, true, "?with_config=yes&with_retry"],
618-
:with_config_and_retry_no => [false, false, "?with_config=no&with_retry=no"])
619+
data(:with_config_and_retry_yes => [true, true, "?with_config=no&with_retry=no"],
620+
:with_config_and_retry_no => [false, false, "?with_config=yes&with_retry=yes"])
619621
test "/api/plugins.json with query parameter. query parameter is preferred than include_config" do |(with_config, with_retry, query_param)|
620622

621623
d = create_driver("
622624
@type monitor_agent
623625
bind '127.0.0.1'
624626
port #{@port}
625627
tag monitor
628+
include_config #{with_config}
629+
include_retry #{with_retry}
626630
")
627631
d.instance.start
628632
expected_test_in_response = {
@@ -659,6 +663,7 @@ def get(uri, header = {})
659663
}
660664
expected_null_response["config"] = {"@id" => "null", "@type" => "null"} if with_config
661665
expected_null_response["retry"] = {} if with_retry
666+
# The values are retrieved based on the configuration settings, regardless of query parameters.
662667
response = JSON.parse(get("http://127.0.0.1:#{@port}/api/plugins.json#{query_param}").body)
663668
test_in_response = response["plugins"][0]
664669
null_response = response["plugins"][5]
@@ -674,6 +679,7 @@ def get(uri, header = {})
674679
bind '127.0.0.1'
675680
port #{@port}
676681
tag monitor
682+
include_debug_info true
677683
")
678684
d.instance.start
679685
expected_test_in_response = {
@@ -710,7 +716,7 @@ def get(uri, header = {})
710716
"flush_time_count" => Integer,
711717
"drop_oldest_chunk_count" => Integer,
712718
}
713-
response = JSON.parse(get("http://127.0.0.1:#{@port}/api/plugins.json?with_config=no&with_retry=no&with_ivars=id,num_errors").body)
719+
response = JSON.parse(get("http://127.0.0.1:#{@port}/api/plugins.json?with_config=no&with_retry=no&with_ivars=id,non_existed_ivar").body)
714720
test_in_response = response["plugins"][0]
715721
null_response = response["plugins"][5]
716722
assert_equal(expected_test_in_response, test_in_response)
@@ -758,6 +764,7 @@ def get(uri, header = {})
758764
bind '127.0.0.1'
759765
port #{@port}
760766
tag monitor
767+
include_debug_info true
761768
")
762769
d.instance.start
763770
# To check pretty print
@@ -823,7 +830,8 @@ def write(chunk)
823830
@type monitor_agent
824831
bind '127.0.0.1'
825832
port #{@port}
826-
include_config no
833+
include_config false
834+
include_retry true
827835
")
828836
d.instance.start
829837
output = @ra.outputs[0]
@@ -968,4 +976,86 @@ def filter(tag, time, record)
968976
d.instance_shutdown
969977
end
970978
end
979+
980+
sub_test_case 'debug information exposure configurations' do
981+
setup do
982+
@port = unused_port(protocol: :tcp)
983+
984+
conf = <<~EOC
985+
<source>
986+
@type test_in
987+
@id test_in
988+
</source>
989+
EOC
990+
@ra = Fluent::RootAgent.new(log: $log)
991+
stub(Fluent::Engine).root_agent { @ra }
992+
@ra = configure_ra(@ra, conf)
993+
end
994+
995+
def config(include_debug_info:)
996+
<<~"CONFIG"
997+
@type monitor_agent
998+
bind '127.0.0.1'
999+
port #{@port}
1000+
tag monitor
1001+
include_debug_info #{include_debug_info}
1002+
CONFIG
1003+
end
1004+
1005+
sub_test_case "include_debug_info true" do
1006+
test "'/api/plugins.json?with_ivars=xxx' expose instance variables" do
1007+
d = create_driver(config(include_debug_info: true))
1008+
d.instance.start
1009+
1010+
response = JSON.parse(get("http://127.0.0.1:#{@port}/api/plugins.json?with_ivars=id").body)
1011+
test_in_response = response["plugins"][0]
1012+
assert_equal({"id" => "test_in"} , test_in_response["instance_variables"])
1013+
1014+
d.instance_shutdown
1015+
end
1016+
1017+
test "'/api/plugins.json?debug=1' expose instance variables" do
1018+
d = create_driver(config(include_debug_info: true))
1019+
d.instance.start
1020+
1021+
response = JSON.parse(get("http://127.0.0.1:#{@port}/api/plugins.json?debug=1").body)
1022+
test_in_response = response["plugins"][0]
1023+
assert_true(test_in_response.has_key?("instance_variables"))
1024+
# check existence about one of typical debug element
1025+
assert_false(test_in_response["instance_variables"]["log"].nil?)
1026+
1027+
d.instance_shutdown
1028+
end
1029+
end
1030+
1031+
sub_test_case "include_debug_info false" do
1032+
test "'/api/plugins.json?with_ivars=xxx' does not expose instance variables" do
1033+
d = create_driver(config(include_debug_info: false))
1034+
d.instance.start
1035+
1036+
response = JSON.parse(get("http://127.0.0.1:#{@port}/api/plugins.json?with_ivars=id").body)
1037+
test_in_response = response["plugins"][0]
1038+
assert_false(test_in_response.has_key?("instance_variables"))
1039+
1040+
no_query_response = JSON.parse(get("http://127.0.0.1:#{@port}/api/plugins.json").body)
1041+
assert_equal(response, no_query_response)
1042+
1043+
d.instance_shutdown
1044+
end
1045+
1046+
test "'/api/plugins.json?debug=1' does not expose instance variables" do
1047+
d = create_driver(config(include_debug_info: false))
1048+
d.instance.start
1049+
1050+
response = JSON.parse(get("http://127.0.0.1:#{@port}/api/plugins.json?debug=1").body)
1051+
test_in_response = response["plugins"][0]
1052+
assert_false(test_in_response.has_key?("instance_variables"))
1053+
1054+
no_query_response = JSON.parse(get("http://127.0.0.1:#{@port}/api/plugins.json").body)
1055+
assert_equal(response, no_query_response)
1056+
1057+
d.instance_shutdown
1058+
end
1059+
end
1060+
end
9711061
end

0 commit comments

Comments
 (0)