Skip to content

Conversation

@GromNaN
Copy link
Member

@GromNaN GromNaN commented Nov 26, 2025

Q A
Branch? 3.x
Bug fix? no
New feature? yes
Deprecations? no (not yet)
Issues Fix #558
License MIT

This is a partial PR to gather feedbacks.

  • Each Monolog handler has its own configuration and extension class
  • Some handlers can share the same with different properties (name, handler class)
  • They are all registered in the MonologExtension
  • 3rd party bundles can add their own handler extension classes

@GromNaN GromNaN changed the base branch from 4.x to 3.x November 26, 2025 09:29
Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

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

I don't think we want external bundles to extend the MonologBundle configuration class. This creates extra complexity:

  • depending on where the Configuration class is accessed, the extensions might be registered or no (we have that issue in SecurityBundle, which is the only bundle I'm aware of doing such thing
  • it makes it harder to ensure BC if we don't control the config tree
  • most of our handlers supported in core don't group their config settings in a dedicated sub-node in the config tree, which means they cannot be migrated to this new system (at least not without a complex BC layer to deprecate the existing nodes, which will be very hard to implement properly given that some of our config settings are used by multiple handlers).

We already have the service type to allow configuring handlers that are not supported in core.

* bubble: bool,
* formatter: string|null,
* } $config Generic options
* @param array{} $handler Specific handler options
Copy link
Member

Choose a reason for hiding this comment

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

this is wrong. array{} is the shape representing an empty array.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is on purpose. By default this is an empty array. You have to specify another array shape in subclasses.

Copy link
Member

Choose a reason for hiding this comment

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

That's not the proper way to describe such API. We would need to use a generic type with THandlerConfig of array<string, mixed>, with child classes describing their array shape as the template type.

@mbabker
Copy link

mbabker commented Dec 2, 2025

  • depending on where the Configuration class is accessed, the extensions might be registered or no (we have that issue in SecurityBundle, which is the only bundle I'm aware of doing such thing

knplabs/knp-gaufrette-bundle, liip/imagine-bundle, payum/payum-bundle, and sylius/theme-bundle all do something similar

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.

Make each handler type have its own configuration and enable custom handler types

3 participants