Description
We should try to enforce user's correct configuration as commented here.
Why? Because I've found a tricky misconfiguration that leaded to wrong parsing of my logs and in loosing them.
That was the (wrong) configuration stripped off the not congruous parts in order to avoid noise, please don't focus on meaning but keep attention on technical matters
monolog:
channels: ['foo_channel']
handlers:
foo_channel_handler:
channels: [foo_channel]
type: stream # <--- THE "SILENT ERROR"
level: error
handler: grouped
grouped:
type: group
members: [streamed]
streamed:
type: stream
path: "%kernel.logs_dir%/%kernel.environment%.log"
level: debug
formatter: monolog.formatter.session_request
monolog.formatter.session_request:
class: Monolog\Formatter\LineFormatter
arguments:
- "[%%datetime%%] [%%extra.token%%] %%channel%%.%%level_name%%: %%message%% \n"
This configuration is clearly wrong (type: stream
tries to use filter
wrapper handler for a configuration error) but Symfony didn't raise an exception.
Why this could be a problem?
In this particular case I had two default values explicitated:
path
--> https://github.com/symfony/monolog-bundle/blob/master/DependencyInjection/Configuration.php#L377
formatter
(I mean, the Class) --> Monolog\Formatter\LineFormatter
(AFAIK is the default one)
So in my tests the log was created at the right location using the desired formatter, except that I dind't noticed that [%%extra.token%%]
was never included. This is due to the fact that, correctly, stream
type never "pass" control to grouped
and so to streamed
.
That was nasty to debug and lost several hours.
We have two chances, both seems to introduce a BC break:
- Handlers
type
as configuration keys
Specifying the type of the handler as key and checking that at least one of the the types is specified under handler's name we can better state what to expect and what should not be nested there. One of the things that worry me the most with this approach is we should carry every acceptable value under each type
key in the configuraton three.
Moreover this would introduce a new way of declaring (and so checking) custom handlers, something like "force" somehow the user to give a definition for its type.
- Performing semantic checks in
MonologExtension
This seems the best way from my standpoint. We can easily check here that all expected attributes (and no more that those) are carried by the configuration, raising and error otherwise.
I think that both methods introduce a BC break as in the first case everyone needs to change configuration files whereas in the latter case we were accepting wrong configuration that could possibly not working anymore.
If this makes sense to you I'm willing to work in order to reach what I've shown above.
I'm also open to other ideas.
Let me know what do you think.