@mwithheld opened this Pull Request on July 13th 2021 Contributor

Description:

Issue #9400 Add log handlers syslog and errorlog; Add config for syslog ident; Document options.

Review

  • [ ] Functional review done
  • [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@mwithheld commented on July 13th 2021 Contributor

This is an update to PR #17745

@diosmosis commented on July 14th 2021 Member

@mwithheld Thanks for fixing the issues! I left one other comment. After it's addressed it should be good to merge.

@mwithheld commented on July 15th 2021 Contributor

Added commit 14992a8 ErrorLogHandler constructor: Avoid null

@mwithheld commented on July 16th 2021 Contributor

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
https://github.com/matomo-org/matomo/pull/17764#discussion_r670919393:

@@ -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
https://github.com/matomo-org/matomo/pull/17764#pullrequestreview-707953192,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAGCSOXPWKB6RSVBKTMBCRDTX6JQRANCNFSM5AKBNYHA
.

@diosmosis commented on July 18th 2021 Member

@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.

@mattab commented on July 26th 2021 Member

@diosmosis could you please document this new capability in https://matomo.org/faq/troubleshooting/faq_115/ ? Thanks

@diosmosis commented on July 26th 2021 Member

documented

This Pull Request was closed on July 18th 2021
Powered by GitHub Issue Mirror