Matomo application logs can now be written in syslog and errorlog #17764
Conversation
|
This is an update to PR #17745 |
plugins/Monolog/config/config.php
Outdated
| @@ -97,6 +99,13 @@ | |||
| 'Piwik\Plugins\Monolog\Handler\FileHandler' => DI\create() | |||
| ->constructor(DI\get('log.file.filename'), DI\get('log.level.file')) | |||
| ->method('setFormatter', DI\get('log.lineMessageFormatter.file')), | |||
|
|
|||
| '\Monolog\Handler\ErrorLogHandler' => DI\create() | |||
| ->constructor(null, DI\get('log.level.errorlog')) | |||
There was a problem hiding this comment.
Looking at the ErrorHandler constructor, it seems like this shouldn't be set to null. If you want to only set the level parameter, and use defaults for the rest, you can use:
->constructorParameter('level', DI\get('log.level.errorlog'))
There was a problem hiding this comment.
That is one nifty dependency injection system you've got going! So would it be like this?
'\Monolog\Handler\ErrorLogHandler' => DI\create()
->constructorParameter('level', DI\get('log.level.errorlog'))
->method('setFormatter', DI\get('log.lineMessageFormatter.file')),
Thanks so much for the guidance dizzy!
There was a problem hiding this comment.
@mwithheld yes, that looks correct! And yes it is definitely a nifty DI system! The library is called php-di and the creator, @/mnapoli, worked w/ us for a while so our integration works rather well.
|
@mwithheld Thanks for fixing the issues! I left one other comment. After it's addressed it should be good to merge. |
Avoid null in ErrorLogHandler constructor call: Use level param; default others
|
Added commit 14992a8 ErrorLogHandler constructor: Avoid null |
plugins/Monolog/config/config.php
Outdated
| ->method('setFormatter', DI\get('log.lineMessageFormatter.file')), | ||
|
|
||
| '\Monolog\Handler\SyslogHandler' => DI\create() | ||
| ->constructor(DI\get('log.syslog.ident'), 'syslog', DI\get('log.level.syslog')), |
There was a problem hiding this comment.
Apologies for the back and forth, but one last question here: was it intentional not to use ->method('setFormatter', DI\get('log.lineMessageFormatter.file')) here?
|
Good catch - the discussion is welcome :) That was not intended - I'm just
not used to using GitHub for editing PRs.
…On Thu, Jul 15, 2021 at 7:20 PM dizzy ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In plugins/Monolog/config/config.php
<#17764 (comment)>:
> @@ -97,6 +99,13 @@
'Piwik\Plugins\Monolog\Handler\FileHandler' => DI\create()
->constructor(DI\get('log.file.filename'), DI\get('log.level.file'))
->method('setFormatter', DI\get('log.lineMessageFormatter.file')),
+
+ '\Monolog\Handler\ErrorLogHandler' => DI\create()
+ ->constructorParameter('level', DI\get('log.level.errorlog'))
+ ->method('setFormatter', DI\get('log.lineMessageFormatter.file')),
+
+ '\Monolog\Handler\SyslogHandler' => DI\create()
+ ->constructor(DI\get('log.syslog.ident'), 'syslog', DI\get('log.level.syslog')),
Apologies for the back and forth, but one last question here: was it
intentional not to use ->method('setFormatter',
DI\get('log.lineMessageFormatter.file')) here?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17764 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGCSOXPWKB6RSVBKTMBCRDTX6JQRANCNFSM5AKBNYHA>
.
|
|
@mwithheld I've merged the PR, thanks for going through the trouble of creating the PR and following up on it! The contribution is greatly appreciated. |
|
@diosmosis could you please document this new capability in https://matomo.org/faq/troubleshooting/faq_115/ ? Thanks |
|
documented |
| ->method('setFormatter', DI\get('log.lineMessageFormatter.file')), | ||
|
|
||
| '\Monolog\Handler\SyslogHandler' => DI\create() | ||
| ->constructor(DI\get('log.syslog.ident'), 'syslog', DI\get('log.level.syslog')) |
There was a problem hiding this comment.
Is this logging to the "syslog" facility? I would guess that "user" is most appropriate, but "syslog" is definitely not correct =)
(Discovered while trying to match on the facility and program name using syslog-ng.)
Lazy reference: https://en.wikipedia.org/wiki/Syslog#Facility
There was a problem hiding this comment.
You are correct this should be 'user'.
Should probably use
->constructorParameter('ident'=>DI\get('log.syslog.ident'))
->constructorParameter('level'=>DI\get('log.level.syslog'))
|
Hi @mwithheld, thanks for creating the change so quickly! Would you be able to put it in a new pull request? This one has already been merged so it's no longer useful to push here. |
|
Ok, thanks! And thanks for adding syslog support in the first place. Obviously, I was waiting for it =) |
* Add log handlers syslog and errorlog Issue#9400 * Add log handlers syslog and errorlog * syslog/errorlog: fix namespace; add log.syslog.ident * syslog/errorlog: Document options * ErrorLogHandler constructor: Avoid null Avoid null in ErrorLogHandler constructor call: Use level param; default others * missing comma * fix two di definitions * Use Monolog constructor-default facility Issue #17764 * #17855 SyslogHandler: Fix typo * Update plugins/Monolog/config/config.php Co-authored-by: diosmosis <diosmosis@users.noreply.github.com> Co-authored-by: root <root@127.0.0.1>
Description:
Issue #9400 Add log handlers syslog and errorlog; Add config for syslog ident; Document options.
Review