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

Convert all log messages to PSR-3 format with placeholders #15753

Closed
hason opened this issue Sep 10, 2015 · 7 comments
Closed

Convert all log messages to PSR-3 format with placeholders #15753

hason opened this issue Sep 10, 2015 · 7 comments

Comments

@hason
Copy link
Contributor

hason commented Sep 10, 2015

It should be used for logging context instead of sprintf.

Before:

$this->logger->info(sprintf('Matched route "%s".', $parameters['_route'] ?? 'n/a'))

After:

$this->logger->info('Matched route "{route}".', ['route' => $parameters['_route'] ?? 'n/a'])

It is still possible to log human-readable messages with PsrLogMessageProcessor, which can be activated on a per-handler basis.

This can be really useful for subsequent processing in logfile monitoring systems as it makes it easier to identify similar messages.

@javiereguiluz
Copy link
Member

@hason we must use "old PHP", so the new code would be instead:

$this->logger->info(
    'Matched route "{route}".',
    array('route' => isset($parameters['_route']) ? $parameters['_route'] : 'n/a')
);

@Pierstoval
Copy link
Contributor

+1 for Javier.

This would turn into BC breaks, so I suggest thinking about this great idea for the 3.0 branch.

I'm 👍 on this 😄

@hason
Copy link
Contributor Author

hason commented Sep 14, 2015

@javiereguiluz @Pierstoval I've used the new syntax for its brevity. It is used even in the Before sample.

@derrabus
Copy link
Member

Thinking out loud: This semantic could also be used to highlight the variable parts of a log message when rendering the log in the Symfony Profiler.

@javiereguiluz
Copy link
Member

@derrabus highlighting log messages is on my TODO list for the profiler. Now that it's been merged, I'll think about it again!

@derrabus
Copy link
Member

My point is: Highlighting is hard, if you just have plain text messages, like when composing the messages with sprinf() or string concatenation. You would have to guess, what could be the important part of a log message. Okay, in the example above, the quotes would be a good indicator, but that assumption won't always work.

But if you get a template of the log message with variables to fill in, highlighting the variable part would probably improve the readability.

@fabpot fabpot closed this as completed Jan 14, 2016
fabpot added a commit that referenced this issue Jan 14, 2016
This PR was merged into the 3.1-dev branch.

Discussion
----------

Add placeholders into log messages

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #15753
| License       | MIT
| Doc PR        |

Commits
-------

c92fcdb Added placeholders to all log messages instead of hardcoded values
@lyrixx
Copy link
Member

lyrixx commented Aug 17, 2016

May be it's too late to discuss about this PR / issue but I'm against it. I really don't see the value added by this (all stuff related into the PSR + Monolog).

My point of view:

It's very useful for message with a level superior to the level that trigger the cross finger (ie warning / error or more). But for others (debug / info) errors it's IMHO useless. It's useless because there is no need to aggregate log message like Notified event "{event}" to listener "{listener}". You have to register a new processor to do the strtr jobs in order to be able to read the logs.

More over, without code it's not possible to strtr notice / info / debug message but to let error+ message.

Could we consider re-opening this this ?

fabpot added a commit to symfony/monolog-bundle that referenced this issue Nov 6, 2016
… PSR-3 (ged15)

This PR was submitted for the master branch but it was merged into the 2.x branch instead (closes #173).

Discussion
----------

ability to configure whether messages should be formatted as PSR-3

This PR symfony/symfony#17166 (reasons described in symfony/symfony#15753) has changed the way messages are logged in Symfony. Now log messages contain placeholders (see https://github.com/symfony/symfony-standard/issues/981).
While this could be a useful feature, IMO by default in SE messages should be preformatted.
This PR introduces a new optional config option that makes the messages be preformatted by default but this behaviour can also be disabled.

Commits
-------

6c8a44b ability to configure whether messages should be formatted as PSR-3
44f3daf minor #179 removed obsolete code (fabpot)
7d28c5a Merge branch '2.x'
b6baaaf Merge branch '2.x'
57b33d6 removed obsolete code
a45682c updated CHANGELOG
bcbf4c5 fixed tests
079c3d1 feature #170 remove class parameters (avant1)
6398efc remove class parameters
885da32 added a CHANGELOG
d8b3f8a feature #169 Deprecate the NotFoundActivationStrategy class (BPScott)
b77bdf0 Deprecate the NotFoundActivationStrategy class
ce169ee bumped version to 3.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants