Since the change to PSR-3 the Flow framework loggers were in a part floating space on how to correctly use them. There were intermediary b/c interfaces PsrSystemLoggerInterface and PsrSecurityLoggerInterface which were marked deprecated and the default configuration when injecting the Psr\Log\LoggerInterface was the systemLogger, though that was totally undocumented, let alone how to get hold of any of the other loggers.
With the “virtual objects” feature we introduced, we have a way to configure multiple instances of a single interface or class, which seemed like a perfect fit for the loggers use case. But @daniellienert rightfully brought up a concern, that virtual objects lack autocomplete support and typing the additional string name is worse than only typing the interface you need injected.
So we’re at the point where we should make a decision about which way to follow and document as the defacto standard of creating and injecting different loggers for the longer run:
IMO the main PRO for the interface is the autocompletion in the IDE, the CON is that the interfaces are abused as markers and not a promise for the methods available (only via inheritance extends \Psr\Log\LoggerInterface).
Can I vote neither? I am really not sure and would hold off for now. I am definitely against marker interfaces, not sure if I am for virtual object configs though…
I think the interface with additional arguments at @Flow\Inject(name... is more expressive and makes it easy to get what actually is happening: A class implementing the LoggerInterface is required but a specific one.
I voted VirtualObjects - I love the idea of sticking to the PSR interface being injected, based on your configured implementation (defined by the “name” of the injection)
As written in the PR I would suggest to just refer to the \Psr\Log\LoggerInterface when needing the default (i.e. System) logger.
(of course that can still be changed via Objects.yaml)
I still like the virtual objects in order to create and re-use these custom loggers (including SecurityLogger and ones I might want to define in my own application)
And IMO we should also clearly communicate that constructor injection is still the more interoperable and testable way to go (like you did in the PR).
In my domain code I personally avoid any @Flow\Inject annotations and go for
public function __constructor(LoggerInterface $someLogger) {
}
…potentially with some Objects.yaml config to inject a custom logge
Argh, I’m too inexperienced with polls. Should have thought of a “abstain” choice. Anyway, if you see any other way how to make the loggers available, I’m totally open for other choices. I mean, technically we could just do “neither - use the factory instead!” way, but not sure the dev experience would justify avoiding these two options.
That’s totally possible with the virtual objects choice as well as the interfaces choice and just a matter of documentation The PSR LoggerInterface would just be the “I don’t care, just give me a logger please!” injection
I know, but since you asked for opinions on the documentation, this is mine:
Use the PSR LoggerInterface as synonym for the System Logger (i.e. don’t use the virtual object for it in the core and examples in the docs)
Document how to use the virtual objects to inject other core loggers and how to create ones for custom loggers especially
Document the potential drawbacks of virtual objects (i.e. framework dependency) and how to use constructor injection and some Objects.yaml config to achieve the above alternatively
Use the PSR LoggerInterface as synonym for the System Logger (i.e. don’t use the virtual object for it in the core and examples in the docs)
I would inject all SystemLoggers in the core explicitly, because I think that if the user rewires the Psr\Log\LoggerInterface to a custom logger, it should not receive all Framework logs by default.
Document how to use the virtual objects to inject other core loggers and how to create ones for custom loggers especially
Document the potential drawbacks of virtual objects (i.e. framework dependency) and how to use constructor injection and some Objects.yaml config to achieve the above alternatively
Would you be up to suggest a small doc section in the PR for that (a section about avoiding the framework and using Objects.yaml already exists)?
I beg to differ. I would think that it’s perfectly fine to assume that the LoggerInterface is treated as the default logger (i.e. system logger) if not specified otherwise explicitly.
I would totally agree, iff the namespace of the LoggerInterface were Neos\Flow\Psr\
, but we are talking about the generic Psr\Log namespace, which is likely used outside the framework as well.
Yes, but the question is, should we use the explicit name="Neos.Flow:SystemLogger" attribute on the injections in core or not? The latter would mean, that rewiring Psr\Log\LoggerInterface automatically leads to receiving all framework logs, which I still think is not always intended. If it is intended, it is possible to also rewire Neos.Flow:SystemLogger to the same logger implementation. So would you argue for or against it?
I would prefer the most simple solution and injecting the plain logger when you want to add something to the system logger sounds simple to me.
The other cases are valid but i would only consider them a problem if someone could run into this late during development which is not the case. If you start configuring the logger anyone will notice that this is the system logger and can configure a specific logger as he likes.
I think I might have worded badly once again. I should receive an achievement by now
injecting the plain logger when you want to add something to the system logger sounds simple to me
Yes, that part is not in dispute. Wether we use virtual objects or interfaces, and/or wether we change all core usages to explicit virtual objects or not, the plain logger resolving to system logger by default will stay.
The thing I object to is
(i.e. don’t use the virtual object for it in the core and examples in the docs)
My argument against it is that there is then no escape hatch to rewire Psr\Log\LoggerInterfacewithout receiving all framework logs also. We effectively force bind the PSR interface to our framework logs.
If we bind all internal usages of system logger to the virtual object instead, then rewiring the interface will allow the developer to simply inject the interface wherever he wants and receive his own logger, while not receiving logs from the framework into that. He can still opt-into that, if he wants though.
So thanks everyone who voted! Looks like we’ll go with the virtual objects then, following the 6:2 vote. Which is nice, because I won’t have to change the PR
The only thing left to decide is if we use the virtual objects in the core for the system logger then or not (see reply above), but I guess that can just be done in the PR directly, as we could change our decision later without too much problems.
This is the keypoint We have had a good discussion, voting have been made, we came to a conclusion and now we can try and use it in our bleeding edge code
I will be starting a new project based on dev-master next and work on the authentication stuff - I will definetly be injecting loggers of all kind
Thanks for the time to comment, PRs and documentation - it is all coming out so nice
The PSR 