[HttpKernel][2.7] Add request uri to Logger context #13320
Conversation
rvanlaak
changed the title
| @@ -140,7 +140,8 @@ public function onKernelRequest(GetResponseEvent $event) | |||
| } | |||
|
|
|||
| if (null !== $this->logger) { | |||
| $this->logger->info(sprintf('Matched route "%s"', $parameters['_route']), $parameters); | |||
| $logContext = $parameters + array('request-uri' => $request->getUri()); | |||
There was a problem hiding this comment.
please distinguish route params and request uri, i.e. array('route-params' => $parameters, 'request-uri' => $request->getUri())
| $logContext = array( | ||
| 'route-params' => $parameters, | ||
| 'request-uri' => $request->getUri(), | ||
| ); |
There was a problem hiding this comment.
#13418 tries to make context more consistent. So, can you rename route-params to route_parameters and request-uri to request_uri? You can also inline the context instead of creating a temp var here. Thanks.
|
👍 after the minor renaming is done. |
|
Fixed @fabpot will try to squash all commits in one I've been looking for docs about logging and log parameter conventions, but couldn't find them. |
| @@ -140,7 +140,10 @@ public function onKernelRequest(GetResponseEvent $event) | |||
| } | |||
|
|
|||
| if (null !== $this->logger) { | |||
| $this->logger->info(sprintf('Matched route "%s"', $parameters['_route']), $parameters); | |||
| $this->logger->info(sprintf('Matched route "%s"', $parameters['_route']), array( | |||
| 'route_params' => $parameters, | |||
There was a problem hiding this comment.
I would prefer route_parameters as mentioned in my previous comment.
|
👍 |
... so host info does not get lost in the logging. The current situation does not allow the user to determine at which host an error occured. | Q | A | ------------- | --- | Bug fix? | [no] | New feature? | [yes] | BC breaks? | [no] | Deprecations? | [no] | Tests pass? | [yes] | Fixed tickets | | License | MIT | Doc PR | Fixed typo Distinguish route-params from request-uri Update context consistency ... and inline the context variable. Rename `route_params` to `route_parameters`
|
Squashing all commits into one is done. Had to figure out PhpStorm 8.0.3 did not allow me to force a push, because the force option was missing. |
|
Thank you @rvanlaak. |
…vanlaak) This PR was merged into the 2.7 branch. Discussion ---------- [HttpKernel][2.7] Add request uri to Logger context ... so host info does not get lost in the logging. The current situation does not allow the user, that receives a `Monolog` email for instance, to determine at which host an error occurred. | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | - Commits ------- c8f1f19 [HttpKernel] Add request uri to Logger context
... so host info does not get lost in the logging. The current situation does not allow the user, that receives a
Monologemail for instance, to determine at which host an error occurred.