Description
Kyle M Hall
2022-11-01 10:35:05 UTC
Created attachment 142855 [details] [review] Bug 32057: Add optional stack trace to action logs It can be useful to know where in Koha a particular logged action was generated from, and how. We should add the ability to attach a syspref controlled basic stack trace with the caller depth controlled via the syspref. We can the call caller() until the specified depth has been reached. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 4) Restart all the things! 5) Set the new syspref ActionLogsTraceDepth to a number ( e.g. 3 ) 6) Update a syspref or trigger another enabled log action 7) Query the database: SELECT * FROM action_logs WHERE trace IS NOT NULL 8) Note the stack trace was created! Created attachment 142857 [details] [review] Bug 32057: Add optional stack trace to action logs It can be useful to know where in Koha a particular logged action was generated from, and how. We should add the ability to attach a syspref controlled basic stack trace with the caller depth controlled via the syspref. We can the call caller() until the specified depth has been reached. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 4) Restart all the things! 5) Set the new syspref ActionLogsTraceDepth to a number ( e.g. 3 ) 6) Update a syspref or trigger another enabled log action 7) Query the database: SELECT * FROM action_logs WHERE trace IS NOT NULL 8) Note the stack trace was created! The patch no longer applies 8-(... Applying: Bug 32057: Add optional stack trace to action logs Using index info to reconstruct a base tree... M installer/data/mysql/kohastructure.sql M installer/data/mysql/mandatory/sysprefs.sql Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/mandatory/sysprefs.sql Auto-merging installer/data/mysql/kohastructure.sql CONFLICT (content): Merge conflict in installer/data/mysql/kohastructure.sql error: Failed to merge in the changes. Patch failed at 0001 Bug 32057: Add optional stack trace to action logs Created attachment 144734 [details] [review] Bug 32057: Add optional stack trace to action logs It can be useful to know where in Koha a particular logged action was generated from, and how. We should add the ability to attach a syspref controlled basic stack trace with the caller depth controlled via the syspref. We can the call caller() until the specified depth has been reached. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 4) Restart all the things! 5) Set the new syspref ActionLogsTraceDepth to a number ( e.g. 3 ) 6) Update a syspref or trigger another enabled log action 7) Query the database: SELECT * FROM action_logs WHERE trace IS NOT NULL 8) Note the stack trace was created! Created attachment 144735 [details] [review] Bug 32057: Add optional stack trace to action logs It can be useful to know where in Koha a particular logged action was generated from, and how. We should add the ability to attach a syspref controlled basic stack trace with the caller depth controlled via the syspref. We can the call caller() until the specified depth has been reached. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 4) Restart all the things! 5) Set the new syspref ActionLogsTraceDepth to a number ( e.g. 3 ) 6) Update a syspref or trigger another enabled log action 7) Query the database: SELECT * FROM action_logs WHERE trace IS NOT NULL 8) Note the stack trace was created! Created attachment 144736 [details] [review] Bug 32057: Use replace insert query with use of Koha::ActionLogs Created attachment 144737 [details] [review] Bug 32057: Replace insert query with use of Koha::ActionLogs Created attachment 144738 [details] [review] Bug 32057: Replace insert query with use of Koha::ActionLogs After applying the patch, updating the database, and adding a value to ActionLogsTraceDepth, I am now unable to change system preferences. I tried with UseCaseRegisters and under Logs - enabling logging for an item that was currently disabled (such as IssueLog). I get an "Internal Server Error, please reload the page" in the staff interface. In the plack-error.log this is the start of the message: 291 DBIx::Class::Row::store_column(): No such column 'trace' on Koha::Schema::Result::ActionLog at /kohadevbox/koha/Koha/Object.pm line 79 292 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 293 DBIx::Class::Exception::throw('DBIx::Class::Exception', 'No such column \'trace\' on Koha::Schema::Result::ActionLog') called at /usr/share/perl5/DBIx/Class/ResultSourc e.pm line 2380 294 DBIx::Class::ResultSource::throw_exception('DBIx::Class::ResultSource::Table=HASH(0x55a3fd102ef8)', 'No such column \'trace\' on Koha::Schema::Result::ActionLog') calle d at /usr/share/perl5/DBIx/Class/Row.pm line 1569 295 DBIx::Class::Row::throw_exception('Koha::Schema::Result::ActionLog=HASH(0x55a400de53c8)', 'No such column \'trace\' on Koha::Schema::Result::ActionLog') called at /usr/ share/perl5/DBIx/Class/Row.pm line 1215 296 DBIx::Class::Row::store_column('Koha::Schema::Result::ActionLog=HASH(0x55a400de53c8)', 'trace', '[^J {^J "package" : "Koha::Config::SysPref",^J "line" : 77, ^J "subroutine" : "C4::Log::logaction",^J "filename" : "/kohadevbox/koha/Koha/Config/SysPref.pm"^J },^J {^J "filename" : "/kohadevbox/koha/C4/Context.pm" ,^J "line" : 431,^J "subroutine" : "Koha::Config::SysPref::store",^J "package" : "C4::Context"^J },^J {^J "subroutine" : "C4::Context::set_preferenc e",^J "line" : 103,^J "package" : "CGI::Compile::ROOT::kohadevbox_koha_svc_config_systempreferences",^J "filename" : "/kohadevbox/koha/svc/config/systemprefe rences"^J }^J]^J') called at /usr/share/perl5/DBIx/Class/Row.pm line 270 Created attachment 144769 [details] [review] Bug 32057: Update ActionLog schema file [DO NOT PUSH] Created attachment 144786 [details] [review] Bug 32057: Add optional stack trace to action logs It can be useful to know where in Koha a particular logged action was generated from, and how. We should add the ability to attach a syspref controlled basic stack trace with the caller depth controlled via the syspref. We can the call caller() until the specified depth has been reached. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 4) Restart all the things! 5) Set the new syspref ActionLogsTraceDepth to a number ( e.g. 3 ) 6) Update a syspref or trigger another enabled log action 7) Query the database: SELECT * FROM action_logs WHERE trace IS NOT NULL 8) Note the stack trace was created! Signed-off-by: David Nind <david@davidnind.com> Created attachment 144787 [details] [review] Bug 32057: Replace insert query with use of Koha::ActionLogs Signed-off-by: David Nind <david@davidnind.com> Created attachment 144788 [details] [review] Bug 32057: Update ActionLog schema file [DO NOT PUSH] Signed-off-by: David Nind <david@davidnind.com> Thanks Kyle for fixing! I should have thought of running dbic! Now signed off. David Did you think about using Devel::StackTrace / libdevel-stacktrace-perl ? (In reply to Marcel de Rooy from comment #15) > Did you think about using Devel::StackTrace / libdevel-stacktrace-perl ? I did have a look at Devel::StackTrace when I started this bug. Simply put, using caller is the absolute fastest way to do what we need here. If there are other benefits to Devel::StackTrace I'm not aware of we could look into that on followup bugs! Created attachment 147152 [details] [review] Bug 32057: Add optional stack trace to action logs It can be useful to know where in Koha a particular logged action was generated from, and how. We should add the ability to attach a syspref controlled basic stack trace with the caller depth controlled via the syspref. We can the call caller() until the specified depth has been reached. Test Plan: 1) Apply this patch 2) Run updatedatabase.pl 4) Restart all the things! 5) Set the new syspref ActionLogsTraceDepth to a number ( e.g. 3 ) 6) Update a syspref or trigger another enabled log action 7) Query the database: SELECT * FROM action_logs WHERE trace IS NOT NULL 8) Note the stack trace was created! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 147153 [details] [review] Bug 32057: Replace insert query with use of Koha::ActionLogs Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 147154 [details] [review] Bug 32057: Update ActionLog schema file [DO NOT PUSH] Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 147155 [details] [review] Bug 32057: (QA follow-up) Remove pretty flag Not needed for storage. You can always display pretty in the interface (on a follow-up report). Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 147156 [details] [review] Bug 32057: (QA follow-up) Add a simple test Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> A future report should add this field to the interface (log viewer). Pushed to master for 23.05. Nice work everyone, thanks! Created attachment 148162 [details] [review] Bug 32057: (follow-up) Add INSERT IGNORE in dbrev Preventing: ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate entry 'ActionLogsTraceDepth' for key 'PRIMARY' at /usr/share/koha/C4/Installer.pm line 739 Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> @RM: last trivial follow-up Thanks for the follow-up, Marcel. Enhancement - not backporting to 22.11.x Nice work everyone! |