Bug 30242 - REST API: Allow configuring log level
Summary: REST API: Allow configuring log level
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-07 12:55 UTC by Marcel de Rooy
Modified: 2023-06-08 22:26 UTC (History)
2 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 30242: Allow changing Mojolicious log level (1.50 KB, patch)
2022-03-07 12:57 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30242: Allow changing Mojolicious log level (2.22 KB, patch)
2022-08-25 09:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30242: Add test (1.79 KB, patch)
2022-08-25 09:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2022-03-07 12:55:17 UTC
Default Mojo log level is trace. I am seeing various debug API warnings in the plack-error.log where they do not even belong.

We should add something like this in Koha::REST::V1->startup:
    my $level = C4::Context->config('mojo_log_level') || 'warn';
    $self->log->level($level);

And add the mojo_log_level to koha-conf.xml. Something like:
<mojo_log_level>warn</mojo_log_level>

Note that we should not have such debug warnings in production logs. The above lines do not yet move the Mojo warns to an api log btw.

What do you think?
Comment 1 Marcel de Rooy 2022-03-07 12:57:29 UTC
Created attachment 131433 [details] [review]
Bug 30242: Allow changing Mojolicious log level

Just a rudimentary approach: Add a config variable to koha-conf
and set log level when calling ->startup.

Test plan:
Add var to your koha-conf.
Hit the API.
Check your plack-error.log with/without this patch.
Toggle log level, check again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Julian Maurice 2022-03-07 13:01:11 UTC
> Default Mojo log level is trace
Only if the "mode" is development. It is 'info' otherwise (https://docs.mojolicious.org/Mojolicious#log). Not sure where MOJO_MODE should be set (in systemd unit perhaps ?)

Is it possible to redirect the logs to what is configured in log4perl.conf for the API ?
Comment 3 Tomás Cohen Arazi (tcohen) 2022-03-07 14:33:01 UTC
(In reply to Julian Maurice from comment #2)
> > Default Mojo log level is trace
> Only if the "mode" is development. It is 'info' otherwise
> (https://docs.mojolicious.org/Mojolicious#log). Not sure where MOJO_MODE
> should be set (in systemd unit perhaps ?)

The koha-plack script sets either 'deployment' or 'development' depending on the environment (if --debug was passed, or dev install detected).

> Is it possible to redirect the logs to what is configured in log4perl.conf
> for the API ?

$c->log should be initialized with Koha::Logger, right? Should be doable.
Comment 4 Marcel de Rooy 2022-08-24 14:58:21 UTC
Revisiting this one.

koha-plack
    if [ "$DEV_INSTALL" = "1" ]; then
        # Maybe we should switch off debug_mode if DEV_INSTALL is not set?
        environment="development"
    fi

    if [ "$debug_mode" = "yes" ]; then
        environment="development"

koha-conf
<dev_install>1</dev_install>
Comment 5 Marcel de Rooy 2022-08-25 09:12:38 UTC
(In reply to Tomás Cohen Arazi from comment #3)
> (In reply to Julian Maurice from comment #2)
> > > Default Mojo log level is trace
> > Only if the "mode" is development. It is 'info' otherwise
> > (https://docs.mojolicious.org/Mojolicious#log). Not sure where MOJO_MODE
> > should be set (in systemd unit perhaps ?)
> 
> The koha-plack script sets either 'deployment' or 'development' depending on
> the environment (if --debug was passed, or dev install detected).

I would like to keep the configuration option too since my production is using dev_install too ;) But when there is no config entry, I add a fallback to trace or info depending on dev_install.


> > Is it possible to redirect the logs to what is configured in log4perl.conf
> > for the API ?
> 
> $c->log should be initialized with Koha::Logger, right? Should be doable.

Probably, but I need to dive deep down into Mojolicious plugins like Mojolicious::Plugin::Log::Any etc. to do so. That is imo quite outside the scope of this bug. Just noting that this was not implemented in the current REST API. I am seeing these kind of calls:

my $logger = Koha::Logger->get({ interface => 'api' });
$logger->error("Warning: Could not load REST API spec bundle: " . $_);

Please open another bug for moving the logs and use a logger plugin.
Comment 6 Marcel de Rooy 2022-08-25 09:42:05 UTC
Created attachment 139759 [details] [review]
Bug 30242: Allow changing Mojolicious log level

Add an optional config variable to koha-conf and set log level in
sub startup.

Test plan:
Check last line of plack-error.log.
Hit the REST API.
When you have a dev_install, you should see new log line(s).
For a regular install, there may be no new log lines.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2022-08-25 09:42:08 UTC
Created attachment 139760 [details] [review]
Bug 30242: Add test

Test plan:
Run t/db_dependent/Koha/REST/V1.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2022-08-25 09:44:17 UTC
Please test. This comes from March already !
Comment 9 Marcel de Rooy 2022-08-25 11:33:44 UTC
On second thought, I am removing these patches. They actually just serve as a workaround until we implement logging properly in REST API.
Comment 10 Marcel de Rooy 2022-08-25 11:39:05 UTC
See bug 18205 and bug 22678 requesting implementation of Koha::Logger into REST API. One in discussion ;)