Implement a real logging engine about php-resque HOT 18 OPEN

Hi,
This is a architectural issue.
I think that queuing code/Job Classes should do some DI to inject logger and then start managing logs.
but the core lib should not be attached with monolog or any other logging system.

IMO It should applications responsibility to manage log.

from php-resque.

You should implement the PSR-3 LoggerInterface to be compatible with most logging engines.

As @rajibahmed said, the logger should be an optional dependency (injected with a setter or an optional parameter of the contructor).
Here is an example of implementation: https://github.com/henrikbjorn/Bernard/pull/40/files

@Kamisama what events do you want to be logged ?

from php-resque.

I think that queuing code/Job Classes should do some DI to inject logger and then start managing logs.
but the core lib should not be attached with monolog or any other logging system.

IMO It should applications responsibility to manage log.

The problem is that the application is not communicating directly with php-resque, thus it's nearly impossible for a DI.
All the application do is calling a static function inside Resque to queue a job. It's not aware of any workers.

I also like the idea of DI, but even if we inject a Monolog handler, resque must check and validate it to use it, and to some extent, is 'tied' to a hardcoded implementation.

A possible method is to instantiate the monolog handler and pass it along in the autoloader file, and just let php-resque use it, like it's using the QUEUE, COUNT, etc .. variable.
But as it's a 'resource', a check is needed to use it. Can we do that (check that the resource is an instance of monolog) without hardcoding the monolog implementation ?

I think that queuing code/Job Classes should do some DI to inject logger and then start managing logs.

I think that it should be done when creating the workers, like the way it's done now. There's nothing wrong with the current way, I just propose a way to extend it.

@Kamisama what events do you want to be logged ?

The same as right now. I don't seek more event logging, but a way to send them elsewhere that in a local file.

Some of use choose to use php-resque to do some heavy jobs, on another server. I want a way to retrieve the health of my workers on these other servers, and centralize them in a single control panel.
Or to send them directly to a metric tools, for some analytics. I know there's a statsd plugin, but by using the log, we can send more infos, and compute more interesting stats, like the most queues job class, the most uses worker, etc ...

from php-resque.

I really think PSR3 is the way to go, it was written for this exact reason.

from php-resque.

So, we pass a logging instance that's implementing the PSR3 LoggerInterface inside the APP_INCLUDE file, then let php-resque use it if it's valid ? DI via the APP_INCLUDE ?

from php-resque.

I dont think just picking up a class from the include path will work because almost any logging lib will require its own init code, here is the usage code for Monolog:

Usage

<?php

use Monolog\Logger;
use Monolog\Handler\StreamHandler;

// create a log channel
$log = new Logger('name');
$log->pushHandler(new StreamHandler('path/to/your.log', Logger::WARNING));

// add records to the log
$log->addWarning('Foo');
$log->addError('Bar');

From the example above, the $log var would need to be set/injected in the worker startup script.

To do this right would really require a rewrite of the worker bin script, from what I am thinking (which may be overlooking something i'm sure)

from php-resque.

Yeah, I'm aware a rewrite of the logging implementation inside the worker is needed.

From the example above, the $log var would need to be set/injected in the worker startup script.

That's my point. Instanciate a Monolog handler in APP_INCLUDE, to make it visible by the resque bin.
When creating a worker, resque init file (bin/resque) will check for let's say a $Logger variable (like it already checks for COUNT and QUEUE variable), determine if it's implementing the PSR3 LoggerInterface.

If it's valid, pass that instance to the worker, else do nothing.

The worker needs to be refactored to implements the LoggerInterface too, and will still default to writing to sdout, if no $Logger is passed to it.

I'm debating to find the proper way to implement this feature, before sending a PR.

from php-resque.

I don't quite understand how you are proposing we pass a configured logging object to the worker through the command line parameters. the examples you gave (COUNT and QUEUE) are strings and ints, not a fully configured PHP object.

from php-resque.

We use the APP_INCLUDE for that. Instanciate the object inside that file. Drawback is that all logging option (path,, filename, etc ..) is hardcoded inside that file, and can't be decided when starting the worker.

from php-resque.

@Kamisama

I have an idea to solve your problem. If you do setup your main application to open up an api point that is
mapped to your logger. Then do a CURL call from the jobs to log the status of the worker or running jobs inside of your application. We use this idea in our next production app.

#loggerController
$app->get('/log/{level}/{msg}',function(){
    # monolog magic here.
})

from php-resque.

I agree that PSR3 and APP_INCLUDE are a reasonable approach. What might be an idea to improve the situation is to make APP_INCLUDE job specific. You could have a default, fallback APP_INCLUDE specified on the command line when you start the worker, but can override it with a specific one when enqueueing a Job.

from php-resque.

I have made a pull request addressing this, @Kamisama please look it over

from php-resque.

Nice.

So now, we have to do a getenv('VERBOSE') and instantiate the logger with the right verbosity in APP_INCLUDE ourself ?

What happen when the $logger in APP_INCLUDE is an object, but not PSR3 compliant ? Shouldn't you add a check for the $loggerInterface ?

from php-resque.

If you use the default logger setting any of LOGGING, VERBOSE, VVERBOSE will turn on all logging, including INFO and DEBUG levels.

Picking what levels to log would all be in you APP_INCLUDE file as part of the setup.
If this was in your APP_INCLUDE file, then you would only get message with a level of WARNING or higher saved to your log file.

use Monolog\Logger;
use Monolog\Handler\StreamHandler;

// create a log channel
$logger = new Logger('name');
$logger->pushHandler(new StreamHandler('path/to/your.log', Logger::WARNING));

This configuration would log all messages.

use Monolog\Logger;
use Monolog\Handler\StreamHandler;

// create a log channel
$logger = new Logger('name');
$logger->pushHandler(new StreamHandler('path/to/your.log', Logger::DEBUG));

As for enforcing the interface, I though about enforcing that, but opted against it because this could potentially work with a class that only had the log() method as long as it supported the common log levels. In the end, if you know how to use a third party logger, you should be able to figure out if it will work with a PSR3 compliant library or not. With all that said, if everyone elses wants to strictly enforce it, then I can add another check for it.

from php-resque.

I'll be nice to log more informations, by passing more datas to the third argument of log(), and leave to the logger whether to save them.

• I think that each log entry should be attached to a worker
• When processing a job, we should also log all the job's data
• We should also categorize each log, by type. E.g: set $context['type'] => 'sleep' when logging a sleep event. That allow the $logger to sort and save only some events.

from php-resque.

I think changes like that would need more input, and maybe another pull request.

I would love to keep the current PR as simple as possible so we can hopefully get it merged faster, and then open up discussion on expanding what and how things are being logged once we have this core logging functionality finalized and in place.

from php-resque.

Agree.

from php-resque.

IIRC, the PR was merged. Should this discussion be resumed? (Perhaps in a new issue to reduce clutter...)

from php-resque.