spotonlive / laravel-google-ads

Google Ads API for Laravel
MIT License
64 stars 58 forks source link

Logging not using provided path #7

Closed rjksn closed 8 years ago

rjksn commented 8 years ago

Hey,

I entered the path here in the config/google-ads.php, but Google's logger is trying to set a /soap_xml.log as opposed to __LARAVEL_DIR__/storage/logs/soap_xml.log.

    'settings' => [
        'LOGGING' => [
            /*
             * Log directory is either an absolute path, or relative path from the AdWordsUser.php file.
             */
            'PATH_RELATIVE' => '0',
            'LIB_LOG_DIR_PATH' => storage_path().'/logs',
        ],

I ended up setting a path in Google's Logger.php

$fp = fopen(storage_path().'/logs/'.$handleLocation, 'a');

Am I doing something wrong (besides the hack)? Or, …

*I have verified that the $user = new AdWordsUser(); contains those settings, with the proper directory structure.

nikolajlovenhardt commented 8 years ago

I'll look into this asap

david-navarro commented 8 years ago

Hi, AdsUser.php line 140, it works for me changing this:

Logger::LogToFile(Logger::$SOAP_XML_LOG,
        $this->logsDirectory . "/soap_xml.log");
    Logger::LogToFile(Logger::$REQUEST_INFO_LOG,
        $this->logsDirectory . "/request_info.log");

for this:

Logger::LogToFile(Logger::$SOAP_XML_LOG,
        $this->GetLogsDirectory() . "/soap_xml.log");
    Logger::LogToFile(Logger::$REQUEST_INFO_LOG,
        $this->GetLogsDirectory() . "/request_info.log");
nikolajlovenhardt commented 8 years ago

@david-navarro cool, thanks! I'll release a new version.

david-navarro commented 8 years ago

Hi, I really think this is a bug in googleads/googleads-php-lib repository. I'll tell you if my pull request becomes accepted or rejected.

Best.

david-navarro commented 8 years ago

Hi @nikolajlovenhardt . The problem occur when init logs files. I think the best solution is not calling $this->InitLogs();

and just replace this line https://github.com/nikolajlovenhardt/laravel-google-ads/blob/master/src/LaravelGoogleAds/AdWords/AdWordsUser.php#L230

with this code to init logs:

\Logger::LogToFile(\Logger::$SOAP_XML_LOG, $this->logsDirectory . "/soap_xml.log"); \Logger::LogToFile(\Logger::$REQUEST_INFO_LOG, $this->logsDirectory . "/request_info.log"); \Logger::SetLogLevel(\Logger::$SOAP_XML_LOG, \Logger::$FATAL); \Logger::SetLogLevel(\Logger::$REQUEST_INFO_LOG, \Logger::$FATAL);

What do u think about it?

nikolajlovenhardt commented 8 years ago

@david-navarro I prefer to override the InitLogs method instead.

What do you think about this https://github.com/nikolajlovenhardt/laravel-google-ads/commit/44b6d4c6b25a6ab023edfc508b6c922896dd1b4f