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 a WithMonologChannel attribute #1847

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Conversation

stof
Copy link
Contributor

@stof stof commented Oct 24, 2023

This attribute is meant to be used by frameworks / integrations to choose which logger instance to inject in a class when they manage several channels, if they decide to use it.
This attribute will have no effect in Monolog itself as the wiring of the logger in other classes is not managed by Monolog.

This attribute is meant to be used by frameworks / integrations to
choose which logger instance to inject in a class when they manage
several channels, if they decide to use it.
This attribute will have no effect in Monolog itself as the wiring of
the logger in other classes is not managed by Monolog.
@stof
Copy link
Contributor Author

stof commented Oct 24, 2023

the phpstan failure is unrelated to my change. It is fixed in #1848

* Using it with the Monolog library only has no effect at all: wiring the logger instance into
* other classes is not managed by Monolog.
*/
#[\Attribute(\Attribute::TARGET_CLASS)]
Copy link
Contributor

@nicolas-grekas nicolas-grekas Oct 25, 2023

Choose a reason for hiding this comment

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

Should be allow it on parameters also, and possibly properties/methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't work in MonologBundle. The tag would apply to the whole service. It does not support separate tags per parameter.
If you want to inject 2 different loggers in the same class with autowiring, you will need to rely on the named aliases or #[Target]

Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't we turn the attribute into eg a binding in the bundle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw, that's also why I did not make the attribute repeatable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

binding are not per-attribute but per service.

Copy link
Contributor

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Makes sense to me, next to AsMonologProcessor.

@Seldaek Seldaek added this to the 3.x milestone Oct 25, 2023
@Seldaek Seldaek merged commit e545d0f into Seldaek:main Oct 25, 2023
13 of 14 checks passed
@Seldaek
Copy link
Owner

Seldaek commented Oct 25, 2023

Yeah I guess why not have this here as well

@stof stof deleted the channel_attribute branch October 25, 2023 13:26
@stof
Copy link
Contributor Author

stof commented Oct 26, 2023

@Seldaek do you know when the 3.5 release will be done with this attribute included ? I'm adding support for this attribute in monolog-bundle.

@Seldaek
Copy link
Owner

Seldaek commented Oct 26, 2023

I'll try to do this soon but it might be a week or two.. Would like to go through all open stuff as it's been a while

@Seldaek
Copy link
Owner

Seldaek commented Oct 27, 2023

Ok 3.5.0 is out now https://github.com/Seldaek/monolog/releases/tag/3.5.0

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.

3 participants