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

Add handler_default_channels configuration option #454

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented Mar 4, 2023

When configuring multiple handlers, one annoying problem I often come across is having to add every channel that is meant to be used with a specific handler to all handlers with an exclusive list, eg:

monolog:
    handlers:
        main:
            type: fingers_crossed
            action_level: error
            handler: nested
            channels: [ "!one", "!two", "!three" ]
        nested:
            type: stream
            path: php://stderr
            level: debug
            formatter: monolog.formatter.json
        console:
            type: console
            process_psr_3_messages: false
            channels: [ "!event", "!doctrine", "!one", "!two", "!three" ]

The idea behind this PR is to be able to set a list of default channels that is applied to all handlers.

monolog:
+   handler_default_channels: [ "!one", "!two", "!three" ]
    handlers:
        main:
            type: fingers_crossed
            action_level: error
            handler: nested
-           channels: [ "!one", "!two", "!three" ]
        nested:
            type: stream
            path: php://stderr
            level: debug
            formatter: monolog.formatter.json
        console:
            type: console
            process_psr_3_messages: false
-           channels: [ "!event", "!doctrine", "!one", "!two", "!three" ]
+           channels: [ "!event", "!doctrine" ]

If a handler has a list of its own and it's the same type as the default one (inclusive/exclusive) the two lists are merged. If the handler's list is a different type, it replaces the default one. Handlers without lists inherit the default one. A use_default_channels handler option was also added so that a certain handler can choose not to use the default list.

 monolog:
     handler_default_channels: [ "!one", "!two", "!three" ]
     handlers:
         one:
             # ...
             channels: [ "one", "four" ] # final list: ["one", "four"]
         two:
             # ...
             use_default_channels: false
             channels: [ "one", "four" ] # final list: ["one", "four"]
         three:
             # ...
             channels: [ "!two", "!three", "!four" ] # final list: ["!one", "!two", "!three", "!four"]
         four:
             # ...
             use_default_channels: false
             channels: [ "!two", "!three", "!four" ] # final list: ["!two", "!three", "!four"]
         five:
             # ...
             # final list: ["!one", "!two", "!three"]
 monolog:
     handler_default_channels: [ "one", "two", "three" ]
     handlers:
         one:
             # ...
             channels: [ "one", "four" ] # final list: ["one", "two", "three", "four"]
         two:
             # ...
             use_default_channels: false
             channels: [ "one", "four" ] # final list: ["one", "four"]
         three:
             # ...
             channels: [ "!two", "!three", "!four" ] # final list: ["!two", "!three", "!four"]
         four:
             # ...
             use_default_channels: false
             channels: [ "!two", "!three", "!four" ] # final list: ["!two", "!three", "!four"]
         five:
         # ...
         # final list: ["one", "two", "three"]

@Seldaek
Copy link
Member

Seldaek commented Mar 10, 2023

I kinda get the problem you're trying to solve but IMO the solution is a bit unclear. Are those auto excluded channels excluded from all handlers, or only those with exclusion lists? I think it's the latter but I don't really see why..

Also name nitpick, perhaps default_excluded_channels instead of auto. Having default_channels might be even more sensible so you can pick or exclude like the rest of channels, and it wouldn't be an inverted option.

@HypeMC HypeMC force-pushed the autoexcludedchannelspass branch from 5bd9dd9 to f5cdcad Compare March 13, 2023 00:51
@HypeMC HypeMC changed the title Add auto_excluded_channels configuration option Add default_excluded_channels configuration option Mar 13, 2023
@HypeMC
Copy link
Contributor Author

HypeMC commented Mar 13, 2023

I kinda get the problem you're trying to solve but IMO the solution is a bit unclear. Are those auto excluded channels excluded from all handlers, or only those with exclusion lists? I think it's the latter but I don't really see why..

Hi, they're excluded only from handlers with an exclusion list. I don't think it would make since for the channels to be excluded from inclusion lists since the channels in those lists are explicitly added through the configuration. I'm not sure how to make this a little more clear.

Also name nitpick, perhaps default_excluded_channels instead of auto.

Your suggestion makes sense, I've updated the name.

Having default_channels might be even more sensible so you can pick or exclude like the rest of channels, and it wouldn't be an inverted option.

Just to see if I've understood you correctly, the default_channels option would be a list of channels to be included to or excluded from every handler. If that's the idea, I like it a lot. How would this work in terms of precedence though? Here's my guess:

  1. if default_channels is an exclusion list and the handlers channels list is an inclusion list, or, default_channels is an inclusion list and the handlers channels list is an exclusion list, the handlers list would take precedence:
    monolog:
        default_channels: [ "!one", "!two", "!three" ]
        handlers:
            main:
                # ...
                channels: [ "one", "four" ] # final list: ["one", "four"]
    monolog:
        default_channels: [ "one", "two", "three" ]
        handlers:
            main:
                # ...
                channels: [ "!one", "!four" ] # final list: ["!one", "!four"]
  2. if both default_channels and the handlers channels list are exclusion lists, or, both are inclusion lists, the lists would get merged:
    monolog:
        default_channels: [ "!one", "!two" ]
        handlers:
            main:
                # ...
                channels: [ "!two", "!three", "!four" ] # final list: ["!one", "!two", "!three", "!four"]
    monolog:
        default_channels: [ "one", "two" ]
        handlers:
            main:
                # ...
                channels: [ "two", "three", "four" ] # final list: ["one", "two", "three", "four"]

In other words, if the lists are the same type, they'd get merged, otherwise, if they're not, the handlers channels list would take precedance.

Also, what about cases such as whitelisting a default excluded channel on some handler or blacklisting a default included one on another? Should there be some special syntax for that, eg my !! proposal:

monolog:
    default_channels: [ "!one", "!two" ]
    handlers:
        main:
            # ...
            channels: [ "!!two", "!three", "!four" ] # final list: ["!one", "!three", "!four"]

or would adding a use_default_channels option make more sense. The option would be true by default, but could be set to false on a per-handler basis:

monolog:
    default_channels: [ "!one", "!two" ]
    handlers:
        main:
            # ...
            use_default_channels: false
            channels: [ "!one", "!three", "!four" ] # final list: ["!one", "!three", "!four"]

@Seldaek Please let me know your thoughts on this and I'll update the PR accordingly.

@HypeMC HypeMC force-pushed the autoexcludedchannelspass branch from f5cdcad to 81c547c Compare March 21, 2023 07:57
@HypeMC HypeMC changed the title Add default_excluded_channels configuration option Add handler_default_channels configuration option Mar 21, 2023
@HypeMC
Copy link
Contributor Author

HypeMC commented Mar 21, 2023

I've updated the PR according to my proposal.

@HypeMC HypeMC force-pushed the autoexcludedchannelspass branch from 81c547c to 402f0fc Compare August 6, 2024 10:18
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

Successfully merging this pull request may close these issues.

2 participants