ZF-9790: allow to attach a formatter from .ini configuration and do not hardcode formatter class in Log_Writer class

Description

It would be great if we could attach a formatter directly through application.ini file AND without having to subclass a writer either.

Currently we can define the following entries for Zend_Log:


resources.log.stream.writerName = "Stream"
resources.log.stream.writerParams.stream = APPLICATION_PATH "/../logs/app.log"
resources.log.stream.writerNamespace = "Library_Log_Writer"
resources.log.stream.filterName = "Priority"
resources.log.stream.filterParams.priority = Zend_Log::WARN

// It would be great if we could add something like:
resources.log.stream.formatterName = "MyFormaterName"
resources.log.stream.formatterNamespace = "My_Formater_Namespace"

This would improve logging customization a lot and make it easier. It would be absolutely necessary to remove currently hardcoded formatter classes within Writer classes.


        $this->_formatter = new Zend_Log_Formatter_Simple();

=> currently, to attach your own formatter, you have to create your own subclass of Writter_Stream... Not very flexible...

Comments

Not sure if is really necessary to extend a Writer. Just setting a Formatter to the current Writer doesn't does the trick?

Here's a patch against 1.10.4 that simply duplicates the way writers and filters are generated as application resources.

I tested only:

resources.log.db.formatterName = "Simple"
resources.log.db.formatterParams.format = '%timestamp%: %message%'

Not the testing of Namespace.

r23599

Benoit, thanks for taking this on.

Perhaps, this should be added to the existing tests for Zend_Application_Resource_Log?

Index: trunk/tests/Zend/Application/Resource/LogTest.php
===================================================================
--- trunk/tests/Zend/Application/Resource/LogTest.php   (revision 23599)
+++ trunk/tests/Zend/Application/Resource/LogTest.php   (working copy)
@@ -109,5 +109,13 @@
             'writerParams' => array(
                 'stream' => $stream,
-            )
+            ),
+            'filterName'   => 'Priority',
+            'filterParams' => array(
+                'priority' => Zend_Log::INFO,
+            ),
+            'formatterName'   => 'Simple',
+            'formatterParams' => array(
+                'format' => '%timestamp%: %message%',
+            ),
         ));
 
@@ -121,5 +129,7 @@
         $log->log($message = 'logged-message', Zend_Log::INFO);
         rewind($stream);
-        $this->assertContains($message, stream_get_contents($stream));
+        $contents = stream_get_contents($stream);
+        $this->assertStringEndsWith($message, $contents);
+        $this->assertRegexp('/\d\d:\d\d:\d\d/', $contents);
     }

Benoit -- patch looks good, but I also think Aaron's point is valid. Once the Log resource test is added, feel free to merge to the 1.11 release branch.

I have committed the unit test of Aaron in the trunk r23642. And, I added documentation in r23675.

Documentation appears as:

filterParams -> format

but you must to use:

formatterParams -> format

I'm using tag http://framework.zend.com/svn/framework/…. this setting in application.ini just gets ignored: resources.log.stream.formatterNamespace = "Model_Log_Formatter"

the reason for it is that Zend_Log calls this on line 225: $formatter = $this->_constructFromConfig('formatter', $config, $this->_defaultFormatterNamespace);

has this fix not been included in release yet ?

peter

What fix are you talking about?

fix which would allow to set a namespace from through config file. the patch attached above doesn't allow that as it does: protected function _constructFormatterFromConfig($config) + { + $formatter = $this->_constructFromConfig('formatter', $config, $this->_defaultFormatterNamespace);

$this->_defaultFormatterNamespace should probably pick up formatterNamespace from config first and if not set then use default one.

This feature is added since version 1.11.3. Would you have a sample of a configuration that does not work?

You make a misreading of the source code. You should read the getClassName method: a default namespace is used only if the configuration does not provide to complete the short name.


        // line 295
        $namespace = $defaultNamespace;
        if (isset($config[ $type . 'Namespace' ])) {
            $namespace = $config[ $type . 'Namespace' ];
        }

        $fullClassName = $namespace . '_' . $className;

You should open a new issue as bug to discuss if there really is a bug on this feature.

you're right, it works fine. I was extending formatter and i didn't overload factory method so it was returning wrong class.

thanx for your time ;)