monolog-wordpress icon indicating copy to clipboard operation
monolog-wordpress copied to clipboard

Extend Additional Field Settings and Added IP Address Support

Open cyberwani opened this issue 7 years ago • 1 comments

Added support to provide additional parameters for Additional and Extra fields. With this user can provide extra details for each field. So, extra fields can be more flexible. Added support to store IP address with log.

cyberwani avatar Jan 05 '18 09:01 cyberwani

Hi!

The part about the the additional fields can be done, but line 116 must be

$additionalFields.=",\n`$af` TEXT NULL DEFAULT NULL";

instead of

$additionalFields.=",\n{$af['name']} TEXT NULL DEFAULT NULL";

to avoid incompatibility with exiting implementations. This means if 'type' is provided, it is used, but if not, there is no reason to expect 'name' to be set. If there's no 'type' the value itself is the column name, just like as it is now. (Also note the ` around the column name, which is needed for better compatibility. See here.)

As for the $record['extra'] handling this need to be demonstrated to be compatible with Monolog! I think it is not, as by design when an array is passed as one of the items in 'extra', the array is going to be converted to a string in some way to be logged. Users expect their 'extra' array to be logged as it is, and not to be interpreted in any special ways depending on the contents. However this should be no issue, as you could simply use $this->additionalFields to declare your 'extra' columns with custom types instead.

More generally, the column types used by monolog-wordpress can become something customized in WordPressHandler as Monolog does not care how the data is stored by Handlers (classes implementing HandlerInterface). However a Handler cannot implement functionality that introduces ambiguity to HandlerInterface. In other words, anyone should be able to replace any Handler with any other Handlers. Such change may affect where and how the extra data is being stored, but should not affect what parts of the extra data are being stored. Trying to interpret certain array keys (like 'name' and 'type') in a special way violates this principle.

Regarding your changes to log the IP addresses, that is certainly not something all users would need or like, and it's not reasonable to force such specific behaviour onto others. For the very same reason our talented friends at Monolog have designed the architecture of the logging stack in a way, that anyone can implement their own 'automatically add some custom data to my log entries' needs. The solution is called Processors. You should implement a custom processor that adds the IP address as an extra field, and push that Processor to your WordPressHandler instance as demonstrated in the example. Actually Monolog\Processor\WebProcessor already logs the client IP, although your method is more advanced, so you may extend that class or just build your own.

PS: it may be better to submit unrelated features as separate PRs, so they can be discussed and approved independently.

BenceSzalai avatar Apr 11 '20 21:04 BenceSzalai