[MonologBridge] Add monolog processors adding route and command info #28330

trakos · 2018-08-31T22:03:50Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR	symfony/symfony-docs#10244

This PR adds two simple processors that add context to every log entry.

RouteProcessor adds routing information:
app.INFO: Some log text {"someContext":"ctx"} {"route":{"controller":"App\\Controller\\SomeController::number","route":"index","route_params":[]}

ConsoleCommandProcessors adds current command information:
app.INFO: Some log text {"someContext":"ctx"} {"command":{"name":"app:some-command","arguments":{"command":"app:some-command","some-argument":10}}}

For ConsoleCommandProcessor I've decided against including command options by default, because there's a lot of default ones:
"options":{"help":false,"quiet":false,"verbose":false,"version":false,"ansi":false,"no-ansi":false,"no-interaction":false,"env":"dev","no-debug":false}. This behavior can be changed with a constructor argument.

lyrixx · 2018-09-03T13:21:40Z

I like it but I have some comments

The RouteProcessor does not work well. The "RouteData" should be unset on "kernel.finish_request".
The RoutePRocessor might be merged with the https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Monolog/Processor/WebProcessor.php ?

trakos · 2018-09-03T20:31:45Z

Yes, you're right, I didn't consider subrequests. I've updated it - I've made it so that it includes an array of (sub)requests, kind of like a stacktrace:

[
    {
        "controller": "App\\Controller\\SomeController::number",
        "route": "index",
        "route_params": []
    },
    {
        "controller": "App\\Controller\\SomeController::other",
        "route": "other",
        "route_params": {
            "page": "235"
        }
    }
]

The last one is the current one. It pops last entry during "kernel.finish_request". What do you think?

I can combine them if you prefer. There's some things to consider:
- Users won't be aware that update will make their log messages longer
- WebProcessor is an extend of monolog's WebProcessor, I think it's supposed to add the same information to the log
- There's probably some users that enable WebProcessor just to have IP logged and won't be happy about lengthier messages

@lyrixx What do you think?

fabpot · 2018-10-10T10:18:33Z

@lyrixx Can you review the latest version of this PR? Thank you.

lyrixx

It seems better. 👏
I left some comments.
Thanks for your work.

src/Symfony/Bridge/Monolog/Processor/RouteProcessor.php

src/Symfony/Bridge/Monolog/Tests/Processor/ConsoleCommandProcessorTest.php

trakos · 2018-10-12T14:52:54Z

Thanks for your feedback @lyrixx . I've updated the changes.

In RouteProcessor I've decided to use spl_object_id instead of SplObjectStorage, there's less code that way. Is that OK?

nicolas-grekas · 2018-11-10T20:00:16Z

meanwhile, ProcessorInterface has been removed from the bridge and added to Monolog directly in Seldaek/monolog#1204
If we don't want to bump the minimum version of Monolog, we should register the new classes for autoconfiguration, see symfony/monolog-bundle#285

trakos · 2018-11-10T20:46:15Z

Would you like me to create PR in symfony/monolog-bundle registering this classes, or should I wait till some decision is made?

nicolas-grekas · 2018-11-10T20:55:36Z

Let's first finish this one :)

lyrixx

LGTM except one tiny comment

src/Symfony/Bridge/Monolog/CHANGELOG.md

trakos · 2018-11-13T19:05:41Z

@lyrixx thanks, I've made the change. I've also removed extending ProcessorInterface, since it's no longer there. It should be ready now.

lyrixx · 2019-03-16T01:00:21Z

let's merge this one (except for native English, I can not tell)

fabpot · 2019-03-17T07:10:04Z

Thank you @trakos.

…d command info (trakos) This PR was squashed before being merged into the 4.3-dev branch (closes #28330). Discussion ---------- [MonologBridge] Add monolog processors adding route and command info | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | symfony/symfony-docs#10244 This PR adds two simple processors that add context to every log entry. RouteProcessor adds routing information: `app.INFO: Some log text {"someContext":"ctx"} {"route":{"controller":"App\\Controller\\SomeController::number","route":"index","route_params":[]}` ConsoleCommandProcessors adds current command information: `app.INFO: Some log text {"someContext":"ctx"} {"command":{"name":"app:some-command","arguments":{"command":"app:some-command","some-argument":10}}}` For ConsoleCommandProcessor I've decided against including command options by default, because there's a lot of default ones: `"options":{"help":false,"quiet":false,"verbose":false,"version":false,"ansi":false,"no-ansi":false,"no-interaction":false,"env":"dev","no-debug":false}`. This behavior can be changed with a constructor argument. Commits ------- 669f6b2 [MonologBridge] Add monolog processors adding route and command info

This PR was merged into the master branch. Discussion ---------- Add documentation on new monolog processors Documentation supporting symfony/symfony#28330 Commits ------- 3421c6c [MonologBridge] Add documentation on new processors

carsonbot added Status: Needs Review MonologBridge Feature labels Aug 31, 2018

This was referenced Aug 31, 2018

[WCM] Add documentation on new monolog processors trakos/symfony-docs#1

Closed

Add documentation on new monolog processors symfony/symfony-docs#10244

Merged

trakos force-pushed the monolog_processors_command_and_route branch from 27f07bc to 62ee940 Compare August 31, 2018 22:31

nicolas-grekas added this to the next milestone Sep 2, 2018

nicolas-grekas requested a review from lyrixx September 2, 2018 05:11

trakos force-pushed the monolog_processors_command_and_route branch 2 times, most recently from e6dee3f to 803dc25 Compare September 3, 2018 20:44

lyrixx requested changes Oct 11, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Oct 11, 2018

trakos force-pushed the monolog_processors_command_and_route branch from 329933c to 228511e Compare October 12, 2018 14:33

lyrixx reviewed Nov 13, 2018

View reviewed changes

src/Symfony/Bridge/Monolog/CHANGELOG.md Outdated Show resolved Hide resolved

trakos force-pushed the monolog_processors_command_and_route branch 3 times, most recently from ac92e84 to d331569 Compare November 13, 2018 18:47

carsonbot removed the Status: Needs Work label Mar 16, 2019

lyrixx approved these changes Mar 16, 2019

View reviewed changes

carsonbot added the Status: Reviewed label Mar 16, 2019

fabpot approved these changes Mar 17, 2019

View reviewed changes

[MonologBridge] Add monolog processors adding route and command info

669f6b2

fabpot force-pushed the monolog_processors_command_and_route branch from e17ae9a to 669f6b2 Compare March 17, 2019 07:08

fabpot merged commit 669f6b2 into symfony:master Mar 17, 2019

trakos mentioned this pull request Mar 19, 2019

logging/processors.rst: no mentions of built-in processors symfony/symfony-docs#11185

Closed

nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019

fabpot mentioned this pull request May 9, 2019

Release v4.3.0-BETA1 #31435

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MonologBridge] Add monolog processors adding route and command info #28330

[MonologBridge] Add monolog processors adding route and command info #28330

trakos commented Aug 31, 2018 •

edited

Loading

lyrixx commented Sep 3, 2018

trakos commented Sep 3, 2018 •

edited

Loading

fabpot commented Oct 10, 2018

lyrixx left a comment

trakos commented Oct 12, 2018

nicolas-grekas commented Nov 10, 2018

trakos commented Nov 10, 2018

nicolas-grekas commented Nov 10, 2018

lyrixx left a comment

trakos commented Nov 13, 2018

lyrixx commented Mar 16, 2019 •

edited

Loading

fabpot commented Mar 17, 2019

[MonologBridge] Add monolog processors adding route and command info #28330

[MonologBridge] Add monolog processors adding route and command info #28330

Conversation

trakos commented Aug 31, 2018 • edited Loading

lyrixx commented Sep 3, 2018

trakos commented Sep 3, 2018 • edited Loading

fabpot commented Oct 10, 2018

lyrixx left a comment

Choose a reason for hiding this comment

trakos commented Oct 12, 2018

nicolas-grekas commented Nov 10, 2018

trakos commented Nov 10, 2018

nicolas-grekas commented Nov 10, 2018

lyrixx left a comment

Choose a reason for hiding this comment

trakos commented Nov 13, 2018

lyrixx commented Mar 16, 2019 • edited Loading

fabpot commented Mar 17, 2019

trakos commented Aug 31, 2018 •

edited

Loading

trakos commented Sep 3, 2018 •

edited

Loading

lyrixx commented Mar 16, 2019 •

edited

Loading