mschilli / log4perl

Log4j Implementation For Perl
http://log4perl.com
Other
116 stars 66 forks source link

Document how to set "undef_column_value" #96

Open Laz80UK opened 5 years ago

Laz80UK commented 5 years ago

Hi,

I couldn't find any documentation on how to set the "undef_column_value". It causes problems when trying to convert the output to JSON as my expected field type is an integer which could be NULL if not available, however Log::Log4perl::Layout::PatternLayout converts everything that's undef to a string, i.e. "[undef]". Many thanks.

mohawk2 commented 4 years ago

A quick git grep shows me it's used in Log::Log4perl::Layout::PatternLayout, and its value from Log::Log4perl::Appender::DBI always appears to be undef. I think this needs a bit of docs and testing (and maybe fixing)! Want to help out?

Laz80UK commented 4 years ago

Kinda busy here, but trying my best... I documented and suggested a workaround here: https://github.com/mschout/Log-Log4perl-Layout-JSON/issues/6

mohawk2 commented 4 years ago

This seems to me like an undef is getting prematurely coerced into a string, which isn't always appropriate as you identify. My feeling is that it should be left as-is, or at least be overridable to a real undef. I'm very open to a backwards-compatible PR implementing this! (with tests and docs, naturally)

abraxxa commented 2 years ago

We'd need this to be able to prefix the message by the NDC which should default to an empty string in case there is no NDC set.

abraxxa commented 2 years ago

My search for a workaround showed that NDC doesn't care about this config option: https://metacpan.org/dist/Log-Log4perl/source/lib/Log/Log4perl/NDC.pm#L19

abraxxa commented 2 years ago

This is what I've come up with:

use Log::Log4perl;
use Log::Log4perl::Layout::PatternLayout;
$Log::Log4perl::ALLOW_CODE_IN_CONFIG_FILE = 1;
Log::Log4perl::Layout::PatternLayout::add_global_cspec('U', sub {
    my $ndc = Log::Log4perl::NDC->get();
    return $ndc eq '[undef]'
        ? ''
        : "$ndc ";
});
$Log::Log4perl::ALLOW_CODE_IN_CONFIG_FILE = 0;

And use %U instead of %x, with no whitespace to the next placeholder, which is %m in my case. I haven't decided if that's ugly or genius...

mohawk2 commented 2 years ago

@abraxxa Doc patches welcome? :-)