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" <adrian.c...@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" <adrian.c...@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, jler...@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)) { >>>> >>>> >
