Leverage AsMonologProcessor attribute #428
Conversation
af9e2ed to
c9fea7a
Compare
|
Shall we add the attribute to the Monolog Bridge instead? |
|
To use it on the Bridge's processors? Since the tag and its attributes are only consumed by the bundle, I thought it belonged here. |
The same is true for any other |
|
Yes but this bundle is outside symfony/symfony, doesn't it make it different? Adding the attribute in symfony/symfony would have to be done on 6.1 as a new feature I guess. Therefore, the whole feature would only be available once 6.1 is out and only with symfony/monolog-bridge >= 6.1 😅 If everything is done here, it can be released earlier and it will be available with symfony/di >= 5.4 🤷♂️ |
I don't think so. As long as we have the bridge, this is where the attribute belongs. The attribute is leveraged by your integration in this PR, but this is not the only way it could be used.
Yes.
Correct. We don't deliver features for Symfony 5 anymore, so that limitation would be just fine. |
|
I think we should submit the attribute to monolog! When the attribute is set, we should also configure the binding. Here is where it's done for the corresponding marker interface. |
|
Thanks @derrabus and @nicolas-grekas for your explanations. @derrabus, (and maybe @Seldaek), WDYT of adding the attribute directly in Monolog? |
|
If the attribute is added to Monolog, great. If not, I'm still in favor of adding it to the bridge. 🙂 |
|
I'm not sure how much sense this makes in Monolog though as it seems really specific to the DI autowiring here, but on the other hand it's a tiny zero-maintenance piece of code so I'm also not strictly opposed to it. The channel/handler are optional is that correct? And method is only needed if the attribute is set to a class? |
|
Everything is optional on the attribute. If the attribute is set on a class and the method is not specified, |
|
Oh right of course, well I guess send a PR, but please add some docs explaining the intent of each property, and a class level docblock should explain what this is meant for but that this offers no guarantee that a processor is seen as such when using Monolog alone. Or maybe we could support it too in addProcessor if the class does not implement ProcessorInterface try to read the attribute to see if a method property is defined? Anyway let's see the details on the monolog repo. |
c9fea7a to
f1d87f0
Compare
|
Thanks |
This feature allows configuring Monolog processors with an attribute. I basically copied
AsEventListener.