Small fixes testagent

[hannaeko]

Purpose

Fix a couple of inconsistencies in the test agent script

Context

Changes

• Uses Log::Dispatch log levels for validation (more or less a subset the Log::Any levels), to avoid a crash because of a bad log level.
• Run preflight checks on both start and foreground.

How to test this PR

• perl script/zonemaster_backend_testagent --logfile=- --outfile=/tmp/zonemaster_testagent.out --loglevel=trace foreground Return Error: Unrecognized --loglevel trace and not a stack trace.
• perl script/zonemaster_backend_testagent --logfile=- --outfile=/tmp/zonemaster_testagent.out --loglevel=debug foreground Show the Starting pre-flight check log message.

[mattias-p]

I believe this is the wrong set of level names. The ones we're supporting are defined here: https://metacpan.org/pod/Log::Any#LOG-LEVELS. We're only hooking up Log::Dispatch as a sink for Log::Any.

Also $CanonicalLevelNames seems to be undocumented. I always think twice before using undocumented features for fear that they could be unstable too.

[hannaeko]

I believe this is the wrong set of level names. The ones we're supporting are defined here: https://metacpan.org/pod/Log::Any#LOG-LEVELS.

Actually that $loglevel is then passed as min_level when creating the Log::Dispatch object. As per documentation this should be one of https://metacpan.org/pod/Log::Dispatch#LOG-LEVELS. Those log level names differ from the ones of Log::Any and using one that is not defined in Log::Dispatch will result in error, e.g.:

% perl script/zonemaster_backend_testagent --logfile=- --outfile=/tmp/zonemaster_testagent.out --loglevel=trace foreground
Validation failed for type named LogLevel declared in package Log::Dispatch::Types (/home/gbm/perl5/lib/perl5/Log/Dispatch/Types.pm) at line 60 in sub named (eval) with value "trace"

Trace begun at Specio::Exception->new line 57
Specio::Exception::throw('Specio::Exception', 'message', 'Validation failed for type named LogLevel declared in package Log::Dispatch::Types (/home/gbm/perl5/lib/perl5/Log/Dispatch/Types.pm) at line 60 in sub named (eval) with value "trace"', 'type', 'Specio::Constraint::Simple=HASH(0x556bdf410ea8)', 'value', 'trace') called at (eval 581) line 101
Eval::Closure::Sandbox_315::__ANON__('callbacks', 'CODE(0x556bdf344188)', 'newline', 1, 'min_level', 'trace') called at /home/gbm/perl5/lib/perl5/Log/Dispatch/Output.pm line 99
Log::Dispatch::Output::_basic_init('Log::Dispatch::Handle=HASH(0x556bdf312238)', 'callbacks', 'CODE(0x556bdf344188)', 'newline', 1, 'min_level', 'trace') called at /home/gbm/perl5/lib/perl5/Log/Dispatch/Handle.pm line 24
Log::Dispatch::Handle::new('Log::Dispatch::Handle', 'handle', 'IO::Handle=GLOB(0x556bda65db98)', 'min_level', 'trace', 'newline', 1, 'callbacks', 'CODE(0x556bdf344188)') called at /home/gbm/perl5/lib/perl5/Log/Dispatch.pm line 112
Log::Dispatch::_add_output('Log::Dispatch=HASH(0x556bda6264a0)', 'Handle', 'handle', 'IO::Handle=GLOB(0x556bda65db98)', 'min_level', 'trace', 'newline', 1, 'callbacks', 'CODE(0x556bdf344188)') called at /home/gbm/perl5/lib/perl5/Log/Dispatch.pm line 81
Log::Dispatch::new('Log::Dispatch', 'outputs', 'ARRAY(0x556bda62ea38)') called at script/zonemaster_backend_testagent line 87
main::log_dispatcher_dup_stdout('trace') called at script/zonemaster_backend_testagent line 215

Also $CanonicalLevelNames seems to be undocumented. I always think twice before using undocumented features for fear that they could be unstable too.

I could use a regex from the values in the documentation if you think that is better.

[mattias-p]

Oh, you're right! This means that the usage documentation for --loglevel doesn't describe the implementation. Though I'd argue that for pragmatic reasons it's better to update the implementation to match the documentation than vice versa.

The reason I'm saying this is that I think it would be a good thing if we were able to switch to another log adapter implementation without affecting our own public interfaces. The reasonable way to do this it to either define our own level names or to adopt the level names from Log::Any.

As you point out this is not how it works today. A better way would be to employ Log::Any for the level filtering. Unfortunately the design of that feature is flawed. It's not possible to specify a global log level filter. Instead you have to do it in every module you emit log messages from, but this is not acceptable. (There's a feature request that would allow setting a global level filter in Log::Any.)

One possibility I haven't fully explored would be to use Log::Any's built-in adapters instead of Log::Dispatch. In principle that wouldn't be an ultimate solution since we'd still rely on the adapters to do the log filtering, but it would have a couple of other desirable consequences. First we could keep referring to the Log::Any log levels in our public interface, the log levels we implement would match the ones we declare, and we could drop a couple of dependencies.

I realize this was supposed to be a small fix, but it kind of hit the tip of an iceberg. We don't need to solve everything here but we should probably create an issue for solving whatever problems remain after this PR.

[matsduf]

Also $CanonicalLevelNames seems to be undocumented. I always think twice before using undocumented features for fear that they could be unstable too.

I could use a regex from the values in the documentation if you think that is better.

Yes, I think that would be better.

[hannaeko]

One possibility I haven't fully explored would be to use Log::Any's built-in adapters instead of Log::Dispatch. In principle that wouldn't be an ultimate solution since we'd still rely on the adapters to do the log filtering, but it would have a couple of other desirable consequences. First we could keep referring to the Log::Any log levels in our public interface, the log levels we implement would match the ones we declare, and we could drop a couple of dependencies.

I decided to go with that, but with a custom adapter since builtin adapter don't offer everything we are using (and also that) see #966

[matsduf]

Also $CanonicalLevelNames seems to be undocumented. I always think twice before using undocumented features for fear that they could be unstable too.

I could use a regex from the values in the documentation if you think that is better.

Yes, I think that would be better.

[hannaeko]

Replaced by #966 and #975.