You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2013/05/23 09:55:14 UTC

svn commit: r1485601 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/metrics/ webapp/config/ webapp/dtd/ webapp/src/org/ofbiz/webapp/control/

Author: jleroux
Date: Thu May 23 07:55:14 2013
New Revision: 1485601

URL: http://svn.apache.org/r1485601
Log:
Add a mean to collect and show the Requests Events durations using Metrics  https://issues.apache.org/jira/browse/OFBIZ-5198
Fulfill the goal with some more:
* makes the Metrics main parameters (estimationSize, estimationTime and smoothing) dynamically changeable (added in serverstats.properties)
* there is no threshold handling for event metrics

Modified:
    ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
    ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
    ofbiz/trunk/framework/webapp/config/serverstats.properties
    ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
    ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
    ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java?rev=1485601&r1=1485600&r2=1485601&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java (original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java Thu May 23 07:55:14 2013
@@ -28,6 +28,7 @@
  */
 package org.ofbiz.base.metrics;
 
+
 /**
  * An object that tracks service metrics.
  * <p>This interface and its default implementation are based on the
@@ -36,11 +37,6 @@ package org.ofbiz.base.metrics;
  * @see <a href="http://www.eecs.harvard.edu/~mdw/proj/seda/">SEDA</a>
  */
 public interface Metrics {
-
-    public static final int ESTIMATION_SIZE = 100;
-    public static final long ESTIMATION_TIME = 1000;
-    public static final double SMOOTHING = 0.7;
-
     /**
      * Returns the name of the metric.
      */

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java?rev=1485601&r1=1485600&r2=1485601&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java (original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java Thu May 23 07:55:14 2013
@@ -34,6 +34,7 @@ import java.util.TreeSet;
 import org.ofbiz.base.lang.LockedBy;
 import org.ofbiz.base.lang.ThreadSafe;
 import org.ofbiz.base.util.Assert;
+import org.ofbiz.base.util.UtilProperties;
 import org.ofbiz.base.util.cache.UtilCache;
 import org.w3c.dom.Element;
 
@@ -42,7 +43,6 @@ import org.w3c.dom.Element;
  */
 @ThreadSafe
 public final class MetricsFactory {
-
     private static final UtilCache<String, Metrics> METRICS_CACHE = UtilCache.createUtilCache("base.metrics", 0, 0);
     /**
      * A "do-nothing" <code>Metrics</code> instance.
@@ -73,17 +73,17 @@ public final class MetricsFactory {
         Assert.notEmpty("name attribute", name);
         Metrics result = METRICS_CACHE.get(name);
         if (result == null) {
-            int estimationSize = Metrics.ESTIMATION_SIZE;
+            int estimationSize = UtilProperties.getPropertyAsInteger("serverstats", "metrics.estimation.size", 100); 
             String attributeValue = element.getAttribute("estimation-size");
             if (!attributeValue.isEmpty()) {
                 estimationSize = Integer.parseInt(attributeValue);
             }
-            long estimationTime = Metrics.ESTIMATION_TIME;
+            long estimationTime = UtilProperties.getPropertyAsLong("serverstats", "metrics.estimation.time", 1000);
             attributeValue = element.getAttribute("estimation-time");
             if (!attributeValue.isEmpty()) {
                 estimationTime = Long.parseLong(attributeValue);
             }
-            double smoothing = Metrics.SMOOTHING;
+            double smoothing = UtilProperties.getPropertyNumber("serverstats", "metrics.smoothing.factor", 0.7);
             attributeValue = element.getAttribute("smoothing");
             if (!attributeValue.isEmpty()) {
                 smoothing = Double.parseDouble(attributeValue);
@@ -151,9 +151,9 @@ public final class MetricsFactory {
         @LockedBy("this")
         private long cumulativeEvents;
         private final String name;
-        private final int estimationSize;
-        private final long estimationTime;
-        private final double smoothing;
+        private int estimationSize;
+        private long estimationTime;
+        private double smoothing;
         private final double threshold;
 
         private MetricsImpl(String name, int estimationSize, long estimationTime, double smoothing, double threshold) {

Modified: ofbiz/trunk/framework/webapp/config/serverstats.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/config/serverstats.properties?rev=1485601&r1=1485600&r2=1485601&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/config/serverstats.properties (original)
+++ ofbiz/trunk/framework/webapp/config/serverstats.properties Thu May 23 07:55:14 2013
@@ -50,3 +50,11 @@ stats.persist.SERVICE.hit=false
 # Specify whether a proxy sits in front of this app server
 # This allows VisitHandler to collect the client's real ip
 stats.proxy.enabled=false
+
+### Metric parameters (moving average)
+# size of the considered subset (defines the window size)
+metrics.estimation.size=100
+# minimum time considered between 2 samplings for taking a record or not (must be > to metrics.estimation.time)  
+metrics.estimation.time=1000
+# used to smooth the differences between calculations. A value of "1" disables smoothing
+metrics.smoothing.factor=0.7

Modified: ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/dtd/site-conf.xsd?rev=1485601&r1=1485600&r2=1485601&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/dtd/site-conf.xsd (original)
+++ ofbiz/trunk/framework/webapp/dtd/site-conf.xsd Thu May 23 07:55:14 2013
@@ -342,8 +342,8 @@ under the License.
     <xs:element name="metric">
         <xs:annotation>
             <xs:documentation>
-                Calculate and maintain an average response time for this request. Request metrics can be used
-                for monitoring and reporting. Metrics can also be used to trigger an alternate
+                Calculate and maintain a moving average response time for a Request or Event. Request Metrics can be used
+                for monitoring and reporting, Event Metrics only for reporting. Request Metrics can also be used to trigger an alternate
                 response if the optional threshold attribute is used.
                 
                 The metric works by gathering statistics until a configurable maximum is reached (number of
@@ -402,6 +402,9 @@ under the License.
             </xs:documentation>
         </xs:annotation>
         <xs:complexType>
+            <xs:sequence>
+                <xs:element minOccurs="0" ref="metric"/>
+            </xs:sequence>
             <xs:attributeGroup ref="attlist.event"/>
         </xs:complexType>
     </xs:element>

Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java?rev=1485601&r1=1485600&r2=1485601&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java (original)
+++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java Thu May 23 07:55:14 2013
@@ -588,12 +588,18 @@ public class ConfigXMLReader {
         public String path;
         public String invoke;
         public boolean globalTransaction = true;
+        public Metrics metrics = null;
 
         public Event(Element eventElement) {
             this.type = eventElement.getAttribute("type");
             this.path = eventElement.getAttribute("path");
             this.invoke = eventElement.getAttribute("invoke");
             this.globalTransaction = !"false".equals(eventElement.getAttribute("global-transaction"));
+            // Get metrics.
+            Element metricsElement = UtilXml.firstChildElement(eventElement, "metric");
+            if (metricsElement != null) {
+                this.metrics = MetricsFactory.getInstance(metricsElement);
+            }
         }
 
         public Event(String type, String path, String invoke, boolean globalTransaction) {

Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java?rev=1485601&r1=1485600&r2=1485601&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java (original)
+++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java Thu May 23 07:55:14 2013
@@ -170,6 +170,7 @@ public class RequestHandler {
         }
         ConfigXMLReader.RequestMap originalRequestMap = requestMap; // Save this so we can update the correct performance metrics.
 
+
         boolean interruptRequest = false;
 
         // Check for chained request.
@@ -416,6 +417,10 @@ public class RequestHandler {
 
                     // run the request event
                     eventReturn = this.runEvent(request, response, requestMap.event, requestMap, "request");
+                                        
+                    if (requestMap.event.metrics != null) {
+                        requestMap.event.metrics.recordServiceRate(1, System.currentTimeMillis() - startTime);
+                    }                    
 
                     // save the server hit for the request event
                     if (this.trackStats(request)) {



Re: svn commit: r1485601 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/metrics/ webapp/config/ webapp/dtd/ webapp/src/org/ofbiz/webapp/control/

Posted by Jacques Le Roux <ja...@les7arts.com>.
Yes, I should have thought about using the model, rather than mucking around with Metric classes. But it was a bit more complicated so I went this wrong way.

BTW, I just thought about something.
Convention above configuration is often good, but here the metrics names rely on hand writing. So wrong C/Ps or typos can introduce challenging wrong results.
I will see if we can rather grab them from context.
Controllers names might be a challenge. We can alway use the component name but it will not be enough, I will see

Jacques

From: "Adrian Crum" <ad...@sandglass-software.com>
> Thanks.
> 
> As you can see, adding metrics to OFBiz is fairly simple. That is why I 
> objected to adding redundant code to the Metrics classes.
> 
> -Adrian
> 
> On 5/23/2013 9:57 AM, Jacques Le Roux wrote:
>> Yes sorry, this was a 1st try I did before changing directly in factory, I forgot to remove, I will.
>> Always better to have at least 2 pairs of  eyes...
>>
>> Jacques
>>
>> From: "Adrian Crum" <ad...@sandglass-software.com>
>>> This looks good, but please leave the thread-safe fields as final. See
>>> inline comment...
>>>
>>> On 5/23/2013 8:55 AM, jleroux@apache.org wrote:
>>>> Author: jleroux
>>>> Date: Thu May 23 07:55:14 2013
>>>> New Revision: 1485601
>>>>
>>>> URL: http://svn.apache.org/r1485601
>>>> Log:
>>>> Add a mean to collect and show the Requests Events durations using Metrics  https://issues.apache.org/jira/browse/OFBIZ-5198
>>>> Fulfill the goal with some more:
>>>> * makes the Metrics main parameters (estimationSize, estimationTime and smoothing) dynamically changeable (added in serverstats.properties)
>>>> * there is no threshold handling for event metrics
>>>>
>>>> Modified:
>>>>       ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
>>>>       ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
>>>>       ofbiz/trunk/framework/webapp/config/serverstats.properties
>>>>       ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>>>>       ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>>>>       ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>>>>
>>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java (original)
>>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java Thu May 23 07:55:14 2013
>>>> @@ -28,6 +28,7 @@
>>>>     */
>>>>    package org.ofbiz.base.metrics;
>>>>    
>>>> +
>>>>    /**
>>>>     * An object that tracks service metrics.
>>>>     * <p>This interface and its default implementation are based on the
>>>> @@ -36,11 +37,6 @@ package org.ofbiz.base.metrics;
>>>>     * @see <a href="http://www.eecs.harvard.edu/~mdw/proj/seda/">SEDA</a>
>>>>     */
>>>>    public interface Metrics {
>>>> -
>>>> -    public static final int ESTIMATION_SIZE = 100;
>>>> -    public static final long ESTIMATION_TIME = 1000;
>>>> -    public static final double SMOOTHING = 0.7;
>>>> -
>>>>        /**
>>>>         * Returns the name of the metric.
>>>>         */
>>>>
>>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java (original)
>>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java Thu May 23 07:55:14 2013
>>>> @@ -34,6 +34,7 @@ import java.util.TreeSet;
>>>>    import org.ofbiz.base.lang.LockedBy;
>>>>    import org.ofbiz.base.lang.ThreadSafe;
>>>>    import org.ofbiz.base.util.Assert;
>>>> +import org.ofbiz.base.util.UtilProperties;
>>>>    import org.ofbiz.base.util.cache.UtilCache;
>>>>    import org.w3c.dom.Element;
>>>>    
>>>> @@ -42,7 +43,6 @@ import org.w3c.dom.Element;
>>>>     */
>>>>    @ThreadSafe
>>>>    public final class MetricsFactory {
>>>> -
>>>>        private static final UtilCache<String, Metrics> METRICS_CACHE = UtilCache.createUtilCache("base.metrics", 0, 0);
>>>>        /**
>>>>         * A "do-nothing" <code>Metrics</code> instance.
>>>> @@ -73,17 +73,17 @@ public final class MetricsFactory {
>>>>            Assert.notEmpty("name attribute", name);
>>>>            Metrics result = METRICS_CACHE.get(name);
>>>>            if (result == null) {
>>>> -            int estimationSize = Metrics.ESTIMATION_SIZE;
>>>> +            int estimationSize = UtilProperties.getPropertyAsInteger("serverstats", "metrics.estimation.size", 100);
>>>>                String attributeValue = element.getAttribute("estimation-size");
>>>>                if (!attributeValue.isEmpty()) {
>>>>                    estimationSize = Integer.parseInt(attributeValue);
>>>>                }
>>>> -            long estimationTime = Metrics.ESTIMATION_TIME;
>>>> +            long estimationTime = UtilProperties.getPropertyAsLong("serverstats", "metrics.estimation.time", 1000);
>>>>                attributeValue = element.getAttribute("estimation-time");
>>>>                if (!attributeValue.isEmpty()) {
>>>>                    estimationTime = Long.parseLong(attributeValue);
>>>>                }
>>>> -            double smoothing = Metrics.SMOOTHING;
>>>> +            double smoothing = UtilProperties.getPropertyNumber("serverstats", "metrics.smoothing.factor", 0.7);
>>>>                attributeValue = element.getAttribute("smoothing");
>>>>                if (!attributeValue.isEmpty()) {
>>>>                    smoothing = Double.parseDouble(attributeValue);
>>>> @@ -151,9 +151,9 @@ public final class MetricsFactory {
>>>>            @LockedBy("this")
>>>>            private long cumulativeEvents;
>>>>            private final String name;
>>> Please keep these final.
>>>
>>>> -        private final int estimationSize;
>>>> -        private final long estimationTime;
>>>> -        private final double smoothing;
>>>> +        private int estimationSize;
>>>> +        private long estimationTime;
>>>> +        private double smoothing;
>>>>            private final double threshold;
>>>>    
>>>>            private MetricsImpl(String name, int estimationSize, long estimationTime, double smoothing, double threshold) {
>>>>
>>>> Modified: ofbiz/trunk/framework/webapp/config/serverstats.properties
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/config/serverstats.properties?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/webapp/config/serverstats.properties (original)
>>>> +++ ofbiz/trunk/framework/webapp/config/serverstats.properties Thu May 23 07:55:14 2013
>>>> @@ -50,3 +50,11 @@ stats.persist.SERVICE.hit=false
>>>>    # Specify whether a proxy sits in front of this app server
>>>>    # This allows VisitHandler to collect the client's real ip
>>>>    stats.proxy.enabled=false
>>>> +
>>>> +### Metric parameters (moving average)
>>>> +# size of the considered subset (defines the window size)
>>>> +metrics.estimation.size=100
>>>> +# minimum time considered between 2 samplings for taking a record or not (must be > to metrics.estimation.time)
>>>> +metrics.estimation.time=1000
>>>> +# used to smooth the differences between calculations. A value of "1" disables smoothing
>>>> +metrics.smoothing.factor=0.7
>>>>
>>>> Modified: ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/dtd/site-conf.xsd?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/webapp/dtd/site-conf.xsd (original)
>>>> +++ ofbiz/trunk/framework/webapp/dtd/site-conf.xsd Thu May 23 07:55:14 2013
>>>> @@ -342,8 +342,8 @@ under the License.
>>>>        <xs:element name="metric">
>>>>            <xs:annotation>
>>>>                <xs:documentation>
>>>> -                Calculate and maintain an average response time for this request. Request metrics can be used
>>>> -                for monitoring and reporting. Metrics can also be used to trigger an alternate
>>>> +                Calculate and maintain a moving average response time for a Request or Event. Request Metrics can be used
>>>> +                for monitoring and reporting, Event Metrics only for reporting. Request Metrics can also be used to trigger an alternate
>>>>                    response if the optional threshold attribute is used.
>>>>                    
>>>>                    The metric works by gathering statistics until a configurable maximum is reached (number of
>>>> @@ -402,6 +402,9 @@ under the License.
>>>>                </xs:documentation>
>>>>            </xs:annotation>
>>>>            <xs:complexType>
>>>> +            <xs:sequence>
>>>> +                <xs:element minOccurs="0" ref="metric"/>
>>>> +            </xs:sequence>
>>>>                <xs:attributeGroup ref="attlist.event"/>
>>>>            </xs:complexType>
>>>>        </xs:element>
>>>>
>>>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java (original)
>>>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java Thu May 23 07:55:14 2013
>>>> @@ -588,12 +588,18 @@ public class ConfigXMLReader {
>>>>            public String path;
>>>>            public String invoke;
>>>>            public boolean globalTransaction = true;
>>>> +        public Metrics metrics = null;
>>>>    
>>>>            public Event(Element eventElement) {
>>>>                this.type = eventElement.getAttribute("type");
>>>>                this.path = eventElement.getAttribute("path");
>>>>                this.invoke = eventElement.getAttribute("invoke");
>>>>                this.globalTransaction = !"false".equals(eventElement.getAttribute("global-transaction"));
>>>> +            // Get metrics.
>>>> +            Element metricsElement = UtilXml.firstChildElement(eventElement, "metric");
>>>> +            if (metricsElement != null) {
>>>> +                this.metrics = MetricsFactory.getInstance(metricsElement);
>>>> +            }
>>>>            }
>>>>    
>>>>            public Event(String type, String path, String invoke, boolean globalTransaction) {
>>>>
>>>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java (original)
>>>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java Thu May 23 07:55:14 2013
>>>> @@ -170,6 +170,7 @@ public class RequestHandler {
>>>>            }
>>>>            ConfigXMLReader.RequestMap originalRequestMap = requestMap; // Save this so we can update the correct performance metrics.
>>>>    
>>>> +
>>>>            boolean interruptRequest = false;
>>>>    
>>>>            // Check for chained request.
>>>> @@ -416,6 +417,10 @@ public class RequestHandler {
>>>>    
>>>>                        // run the request event
>>>>                        eventReturn = this.runEvent(request, response, requestMap.event, requestMap, "request");
>>>> +
>>>> +                    if (requestMap.event.metrics != null) {
>>>> +                        requestMap.event.metrics.recordServiceRate(1, System.currentTimeMillis() - startTime);
>>>> +                    }
>>>>    
>>>>                        // save the server hit for the request event
>>>>                        if (this.trackStats(request)) {
>>>>
>>>>
>

Re: svn commit: r1485601 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/metrics/ webapp/config/ webapp/dtd/ webapp/src/org/ofbiz/webapp/control/

Posted by Jacques Le Roux <ja...@les7arts.com>.
Yes, I should have thought about using the model, rather than mucking around with Metric classes. But it was a bit more complicated so I got this wrong way.

BTW, I just thought about something.
Convention above configuration is often good, but here the metrics names rely on hand writing. So wrong C/Ps or typos can introduce challenging wrong results.
I will see if we can rather grab them from context.
Controllers names might be a challenge. We can alway use the component name but it will not be enough, I will see

Jacques

From: "Adrian Crum" <ad...@sandglass-software.com>
> Thanks.
> 
> As you can see, adding metrics to OFBiz is fairly simple. That is why I 
> objected to adding redundant code to the Metrics classes.
> 
> -Adrian
> 
> On 5/23/2013 9:57 AM, Jacques Le Roux wrote:
>> Yes sorry, this was a 1st try I did before changing directly in factory, I forgot to remove, I will.
>> Always better to have at least 2 pairs of  eyes...
>>
>> Jacques
>>
>> From: "Adrian Crum" <ad...@sandglass-software.com>
>>> This looks good, but please leave the thread-safe fields as final. See
>>> inline comment...
>>>
>>> On 5/23/2013 8:55 AM, jleroux@apache.org wrote:
>>>> Author: jleroux
>>>> Date: Thu May 23 07:55:14 2013
>>>> New Revision: 1485601
>>>>
>>>> URL: http://svn.apache.org/r1485601
>>>> Log:
>>>> Add a mean to collect and show the Requests Events durations using Metrics  https://issues.apache.org/jira/browse/OFBIZ-5198
>>>> Fulfill the goal with some more:
>>>> * makes the Metrics main parameters (estimationSize, estimationTime and smoothing) dynamically changeable (added in serverstats.properties)
>>>> * there is no threshold handling for event metrics
>>>>
>>>> Modified:
>>>>       ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
>>>>       ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
>>>>       ofbiz/trunk/framework/webapp/config/serverstats.properties
>>>>       ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>>>>       ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>>>>       ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>>>>
>>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java (original)
>>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java Thu May 23 07:55:14 2013
>>>> @@ -28,6 +28,7 @@
>>>>     */
>>>>    package org.ofbiz.base.metrics;
>>>>    
>>>> +
>>>>    /**
>>>>     * An object that tracks service metrics.
>>>>     * <p>This interface and its default implementation are based on the
>>>> @@ -36,11 +37,6 @@ package org.ofbiz.base.metrics;
>>>>     * @see <a href="http://www.eecs.harvard.edu/~mdw/proj/seda/">SEDA</a>
>>>>     */
>>>>    public interface Metrics {
>>>> -
>>>> -    public static final int ESTIMATION_SIZE = 100;
>>>> -    public static final long ESTIMATION_TIME = 1000;
>>>> -    public static final double SMOOTHING = 0.7;
>>>> -
>>>>        /**
>>>>         * Returns the name of the metric.
>>>>         */
>>>>
>>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java (original)
>>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java Thu May 23 07:55:14 2013
>>>> @@ -34,6 +34,7 @@ import java.util.TreeSet;
>>>>    import org.ofbiz.base.lang.LockedBy;
>>>>    import org.ofbiz.base.lang.ThreadSafe;
>>>>    import org.ofbiz.base.util.Assert;
>>>> +import org.ofbiz.base.util.UtilProperties;
>>>>    import org.ofbiz.base.util.cache.UtilCache;
>>>>    import org.w3c.dom.Element;
>>>>    
>>>> @@ -42,7 +43,6 @@ import org.w3c.dom.Element;
>>>>     */
>>>>    @ThreadSafe
>>>>    public final class MetricsFactory {
>>>> -
>>>>        private static final UtilCache<String, Metrics> METRICS_CACHE = UtilCache.createUtilCache("base.metrics", 0, 0);
>>>>        /**
>>>>         * A "do-nothing" <code>Metrics</code> instance.
>>>> @@ -73,17 +73,17 @@ public final class MetricsFactory {
>>>>            Assert.notEmpty("name attribute", name);
>>>>            Metrics result = METRICS_CACHE.get(name);
>>>>            if (result == null) {
>>>> -            int estimationSize = Metrics.ESTIMATION_SIZE;
>>>> +            int estimationSize = UtilProperties.getPropertyAsInteger("serverstats", "metrics.estimation.size", 100);
>>>>                String attributeValue = element.getAttribute("estimation-size");
>>>>                if (!attributeValue.isEmpty()) {
>>>>                    estimationSize = Integer.parseInt(attributeValue);
>>>>                }
>>>> -            long estimationTime = Metrics.ESTIMATION_TIME;
>>>> +            long estimationTime = UtilProperties.getPropertyAsLong("serverstats", "metrics.estimation.time", 1000);
>>>>                attributeValue = element.getAttribute("estimation-time");
>>>>                if (!attributeValue.isEmpty()) {
>>>>                    estimationTime = Long.parseLong(attributeValue);
>>>>                }
>>>> -            double smoothing = Metrics.SMOOTHING;
>>>> +            double smoothing = UtilProperties.getPropertyNumber("serverstats", "metrics.smoothing.factor", 0.7);
>>>>                attributeValue = element.getAttribute("smoothing");
>>>>                if (!attributeValue.isEmpty()) {
>>>>                    smoothing = Double.parseDouble(attributeValue);
>>>> @@ -151,9 +151,9 @@ public final class MetricsFactory {
>>>>            @LockedBy("this")
>>>>            private long cumulativeEvents;
>>>>            private final String name;
>>> Please keep these final.
>>>
>>>> -        private final int estimationSize;
>>>> -        private final long estimationTime;
>>>> -        private final double smoothing;
>>>> +        private int estimationSize;
>>>> +        private long estimationTime;
>>>> +        private double smoothing;
>>>>            private final double threshold;
>>>>    
>>>>            private MetricsImpl(String name, int estimationSize, long estimationTime, double smoothing, double threshold) {
>>>>
>>>> Modified: ofbiz/trunk/framework/webapp/config/serverstats.properties
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/config/serverstats.properties?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/webapp/config/serverstats.properties (original)
>>>> +++ ofbiz/trunk/framework/webapp/config/serverstats.properties Thu May 23 07:55:14 2013
>>>> @@ -50,3 +50,11 @@ stats.persist.SERVICE.hit=false
>>>>    # Specify whether a proxy sits in front of this app server
>>>>    # This allows VisitHandler to collect the client's real ip
>>>>    stats.proxy.enabled=false
>>>> +
>>>> +### Metric parameters (moving average)
>>>> +# size of the considered subset (defines the window size)
>>>> +metrics.estimation.size=100
>>>> +# minimum time considered between 2 samplings for taking a record or not (must be > to metrics.estimation.time)
>>>> +metrics.estimation.time=1000
>>>> +# used to smooth the differences between calculations. A value of "1" disables smoothing
>>>> +metrics.smoothing.factor=0.7
>>>>
>>>> Modified: ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/dtd/site-conf.xsd?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/webapp/dtd/site-conf.xsd (original)
>>>> +++ ofbiz/trunk/framework/webapp/dtd/site-conf.xsd Thu May 23 07:55:14 2013
>>>> @@ -342,8 +342,8 @@ under the License.
>>>>        <xs:element name="metric">
>>>>            <xs:annotation>
>>>>                <xs:documentation>
>>>> -                Calculate and maintain an average response time for this request. Request metrics can be used
>>>> -                for monitoring and reporting. Metrics can also be used to trigger an alternate
>>>> +                Calculate and maintain a moving average response time for a Request or Event. Request Metrics can be used
>>>> +                for monitoring and reporting, Event Metrics only for reporting. Request Metrics can also be used to trigger an alternate
>>>>                    response if the optional threshold attribute is used.
>>>>                    
>>>>                    The metric works by gathering statistics until a configurable maximum is reached (number of
>>>> @@ -402,6 +402,9 @@ under the License.
>>>>                </xs:documentation>
>>>>            </xs:annotation>
>>>>            <xs:complexType>
>>>> +            <xs:sequence>
>>>> +                <xs:element minOccurs="0" ref="metric"/>
>>>> +            </xs:sequence>
>>>>                <xs:attributeGroup ref="attlist.event"/>
>>>>            </xs:complexType>
>>>>        </xs:element>
>>>>
>>>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java (original)
>>>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java Thu May 23 07:55:14 2013
>>>> @@ -588,12 +588,18 @@ public class ConfigXMLReader {
>>>>            public String path;
>>>>            public String invoke;
>>>>            public boolean globalTransaction = true;
>>>> +        public Metrics metrics = null;
>>>>    
>>>>            public Event(Element eventElement) {
>>>>                this.type = eventElement.getAttribute("type");
>>>>                this.path = eventElement.getAttribute("path");
>>>>                this.invoke = eventElement.getAttribute("invoke");
>>>>                this.globalTransaction = !"false".equals(eventElement.getAttribute("global-transaction"));
>>>> +            // Get metrics.
>>>> +            Element metricsElement = UtilXml.firstChildElement(eventElement, "metric");
>>>> +            if (metricsElement != null) {
>>>> +                this.metrics = MetricsFactory.getInstance(metricsElement);
>>>> +            }
>>>>            }
>>>>    
>>>>            public Event(String type, String path, String invoke, boolean globalTransaction) {
>>>>
>>>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java (original)
>>>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java Thu May 23 07:55:14 2013
>>>> @@ -170,6 +170,7 @@ public class RequestHandler {
>>>>            }
>>>>            ConfigXMLReader.RequestMap originalRequestMap = requestMap; // Save this so we can update the correct performance metrics.
>>>>    
>>>> +
>>>>            boolean interruptRequest = false;
>>>>    
>>>>            // Check for chained request.
>>>> @@ -416,6 +417,10 @@ public class RequestHandler {
>>>>    
>>>>                        // run the request event
>>>>                        eventReturn = this.runEvent(request, response, requestMap.event, requestMap, "request");
>>>> +
>>>> +                    if (requestMap.event.metrics != null) {
>>>> +                        requestMap.event.metrics.recordServiceRate(1, System.currentTimeMillis() - startTime);
>>>> +                    }
>>>>    
>>>>                        // save the server hit for the request event
>>>>                        if (this.trackStats(request)) {
>>>>
>>>>
>

Re: svn commit: r1485601 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/metrics/ webapp/config/ webapp/dtd/ webapp/src/org/ofbiz/webapp/control/

Posted by Adrian Crum <ad...@sandglass-software.com>.
Thanks.

As you can see, adding metrics to OFBiz is fairly simple. That is why I 
objected to adding redundant code to the Metrics classes.

-Adrian

On 5/23/2013 9:57 AM, Jacques Le Roux wrote:
> Yes sorry, this was a 1st try I did before changing directly in factory, I forgot to remove, I will.
> Always better to have at least 2 pairs of  eyes...
>
> Jacques
>
> From: "Adrian Crum" <ad...@sandglass-software.com>
>> This looks good, but please leave the thread-safe fields as final. See
>> inline comment...
>>
>> On 5/23/2013 8:55 AM, jleroux@apache.org wrote:
>>> Author: jleroux
>>> Date: Thu May 23 07:55:14 2013
>>> New Revision: 1485601
>>>
>>> URL: http://svn.apache.org/r1485601
>>> Log:
>>> Add a mean to collect and show the Requests Events durations using Metrics  https://issues.apache.org/jira/browse/OFBIZ-5198
>>> Fulfill the goal with some more:
>>> * makes the Metrics main parameters (estimationSize, estimationTime and smoothing) dynamically changeable (added in serverstats.properties)
>>> * there is no threshold handling for event metrics
>>>
>>> Modified:
>>>       ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
>>>       ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
>>>       ofbiz/trunk/framework/webapp/config/serverstats.properties
>>>       ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>>>       ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>>>       ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>>>
>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java (original)
>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java Thu May 23 07:55:14 2013
>>> @@ -28,6 +28,7 @@
>>>     */
>>>    package org.ofbiz.base.metrics;
>>>    
>>> +
>>>    /**
>>>     * An object that tracks service metrics.
>>>     * <p>This interface and its default implementation are based on the
>>> @@ -36,11 +37,6 @@ package org.ofbiz.base.metrics;
>>>     * @see <a href="http://www.eecs.harvard.edu/~mdw/proj/seda/">SEDA</a>
>>>     */
>>>    public interface Metrics {
>>> -
>>> -    public static final int ESTIMATION_SIZE = 100;
>>> -    public static final long ESTIMATION_TIME = 1000;
>>> -    public static final double SMOOTHING = 0.7;
>>> -
>>>        /**
>>>         * Returns the name of the metric.
>>>         */
>>>
>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java (original)
>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java Thu May 23 07:55:14 2013
>>> @@ -34,6 +34,7 @@ import java.util.TreeSet;
>>>    import org.ofbiz.base.lang.LockedBy;
>>>    import org.ofbiz.base.lang.ThreadSafe;
>>>    import org.ofbiz.base.util.Assert;
>>> +import org.ofbiz.base.util.UtilProperties;
>>>    import org.ofbiz.base.util.cache.UtilCache;
>>>    import org.w3c.dom.Element;
>>>    
>>> @@ -42,7 +43,6 @@ import org.w3c.dom.Element;
>>>     */
>>>    @ThreadSafe
>>>    public final class MetricsFactory {
>>> -
>>>        private static final UtilCache<String, Metrics> METRICS_CACHE = UtilCache.createUtilCache("base.metrics", 0, 0);
>>>        /**
>>>         * A "do-nothing" <code>Metrics</code> instance.
>>> @@ -73,17 +73,17 @@ public final class MetricsFactory {
>>>            Assert.notEmpty("name attribute", name);
>>>            Metrics result = METRICS_CACHE.get(name);
>>>            if (result == null) {
>>> -            int estimationSize = Metrics.ESTIMATION_SIZE;
>>> +            int estimationSize = UtilProperties.getPropertyAsInteger("serverstats", "metrics.estimation.size", 100);
>>>                String attributeValue = element.getAttribute("estimation-size");
>>>                if (!attributeValue.isEmpty()) {
>>>                    estimationSize = Integer.parseInt(attributeValue);
>>>                }
>>> -            long estimationTime = Metrics.ESTIMATION_TIME;
>>> +            long estimationTime = UtilProperties.getPropertyAsLong("serverstats", "metrics.estimation.time", 1000);
>>>                attributeValue = element.getAttribute("estimation-time");
>>>                if (!attributeValue.isEmpty()) {
>>>                    estimationTime = Long.parseLong(attributeValue);
>>>                }
>>> -            double smoothing = Metrics.SMOOTHING;
>>> +            double smoothing = UtilProperties.getPropertyNumber("serverstats", "metrics.smoothing.factor", 0.7);
>>>                attributeValue = element.getAttribute("smoothing");
>>>                if (!attributeValue.isEmpty()) {
>>>                    smoothing = Double.parseDouble(attributeValue);
>>> @@ -151,9 +151,9 @@ public final class MetricsFactory {
>>>            @LockedBy("this")
>>>            private long cumulativeEvents;
>>>            private final String name;
>> Please keep these final.
>>
>>> -        private final int estimationSize;
>>> -        private final long estimationTime;
>>> -        private final double smoothing;
>>> +        private int estimationSize;
>>> +        private long estimationTime;
>>> +        private double smoothing;
>>>            private final double threshold;
>>>    
>>>            private MetricsImpl(String name, int estimationSize, long estimationTime, double smoothing, double threshold) {
>>>
>>> Modified: ofbiz/trunk/framework/webapp/config/serverstats.properties
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/config/serverstats.properties?rev=1485601&r1=1485600&r2=1485601&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/webapp/config/serverstats.properties (original)
>>> +++ ofbiz/trunk/framework/webapp/config/serverstats.properties Thu May 23 07:55:14 2013
>>> @@ -50,3 +50,11 @@ stats.persist.SERVICE.hit=false
>>>    # Specify whether a proxy sits in front of this app server
>>>    # This allows VisitHandler to collect the client's real ip
>>>    stats.proxy.enabled=false
>>> +
>>> +### Metric parameters (moving average)
>>> +# size of the considered subset (defines the window size)
>>> +metrics.estimation.size=100
>>> +# minimum time considered between 2 samplings for taking a record or not (must be > to metrics.estimation.time)
>>> +metrics.estimation.time=1000
>>> +# used to smooth the differences between calculations. A value of "1" disables smoothing
>>> +metrics.smoothing.factor=0.7
>>>
>>> Modified: ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/dtd/site-conf.xsd?rev=1485601&r1=1485600&r2=1485601&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/webapp/dtd/site-conf.xsd (original)
>>> +++ ofbiz/trunk/framework/webapp/dtd/site-conf.xsd Thu May 23 07:55:14 2013
>>> @@ -342,8 +342,8 @@ under the License.
>>>        <xs:element name="metric">
>>>            <xs:annotation>
>>>                <xs:documentation>
>>> -                Calculate and maintain an average response time for this request. Request metrics can be used
>>> -                for monitoring and reporting. Metrics can also be used to trigger an alternate
>>> +                Calculate and maintain a moving average response time for a Request or Event. Request Metrics can be used
>>> +                for monitoring and reporting, Event Metrics only for reporting. Request Metrics can also be used to trigger an alternate
>>>                    response if the optional threshold attribute is used.
>>>                    
>>>                    The metric works by gathering statistics until a configurable maximum is reached (number of
>>> @@ -402,6 +402,9 @@ under the License.
>>>                </xs:documentation>
>>>            </xs:annotation>
>>>            <xs:complexType>
>>> +            <xs:sequence>
>>> +                <xs:element minOccurs="0" ref="metric"/>
>>> +            </xs:sequence>
>>>                <xs:attributeGroup ref="attlist.event"/>
>>>            </xs:complexType>
>>>        </xs:element>
>>>
>>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java (original)
>>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java Thu May 23 07:55:14 2013
>>> @@ -588,12 +588,18 @@ public class ConfigXMLReader {
>>>            public String path;
>>>            public String invoke;
>>>            public boolean globalTransaction = true;
>>> +        public Metrics metrics = null;
>>>    
>>>            public Event(Element eventElement) {
>>>                this.type = eventElement.getAttribute("type");
>>>                this.path = eventElement.getAttribute("path");
>>>                this.invoke = eventElement.getAttribute("invoke");
>>>                this.globalTransaction = !"false".equals(eventElement.getAttribute("global-transaction"));
>>> +            // Get metrics.
>>> +            Element metricsElement = UtilXml.firstChildElement(eventElement, "metric");
>>> +            if (metricsElement != null) {
>>> +                this.metrics = MetricsFactory.getInstance(metricsElement);
>>> +            }
>>>            }
>>>    
>>>            public Event(String type, String path, String invoke, boolean globalTransaction) {
>>>
>>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java (original)
>>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java Thu May 23 07:55:14 2013
>>> @@ -170,6 +170,7 @@ public class RequestHandler {
>>>            }
>>>            ConfigXMLReader.RequestMap originalRequestMap = requestMap; // Save this so we can update the correct performance metrics.
>>>    
>>> +
>>>            boolean interruptRequest = false;
>>>    
>>>            // Check for chained request.
>>> @@ -416,6 +417,10 @@ public class RequestHandler {
>>>    
>>>                        // run the request event
>>>                        eventReturn = this.runEvent(request, response, requestMap.event, requestMap, "request");
>>> +
>>> +                    if (requestMap.event.metrics != null) {
>>> +                        requestMap.event.metrics.recordServiceRate(1, System.currentTimeMillis() - startTime);
>>> +                    }
>>>    
>>>                        // save the server hit for the request event
>>>                        if (this.trackStats(request)) {
>>>
>>>


Re: svn commit: r1485601 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/metrics/ webapp/config/ webapp/dtd/ webapp/src/org/ofbiz/webapp/control/

Posted by Jacques Le Roux <ja...@les7arts.com>.
Yes sorry, this was a 1st try I did before changing directly in factory, I forgot to remove, I will. 
Always better to have at least 2 pairs of  eyes...

Jacques

From: "Adrian Crum" <ad...@sandglass-software.com>
> This looks good, but please leave the thread-safe fields as final. See 
> inline comment...
> 
> On 5/23/2013 8:55 AM, jleroux@apache.org wrote:
>> Author: jleroux
>> Date: Thu May 23 07:55:14 2013
>> New Revision: 1485601
>>
>> URL: http://svn.apache.org/r1485601
>> Log:
>> Add a mean to collect and show the Requests Events durations using Metrics  https://issues.apache.org/jira/browse/OFBIZ-5198
>> Fulfill the goal with some more:
>> * makes the Metrics main parameters (estimationSize, estimationTime and smoothing) dynamically changeable (added in serverstats.properties)
>> * there is no threshold handling for event metrics
>>
>> Modified:
>>      ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
>>      ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
>>      ofbiz/trunk/framework/webapp/config/serverstats.properties
>>      ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java Thu May 23 07:55:14 2013
>> @@ -28,6 +28,7 @@
>>    */
>>   package org.ofbiz.base.metrics;
>>   
>> +
>>   /**
>>    * An object that tracks service metrics.
>>    * <p>This interface and its default implementation are based on the
>> @@ -36,11 +37,6 @@ package org.ofbiz.base.metrics;
>>    * @see <a href="http://www.eecs.harvard.edu/~mdw/proj/seda/">SEDA</a>
>>    */
>>   public interface Metrics {
>> -
>> -    public static final int ESTIMATION_SIZE = 100;
>> -    public static final long ESTIMATION_TIME = 1000;
>> -    public static final double SMOOTHING = 0.7;
>> -
>>       /**
>>        * Returns the name of the metric.
>>        */
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java Thu May 23 07:55:14 2013
>> @@ -34,6 +34,7 @@ import java.util.TreeSet;
>>   import org.ofbiz.base.lang.LockedBy;
>>   import org.ofbiz.base.lang.ThreadSafe;
>>   import org.ofbiz.base.util.Assert;
>> +import org.ofbiz.base.util.UtilProperties;
>>   import org.ofbiz.base.util.cache.UtilCache;
>>   import org.w3c.dom.Element;
>>   
>> @@ -42,7 +43,6 @@ import org.w3c.dom.Element;
>>    */
>>   @ThreadSafe
>>   public final class MetricsFactory {
>> -
>>       private static final UtilCache<String, Metrics> METRICS_CACHE = UtilCache.createUtilCache("base.metrics", 0, 0);
>>       /**
>>        * A "do-nothing" <code>Metrics</code> instance.
>> @@ -73,17 +73,17 @@ public final class MetricsFactory {
>>           Assert.notEmpty("name attribute", name);
>>           Metrics result = METRICS_CACHE.get(name);
>>           if (result == null) {
>> -            int estimationSize = Metrics.ESTIMATION_SIZE;
>> +            int estimationSize = UtilProperties.getPropertyAsInteger("serverstats", "metrics.estimation.size", 100);
>>               String attributeValue = element.getAttribute("estimation-size");
>>               if (!attributeValue.isEmpty()) {
>>                   estimationSize = Integer.parseInt(attributeValue);
>>               }
>> -            long estimationTime = Metrics.ESTIMATION_TIME;
>> +            long estimationTime = UtilProperties.getPropertyAsLong("serverstats", "metrics.estimation.time", 1000);
>>               attributeValue = element.getAttribute("estimation-time");
>>               if (!attributeValue.isEmpty()) {
>>                   estimationTime = Long.parseLong(attributeValue);
>>               }
>> -            double smoothing = Metrics.SMOOTHING;
>> +            double smoothing = UtilProperties.getPropertyNumber("serverstats", "metrics.smoothing.factor", 0.7);
>>               attributeValue = element.getAttribute("smoothing");
>>               if (!attributeValue.isEmpty()) {
>>                   smoothing = Double.parseDouble(attributeValue);
>> @@ -151,9 +151,9 @@ public final class MetricsFactory {
>>           @LockedBy("this")
>>           private long cumulativeEvents;
>>           private final String name;
> 
> Please keep these final.
> 
>> -        private final int estimationSize;
>> -        private final long estimationTime;
>> -        private final double smoothing;
>> +        private int estimationSize;
>> +        private long estimationTime;
>> +        private double smoothing;
>>           private final double threshold;
>>   
>>           private MetricsImpl(String name, int estimationSize, long estimationTime, double smoothing, double threshold) {
>>
>> Modified: ofbiz/trunk/framework/webapp/config/serverstats.properties
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/config/serverstats.properties?rev=1485601&r1=1485600&r2=1485601&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/webapp/config/serverstats.properties (original)
>> +++ ofbiz/trunk/framework/webapp/config/serverstats.properties Thu May 23 07:55:14 2013
>> @@ -50,3 +50,11 @@ stats.persist.SERVICE.hit=false
>>   # Specify whether a proxy sits in front of this app server
>>   # This allows VisitHandler to collect the client's real ip
>>   stats.proxy.enabled=false
>> +
>> +### Metric parameters (moving average)
>> +# size of the considered subset (defines the window size)
>> +metrics.estimation.size=100
>> +# minimum time considered between 2 samplings for taking a record or not (must be > to metrics.estimation.time)
>> +metrics.estimation.time=1000
>> +# used to smooth the differences between calculations. A value of "1" disables smoothing
>> +metrics.smoothing.factor=0.7
>>
>> Modified: ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/dtd/site-conf.xsd?rev=1485601&r1=1485600&r2=1485601&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/webapp/dtd/site-conf.xsd (original)
>> +++ ofbiz/trunk/framework/webapp/dtd/site-conf.xsd Thu May 23 07:55:14 2013
>> @@ -342,8 +342,8 @@ under the License.
>>       <xs:element name="metric">
>>           <xs:annotation>
>>               <xs:documentation>
>> -                Calculate and maintain an average response time for this request. Request metrics can be used
>> -                for monitoring and reporting. Metrics can also be used to trigger an alternate
>> +                Calculate and maintain a moving average response time for a Request or Event. Request Metrics can be used
>> +                for monitoring and reporting, Event Metrics only for reporting. Request Metrics can also be used to trigger an alternate
>>                   response if the optional threshold attribute is used.
>>                   
>>                   The metric works by gathering statistics until a configurable maximum is reached (number of
>> @@ -402,6 +402,9 @@ under the License.
>>               </xs:documentation>
>>           </xs:annotation>
>>           <xs:complexType>
>> +            <xs:sequence>
>> +                <xs:element minOccurs="0" ref="metric"/>
>> +            </xs:sequence>
>>               <xs:attributeGroup ref="attlist.event"/>
>>           </xs:complexType>
>>       </xs:element>
>>
>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java (original)
>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java Thu May 23 07:55:14 2013
>> @@ -588,12 +588,18 @@ public class ConfigXMLReader {
>>           public String path;
>>           public String invoke;
>>           public boolean globalTransaction = true;
>> +        public Metrics metrics = null;
>>   
>>           public Event(Element eventElement) {
>>               this.type = eventElement.getAttribute("type");
>>               this.path = eventElement.getAttribute("path");
>>               this.invoke = eventElement.getAttribute("invoke");
>>               this.globalTransaction = !"false".equals(eventElement.getAttribute("global-transaction"));
>> +            // Get metrics.
>> +            Element metricsElement = UtilXml.firstChildElement(eventElement, "metric");
>> +            if (metricsElement != null) {
>> +                this.metrics = MetricsFactory.getInstance(metricsElement);
>> +            }
>>           }
>>   
>>           public Event(String type, String path, String invoke, boolean globalTransaction) {
>>
>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java?rev=1485601&r1=1485600&r2=1485601&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java (original)
>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java Thu May 23 07:55:14 2013
>> @@ -170,6 +170,7 @@ public class RequestHandler {
>>           }
>>           ConfigXMLReader.RequestMap originalRequestMap = requestMap; // Save this so we can update the correct performance metrics.
>>   
>> +
>>           boolean interruptRequest = false;
>>   
>>           // Check for chained request.
>> @@ -416,6 +417,10 @@ public class RequestHandler {
>>   
>>                       // run the request event
>>                       eventReturn = this.runEvent(request, response, requestMap.event, requestMap, "request");
>> +
>> +                    if (requestMap.event.metrics != null) {
>> +                        requestMap.event.metrics.recordServiceRate(1, System.currentTimeMillis() - startTime);
>> +                    }
>>   
>>                       // save the server hit for the request event
>>                       if (this.trackStats(request)) {
>>
>>
>

Re: svn commit: r1485601 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/metrics/ webapp/config/ webapp/dtd/ webapp/src/org/ofbiz/webapp/control/

Posted by Adrian Crum <ad...@sandglass-software.com>.
This looks good, but please leave the thread-safe fields as final. See 
inline comment...

On 5/23/2013 8:55 AM, jleroux@apache.org wrote:
> Author: jleroux
> Date: Thu May 23 07:55:14 2013
> New Revision: 1485601
>
> URL: http://svn.apache.org/r1485601
> Log:
> Add a mean to collect and show the Requests Events durations using Metrics  https://issues.apache.org/jira/browse/OFBIZ-5198
> Fulfill the goal with some more:
> * makes the Metrics main parameters (estimationSize, estimationTime and smoothing) dynamically changeable (added in serverstats.properties)
> * there is no threshold handling for event metrics
>
> Modified:
>      ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
>      ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
>      ofbiz/trunk/framework/webapp/config/serverstats.properties
>      ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
>
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java?rev=1485601&r1=1485600&r2=1485601&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/Metrics.java Thu May 23 07:55:14 2013
> @@ -28,6 +28,7 @@
>    */
>   package org.ofbiz.base.metrics;
>   
> +
>   /**
>    * An object that tracks service metrics.
>    * <p>This interface and its default implementation are based on the
> @@ -36,11 +37,6 @@ package org.ofbiz.base.metrics;
>    * @see <a href="http://www.eecs.harvard.edu/~mdw/proj/seda/">SEDA</a>
>    */
>   public interface Metrics {
> -
> -    public static final int ESTIMATION_SIZE = 100;
> -    public static final long ESTIMATION_TIME = 1000;
> -    public static final double SMOOTHING = 0.7;
> -
>       /**
>        * Returns the name of the metric.
>        */
>
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java?rev=1485601&r1=1485600&r2=1485601&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/metrics/MetricsFactory.java Thu May 23 07:55:14 2013
> @@ -34,6 +34,7 @@ import java.util.TreeSet;
>   import org.ofbiz.base.lang.LockedBy;
>   import org.ofbiz.base.lang.ThreadSafe;
>   import org.ofbiz.base.util.Assert;
> +import org.ofbiz.base.util.UtilProperties;
>   import org.ofbiz.base.util.cache.UtilCache;
>   import org.w3c.dom.Element;
>   
> @@ -42,7 +43,6 @@ import org.w3c.dom.Element;
>    */
>   @ThreadSafe
>   public final class MetricsFactory {
> -
>       private static final UtilCache<String, Metrics> METRICS_CACHE = UtilCache.createUtilCache("base.metrics", 0, 0);
>       /**
>        * A "do-nothing" <code>Metrics</code> instance.
> @@ -73,17 +73,17 @@ public final class MetricsFactory {
>           Assert.notEmpty("name attribute", name);
>           Metrics result = METRICS_CACHE.get(name);
>           if (result == null) {
> -            int estimationSize = Metrics.ESTIMATION_SIZE;
> +            int estimationSize = UtilProperties.getPropertyAsInteger("serverstats", "metrics.estimation.size", 100);
>               String attributeValue = element.getAttribute("estimation-size");
>               if (!attributeValue.isEmpty()) {
>                   estimationSize = Integer.parseInt(attributeValue);
>               }
> -            long estimationTime = Metrics.ESTIMATION_TIME;
> +            long estimationTime = UtilProperties.getPropertyAsLong("serverstats", "metrics.estimation.time", 1000);
>               attributeValue = element.getAttribute("estimation-time");
>               if (!attributeValue.isEmpty()) {
>                   estimationTime = Long.parseLong(attributeValue);
>               }
> -            double smoothing = Metrics.SMOOTHING;
> +            double smoothing = UtilProperties.getPropertyNumber("serverstats", "metrics.smoothing.factor", 0.7);
>               attributeValue = element.getAttribute("smoothing");
>               if (!attributeValue.isEmpty()) {
>                   smoothing = Double.parseDouble(attributeValue);
> @@ -151,9 +151,9 @@ public final class MetricsFactory {
>           @LockedBy("this")
>           private long cumulativeEvents;
>           private final String name;

Please keep these final.

> -        private final int estimationSize;
> -        private final long estimationTime;
> -        private final double smoothing;
> +        private int estimationSize;
> +        private long estimationTime;
> +        private double smoothing;
>           private final double threshold;
>   
>           private MetricsImpl(String name, int estimationSize, long estimationTime, double smoothing, double threshold) {
>
> Modified: ofbiz/trunk/framework/webapp/config/serverstats.properties
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/config/serverstats.properties?rev=1485601&r1=1485600&r2=1485601&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/webapp/config/serverstats.properties (original)
> +++ ofbiz/trunk/framework/webapp/config/serverstats.properties Thu May 23 07:55:14 2013
> @@ -50,3 +50,11 @@ stats.persist.SERVICE.hit=false
>   # Specify whether a proxy sits in front of this app server
>   # This allows VisitHandler to collect the client's real ip
>   stats.proxy.enabled=false
> +
> +### Metric parameters (moving average)
> +# size of the considered subset (defines the window size)
> +metrics.estimation.size=100
> +# minimum time considered between 2 samplings for taking a record or not (must be > to metrics.estimation.time)
> +metrics.estimation.time=1000
> +# used to smooth the differences between calculations. A value of "1" disables smoothing
> +metrics.smoothing.factor=0.7
>
> Modified: ofbiz/trunk/framework/webapp/dtd/site-conf.xsd
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/dtd/site-conf.xsd?rev=1485601&r1=1485600&r2=1485601&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/webapp/dtd/site-conf.xsd (original)
> +++ ofbiz/trunk/framework/webapp/dtd/site-conf.xsd Thu May 23 07:55:14 2013
> @@ -342,8 +342,8 @@ under the License.
>       <xs:element name="metric">
>           <xs:annotation>
>               <xs:documentation>
> -                Calculate and maintain an average response time for this request. Request metrics can be used
> -                for monitoring and reporting. Metrics can also be used to trigger an alternate
> +                Calculate and maintain a moving average response time for a Request or Event. Request Metrics can be used
> +                for monitoring and reporting, Event Metrics only for reporting. Request Metrics can also be used to trigger an alternate
>                   response if the optional threshold attribute is used.
>                   
>                   The metric works by gathering statistics until a configurable maximum is reached (number of
> @@ -402,6 +402,9 @@ under the License.
>               </xs:documentation>
>           </xs:annotation>
>           <xs:complexType>
> +            <xs:sequence>
> +                <xs:element minOccurs="0" ref="metric"/>
> +            </xs:sequence>
>               <xs:attributeGroup ref="attlist.event"/>
>           </xs:complexType>
>       </xs:element>
>
> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java?rev=1485601&r1=1485600&r2=1485601&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java (original)
> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ConfigXMLReader.java Thu May 23 07:55:14 2013
> @@ -588,12 +588,18 @@ public class ConfigXMLReader {
>           public String path;
>           public String invoke;
>           public boolean globalTransaction = true;
> +        public Metrics metrics = null;
>   
>           public Event(Element eventElement) {
>               this.type = eventElement.getAttribute("type");
>               this.path = eventElement.getAttribute("path");
>               this.invoke = eventElement.getAttribute("invoke");
>               this.globalTransaction = !"false".equals(eventElement.getAttribute("global-transaction"));
> +            // Get metrics.
> +            Element metricsElement = UtilXml.firstChildElement(eventElement, "metric");
> +            if (metricsElement != null) {
> +                this.metrics = MetricsFactory.getInstance(metricsElement);
> +            }
>           }
>   
>           public Event(String type, String path, String invoke, boolean globalTransaction) {
>
> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java?rev=1485601&r1=1485600&r2=1485601&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java (original)
> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java Thu May 23 07:55:14 2013
> @@ -170,6 +170,7 @@ public class RequestHandler {
>           }
>           ConfigXMLReader.RequestMap originalRequestMap = requestMap; // Save this so we can update the correct performance metrics.
>   
> +
>           boolean interruptRequest = false;
>   
>           // Check for chained request.
> @@ -416,6 +417,10 @@ public class RequestHandler {
>   
>                       // run the request event
>                       eventReturn = this.runEvent(request, response, requestMap.event, requestMap, "request");
> +
> +                    if (requestMap.event.metrics != null) {
> +                        requestMap.event.metrics.recordServiceRate(1, System.currentTimeMillis() - startTime);
> +                    }
>   
>                       // save the server hit for the request event
>                       if (this.trackStats(request)) {
>
>