Error event logs should include plugin id

[DavidKnacker]

Error event logs should include plugin id

[DavidKnacker]

@Athishpranav2003 to come back to your question, I do see the @id field for logs generated by the input plugin.

[Athishpranav2003]

@DavidKnacker is the id missing even for logs generated by input plugin?

[DavidKnacker]

@Athishpranav2003 I haven't checked it for error logs thrown by an input plugin yet.
But am I correct to assume that using @id to track errors should work for all kind of plugins? (or at least the ones that are shipped by default with fluentd?)

[Athishpranav2003]

Ideally yes but since it's not behaving in that way I was checking what's the difference in underlying code
Will check it meanwhile

Just ping me later with the case of input plugin

[DavidKnacker]

👍
will check the behavior for the input plugin tomorrow and report back

[kenhys]

another scope of Idea: track plugin which emit error event in router side, then emit more user friendly error message.

[daipom]

Thanks for your report.
This is Fluentd's specification.

This dump an error event log is generated by RootAgent (Fluentd's common logic).
The filter_parser plugin just emits the error event to the router.
So, this log does not have its id.

fluentd/lib/fluent/root_agent.rb

Lines 344 to 354 in e763c07

	def emit_error_event(tag, time, record, error)
	error_info = {error: error, location: (error.backtrace ? error.backtrace.first : nil), tag: tag, time: time}
	if @error_collector
	# A record is not included in the logs because <@ERROR> handles it. This warn is for the notification
	log.warn "send an error event to @ERROR:", error_info
	@error_collector.emit(tag, time, record)
	else
	error_info[:record] = record
	log.warn "dump an error event:", error_info
	end
	end

Although this is the current specification, it looks like not so good.
Maybe it could be improved somehow.

[daipom]

Although this is the current specification, it looks like not so good.
Maybe it could be improved somehow.

It would be better to include the id of the plugin that emits the error event.

It may be easy to manually include the id of filter_parser in the error info that it emits.

fluentd/lib/fluent/plugin/filter_parser.rb

Lines 101 to 103 in e763c07

	if @emit_invalid_record_to_error
	router.emit_error_event(tag, time, record, Fluent::Plugin::Parser::ParserError.new("pattern not matched with data '#{raw_value}'"))
	end

However, it is not so good because every plugin that emits error events needs to consider this issue.

As @kenhys says in #4567 (comment), it would be great if we could create a common mechanism for this.

another scope of Idea: track plugin which emit error event in router side, then emit more user friendly error message.

[Athishpranav2003]

@daipom so basically we need to avoid change in function signature but capture the caller plugin id?

[daipom]

so basically we need to avoid change in function signature but capture the caller plugin id?

Yes.
We can't change the signature of emit_error_event() because it is related to Event Emitter plugin helper API.
We need to keep compatibility for the plugins that use this function.

[Athishpranav2003]

Why don't we use log.error method to generate the error log than emitting it as event(It's indirect but works the same way right)?

[Athishpranav2003]

Why don't we use log.error method to generate the error log than emitting it as event(It's indirect but works the same way right)?

@daipom what about this?