Skip Menu |

This queue is for tickets about the Log-Log4perl CPAN distribution.

Report information
The Basics
Id: 79960
Status: resolved
Priority: 0/
Queue: Log-Log4perl

People
Owner: Nobody in particular
Requestors: FGA [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 1.38
Fixed in: (no value)



Subject: Expanding %X{nosuchkey} to "[undef]" can be undesirable behavior
Hello, I'm using the MDC to stash interesting data for more searchable logging. In particular I have a method in my DBIC schema that users can call to set the "current running application" and be able to identify the application later. It basically looks like $schema->register_logged_application("foo"); The issue is that logging can happen before the schema has been instantiated, so the application is not registered yet. The DBI appender gets %X{application_run} for insertion into a table with a foreign key constraint, but it's undef, so it tries to insert "[undef]" into the INTEGER FOREIGN KEY column, which causes an error. I expected the appender to insert NULL instead, as you'd get if you used DBI directly. This is obviously because the appender uses PatternLayout to handle the params. Attached is my L4P configuration. Is there a workaround for this?
Subject: l4p_anonymized.conf
Download l4p_anonymized.conf
application/octet-stream 1.2k

Message body not shown because it is not plain text.

On Tue Oct 02 07:29:15 2012, FGA wrote: Show quoted text
> Is there a workaround for this?
Now there is :). I've just put in a fix at: https://github.com/mschilli/log4perl/commit/d7066113066b3db3f7264c812e24e04fd8592b 9c which lets you define $Log::Log4perl::Layout::PatternLayout::UNDEF_COLUMN_VALUE = undef; which changes the PatternLayout to return undef (and not the string "[undef]") in these cases. t/034DBI.t has a test case for it, please check the end of the file for details. You can download a test version at https://github.com/mschilli/log4perl/tarball/rt-79960 Let me know if that works for you! -- Mike
On Sun, 7 Oct 2012 01:18:17 -0400, MSCHILLI wrote: Show quoted text
> $Log::Log4perl::Layout::PatternLayout::UNDEF_COLUMN_VALUE = undef; > > which changes the PatternLayout to return undef (and not the string > "[undef]") in these cases. > t/034DBI.t has a test case for it, please check the end of the file > for details.
OK, I cloned your repo and installed it. Now all my tests pass, the script doesn't break, and my logs are everything they should be. It works brilliantly, thanks! My only complaint is that I need to do the UNDEF_COLUMN_VALUE incantation in all the applications that use Log::Log4perl (previously, changes to the logging behavior took place in the DB schema and the "register_logged_application" method, and then all apps could log to the new database schema without changing anything. Also, other appenders (e.g. File) will, I assume, try to interpolate undef somewhere and generate warnings? (which was probably why "[undef]" was introduced in the first place, right?) I'll try various things and send you a pull request if I come up with something that looks good. Thanks again!
On Thu Oct 11 08:16:30 2012, FGA wrote: Show quoted text
> previously, > changes to the logging behavior took place in the DB schema and the > "register_logged_application" method, and then all apps could log > to the new database schema without changing anything.
Yeah, good point, we should rather keep that change local to the DBI appender. I've added as a paramter to PatternLayout, and DBI sets it appropriately, and since it's a reasonable default in the DBI appender (I think), I've made it the default: https://github.com/mschilli/log4perl/commit/0bfe34fb3e78c356055a294ebb74 a8589cc9b381 Please give this tarball a try now, it should work right out of the box for you without having to set any variables: https://github.com/mschilli/log4perl/tarball/rt-79960-2 -- Mike
On Tue, 23 Oct 2012 00:17:37 -0400, MSCHILLI wrote: Show quoted text
> On Thu Oct 11 08:16:30 2012, FGA wrote:
> > previously, > > changes to the logging behavior took place in the DB schema and the > > "register_logged_application" method, and then all apps could log > > to the new database schema without changing anything.
> > Yeah, good point, we should rather keep that change local to the DBI > appender. I've added as a paramter to PatternLayout, and DBI sets it > appropriately, and since it's a reasonable default in the DBI appender > (I think), I've made it the default: > > https://github.com/mschilli/log4perl/commit/0bfe34fb3e78c356055a294ebb74 > a8589cc9b381 > > Please give this tarball a try now, it should work right out of the box > for you without having to set any variables: > > https://github.com/mschilli/log4perl/tarball/rt-79960-2 > > -- Mike
Yup, this works out of the box now. I've injected this version into our minicpan repository and updated all relevant apps to the new DB schema, everything's working as intended. I'm marking this resolved, thanks!