Skip to content

[HttpKernel][FrameworkBundle] Add a minimalist default PSR-3 logger #24300

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

Closed
wants to merge 4 commits into from

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Sep 23, 2017

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

This PR provides a minimalist PSR-3 logger that is always available when FrameworkBundle is installed.
By default, it writes errors on stderr, regular logs on stdout and discards debug data (this is configurable).

This approach has several benefits:

  • It's what expect from an app logging systems of major containerization and orchestration tools including Docker and Kubernetes, as well as most cloud providers such as Heroku and Google Container Engine. If the app follows this standard (and it's not currently the case with Symfony by default) logs will be automatically collected, aggregated and stored.
  • It's in sync with the "back to Unix roots" philosophy of Flex
  • Logs are directly displayed in the console when running the integrated PHP web server (bin/console server:start or Flex's make serve), Create React App also do that for instance.
  • It fixes a common problem when installing Flex recipes: many bundles expect a logger service but currently there is none available by default, and you usually get a "logger" service not found error (because packages depend of the PSR, but the PSR doesn't provide a logger service).

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I like this idea :)

@@ -303,6 +304,10 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('web_link.xml');
}

$loggerDefinition = $container->register(LoggerInterface::class, Logger::class);
$loggerDefinition->setPublic(false);
$container->setAlias('logger', LoggerInterface::class);
Copy link
Member

Choose a reason for hiding this comment

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

the alias also needs to be private.
for other services, we create the "noun" as a real definition, and the "fqcn" as the alias
for consistency, shouldn't we do the same here also?

final class Logger extends AbstractLogger
{
/**
* @var array
Copy link
Member

Choose a reason for hiding this comment

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

should be removed

*
* @author Kévin Dunglas <dunglas@gmail.com>
*
* @see http://www.php-fig.org/psr/psr-3/
Copy link
Member

Choose a reason for hiding this comment

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

no need for that imho

private $formatPattern;

/**
* @var bool
Copy link
Member

Choose a reason for hiding this comment

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

should be removed


/**
* @param array $levelToStream
* @param string $formatPattern
Copy link
Member

Choose a reason for hiding this comment

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

almost no value also, should be removed imho

*/
public function __construct(array $levelToStream = array(), $formatPattern = '[%s] %s')
{
$this->levelToStream = $levelToStream + self::$defaultLevelToStream;
Copy link
Member

Choose a reason for hiding this comment

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

destByLevel? outputByLevel?

throw new InvalidArgumentException(sprintf('The log level "%s" does not exist.', $level));
}

if (!$this->errored) {
Copy link
Member

Choose a reason for hiding this comment

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

in fact, I'm skeptical about the need to write conditionally to stderr vs stdout.
despite its name, stderr is not only for errors but is for also for diagnostics, which all logs are to me,
especially in the simple logger we target.

}

if ($this->levelToStream[$level]) {
file_put_contents($this->levelToStream[$level], sprintf($this->formatPattern, $level, $this->interpolate($message, $context)).PHP_EOL, FILE_APPEND);
Copy link
Member

Choose a reason for hiding this comment

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

could it be "fwrite" for perf (ie the stream should be opened once?)

*
* @return bool
*/
public function hasErrored()
Copy link
Member

Choose a reason for hiding this comment

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

not sure we need the method?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it only exists here because the same already exists in ConsoleLogger. But what is the use case?

Copy link
Member Author

Choose a reason for hiding this comment

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

In ConsoleLogger, it has been introduced by Nicolas in #19090. The use case is to be able to detect if an error has occurred and to set the appropriate exit code. But it doesn't look to be used in the Symfony code base and the PR doesn't give more information.
I've added this for the sake of consistency but maybe should it be removed?

Copy link
Member

Choose a reason for hiding this comment

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

OK, let's keep it :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, can you give me a bit more information about the use case? (this method isn't in any interface)

Copy link
Member

Choose a reason for hiding this comment

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

I dig a bit and we used to use it in the Blackfire Player - we don't anymore.
So we can consider it a non-common case and remove it from the simple logger we want here.
Let's drop :)

}

// interpolate replacement values into the message and return
return strtr($message, $replace);
Copy link
Member

Choose a reason for hiding this comment

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

what about this for perf?

        $replace = array();
        foreach ($context as $key => $val) {
            if (!\is_array($val) && (!\is_object($val) || method_exists($val, '__toString'))) {
                $replace["{{$key}}"] = $val;
            }
        }

        // interpolate replacement values into the message and return
        return $replace ? strtr($message, $replace) : $message;

Copy link
Contributor

@ogizanagi ogizanagi Sep 24, 2017

Choose a reason for hiding this comment

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

Couldn't we reuse the same code as in PsrLogMessageProcessor? (and update ConsoleLogger btw)

Copy link
Member Author

Choose a reason for hiding this comment

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

I've merged both proposals and added some perf optims. I'll open a PR to update existing implementations like the one in ConsoleLogger.

Copy link
Contributor

@ogizanagi ogizanagi left a comment

Choose a reason for hiding this comment

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

So, this PR aims to provide a sensible default logger, when not using MonologBundle, for instance when starting a simple project with flex.
I like it :)

I wonder: would it be interesting to allow not interpolating messages, but inline context at the end (see Monolog's LineFormatter)? It'll allow better aggregation, but I don't know if the logging systems you mentioned could leverage it easily.

*
* @return bool
*/
public function hasErrored()
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it only exists here because the same already exists in ConsoleLogger. But what is the use case?

LogLevel::ALERT => 'php://stderr',
LogLevel::CRITICAL => 'php://stderr',
LogLevel::ERROR => 'php://stderr',
LogLevel::WARNING => 'php://stdout',
Copy link
Contributor

Choose a reason for hiding this comment

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

All defaults should be php::stderr IMHO (appart DEBUG => false).

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, all logs should go to stderr IMHO.

@@ -303,6 +304,10 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('web_link.xml');
}

$loggerDefinition = $container->register(LoggerInterface::class, Logger::class);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be in a pass and only if there is no logger service/alias already registered? (in other words, won't it conflict with the MonologBundle depending on the order the bundles are registered in the kernel? FrameworkBundle usually come first, but still)

Copy link
Member

Choose a reason for hiding this comment

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

The idea is indeed to provide this simple logger only when monolog (or any other logger) is not registered. Even if Flex tries to keep FrameworkBundle first, we should not rely on this.

fabpot
fabpot previously requested changes Sep 24, 2017
@@ -303,6 +304,10 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('web_link.xml');
}

$loggerDefinition = $container->register(LoggerInterface::class, Logger::class);
Copy link
Member

Choose a reason for hiding this comment

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

The idea is indeed to provide this simple logger only when monolog (or any other logger) is not registered. Even if Flex tries to keep FrameworkBundle first, we should not rely on this.

LogLevel::ALERT => 'php://stderr',
LogLevel::CRITICAL => 'php://stderr',
LogLevel::ERROR => 'php://stderr',
LogLevel::WARNING => 'php://stdout',
Copy link
Member

Choose a reason for hiding this comment

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

Indeed, all logs should go to stderr IMHO.

LogLevel::ERROR => 'php://stderr',
LogLevel::WARNING => 'php://stdout',
LogLevel::NOTICE => 'php://stdout',
LogLevel::INFO => 'php://stdout',
Copy link
Member

Choose a reason for hiding this comment

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

I haven't had a look carefully, but I'm not sure we need to output info/notice logs. Intuitively, I would have skipped them.

@dunglas
Copy link
Member Author

dunglas commented Sep 24, 2017

I've changed the way levels are handled:

  • Only stderr is used by default
  • Levels less critical than warning are ignored by default
  • if kernel.debug is set, all levels enabled (in the compiled pass, and sent to stderr)

I've also fixed most comments, thanks everybody for the review.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

the logger should have a "reset" method, and be tagged "kernel.reset".

}

if (!$this->errored) {
$this->errored = in_array($level, array(LogLevel::EMERGENCY, LogLevel::ALERT, LogLevel::CRITICAL, LogLevel::ERROR));
Copy link
Member

Choose a reason for hiding this comment

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

can be \in_array + true as 3rd arg to trigger compile-time optims by the php enging

*
* @return bool
*/
public function hasErrored()
Copy link
Member

Choose a reason for hiding this comment

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

OK, let's keep it :)

*/
public function process(ContainerBuilder $container)
{
$loggerDefinition = $container->register('logger', Logger::class);
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be set after an "if" somewhere? where is the "if necessary" condition?

*/
public function process(ContainerBuilder $container)
{
$loggerDefinition = $container->register('logger', Logger::class);
Copy link
Contributor

Choose a reason for hiding this comment

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

It must check if there is no existing def/alias for logger and LoggerInterface.

{
$loggerDefinition = $container->register('logger', Logger::class);
$loggerDefinition->setPublic(false);
if ($container->getParameter('kernel.debug')) {
Copy link
Member

Choose a reason for hiding this comment

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

How does that work with the console. Wouldn't we have too much output now?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure of what the best behavior is.

Imagine the following scenario:

  • I've a problem in production
  • I update the env var APP_DEBUG to true

I want all the available level of logs to be able to debug. (the current behavior).

However, Symfony is very verbose, APP_DEBUG is set to true for the dev env, and dev is the default env when the framework is installed. I agree that having a bunch of useless logs in the console when just running make serve is not a good developer experience.

What do you think about providing a custom logger definition (with less output) in the Flex recipe for just the dev env? We can also disable logs in the test env.

$replacements["{{$key}}"] = $val;
} elseif ($val instanceof \DateTimeInterface) {
$replacements["{{$key}}"] = $val->format($this->dateFormat);
} elseif (is_object($val)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs leading \ too

$this->handles[$stream] = \fopen($stream, 'a');
}

\fwrite($this->handles[$stream], \sprintf($this->format, $level, $this->interpolate($message, $context)).PHP_EOL);
Copy link
Contributor

Choose a reason for hiding this comment

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

PHP_EOL needs a leading \ too

Copy link
Member

Choose a reason for hiding this comment

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

we don't want to root-namespace everything, see PHP-CS-Fixer/PHP-CS-Fixer#3048 for some background.

Copy link
Member

Choose a reason for hiding this comment

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

Well, for constants, it would make sense though (as they are resolved at compile time for internal constants)


if ($stream = $this->outputByLevel[$level]) {
if (!isset($this->handles[$stream])) {
$this->handles[$stream] = \fopen($stream, 'a');
Copy link
Member

Choose a reason for hiding this comment

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

\ should be removed in fact, sorry for the confusion, only a short list of functions really benefit from \
see PHP-CS-Fixer/PHP-CS-Fixer#3048

@dunglas
Copy link
Member Author

dunglas commented Sep 24, 2017

@ogizanagi

I wonder: would it be interesting to allow not interpolating messages, but inline context at the end (see Monolog's LineFormatter)? It'll allow better aggregation, but I don't know if the logging systems you mentioned could leverage it easily.

AFAIK, most platforms doesn't support that unless you use their proprietary API (ex for Stackdriver: https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry).
I'm not aware of a standard way to do that (but if you have a hint, I'm very interested).

IMO we should keep this builtin logger as small and standard as possible and recommend to use other libraries such as Monolog for more advanced uses like this one... until a standard appears.

*
* @return string
*/
private function interpolate($message, array $context)
Copy link
Contributor

Choose a reason for hiding this comment

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

This PR is awesome ! But any chance to move this behavior in a dedicated class behind interface ?
Like the Formatter of Monolog. I know you want to keep it simple but it is a big lack to be used imo.

We look for this kind of logger as it is our main usage (docker). But our logs are parsed into an ELK and without custom formatting we will not be able to use it (or change all other apps not in PHP that output logs and logstash config...).

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe can we just allow a callable in format instead of a string?

Copy link
Contributor

Choose a reason for hiding this comment

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

interpolate is added here only because it is part of the PSR-3 standard AFAIK. For custom advanced formatting, Monolog should be used

Copy link
Contributor

@tyx tyx Sep 25, 2017

Choose a reason for hiding this comment

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

I'm not sure a callable only for format would help (or I miss anything?).

We would need both message and context separate as @ogizanagi said (I just realized he said the same thing).

I don't see any problem to not having this behavior embed by default, but if we could make it possible by design, we will definitively use it. Like a callable on the whole sprintf with message level context as arguments ?

But apparently other people are not enthusiastic about adding more configuration 😭 I guess we could just copy/paste the code also.

Copy link
Member Author

@dunglas dunglas Sep 25, 2017

Choose a reason for hiding this comment

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

The callable will take the message and the context in parameters, and wil return a string. if none is passed, the current sprintf will be used.

It has several benefits:

  • on one hand it's very flexible for the end user, allowing all kind of customizations
  • on the other hand, we can remove all configuration options and it's very solid

WDYT @fabpot @javiereguiluz?

Copy link
Member

Choose a reason for hiding this comment

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

No configuration seems better to me. Any configuration should involve using Monolog.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. Let's forgot about this.

Copy link
Member Author

@dunglas dunglas Sep 25, 2017

Choose a reason for hiding this comment

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

I've added a callable formatter implementation in a separate commit. It's just 1 line of code but it opens interesting possibilities in a cloud or Docker environment (see the json formatter in tests). IMO it is worth it.

Copy link

Choose a reason for hiding this comment

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

Without the formatter callable, I guess everybody will keep using monolog in fact, all the time. So in this case I'm not sure it's a good move to merge this PR at all.

Either monolog remains the log dependency - no need for this Logger - either the formatter should be added and we can sometimes decide to require monolog or not depending of the complexity of the wanted log stack. WDYT?

private $dateFormat;
private $handles = array();

public function __construct(array $outputByLevel = array(), $format = '[%s] %s', $dateFormat = \DateTime::RFC3339)
Copy link
Member

Choose a reason for hiding this comment

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

Given that this is a minimalist logger, could we make some decisions about the formats instead of making them configurable? In other words, hardcode [%s] %s as $format and \DateTime::RFC3339 as $dateFormat. If someone wants configurability, use a full-featured logger such as Monolog.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not, I've done that because it comes almost for free but we can be more restrictive. Exposing $format looks useful anyway.

Copy link
Member

Choose a reason for hiding this comment

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

I think I agree with @javiereguiluz. Even if it's free, it should be crystal clear that this logger is just a default/simple one. For any configurability, people still need to install Monolog. So, I would remove any way that allow configuration of this class.

/**
* Minimalist PSR-3 logger designed to write in stdout, stderr or any other stream.
*
* @author Kévin Dunglas <dunglas@gmail.com>
Copy link
Member

Choose a reason for hiding this comment

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

Could be marked as @internal, right?

Copy link
Member Author

@dunglas dunglas Sep 25, 2017

Choose a reason for hiding this comment

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

Indeed. It may be useful in other contexts than FrameworkBundle, but this class is so simple that a copy/paste should be good enough.

private $handles = array();
private $formatter;

public function __construct(array $outputByLevel = array(), callable $formatter = null)
Copy link
Member

Choose a reason for hiding this comment

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

Why are you still allowing a formatter callable here, which is never used when wiring the service ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -22,6 +22,7 @@
use Symfony\Bundle\FrameworkBundle\Command\YamlLintCommand;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Bundle\FrameworkBundle\Logger\Logger;
Copy link
Member

Choose a reason for hiding this comment

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

what for?

LogLevel::WARNING => 'php://stderr',
LogLevel::NOTICE => false,
LogLevel::INFO => false,
LogLevel::DEBUG => false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using using by-level output streams I would defined an unique stream and a minimum logged level, I don't think logging different levels to separate streams is a common enough use case for a minimal logger.

Copy link
Member Author

@dunglas dunglas Sep 26, 2017

Choose a reason for hiding this comment

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

I prefer to keep this feature, for instance it allows to redirect some levels on stdout and some others on strerr, or to create a file per level on disk.
This level to stream mapping and the formatter feature allows to have a very flexible solution in just 100 LOC (100 LOC is what we can call minimal). For all more advanced use cases, Monolog must be installed and that's is.

Copy link
Member

Choose a reason for hiding this comment

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

redirecting logs to stdout is not a good practice and should not be encouraged, so that's not a valid use case IMHO. Creating a file is also not a use case. I tend to agree with @jvasseur, let's keep it minimal. It's not "Monolog should be installed for advanced use cases", it's more like "Monolog must be installed for any configurable logger". If not, drawing the line won't be easy.

Copy link
Member Author

@dunglas dunglas Sep 26, 2017

Choose a reason for hiding this comment

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

I was looking for feature parity with the log package of the Go language (that is minimalist but very convenient with Docker). This package has the ability to change the output (https://golang.org/pkg/log/#SetOutput) and to set the format. That's all.

Copy link
Member

@nicolas-grekas nicolas-grekas Sep 26, 2017

Choose a reason for hiding this comment

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

Go only allows configuring one single output, not one per level, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

Copy link

Choose a reason for hiding this comment

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

Makes sense to configure the output globally, not by level indeed.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

👍 as is :)

@nicolas-grekas
Copy link
Member

when test are reasonably green :)

@@ -18,6 +18,7 @@
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\CachePoolClearerPass;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\CachePoolPrunerPass;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\DataCollectorTranslatorPass;
use Symfony\Component\HttpKernel\DependencyInjection\LoggerPass;
Copy link
Member

Choose a reason for hiding this comment

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

alpha order

@dunglas dunglas force-pushed the minimal-logger branch 3 times, most recently from 1fd3bb4 to 5efbd75 Compare September 29, 2017 06:39
$this->logger->log(LogLevel::DEBUG, 'test', array('user' => 'Bob'));

// Will always be true, but asserts than an exception isn't thrown
$this->assertLogsMatch(array(), $this->getLogs());
Copy link
Member

Choose a reason for hiding this comment

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

should be assertSame(array(), ...

@fabpot
Copy link
Member

fabpot commented Sep 29, 2017

Thank you @dunglas

fabpot added a commit that referenced this pull request Sep 29, 2017
… PSR-3 logger (dunglas)

This PR was squashed before being merged into the 3.4 branch (closes #24300).

Discussion
----------

[HttpKernel][FrameworkBundle] Add a minimalist default PSR-3 logger

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

This PR provides a minimalist PSR-3 logger that is always available when FrameworkBundle is installed.
By default, it writes errors on `stderr`, regular logs on `stdout` and discards debug data (this is configurable).

This approach has several benefits:

- It's what expect from an app logging systems of major containerization and orchestration tools including [Docker](https://docs.docker.com/engine/admin/logging/view_container_logs/) and [Kubernetes](https://kubernetes.io/docs/concepts/cluster-administration/logging/), as well as most cloud providers such as [Heroku](https://devcenter.heroku.com/articles/logging#writing-to-your-log) and [Google Container Engine](https://kubernetes.io/docs/tasks/debug-application-cluster/logging-stackdriver/). If the app follows this standard (and it's not currently the case with Symfony by default) logs will be automatically collected, aggregated and stored.
- It's in sync with the "back to Unix roots" philosophy of Flex
- Logs are directly displayed in the console when running the integrated PHP web server (`bin/console server:start` or Flex's `make serve`), Create React App also do that for instance.
- It fixes a common problem when installing Flex recipes: many bundles expect a logger service but currently there is none available by default, and you usually get a `"logger" service not found error` (because packages depend of the PSR, but the PSR doesn't provide a logger service).

Commits
-------

9a06513 [HttpKernel][FrameworkBundle] Add a minimalist default PSR-3 logger
@fabpot fabpot closed this Sep 29, 2017
fabpot added a commit that referenced this pull request Sep 30, 2017
…s (ogizanagi)

This PR was merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] Register a NullLogger from test kernels

| Q             | A
| ------------- | ---
| Branch?       | 3.4 <!-- see comment below -->
| Bug fix?      | no
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | N/A <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

Relates to #24300

This will avoid unnecessary output on Travis or when running FrameworkBundle tests locally:
- before: https://travis-ci.org/symfony/symfony/jobs/281624658#L3594-L3635
- after: https://travis-ci.org/symfony/symfony/jobs/281643868#L3599-L3617

but also needed for anyone running functional tests on their project and using the default logger, in order to not get garbage output.

Do we need to find a more generic solution (like exposing a `framework.default_logger` option so users can set it to false for test)? Or just documenting this?

Commits
-------

c109dcd [FrameworkBundle] Register a NullLogger from test kernels
This was referenced Oct 18, 2017
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Sep 19, 2018
This PR was squashed before being merged into the 3.4 branch (closes #10321).

Discussion
----------

Minimalist default PSR-3 logger

symfony/symfony#24300

Commits
-------

da84567 Minimalist default PSR-3 logger
fabpot added a commit that referenced this pull request Jan 4, 2025
…owiring alias in `LoggerPass` (nicolas-grekas)

This PR was merged into the 6.4 branch.

Discussion
----------

[HttpKernel] Don't override existing `LoggerInterface` autowiring alias in `LoggerPass`

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Something we missed in #24300

Explicit configuration should always have highest precedence, yet LoggerPass ignores the existing autowiring alias at the moment.

Commits
-------

771a79d [HttpKernel] Don't override existing LoggerInterface autowiring alias in LoggerPass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.