-
Notifications
You must be signed in to change notification settings - Fork 69
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
Enhance plugin logging to provide more context information and request IDs #9853
base: develop
Are you sure you want to change the base?
Changes from all commits
1854c5e
e02b9fd
6a97436
62a094d
c739949
f48b470
82602b9
819ef17
b70f125
708e13f
0f697a6
7f5955d
fdbcfd7
e4b1b11
6dd09ed
7e20321
3927d38
d16d5c5
c47ea0c
9b5374f
13933bb
c2a0813
269930d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Significance: minor | ||
Type: dev | ||
|
||
Enhance log file format to provide more information about the request flow. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?php | ||
/** | ||
* Class LoggerContext | ||
* | ||
* @package WooCommerce\Payments | ||
*/ | ||
|
||
namespace WCPay; | ||
|
||
use WCPay\Internal\LoggerContext as InternalLoggerContext; | ||
|
||
defined( 'ABSPATH' ) || exit; // block direct access. | ||
|
||
/** | ||
* A wrapper class for accessing LoggerContext as a singletone. | ||
*/ | ||
class Logger_Context { | ||
/** | ||
* Sets a context value. | ||
* | ||
* @param string $key The key to set. | ||
* @param string|int|float|bool|null $value The value to set. Null removes value. | ||
* | ||
* @return void | ||
*/ | ||
public static function set_value( $key, $value ) { | ||
wcpay_get_container()->get( InternalLoggerContext::class )->set_value( $key, $value ); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,20 +23,21 @@ class Logger { | |
* we need this to access the plugins debug setting to figure out if the setting | ||
* is turned on. | ||
* | ||
* @param string $message Log message. | ||
* @param string $message Log message. | ||
* | ||
* @param string $level One of the following: | ||
* 'emergency': System is unusable. | ||
* 'alert': Action must be taken immediately. | ||
* 'critical': Critical conditions. | ||
* 'error': Error conditions. | ||
* 'warning': Warning conditions. | ||
* 'notice': Normal but significant condition. | ||
* 'info': Informational messages. | ||
* 'debug': Debug-level messages. | ||
* @param string $level One of the following: | ||
* 'emergency': System is unusable. | ||
* 'alert': Action must be taken immediately. | ||
* 'critical': Critical conditions. | ||
* 'error': Error conditions. | ||
* 'warning': Warning conditions. | ||
* 'notice': Normal but significant condition. | ||
* 'info': Informational messages. | ||
* 'debug': Debug-level messages. | ||
* @param array<string, string> $context Context data. | ||
*/ | ||
public static function log( $message, $level = 'info' ) { | ||
wcpay_get_container()->get( InternalLogger::class )->log( $message, $level ); | ||
public static function log( $message, $level = 'info', $context = [] ) { | ||
wcpay_get_container()->get( InternalLogger::class )->log( $message, $level, $context ); | ||
RadoslavGeorgiev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
|
@@ -119,4 +120,20 @@ public static function info( $message ) { | |
public static function debug( $message ) { | ||
self::log( $message, 'debug' ); | ||
} | ||
|
||
/** | ||
* Formats an object for logging. | ||
* | ||
* @param string $label Label for the object. | ||
* @param mixed $object Object to format. | ||
* @return string | ||
*/ | ||
public static function format_object( $label, $object ) { | ||
try { | ||
$encoded = wp_json_encode( $object, JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE | JSON_THROW_ON_ERROR ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small suggestion here, but could we add a string like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 13933bb |
||
} catch ( \JsonException $e ) { | ||
return sprintf( 'Error encoding object "%s": %s', $label, $e->getMessage() ); | ||
} | ||
return sprintf( '%s (JSON): %s', $label, $encoded ); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing to do with this PR in particular, but when we started the re-engineering project, and added
src
with the DI container, I was really hoping that we'd be able to refactor the whole plugin to work that way. Thus this feels like a hack, but I understand it is necessary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware about it, but sounds interesting. I think we way
src
is made to work is much easier to follow than the long lists ofinclude
orrequire
operators in the other parts of plugin code.