Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle log output with log4perl #86

Merged
merged 1 commit into from
Apr 11, 2022
Merged

Handle log output with log4perl #86

merged 1 commit into from
Apr 11, 2022

Conversation

sloanebernstein
Copy link
Contributor

Originally, logging to a file was done by piping screen output to
tee(1). This prevented some output from being logged, as only the
systemd service did this. Instead, use a native log4perl file appender
to log to the file.

This eliminates colored output, since there is no direct way to save
color data to the file anymore. Restoring this feature will be explored
in a future issue.

Resolves #71 and #83.

By submitting pull requests to this repo, I agree to the Contributor License Agreement which can be found at: https://github.com/cpanel/elevate/blob/main/docs/cPanel-CLA.pdf

Originally, logging to a file was done by piping screen output to
`tee(1)`. This prevented some output from being logged, as only the
systemd service did this. Instead, use a native log4perl file appender
to log to the file.

This eliminates colored output, since there is no direct way to save
color data to the file anymore. Restoring this feature will be explored
in a future issue.

Resolves #71 and #83.
@sloanebernstein sloanebernstein linked an issue Apr 11, 2022 that may be closed by this pull request
@@ -3195,7 +3192,7 @@ sub install_cpanel_elevate_service {
Type=simple
# want to run it once per boot time
RemainAfterExit=yes
ExecStart=/usr/bin/bash -c 'exec /usr/local/cpanel/scripts/elevate-cpanel --service 2>&1 | /usr/bin/tee -a $log_file'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WTH? why was this bash??

my $program = shift @args;
my ( $callback_out, $callback_err ) = map {
my $label = $_;
Cpanel::IOCallbackWriteLine->new( sub ($line) { chomp $line; INFO("{$label} $line"); } )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INFO autochomps.

@sloanebernstein
Copy link
Contributor Author

sloanebernstein commented Apr 11, 2022

If logging to the systemd journal is not desirable, the Screen appender will need to be disabled when run as a service.

);
INFO(); # Buffer so they can more easily read the output.

return $? = $sr->CHILD_ERROR; ## no critic qw(Variables::RequireLocalizedPunctuationVars) -- emulate return behavior of system()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not wild about us clobbering $? why would we want to set that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of system already clobbered $?, so this just maintains "feature" parity. It doesn't need to stay.

@toddr toddr merged commit e278e34 into main Apr 11, 2022
@toddr toddr deleted the log4perlify-elevate branch April 11, 2022 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Send all output through log4perl? add to log when running with --continue
2 participants