Skip to content

add filter capability - #411

Closed
llaville wants to merge 2 commits into
Seldaek:masterfrom
llaville:FilterFeature
Closed

add filter capability#411
llaville wants to merge 2 commits into
Seldaek:masterfrom
llaville:FilterFeature

Conversation

@llaville

Copy link
Copy Markdown
Contributor

Let me introduce myself.
I'm author of the PEAR Net_Growl package that allow to send notification to Growl from PHP on Mac/OSX and Windows.

More documentation about the package can be found at http://growl.laurent-laville.org/

As I wanted to propose/add a Growl handler to Monolog, but was a bit disappointed by lack of filtering logs capability in Monolog, I've choosen to propose you now this new feature as a PR.

I've of course modifed the source code, but also added unit tests and modified the documentation.
To read it more easily, you can have a look directly on my forked version at https://github.com/llaville/monolog/blob/FilterFeature/doc/usage.md#filtering-log-records

That will explains how to filter logs globally and locally, and allow a fine grain to accept/deny logs messages.

Laurent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you really want to make the Log level (inside $this->level) useless with these filters? This is nothing I would expect if I set a Log level while creating a log handler. The filters should only be additional.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mimmi20 You're free to add or not a filter, and of course you are free to create your own filtering rule.
But the default behavior is kept is you don't add a filter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@llaville If I create a handler with a specific log level I would not expect that a filter may change it. You should add a comment about this into the usage.md.

@llaville

Copy link
Copy Markdown
Contributor Author

@mimmi20 With the latest commit (bbc87f0) it allows to combine the standard level filter with others rules defined by user.

For example, to combine the standard level filter we must define it like

$filter3 = function($record, $handlerLevel) {
    return ($record['level'] >= $handlerLevel);
};

$logger = new Logger('my_logger');
// add the filter rule globally 
$logger->pushFilter($filter3);

Hope it will help to clarify and fix the situation

@llaville

Copy link
Copy Markdown
Contributor Author

I've created a new repository on my account to show a real case (situation) where it could useful to have such filter feature.

https://github.com/llaville/phpunit-LoggerTestListener

@stof

stof commented Aug 26, 2014

Copy link
Copy Markdown
Contributor

Instead of adding a concept of filters in Monolog, I would implement it with a special filtering handler decorating another handler (as done for the FilterHandler which is based on the level only)

@llaville

Copy link
Copy Markdown
Contributor Author

@stof I don't agree with you. First FilterHandler provide only the basic way of level filtering, and did not allow to use any handler.

My concept enlarge the restricted level only criteria, and allow to filter based on each record elements.

If you found a new solution rather than using a special filtering handler, I would like to see it !

Filter concept should be used whatever handlers you want to add on stack, and not a special one.

@stof

stof commented Aug 26, 2014

Copy link
Copy Markdown
Contributor

It can be used with any handler, given it wraps it (and it does not require changing all handlers). And I'm not saying you should use the FilterHandler itself (which indeed only supports filtering based on the level), but a similar approach:

use Monolog\Handler\AbstractHandler;
use Monolog\Handler\HandlerInterface;

class AdvancedFilterHandler extends AbstractHandler
{
    private $filters;
    private $handler;
    private $bubble;

    /**
     * @param callable|HandlerInterface $handler        Handler or factory callable($record, $this).
     * @param callable[]                    $filters A list of filters to apply
     * @param Boolean                   $bubble         Whether the messages that are handled can bubble up the stack or not
     */
    public function __construct($handler, array $filters, $bubble = true)
    {
        $this->filters = $filters;
        $this->handler  = $handler;
        $this->bubble   = $bubble;
    }

    /**
     * {@inheritdoc}
     */
    public function isHandling(array $record)
    {
        foreach ($this->filters as $filter) {
            if (!call_user_func($filter, $record)) {
                return false;
            }
        }

        return true;
    }

// The following methods are copied from FilterHandler

    /**
     * {@inheritdoc}
     */
    public function handle(array $record)
    {
        if (!$this->isHandling($record)) {
            return false;
        }

        // The same logic as in FingersCrossedHandler
        if (!$this->handler instanceof HandlerInterface) {
            if (!is_callable($this->handler)) {
                throw new \RuntimeException(
                    "The given handler (" . json_encode($this->handler)
                    . ") is not a callable nor a Monolog\\Handler\\HandlerInterface object"
                );
            }
            $this->handler = call_user_func($this->handler, $record, $this);
            if (!$this->handler instanceof HandlerInterface) {
                throw new \RuntimeException("The factory callable should return a HandlerInterface");
            }
        }

        if ($this->processors) {
            foreach ($this->processors as $processor) {
                $record = call_user_func($processor, $record);
            }
        }

        $this->handler->handle($record);

        return false === $this->bubble;
    }

    /**
     * {@inheritdoc}
     */
    public function handleBatch(array $records)
    {
        $filtered = array();
        foreach ($records as $record) {
            if ($this->isHandling($record)) {
                $filtered[] = $record;
            }
        }

        $this->handler->handleBatch($filtered);
    }
}

And then using it:

// Create some filters
$filter1 = function($record) {
    return (preg_match('/^add/', $record['message']) === 1);
};
$filter2 = function($record) {
    return ($record['level'] == Logger::NOTICE);
};

// Create some handlers
$stream = new StreamHandler(__DIR__.'/my_app.log', Logger::DEBUG);
$filterStream = new AdvancedFilterHandler(
    $stream,
    [$filter1]
);

$errlog = new AdvancedFilterHandler(
    new ErrorLogHandler(),
    [$filter2]
);



// Create the main logger of the app
$logger = new Logger('my_logger');

$logger->pushHandler($filterStream);
$logger->pushHandler($errlog);

// Stream handler accept this message, and was denied by ErrorLog handler
$logger->addDebug('add a debug message');

// Stream handler deny this message, and was accepted by ErrorLog handler
$logger->addNotice('a notice message');

@Seldaek

Seldaek commented Sep 10, 2014

Copy link
Copy Markdown
Owner

I agree with @stof there is no real benefit to filtering at the handler level when we do everything else with nesting/composition of handlers. GroupHandler + FilterHandler can filter a whole subset of handlers for example. Besides I find filtering to be much more useful when analyzing logs than when logging, because what you filter out can't be found again later, and it's always hard to know which info you'll need until a problem occurs and you're trying to figure out what happened.

But I'd be happy to discuss a more advanced version of the FilterHandler if that makes sense.

@Seldaek Seldaek closed this Sep 10, 2014
@llaville

Copy link
Copy Markdown
Contributor Author

Hello,

I'm back from hollidays two days ago, and I readed your comments.

It's fine for me to use an AdvancedFilterHandler (thanks to @stof to provide the source code).
BTW, I've updated my project https://github.com/llaville/phpunit-LoggerTestListener in version 1.1.0 that used a such AvancedFilterHandler.
Hope your agree with me to include it (with credits, as original author to Christophe). I'm made very little changes. Source code available at https://github.com/llaville/phpunit-LoggerTestListener/blob/master/extra/AdvancedFilterHandler.php

@Seldaek Why not include a such advanced filter version in standard Monolog distribution !

@llaville
llaville deleted the FilterFeature branch September 17, 2014 09:27
@Seldaek

Seldaek commented Sep 18, 2014

Copy link
Copy Markdown
Owner

@llaville yes maybe it would fit in monolog itself. I'll open a new issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants