search

Grinnz / Mojolicious-Plugin-Log-Any

Mojolicious::Plugin::Log::Any - Use other loggers in a Mojolicious application
https://metacpan.org/pod/Mojolicious::Plugin::Log::Any
Other
0 stars 1 forks source link

Mojolicious 9.0 - Handle "context" messages #3

Open jrubinator opened 3 years ago

jrubinator commented 3 years ago

In Mojolicious 9.0 (release date still TBD), context will come through as a separate string message, rather than being prepended to the first message in a string. (commit)

A nice improvement here would be to store that as context data - that change would need to occur near eg. https://metacpan.org/source/DBOOK/Mojolicious-Plugin-Log-Any-v1.0.1/lib/Mojo/Log/Role/AttachLogger.pm#L31

Perhaps it could look something like:

my $request_id;
if (@msgs > 1 && $msgs[0] =~ /^\[[a-f0-9]+\]$/) {
  $request_id = shift @msgs;
}
local $logger->context->{request_id} //= $request_id if $request_id;
...

Thoughts?

Some further background: the context is set in Mojolicious via DefaultHelpers

jrubinator commented 3 years ago

Mojolicious 9.0 is out now: https://mojolicious.io/blog/2021/02/14/announcing-mojolicious-9-0/ https://metacpan.org/pod/Mojolicious

@Grinnz - any thoughts on this?

Grinnz commented 3 years ago

Well the tricky part is that context is not always the request ID, and also not always there. Especially in the role which can be used on any Mojo::Log object. But it's an interesting idea.

jrubinator commented 3 years ago

Oooh, yeah - totally missed supporting other Mojo::Log attachments.

Maybe one more option to attach_logger, like expect_mojolicious_request_id? Then Mojolicious::Plugin::Log::Any could detect whether your at Mojolicious >9.0 before turning that on by default.

As for Mojolicious 9.0+ that doesn't have request ID, it seems highly unlikely that we'd see something of the format eg. $app->log('[abcdef123]', 'That is not a request id'), but if someone does have that use case, maybe they could explicitly turn off the new parameter?