eserte / Doit

a scripting framework
https://metacpan.org/release/Doit
0 stars 0 forks source link

Doit::Log::set_label doesn't propogate to notices on remote machines #4

Open greencoloured opened 5 years ago

greencoloured commented 5 years ago

Hi,

Unless I'm missing something the $current_label for Doit::Log is just a lexically scoped variable at the class level so is not propagated to the target machine when messages are generated by components there.

Would it not make more sense to make a Doit::Logger and store the label inside the $doit object which gets copied on do_connect_ssh?

Then calls to log would look somewhat like:

$doit->set_label("[$host]");
my $ssh = $doit->do_connect_ssh($host) or $doit->error("unable to connect");

# in method on component
$doit->info("working");
# do something
$doit->warning("something didn't run correctly") unless $success;
$doit->error("something went very wrong!") if $very_wrong;

the Doit internal components would need to be updated so that they used the correct label.

greencoloured commented 5 years ago

Something like this:

package Doit::Logger;

use strict;
use warnings;

my %cols = (
    notice  => 'yellow on_black',
    info    => 'green on_black',
    warning => 'red on_black',
);

sub new { bless {}, shift }
sub functions { qw/ 
    notice
    info 
    warning
    error
    set_log_label
    / }

sub _col { 
    require Term::ANSIColor;
    return Term::ANSIColor::colored($_[1], $cols{$_[0]});
};

# added for variety
sub notice {
    my $doit = shift;
    my $label = $doit->{_log_current_label} //= '';
    print STDERR _col(notice => "NOTE$label:"), " ", $_[0], "\n"
}

sub info {
    my $doit = shift;
    my $label = $doit->{_log_current_label} //= '';
    print STDERR _col(info => "INFO$label:"), " ", $_[0], "\n"
}

sub warning {
    my $doit = shift;
    my $label = $doit->{_log_current_label} //= '';
    print STDERR _col(warning => "WARN$label:"), " ", $_[0], "\n"
}

sub error {
    my $doit = shift;
    my $label = $doit->{_log_current_label} //= '';
    require Carp;
    Carp::croak(_col(error => "ERROR$label:"), " ", $_[0]);
}

sub set_log_label {
    my $doit = shift;
    $doit->{_log_current_label} = shift // '';
}

1;
eserte commented 5 years ago

I was already thinking about extending the logging capabilities, e.g. turning the logger class into OO and implement automatic set_label propagation to remote connections.

Currently this is not done. A possible workaround is, after creating a connection or on every set_label change:

$ssh->call('Doit::Log::set_label', "host");

An implementation should probably do two things:

Maybe propagating to other connections is not always wished; so I think having two functions, a simple one and a "propagating" one, would be useful.