Allow to use custom logger

[Justinas-Jurciukonis]

For better integration into other systems, it would be great to have option to set custom logger to be used.

// \NetSuite\NetSuiteClient

use NetSuite\LoggerInterface;
use NetSuite\Logger;

class NetSuiteClient
{
    public function __construct($config = null, $options = [], $client = null, LoggerInterface $logger = null)
    {
        if (is_null($logger)) {
            $logger = new Logger(
                !empty($this->config['log_path']) ? $this->config['log_path'] : NULL,
                !empty($this->config['log_format']) ? $this->config['log_format'] : Logger::DEFAULT_LOG_FORMAT,
                !empty($this->config['log_dateformat']) ? $this->config['log_dateformat'] : Logger::DEFAULT_DATE_FORMAT
            );
        }
    }
}

[jrebs]

Oh, I see, this is an example, not a quote from the code as it is. TBH, I thought that it was already like this, but I guess I was thinking of some other project I work on. That's definitely a good idea. Will implement in near future.

[jrebs]

Yeah, I actually wanted to do this in the logger changes but dropped it to limit the changes to only the format. 100% agree though, this sort of change makes throwing our code on service container make more sense and affords complete customization of the logging including redirecting to external loggers.

@neclimdul I'm finally setting down to do a little refactoring around logging, moving to depending on a PSR-3 implementation to be passed in and upgrading the supplied Logger to conform with this and I'd like your opinion on something.

I expect to throw away the current method of logging every request and response to its own file and instead default to a netsuite-php-{datetime}.log file and writing requests and responses to it with the operation as context because when accepting a PSR-3 logger we will be limited to leveraging the standard logging methods. When I started refactoring the Logger class, it got me thinking that this is really sort of out of the scope of this package and I'd rather just use something like seldaek/monolog but then that creates the requirement of pulling monolog in as a dep for netsuite-php which I also feel is something that is perhaps better left to the end user.

Ultimately, I'm leaning to the position that the most appropriate net result is to:

• Remove the default Logger from this library
• Continue to support logging, if enabled, only if the user provides their own LoggerInterface-compatible logger object

Do you have an opinion on this?

[Justinas-Jurciukonis]

@jrebs Actually no, you don't need to rely on external logger, because you already have it in the way that you want it. Just need to adjust how/when is logging done.

I have created PR for that

[Justinas-Jurciukonis]

And if anyone wants to use any other logger (e.g. I personally use Symfony Monolog logger), then you can pass whatever logger you want

[jrebs]

Actually no, you don't need to rely on external logger, because you already have it in the way that you want it. Just need to adjust how/when is logging done.

I have created PR for that

I know we don't have to rely on an external logger, I'm debating the rationale behind packing in our own logger when there are already many very well developed and well maintained projects that do logging well. I generally favour the classical UNIX mentality of doing one thing and doing it right and for other things, using the appropriate tools that are already available. Before I add code, I like to be sure whether removing code doesn't make more sense. Almost anyone I've ever encountered who uses this toolkit is integrating it into a larger project/framework that already has its own opinions on how/where/when to log data. I already more or less wrote a change much like what you've provided in the PR but decided not to publish it yet and instead commented because I would like to chew on the subject a bit more. So far, I'm not quite convinced that packing and maintaining logging functionality into this library is the best thing for the project but I'm open to arguments. I'd like to hear what James has to say on the matter before making a decision.

[neclimdul]

Haven't dug deep into the PR but that approach makes sense. The idea of providing an out of the box logger basically only as a baseline debugging tool makes sense especially since its actually pretty effective for that.

As far as supporting monolog, I think we should definitely write that implementation. At the very least its something the Laravel package would benefit from. Requiring Monolog feels a bit icky though because its pretty big and while its definitely very common, its not the only logging framework. I'd prefer having it require-dev+suggest or a separate project. I'd expect it to just be the one class so including the implementation in this project probably isn't a big deal.