Skip to content
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

Implement better request/response logging #71

Closed
jedateach opened this issue Nov 16, 2014 · 7 comments
Closed

Implement better request/response logging #71

jedateach opened this issue Nov 16, 2014 · 7 comments
Milestone

Comments

@jedateach
Copy link
Contributor

The current logging approach is rather limited:

  • Only dumping arrays and objects, rather than raw data.
  • Uses SilverStripe debug.log, which could get cluttered with other things.

Solution: make use of existing guzzle logging tools. Such logging has a great amount of flexibility to log in various formats and to various locations.

It would be good if raw data could be stored in db, if desired. This could pose security issues however.

Here's how we can hook directly into the Guzzle client and log raw requests and responses:

Additional composer requirements

composer require guzzle/log *@stable
composer require guzzle/plugin-log *@stable
composer require monolog/monolog *@stable

Add this to mysite/_config.php:

<?php

use Guzzle\Http\Client;
use Guzzle\Log\MessageFormatter;
use Guzzle\Log\MonologLogAdapter;
use Guzzle\Plugin\Log\LogPlugin;
use Monolog\Handler\StreamHandler;
use Monolog\Logger;

$logger = new Logger('client');
$logger->pushHandler(new StreamHandler(BASE_PATH.'/omnipay.log'));
$logAdapter = new MonologLogAdapter($logger);
$logPlugin = new LogPlugin($logAdapter, MessageFormatter::DEBUG_FORMAT);
$client = new Client();
$client->addSubscriber($logPlugin);

PaymentService::set_http_client($client);

Adding to _config.php probably isn't the best idea. Would be better to set up on some closure callback. The static setter isn't that great either.

Cons to this approach

If a gateway doesn't use guzzle, it will need to do its own logging. I suspect this is rare, but it should be noted at least.

@jedateach jedateach added this to the 2.0 milestone Nov 16, 2014
@judgej
Copy link

judgej commented Jan 14, 2015

_config.php where? I would like to try this out to debug a driver.

@jedateach
Copy link
Contributor Author

mysite

@judgej
Copy link

judgej commented Jan 15, 2015

This might be some use:

https://groups.google.com/forum/#!topic/omnipay/O6pX48id2LE

A basic problem with logging, is not being able to add logging to the Guzzle client from the application. If the AbstractRequest allowed access to the Guzzle Client, then all the above logging could be added in the application for any driver. It keeps the logging out of individual drivers. Just a thought, anyway.

@jedateach
Copy link
Contributor Author

Thanks Jason.

I assume by "applictation", you mean your SilverStripe project. Yes, replacing the entire http client isn't the best way to do things here.

I think we need a way to "inject" some customisations to the guzzle client.

@judgej
Copy link

judgej commented Jan 15, 2015

Yes, that's right. I'm coming at this from an OmniPay point of view, rather than specifically SilverStripe. A tend to use it more often with Laravel, but your issue came up in a search as a request to expose the wire messages between OmniPay and the remote gateways. So I kind of jumped in :-)

@bummzack bummzack removed this from the 2.X milestone Jul 6, 2016
@bummzack bummzack added this to the 3.x milestone Feb 19, 2017
@jedateach
Copy link
Contributor Author

jedateach commented Apr 10, 2017

Here's how it could go into a file on it's own, behind a flag. This file could get r by omnipay/_config.php

paymentLogging.php

<?php

use Guzzle\Http\Client;
use Guzzle\Log\MessageFormatter;
use Guzzle\Log\MonologLogAdapter;
use Guzzle\Plugin\Log\LogPlugin;
use Monolog\Handler\StreamHandler;
use Monolog\Logger;

/**
 * Add the following to your _ss_environment.php file:
 * define("SS_PAYMENT_LOG", __DIR__."/payments.log");
 */
if(defined('SS_PAYMENT_LOG')) {
	$logger = new Logger('client');
	$logger->pushHandler(new StreamHandler(SS_PAYMENT_LOG));
	$logAdapter = new MonologLogAdapter($logger);
	$logPlugin = new LogPlugin($logAdapter, MessageFormatter::DEBUG_FORMAT);
	$client = new Client();
	$client->addSubscriber($logPlugin);
	SilverStripe\Omnipay\Service\PaymentService::setHttpClient($client);
}

It still would be better to use use the injector to generate / manipulate the http client (singleton) instance I think.

@bummzack
Copy link
Collaborator

bummzack commented Feb 9, 2018

Fixed with #180
Separate logging can now be configured with SS4/monolog.

@bummzack bummzack closed this as completed Feb 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants