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

Add support to log to redis #292

Closed
wants to merge 44 commits into from

Conversation

ruudvdd
Copy link

@ruudvdd ruudvdd commented Jan 3, 2019

Updated version of #262

@ruudvdd ruudvdd mentioned this pull request Jan 3, 2019
@ruudvdd
Copy link
Author

ruudvdd commented Jan 4, 2019

Got the same deprecation notice as #279 and can't find the reason other than it's a known issue in older versions of phpunit.

Ref: symfony/maker-bundle#330

@Seldaek Seldaek added this to the 3.4 milestone Jan 13, 2019
B-Galati and others added 7 commits January 16, 2019 10:19
This PR was merged into the 3.x-dev branch.

Discussion
----------

Remove duplicate entry

Commits
-------

24291fb Remove duplicate entry
This PR was merged into the 3.x-dev branch.

Discussion
----------

Expose configuration for the ConsoleHandler

Like that we will be able to use this configuration (for example):
```yaml
        console:
            type: console
            process_psr_3_messages: false
            channels: ["!event", "!doctrine", "!console"]
            console_formater_options:
                format: "%%datetime%% %%start_tag%%%%level_name%%%%end_tag%% <comment>[%%channel%%]</> %%message%%%%context%%\n"
                multiline: false
```

see also symfony/symfony#30345

Commits
-------

cc9abf6 Expose configuration for the ConsoleHandler
…s-grekas)

This PR was merged into the 3.x-dev branch.

Discussion
----------

Register processors for autoconfiguration

Replaces symfony/symfony#27801 (which has been reverted meanwhile)
Leverages Seldaek/monolog#1204
Already documented in symfony/symfony-docs#9996 (but doc needs update)

Commits
-------

7fdda67 Register processors for autoconfiguration
This PR was merged into the 3.x-dev branch.

Discussion
----------

Fix typo in PhpDoc

As the `DeduplicationHandler` extends the `BufferHandler` I think the wording for the `handler` parameter should be the same.

Commits
-------

981d74f Fix typo in PhpDoc
Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello
Thanks for your contribution.
I let some smalls comments. Could you have a look at them?

DependencyInjection/Configuration.php Outdated Show resolved Hide resolved
DependencyInjection/Configuration.php Outdated Show resolved Hide resolved
DependencyInjection/MonologExtension.php Show resolved Hide resolved
DependencyInjection/MonologExtension.php Outdated Show resolved Hide resolved
DependencyInjection/MonologExtension.php Outdated Show resolved Hide resolved
@lyrixx
Copy link
Member

lyrixx commented May 16, 2019

I also forgot to ask you to add a note in the CHANGELOG in v3.4.0

dnna and others added 7 commits May 16, 2019 12:20
This PR was squashed before being merged into the 3.x-dev branch (closes symfony#279).

Discussion
----------

Flush loggers on kernel.reset

Fixes php-pm/php-pm-httpkernel#134 and php-pm/php-pm-httpkernel#62

When using [PHP-PM](https://github.com/php-pm/php-pm) the worker does not die at the end of the request, so the shutdown functions registered by monolog are never triggered (so for example `BufferHandler` never flushes). This PR partially fixes the problem by flushing the loggers containing a `flush` function at the end of each request.

I did not use the `close` function (mentioned in Seldaek/monolog#1053) because some loggers use sockets or other connections that would be closed in this case and not be reusable in the next request.

I'm not sure if this is the best solution to this problem, happy to discuss alternatives if there is a better way.

Commits
-------

366f692 Flush loggers on kernel.reset
This PR was merged into the 3.x-dev branch.

Discussion
----------

Deprecate "excluded_404s" option

replaces symfony#275
fixed symfony/symfony#26969

I have fixed confliects + CS + updated the CHANGELOG

Commits
-------

f6c8593 Deprecrate "excluded_404s" option
This PR was merged into the 3.x-dev branch.

Discussion
----------

Support environment option for raven client

In sentry 9, we can filter logs thourgh environments. This PR allows to support this option.

Commits
-------

86fd51e Support environment option for raven client
@lyrixx
Copy link
Member

lyrixx commented May 16, 2019

Also, could you rebase please? thanks

…ler, affects symfony#266 (dhirtzbruch)

This PR was merged into the 3.x-dev branch.

Discussion
----------

Allow setting "ident" parameter for SyslogUdpHandler, affects symfony#266

As outlined in ticket symfony#266:

The SyslogUdpHandler constructor has a sixth parameter "ident", defaulting to "php".
However, the current Monolog Bundle implementation allows setting only the first five. Which defaults to the "ident" being set to "php".
In our case, we are using papertrail as a log backend, and use the same webserver for multiple small projects. In papertrail, that results in the following output:

```
May 05 02:16:30 vagrant php:  app.ERROR: fooo {"exception":"[object] (Exception(code: 0): fooo at /var/customers/webs/fastbolt/https/env1.fastbolt.com/src/Pricecalc/UserBundle/Controller/SecurityController.php:36)","username":"anon.","uri":"http://env1.dev.local/login","level":"critical"} []
May 05 02:18:32 vagrant php:  app.ERROR: fooo {"exception":"[object] (Exception(code: 0): fooo at /var/customers/webs/fastbolt/https/env2.fastbolt.com/src/Pricecalc/UserBundle/Controller/SecurityController.php:36)","username":"anon.","uri":"http://env2.dev.local/login","level":"critical"} []
```

The "php" after "vagrant" is the ident configuration. As you can see, we are not able to determine the source environment, other than guessing from filepath or uri.
However, for filtering using the API, we`d like to be able to change the ident per configuration.

```
monolog:
    handlers:
        syslog:
            type: syslogudp
            host: example.com
            port: 514
            level: error
            ident: env1
```

Using that config, we should be able to achieve the following log:

```
May 05 02:16:30 vagrant env1:  app.ERROR: fooo {"exception":"[object] (Exception(code: 0): fooo at /var/customers/webs/fastbolt/https/env1.fastbolt.com/src/Pricecalc/UserBundle/Controller/SecurityController.php:36)","username":"anon.","uri":"http://env1.dev.local/login","level":"critical"} []
May 05 02:18:32 vagrant env2:  app.ERROR: fooo {"exception":"[object] (Exception(code: 0): fooo at /var/customers/webs/fastbolt/https/env2.fastbolt.com/src/Pricecalc/UserBundle/Controller/SecurityController.php:36)","username":"anon.","uri":"http://env2.dev.local/login","level":"critical"} []
```

Please let me know if there is anything else I should do, as that`s my first PR/issue here.

Commits
-------

9f6f0b6 Allow setting "ident" parameter for SyslogUdpHandler, affects symfony#266
This PR was merged into the 3.x-dev branch.

Discussion
----------

Fixed PHPUnit Warning

Commits
-------

9f8e531 Fixed PHPUnit Warning
This PR was merged into the 3.x-dev branch.

Discussion
----------

Add stream handler lock option

This PR adds the $useLocking StreamHandler argument : https://github.com/Seldaek/monolog/blob/master/src/Monolog/Handler/StreamHandler.php#L44

Commits
-------

00ed2f3 Add stream handler lock option
@ruudvdd
Copy link
Author

ruudvdd commented May 28, 2019

@lyrixx
Thanks for reviewing. I've fixed your comments!

Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

I left 2 minor comments

DependencyInjection/MonologExtension.php Outdated Show resolved Hide resolved
DependencyInjection/MonologExtension.php Outdated Show resolved Hide resolved
DependencyInjection/MonologExtension.php Outdated Show resolved Hide resolved
@lyrixx
Copy link
Member

lyrixx commented May 28, 2019

Oh, you forgot to update the README.md file. Thanks

@lyrixx
Copy link
Member

lyrixx commented Jun 3, 2019

Hello, It looks like something goes wrong with git.
Could you rebase please?
Thanks

@ruudvdd
Copy link
Author

ruudvdd commented Jun 3, 2019

I did a rebase before applying my changes, but I did it again.

I've changed the configuration documentation comment a bit, because it wasn't correct. Also, I reverted the $type change in the MonologExtension because it was correct the first time. It just wasn't documented right. 'redis' is the property in the handler config where the redis host, id, ... are stored. They're both named 'redis' because it's a reference to the technology, not the lib.

Also applied the fixes for your above comments

@lyrixx
Copy link
Member

lyrixx commented Jun 3, 2019

I think you did a merge, and not a rebase. I will try to fix it for you :)

@lyrixx
Copy link
Member

lyrixx commented Jun 3, 2019

Thanks for your hardwork.
I have finished it in #305, I have rebased your commits and squashed them into one.

@lyrixx lyrixx closed this Jun 3, 2019
lyrixx added a commit that referenced this pull request Jun 3, 2019
…en, lyrixx)

This PR was merged into the 3.x-dev branch.

Discussion
----------

Added support for Redis configuration

fixed #292

Commits
-------

9667b1f Fixed previous commit (CS)
4fb28f7 Added support for Redis configuration
dani-danigm pushed a commit to dani-danigm/monolog-bundle that referenced this pull request Jun 15, 2022
…en, lyrixx)

This PR was merged into the 3.x-dev branch.

Discussion
----------

Added support for Redis configuration

fixed symfony/monolog-bundle#292

Commits
-------

072bf26 Fixed previous commit (CS)
c0cb773 Added support for Redis configuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet