Skip to content

Feature Request: Plugin-able Logger Interface #2641

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

Closed
astorm opened this issue Dec 8, 2015 · 6 comments
Closed

Feature Request: Plugin-able Logger Interface #2641

astorm opened this issue Dec 8, 2015 · 6 comments

Comments

@astorm
Copy link

astorm commented Dec 8, 2015

Based on some examination of the Magento 2 codebase, it looks like Magento 2 uses the virtual type

Psr\Log\LoggerInterface

Whenever it needs to grab a logger. This is good. However, it sounds like you can't use plugins on non-Magento classes.

If possible, it would be nice if the there was a universally used logger interface that was plugin-able. This would enable a lot of useful third party customizations/tooling. While Monologger is a great package, it's handler based approach is limited/complicated/creates-hard-to-resolve-issues for extension developers.

@alankent
Copy link

alankent commented Dec 8, 2015

Possibly related, there is an internal proposal for a new logger interface with switches (MAGETWO-41208). Not committed to. But then code can ask for a logger based on flag names. In the Java world, the current class name is typically used, so you would in the constructor say something like:

$this->logger = $logManager->getLogger(get_class($this));

Then to use the logger, you check for NULL before using it.

if ($this->logger != NULL) {
    // Do whatever computation and generation of log messages
    // But only if logger is turned on.
    $message = ....
    $this->logger->debug($message);
}

The interface might be something like the following:

/**
 * This interface is used to get access to a PSR-3 Logger if a log switch has been enabled. Disabling a log switch is
 * useful to controlling the volume of output written to logs.
 */
interface LogSwitchManagerInterface {

    /**
     * Return a PSR-3 logger, if the specified "log name" has been enabled.
     * @param $logSwitch The name of the switch, typically the class name the calling code is within.
     * @return LoggerInterface|null A PSR-3 logger is returned if the flag is enabled, otherwise NULL.
     */
    public function getLogger($logSwitch);

    /**
     * Return true if the specified log switch is enabled.
     * @param string $logSwitch The name of the switch to check.
     * @return bool Returns true if the switch is set, false otherwise.
     */
    public function getLogSwitch($logSwitch);

    /**
     * Enable the specified log switch programatically.
     * @param string $logSwitch The name of the switch to enable/disable.
     * @param bool $enabled Set to true to enable the log switch, false to disable.
     */
    public function setLogSwitch($logSwitch, $enabled);
}

Then the wonderful community could do things like contribute better logging code for URL resolution, theme resolution, fall backs, etc. So a developer can say "I want to trace layout files" and not get all other logging at the same time.

So why is this relevant? If we did such a new interface, should plugins be based on the underlying logger (the above just allows conditional writing to logs), or on the getLogger($name) function so you can control what logger to return for different conditionals?

@alankent
Copy link

alankent commented Dec 8, 2015

I forgot to add, do you want plugins (allowing cascading and nesting) rather than using the di.xml file to replace monolog with something different?

@astorm
Copy link
Author

astorm commented Dec 8, 2015

@alankent I wouldn't presume to dictate an implementation or architecture detail for a team I'm not a member of, and whose consequences could be far reaching for that team.

At a higher level the main thrust of this request is, as a third party end-user-programmer, I'd like the ability to take programatic action whenever a specific piece of information is sent to the logs, and have have the rules for that "specific"-ness also be programatic. (where programatic is defined by run-time PHP rules, not a DSL). From an outsider perspective a plugin-able logger interface would seem to get there, and get there quickly, but I (and I suspect others) would be open to anything that enabled the above.

@Vinai
Copy link
Contributor

Vinai commented Dec 10, 2015

I guess with the current implementation you could create a \Monolog\Handler\HandlerInterface that does what you want and register it via di.xml.

    <type name="Magento\Framework\Logger\Monolog">
        <arguments>
            <argument name="handlers"  xsi:type="array">
                <item name="myHandler" xsi:type="object">My\Module\Logger\Handler\Foo</item>
            </argument>
        </arguments>
    </type>

In that implementation isHandling() would be called for each message, and if that returns true, I guess handle() is called. You might also be able to do something with the $processors array of callable types constructor argument, too, but I haven't looked how they can be used.

Using a handler for some kind of custom logic that doesn't write to a log seems a bit wrong but I'm pretty sure it would work.

@astorm
Copy link
Author

astorm commented Dec 10, 2015

Thanks @Vinai -- w/r/t the specific problem, I was also confused by what ended up being this bug/issue #2674 -- mentioned here mainly for the search engines. My brief look at the Monolog handler system left me with the impression it was a "winner takes all" situation, but I may be incorrect.

Re:

Using a handler for some kind of custom logic that doesn't write to a log seems a bit wrong but I'm pretty sure it would work

I agree that this seems a bit wrong, and isn't something I'd build into a system or application itself -- but I also believe that the point of a plugin system is to expose the inner workings of a larger system and let third party developers play, be creative, and come up with things that the original system developers may never have intended. Allowing things that are a little-bit-wrong-but-still-stable can spark new ideas, which can then be better folded back into the system in a more correct way. I guess my point of view is more "make the plugin system as good, consistent, and stable as possible" and having (what seemed to be) a a non-plugin-able interface that's used all over the place in constructor DI seemed inconsistent. However -- the Psr\Log\LoggerInterface is plugin-able. I was just hitting this bug: #2674

@astorm
Copy link
Author

astorm commented Dec 10, 2015

Re:

If possible, it would be nice if the there was a universally used logger interface that was plugin-able. This would enable a lot of useful third party customizations/tooling.

Closing this issue because this does exist. The Psr\Log\LoggerInterface is plugin-able -- just not in developer mode without a compile.

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

No branches or pull requests

3 participants