Not activate channel on DefinitionDecorator instance

[ZhukV]

Hi.

The channel for logger not activate, if i use DefinitionDecorator instance for create service.

As example:

<service id="api.subscriber.logger_abstract" class="%api.subscriber.logger.class%" abstract="true" public="false">
    <argument type="service" id="logger" />
</service>

And after i create a DefinitionDecorator and add monolog.logger tag for this service and inject to container.

$subscriberDefinition = new DefinitionDecorator('api.subscriber.logger_abstract');
$subscriberDefinition->addTag('monolog.logger', [
    'channel' => $this->getLoggerChannelForHandler($handlerKey)
]);

$container->setDefinition($subscriberId, $subscriberDefinition);

The problem in DefinitionDecorator, because here not have a arguments. Argument and another definition options will be processed on ResolveDefinitionTemplatesPass (Optimization layer on compiler pass).

Maybe add LoggerChannelPass to optimization layer on CompilerPass?:

$container->addCompilerPass($channelPass = new LoggerChannelPass(), PassConfig::TYPE_OPTIMIZE);
$container->addCompilerPass(new DebugHandlerPass($channelPass), PassConfig::TYPE_OPTIMIZE);

Thank.

[Taluu]

Another case which would need to move the LoggerPass while the compiler is optimizing the services (or after ?) rather than before. I have the following services :

<service id="service.abstract" abstract="true">
    <argument type="service" id="logger" />
</service>

<service id="service.child" parent="service.abstract">
    <tag name="monolog.logger" channel="test" />
</service>

As I have the same structure / constructor but for different goals (pulling different things actually), so I need a different channel for each children.

Because the pass is registered on BEFORE_OPTIMIZATION, the service.child's tag monolg.logger is not used to determine which channel to use, and then fall into the app channel.

So we basically need to currently duplicate our definitions instead of having a simple inheritance...

I guess moving this pass to AFTER_OPTIMIZATION would be... well, optimized, but I'm not quire sure ? At least, it should be run after the ResolveDefinitionTemplatePass. I wouldn't mind making the change, but I need to be sure that there won't be any breaks actually (or someone against it)

[stof]

@Taluu moving it after the optimization would break thing, as the compiler pass is changing the graph of dependencies between services, and the optimizations are performed based on this dependency graph. For instance, handlers may get inlined by optimizations if the container thinks they are used by a single logger service, while they are later needed by multiple loggers and so cannot be inlined. It would also make it much harder to change the logger in private services which gets inlined and removed by optimizations, as we would have to look at any service to find inlined arguments and process them too).

[Taluu]

In that case, I think the ResolveDefinitionTemplatePass should maybe be moved into the before optimization pass, so that the dependency are kept, and passes such as the LoggerPass could operate on the "real" and unoptimized (yet) services, but still work fine on such services, and then allowing services to be inlined etc.

But I guess this would also break things though...

So the current solution is the following :

<service id="service.child">
    <1-- other services... //-->
    <argument type="service" id="logger" />    

    <tag name="monolog.logger" channel="test" />
</service>

Which is to actually not use a service inheritance...

[stof]

@Taluu this would be a huge BC break too, as it means that most compiler passes will become unable to use DefinitionDecorator themselves by running after the resolution.

[Taluu]

Mmh so maybe the LoggerPass should kind of copy what the ResolveDefinitionTemplatePass does ? But that would kind of be repetitive (and how would the main one works then ?)

[stof]

I have an idea how this could be solved

[Taluu]

I don't know if it would be optimized, but maybe if, when not finding any logger argument, it could try to lookup in a parent service if there is one (and so on, until it finds one or the service has no parent), and then change that one for the current service ?

But I think it would kind of be complicated to handle (and I'm not quite sure what it would give performance wise)

[chalasr]

I just ran into this issue which is quite annoying (given I have 7 services extending an abstract one with a monolog.logger tag as only difference). Not sure where replacing the argument in the parent service would lead, any hint about the best way to solve this?

[MisatoTremor]

I just wanted to throw the optimization AutowireRequiredMethodsPass into the mix, as it doesn't work with tagging because the LoggerChannelPass is executed earlier.

[lyrixx]

I have an idea how this could be solved

@stof What is your idea?