Conversation
stof
left a comment
stof
left a comment
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
this is wrong. array{} is the shape representing an empty array.
There was a problem hiding this comment.
This is on purpose. By default this is an empty array. You have to specify another array shape in subclasses.
There was a problem hiding this comment.
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.
|
|
Could this also be used to pass the handler's FQCN as 'handlers' => [
'foobar' => [
'type' => StreamHandler::class |
Introduce one HandlerExtensionInterface per handler type, owning its config sub-node (buildArrayNode), cross-field validates (buildHandlerValidates) and service definition (getDefinition). A HandlerContext gives extensions access to the container, handler id and nested-handler bookkeeping. Legacy flat config is preserved: a beforeNormalization migrates flat fields into the per-type sub-node, and type is auto-detected from the sub-node keys when omitted. No breaking change, no deprecation of the flat format. Covers leaf handlers (stream, rotating_file, socket, syslog, syslogudp, cube, error_log, server_log, amqp, logentries, loggly, insightops, flowdock, pushover, telegram, rollbar, newrelic, console, slack, slackwebhook, native_mailer, symfony_mailer, gelf, mongodb) and wrapping handlers (fingers_crossed, filter, buffer, deduplication, sampling, group, whatfailuregroup, fallbackgroup). firephp/chromephp use a dedicated KernelResponseHandlerExtension. Deferred: redis/predis and elasticsearch/elastica/elastic_search share one connection sub-node across several type values, which the current pattern (one sub-node per getName()) cannot express. buildHandlerValidates restores the REQUIRE-style validates (merge-safe: guarded on the final type) and drops the FORBID-style rule on excluded_http_codes, per symfony#502/symfony#556.
e50c7a7 to
db678bb
Compare
4 participants
This is a partial PR to gather feedbacks.