ampproject / amp-toolbox-php

AMP Optimizer PHP library
Apache License 2.0
73 stars 25 forks source link

Add PSR-3 logging #74

Open schlessera opened 3 years ago

schlessera commented 3 years ago

The TransformationEngine should accept a PSR-3 logger implementation and route it through to the transformers. This is a standardized way of letting the consumer of the library decide how and when to accept logging information and is mandatory in larger projects to maintain control over the different parts of the system.

The library should also include a logger implementation that can be used (or will be used by default, TBD) that logs errors into HTML comments, as a sort of last resort to communicating error conditions in an HTTP stack that doesn't come with its own PSR-3 routing.

In the future, the validator and the sanitizer engines should accept such a PSR-3 logger as well.

schlessera commented 3 years ago

We already have the ErrorCollection object which is mandatory and collects actual errors within the Optimizer.

The logger would be optional and maybe complementary to that and collect purely informational and/or debugging information. It could also forward the errors added to the ErrorCollection as well, so that log files contain the complete picture of what the system is going through.

The way a PSR-3 logger is usually handled is that, at the very least, you pass a NullLogger around (see NullObject design pattern) and all the places where logging makes sense just blindly log into whatever logging instance they received.

Then, the outside/consuming code can decide what logging functionality is needed and pass the corresponding implementation into the system via dependency injection.

This is different than the ErrorCollection object that we currently have for errors, because that one:

Food for thought: Is the ErrorCollection object really needed, or would it be preferable to have a PSR-3 logger only which covers the entire spectrum of feedback (debug, info, warnings & errors in several severities)? Is it preferable to use both for semantically separating the two concepts of "providing generic output" & "gathering a list of encountered errors"?