Zend Framework

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

Details

  • Type: Improvement Improvement
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.10.4
  • Fix Version/s: 1.11.3
  • Component/s: Zend_Log
  • Labels:
    None

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:

"application.ini"
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.

"Zend_Log_Writer_Stream::__construct()"
$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...

Issue Links

This issue duplicates:
ZF-9176 Zend_Log::factory() does not support formatters N/A Resolved

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Erick Wilder de Oliveira added a comment -

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

Show
Erick Wilder de Oliveira added a comment - Not sure if is really necessary to extend a Writer. Just setting a Formatter to the current Writer doesn't does the trick?
Hide
Permalink
Aaron S. Hawley added a comment -

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.

Show
Aaron S. Hawley added a comment - 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.
Hide
Permalink
Benoît Durand added a comment -

r23599

Show
Benoît Durand added a comment - r23599
Hide
Permalink
Aaron S. Hawley added a comment -

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);
     }
Show
Aaron S. Hawley added a comment - 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);
     }
Hide
Permalink
Matthew Weier O'Phinney added a comment -

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.

Show
Matthew Weier O'Phinney added a comment - 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.
Hide
Permalink
Benoît Durand added a comment - - edited

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

Show
Benoît Durand added a comment - - edited I have committed the unit test of Aaron in the trunk r23642. And, I added documentation in r23675.
Hide
Permalink
Alberto added a comment -

Documentation appears as:

filterParams -> format

but you must to use:

formatterParams -> format

Show
Alberto added a comment - Documentation appears as: filterParams -> format but you must to use: formatterParams -> format
Hide
Permalink
Peter Culak added a comment -

I'm using tag http://framework.zend.com/svn/framework/standard/tags/release-1.11.7/library/Zend.
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

Show
Peter Culak added a comment - I'm using tag http://framework.zend.com/svn/framework/standard/tags/release-1.11.7/library/Zend. 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
Hide
Permalink
Benoît Durand added a comment -

What fix are you talking about?

Show
Benoît Durand added a comment - What fix are you talking about?
Hide
Permalink
Peter Culak added a comment -

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.

Show
Peter Culak added a comment - 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.
Hide
Permalink
Benoît Durand added a comment - - edited

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.

"Zend_Log::getClassName()"
// 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.

Show
Benoît Durand added a comment - - edited 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.
"Zend_Log::getClassName()"
// 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.
Hide
Permalink
Peter Culak added a comment -

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

Show
Peter Culak added a comment - 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

People

Vote (7)
Watch (7)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.