Adapt Console::Logger to Fluent::Log #3987
Conversation
|
It's still early draft. |
ashie
force-pushed
the
console-logger-adapter
branch
from
71ea8fb to
4d141de
Compare
ashie
changed the title
Signed-off-by: Takuro Ashie <[email protected]>
Signed-off-by: Takuro Ashie <[email protected]>
Signed-off-by: Takuro Ashie <[email protected]>
Signed-off-by: Takuro Ashie <[email protected]>
Signed-off-by: Takuro Ashie <[email protected]>
Signed-off-by: Takuro Ashie <[email protected]>
Signed-off-by: Takuro Ashie <[email protected]>
Signed-off-by: Takuro Ashie <[email protected]>
ashie
force-pushed
the
console-logger-adapter
branch
from
2d94735 to
c3dadf7
Compare
Signed-off-by: Takuro Ashie <[email protected]>
Signed-off-by: Takuro Ashie <[email protected]>
|
CI errors on Ruby Head aren't concerned with this PR. |
|
I'll see this today! |
|
I'm not sure exactly which versions of async, cosole and td-agent are affected yet. |
There was a problem hiding this comment.
I'm seeing this fix, the async gem, and the console gem.
It certainly looks like the async is starting to use the console as of async-1.17, but at that point, the async seems to be using directly the logger passed in the constructor.
- https://github.com/socketry/async/blob/v1.17.1/lib/async/reactor.rb#L66
- https://github.com/socketry/async/blob/v1.17.1/lib/async/reactor.rb#L74-L76
- https://github.com/socketry/async/blob/v1.17.1/lib/async/reactor.rb#L201
I don't know for sure, but from the code, it seems like the behavior would be doubtful as of async-1.29.0.
Anyway, I understand the overall situation. I will see some more.
Yeah, it's been about half a year since I noticed such behaviour change. |
|
Even though the https://github.com/socketry/async/blob/v1.17.1/lib/async/reactor.rb#L201 logger.debug(self) {"Exiting run-loop because #{$! ? $! : 'finished'}."}So, as the fluentd logger, it would log something like this. |
|
Ah, I got it. In the failed test, the test passes Ruby's Logger to The signature of But the signature of Line 344 in 398feda Although the failed test passes incompatible logger, it should pass |
|
But strictly speaking, the signature of |
Wow!! Indeed!!
I agree! The current implementation could be an error in the future. |
Signed-off-by: Takuro Ashie <[email protected]>
Signed-off-by: Takuro Ashie <[email protected]>
|
We can confirm the behavior as following. config <source>
@type monitor_agent
</source>Run Fluentd and the following command. $ curl http://localhost:24220/api/pluginsThe logs being outputted through
|
|
I think this implementation is fine, but I have one question about using It seems that we can create the adapter without it, such as the following. require 'console/logger'
module Fluent
class Log
# Async gem which is used by http_server helper switched logger mechanism to
# Console gem which isn't complatible with Ruby's standard Logger (since
# v1.17). This class adapts it to Fluentd's logger mechanism.
class ConsoleAdapter
def self.wrap(logger)
_, level = Console::Logger::LEVELS.find { |key, value|
if logger.level <= 0
key == :debug
else
value == logger.level - 1
end
}
Console::Logger.new(ConsoleAdapter.new(logger), level: level)
end
def initialize(logger)
@logger = logger
end
def verbose!; end
def call(subject = nil, *arguments, name: nil, severity: 'info', **options, &block)
if subject
@logger.send(severity, *(["#{subject} "] + arguments), &block)
else
@logger.send(severity, *arguments, &block)
end
end
end
end
endThis can output logs similar to the conventional form. In terms of simply connecting to the fluentd logger, it could look like this. On the other hand, through the |
|
I understand! |
The last remaining concern for me is whether we should add our header to each line or not. |
Signed-off-by: Takuro Ashie <[email protected]>
Like this? Is there any advantage in aligning the logs, destroying the original meaning of a single log as multiple lines? |
Lines 474 to 476 in 8df759b This is the motivation I thought it (but probably I won't do it for this). |
I see!! |
|
There are also cases that output multiple lines without header in Fluentd's log, such as dumping Fluentd's config. |
|
Sure! I'm OK too. |
|
Thanks for your review! |
Which issue(s) this PR fixes:
N/A
What this PR does / why we need it:
The log mechanism of Async gem was changed as of async-1.17. Async gem is used in http_server helper of Fluentd. From async-1.17 it uses
Console::Loggerof Console gem which isn't compatible with Ruby's standard Logger. So we need to adapt it to our logger mechanism to output Async's log correctly.For example sometimes following error is occurred on CI without this patch:
#3842 (comment)
Docs Changes:
N/A
Release Note: