Skip to content

Validating configuration #325

Open
@DonCallisto

Description

@DonCallisto

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:

  1. 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.

  1. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions