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 AsMonologProcessor attribute #1637

Merged
merged 1 commit into from
Mar 6, 2022

Conversation

fancyweb
Copy link
Contributor

Ref symfony/monolog-bundle#428

This PR adds an attribute that third-party consumers can leverage to "configure" Monolog processors.

I didn't find any real benefits in supporting it directly in Monolog's pushProcessor() methods:

  1. We would need to widen the type from callable to callable|object to accept any objects potentially using the attribute, and that would be a BC break.
  2. Monolog would only use the method "option" and it doesn't bring any real value, it's already easy to do:
->pushProcessor([$myProcessor, 'myCustomMethod'])

cc @derrabus and @nicolas-grekas

* needed and manually pushed to the loggers and to the processable handlers.
*/
#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
class AsMonologProcessor
Copy link
Contributor Author

@fancyweb fancyweb Feb 24, 2022

Choose a reason for hiding this comment

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

Having Monolog in the name is good from an external POV I think. In a project, it gives more information than just AsProcessor.

class AsMonologProcessor
{
/**
* @param string|null $channel The logging channel the processor should be pushed to.
Copy link
Contributor Author

@fancyweb fancyweb Feb 24, 2022

Choose a reason for hiding this comment

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

In symfony/monolog-bundle, setting both channel and handler will throw. Also setting method if the attribute is used as the method level will throw. Also, the method falls back to __invoke.

I didn't add any hints about this here because I guess it's just easier to accept everything and let each implementation do what they want.

@Seldaek Seldaek added this to the 2.x milestone Mar 6, 2022
@Seldaek Seldaek added the Feature label Mar 6, 2022
@Seldaek Seldaek merged commit 8c58aba into Seldaek:main Mar 6, 2022
@Seldaek
Copy link
Owner

Seldaek commented Mar 6, 2022

Thanks

@fancyweb fancyweb deleted the feat/as-monolog-processor-attribute branch March 7, 2022 07:56
@lyrixx lyrixx mentioned this pull request Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants