http_server: Ready to support Async 2.0 gem
Which issue(s) this PR fixes: Fixes #
What this PR does / why we need it:
http_server plugin helper doesn't work with Async 2.x gem due to using obsolete usage.
This PR updates it to follow current documented way:
https://github.com/socketry/async-http/blob/0a65acd7cf7486e1877f0da86580e1692cd8207b/readme.md#usage
It's applicable both Async 2.x & Async 1.x.
But this PR still stay on Async 1.x because io-event gem which is required by Async 2.x can't build on Windows.
Docs Changes: None
Release Note: Same with the title
Hmm, can't build io-event on Windows: https://github.com/fluent/fluentd/runs/7645045887?check_suite_focus=true
Gem::Ext::BuildError: ERROR: Failed to build gem native extension.
current directory:
C:/hostedtoolcache/windows/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/io-event-1.0.9/ext
C:/hostedtoolcache/windows/Ruby/3.1.2/x64/bin/ruby.exe -I
C:/hostedtoolcache/windows/Ruby/3.1.2/x64/lib/ruby/3.1.0 -r
./siteconf20220803-6988-hlcmr.rb extconf.rb
checking for rb_ext_ractor_safe()... yes
checking for &rb_fiber_transfer()... yes
checking for -luring... no
checking for sys/epoll.h... no
checking for sys/event.h... no
checking for sys/eventfd.h... no
checking for rb_io_descriptor()... yes
checking for &rb_process_status_wait()... no
checking for rb_fiber_current()... yes
checking for &rb_fiber_raise()... yes
checking for ruby/io/buffer.h... yes
creating extconf.h
creating Makefile
current directory:
C:/hostedtoolcache/windows/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/io-event-1.0.9/ext
make.exe DESTDIR\= clean
current directory:
C:/hostedtoolcache/windows/Ruby/3.1.2/x64/lib/ruby/gems/3.1.0/gems/io-event-1.0.9/ext
make.exe DESTDIR\=
generating IO_Event-x64-mingw-ucrt.def
compiling ./io/event/event.c
compiling ./io/event/selector/selector.c
./io/event/selector/selector.c:26:10: fatal error: sys/wait.h: No such file or
directory
26 | #include <sys/wait.h>
| ^~~~~~~~~~~~
compilation terminated.
make: *** [Makefile:246: selector.o] Error 1
make failed, exit code 2
I'm seeing..
Hmm, this log is not outputted. I don't know why...
[debug]: #0 fluent/log.rb:309:debug: 0.0s: Async::IO::Socket
| Binding to #<Addrinfo: 0.0.0.0:24220 TCP>
In master, when launching Fluentd with this config, the log is outputted using ConsoleAdapter.
<source>
@type monitor_agent
</source>
https://github.com/socketry/async-io/blob/v1.34.3/lib/async/io/socket.rb#L157
At this point, somehow Console.logger is Console::Terminal::Logger, not Fluent::Log::ConsoleAdapter.
I'm thinking about the possibility that the logger is not passed to the child task correctly.
It seems that Console.logger = Fluent::Log::ConsoleAdapter.wrap(@logger) is not applied to this thread.
https://github.com/fluent/fluentd/blob/bf7498d6ff2805bfa517bbd6f7c17103c7d879d4/lib/fluent/plugin_helper/http_server.rb#L74-L76
Console.logger= sets the value as Fiber::local::instance.
This is why Console.logger = Fluent::Log::ConsoleAdapter.wrap(@logger) is not applied to other threads.
https://github.com/socketry/console/blob/main/lib/console.rb#L13-L19 https://github.com/socketry/console/blob/main/lib/console/logger.rb#L19-L20 https://github.com/socketry/fiber-local/blob/main/lib/fiber/local.rb#L34-L53
We may have to do something like this... Maybe there is a more correct way...
--- a/lib/fluent/plugin_helper/http_server/server.rb
+++ b/lib/fluent/plugin_helper/http_server/server.rb
@@ -40,7 +40,6 @@ module Fluent
@uri = URI("#{scheme}://#{@addr}:#{@port}").to_s
@router = Router.new(default_app)
@server_task = nil
- Console.logger = Fluent::Log::ConsoleAdapter.wrap(@logger)
opts = if tls_context
{ ssl_context: tls_context }
@@ -55,10 +54,12 @@ module Fluent
end
def start(notify = nil)
+ Console.logger = Fluent::Log::ConsoleAdapter.wrap(@logger)
@logger.debug("Start async HTTP server listening #{@uri}")
Async do |task|
@server_task = task.async do
+ Console.logger = Fluent::Log::ConsoleAdapter.wrap(@logger)
@server.run
end
if notify
We may have to do something like this... Maybe there is a more correct way...
Thanks for checking it. It seems there is no other way to replace them, so I applied the fix. But we don't need to rush merging this. I've reverted the status of this PR to draft. We'll check it later.
Note: We have to consider the possibility of Async::HTTP::Server::run() creating child tasks inside.
(If so, how should we apply our logger to the child tasks' threads?)
Hmm, GitHub "update branch" feature is not what I want. rebased manually.
Hmm, GitHub "update branch" feature is not what I want. rebased manually.
It should be "Update with rebase".