daithi-coombes / api-connection-manager

Manages connections and requests to 3rd party providers or services
0 stars 1 forks source link

Spike - experiment with log4php and sending data to Loggly #12

Closed daithi-coombes closed 11 years ago

daithi-coombes commented 11 years ago

Creating a new pull request for the php4log class

From David in pull request 11: This branch experiments with:

  1. Log4PHP - Replace the custom class-api-con-mngr-log.php with Log4PHP
  2. Capture & log all WordPress events
  3. Write to several different appenders - the existing a text log file in uploads, a HTML log file of the last request (very verbose), also in uploads and sending data to Loggly via Syslog over UDP
daithi-coombes commented 11 years ago

From David in pull request 11: @david-coombes Of the 3 logging options I've enabled, the HTML log file of the last request seems the most useful.

What are your thoughts?

daithi-coombes commented 11 years ago

From David in pull request 11: Here is a screencast of it in action - https://cityindex.viewscreencasts.com/6d1693516241a104b69c17507eba3609

daithi-coombes commented 11 years ago

@mrdavidlaing @ryanholder I've create a new pull request for the log4php

There are issues with the loggly appender, both on my local and remote environment. It seems we may have to disable the loggly option and let the wordpress dev's use them as needed.

Local environment:

[Fri Jan 18 07:31:22 2013] [error] [client 127.0.0.1] PHP Warning:  fsockopen(): unable to connect to udp://logs.loggly.com:23640 (php_network_getaddresses: getaddrinfo failed: Name or service not known) in /var/www/cityindex.loc/api-connection-manager/wp3.5/wp-content/plugins/api-connection-manager/vendor/log4php/appenders/LoggerAppenderLoggly.php on line 411
[Fri Jan 18 07:32:55 2013] [error] [client 127.0.0.1] PHP Warning:  fsockopen(): php_network_getaddresses: getaddrinfo failed: Name or service not known in /var/www/cityindex.loc/api-connection-manager/wp3.5/wp-content/plugins/api-connection-manager/vendor/log4php/appenders/LoggerAppenderLoggly.php on line 411

Remote:

Notice: fwrite() [function.fwrite]: send of 164 bytes failed with errno=1 Operation not permitted in /home/davidcoo/public_html/wp-content/plugins/api-connection-manager/vendor/log4php/appenders/LoggerAppenderLoggly.php on line 413
daithi-coombes commented 11 years ago

Also, after dissabling the loggly appender: https://github.com/david-coombes/api-connection-manager/blob/spike-log4php/log4net-config.xml#L32

I'm getting a permissions warning for the file appender:

Warning: mkdir(): Permission denied in /var/www/cityindex.loc/api-connection-manager/wp3.5/wp-content/plugins/api-connection-manager/vendor/log4php/appenders/LoggerAppenderFile.php on line 89 Warning: log4php: [LoggerAppenderFile:myHTMLFileAppender]: Failed creating target directory [wp-content/uploads]. Closing appender. in /var/www/cityindex.loc/api-connection-manager/wp3.5/wp-content/plugins/api-connection-manager/vendor/log4php/LoggerAppender.php on line 283

Once there's no "fatal" error's these warnings and notices can be surpressed by adding '@' here: https://github.com/david-coombes/api-connection-manager/blob/spike-log4php/class-api-connection-manager.php#L102

$this->log_api = @Logger::getLogger(__CLASS__);

and then we can do our own checks.

We had a system before that would use wordpress's file class to create required folders, in that case it was the api-con-modules folder. This class will automatically ask for ftp details if required for file functions. Before the class was called when the user/admin went to the dashboard settings page for the api-con. If the required files/folders didn't exist the wp file class would then ask for ftp permissions if needed and create the files folders.

mrdavidlaing commented 11 years ago

I vote for shipping the plugin with the majority of logging appenders "commented out". If you want to switch them on, then you can decide where to write the files and sort out the respective permissions.

The issue with the loggly appender is that you need to configure loggly to allow connections from specific IPs. If you want to use you'll need your own (free) loggly account. I should have mentioned that - sorry!

daithi-coombes commented 11 years ago

agreed it is an advantage that should be left in. Given the nature of api-con I'd also vote for the ability to write to files to be default. When admin goes to api-con settings page if the log files aren't created we can call WP_File class to sort out all the nitty gritty bits and create them.

WP_Filesystem, if ftp details are needed they're requested: http://ottopress.com/2011/tutorial-using-the-wp_filesystem/

Then we can have logging of all requests by default (to log file created above) by callign the log4php in the request method of the api-module class:

    API_Con_Mngr_Module::request(){
        $this->log->msg( $slug, "request GET {$url}");
    }

On Fri, Jan 18, 2013 at 9:45 AM, David Laing notifications@github.comwrote:

I vote for shipping the plugin with the majority of logging appenders "commented out". If you want to switch them on, then you can decide where to write the files and sort out the respective permissions.

The issue with the loggly appender is that you need to configure loggly to allow connections from specific IPs. If you want to use you'll need your own (free) loggly account. I should have mentioned that - sorry!

— Reply to this email directly or view it on GitHubhttps://github.com/david-coombes/api-connection-manager/pull/12#issuecomment-12415062.

"Any society that would give up a little liberty to gain a little security, will deserve neither and lose both." - Benjamin Franklin

daithi-coombes commented 11 years ago

... acutally just thinking, using WP_Filesystem, if the ftp details are needed to create the file, then more than likely they'll be needed for every write to the log file.

So best instead to test if write permissions on the upload folder, if not then display warning in the dashboard wp-error box and auto disable logging (don't consrtuct log4php)

mrdavidlaing commented 11 years ago

I think you're just overcomplicating things.

Disabling whether & where you output logs is the responsibility of the Logger configuration (specifically whether you have a FileAppender setup).

Leave the logger instantiated; but by default ship a logger configuration that doesn't log anything.

Put some documentation somewhere explaining how to enable logging; and then let people choose & resolve where they put it themselves.

daithi-coombes commented 11 years ago

^above commit...

The log4php only throws warnings, thankfully, so supressing their messages when constructing log4php allows us to test if the log file was created successfully or not:

        //test logging
        if(!file_exists("wp-content/uploads/api-con-mngr.lastrequest.html"))
            $this->log_api = new WP_Error('API_Connection_Manager: log4php','Unable to create log file');

Then when when logging message can register error in dashboard to fix permissions for logging

/**
     * Log an INFO message
     * @param string $msg The message to log
     * @return none
     */
    public function log($msg){

        //if no log4php
        if(is_wp_error($this->log_api)){
            //register dashboard error message
        }

        //else log message
        else
            $this->log_api->info($msg);
    }