OFBiz › OFBiz - Dev

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

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

5 messages Options

Adrian Crum-3

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

This looks good, but please leave the thread-safe fields as final. See
inline comment...

On 5/23/2013 8:55 AM, [hidden email] 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)) {
>
>

Jacques Le Roux

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

Administrator

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" <[hidden email]>

> This looks good, but please leave the thread-safe fields as final. See
> inline comment...
>
> On 5/23/2013 8:55 AM, [hidden email] 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)) {
>>
>>
>

Adrian Crum-3

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

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" <[hidden email]>
>> This looks good, but please leave the thread-safe fields as final. See
>> inline comment...
>>
>> On 5/23/2013 8:55 AM, [hidden email] 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)) {
>>>
>>>

Jacques Le Roux

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

Administrator

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" <[hidden email]>

> 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" <[hidden email]>
>>> This looks good, but please leave the thread-safe fields as final. See
>>> inline comment...
>>>
>>> On 5/23/2013 8:55 AM, [hidden email] 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)) {
>>>>
>>>>
>

Jacques Le Roux

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

Administrator

In reply to this post by Adrian Crum-3

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" <[hidden email]>