-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Update log events and its handling #1405
Conversation
…events only when engine is running) * fix a bug not to enable log events only with "match fluent.**" sections * fixed code will enable log events only with "match fluent.info" sections * route fluent.** events to @FLUENT_LOG label event router if exists * route these events to top-level fluent.** if @FLUENT_LOG missing * in @FLUENT_LOG section, events not matched will be ignored (assuming log-level-ish behavior) * both in @FLUENT_LOG and in top-level, non-matched log event tags will be warned at startup * disable log events in default, and enable it after starting up Engine * it prevent to raise unexpected errors by emitting events to plugins already shut-down
@repeatedly could you review this change? |
record = map.dup | ||
record.keys.each {|key| | ||
record[key] = record[key].inspect unless record[key].respond_to?(:to_msgpack) | ||
} | ||
record['message'] = message.dup | ||
@engine.push_log_event("#{@tag}.#{level}", time.to_i, record) | ||
@engine.push_log_event("#{LOG_EVENT_TAG_PREFIX}.#{level}", time.to_i, record) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use EventTime
for time
and passing it to push_log_event without to_i
call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right... I added a commit to emit events with Fluent::EventTime.from_time(time)
.
@@ -158,35 +159,5 @@ def emit_error_event(tag, time, record, error) | |||
|
|||
def handle_emits_error(tag, es, error) | |||
end | |||
|
|||
class NoMatchMatch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fluent-plugin-reemit uses this class directly. Should keep with constant value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. Why do the such thing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the code of out_reemit
. It doesn't refer Fluent::NoMatchMatch
.
(It just refer Fluent::EngineClass::NoMatchMatch
in V10Engine
).
https://github.com/sonots/fluent-plugin-reemit/blob/master/lib/fluent/plugin/out_reemit.rb
Isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see.
I missed the if
condition for versions.
unmatched_tags = Fluent::Log.event_tags.select{|t| !@log_event_router.match?(t) } | ||
unless unmatched_tags.empty? | ||
$log.warn "match for some tags of log events are not defined (to be ignored)", tags: unmatched_tags | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check tag typo is better. Currently, there is no warning with following conf.
<label @FLUENT_LOG>
<match fleunt.info>
@type stdout
</match>
</label>
Log example: There is no 'fluent' prefix <match> in <label @FLUENT_LOG>. Check typo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the code to warn that <label @FLUENT_LOG>
doesn't have any matches for fluent.*
tags.
1705933
to
5b87737
Compare
I'll merge this after CI green. |
There are some issues and unexpected behavior about log events (
fluent.**
events).This change is to fix these problems and add features.
<match fluent.**>
sections<match fluent.info>
section doesn't work A counter-intuitive behavior for fluent.(error|warn|info|trace) logs #485Added features and changes:
@FLUENT_LOG
labels if exists#configure
and#start
of each plugiins), and after entering shutdown sequence (#stop
,#shutdown
...)--log-event-verbose
(orlog_event_verbose
in system config) turns log events on even in startup and shutting downFixes #485, #1038