From: "Adrian Crum" <adrian.c...@sandglass-software.com>
I'm going to try one more time to ask nicely, and if that doesn't work,
then I'm prepared to veto your commit.
You added redundant methods to the existing code that do nothing more
than duplicate existing functionality.
"duplicate existing functionality. " Not true for 2 reasons
1) recordServiceRate() provides only the serviceRate variable which
is then used to feed the corresponding column in the webtools form.
I need another column to store the request events durations for the
chained requests. Hence a new requestRate variable is used in
recordRequestRate(). I could have called it EventRequestRate to make
it more clear, because it really measures the event duration which
for chained requests is the only duration which makes sense.
2) As says the javadoc, recordServiceRate() implements as default a
smoothed moving average. I wanted something simpler: a simple
average, that's what recordRequestRate() does.
You are not taking the time to
understand how to use the feature properly.
I did. You are not taking the time to understand my need. I want to
measure chained request events durations, and I want to use a simple
average not a smoothed moving average.
Instead, you are being
argumentative and supporting your view with imaginary users with
imaginary requirements.
I'm an user and I'm not imaginary. So what I want I believe some
other users would like.
I thought about putting it in the original Stats form which lacks a
way to measure chained requests events duration.
But I found Metrics form easier to read, Metrics implementation easy
to understand and using only memory a plus
Please revert your commit, then we can discuss this further.
If I revert I will lose the functionallity I want (chained request
events durations measured with a simple average) and with the current
code it can't be implemented.
I understand that you want to keep the Metrics classes as they are
now but I did not change it, just added what I needed
What I could to if you want is below but I'd then lose the simple
average calculation and things would get blurred rather than simplified.
Index: base/src/org/ofbiz/base/metrics/Metrics.java
===================================================================
--- base/src/org/ofbiz/base/metrics/Metrics.java (revision 1484069)
+++ base/src/org/ofbiz/base/metrics/Metrics.java (working copy)
@@ -76,12 +76,11 @@
void recordServiceRate(int numEvents, long time);
/**
- * Records the request time for <code>numEvents</code> taking
- * <code>time</code> milliseconds to be processed.
- */
- void recordRequestRate(int numEvents, long time);
+ * Records in varName the time for <code>numEvents</code> taking
+ * <code>time</code> milliseconds to be processed.
+ */
+ void recordServiceRate(int numEvents, long time, String varName);
/** Resets all metrics. */
void reset();
-
}
\ No newline at end of file
Index: base/src/org/ofbiz/base/metrics/MetricsFactory.java
===================================================================
--- base/src/org/ofbiz/base/metrics/MetricsFactory.java (revision
1484069)
+++ base/src/org/ofbiz/base/metrics/MetricsFactory.java (working copy)
@@ -209,6 +209,10 @@
@Override
public synchronized void recordServiceRate(int numEvents,
long time) {
+ recordServiceRate(numEvents, time, "serviceRate");
+ }
+
+ public synchronized void recordServiceRate(int numEvents,
long time, String varName) {
totalEvents += numEvents;
cumulativeEvents += numEvents;
totalServiceTime += time;
@@ -219,20 +223,18 @@
totalEvents = 1;
}
double rate = totalServiceTime / totalEvents;
- serviceRate = (rate * smoothing) + (serviceRate *
(1.0 - smoothing));
+ rate = (rate * smoothing) + (serviceRate * (1.0 -
smoothing));
+ if ("serviceRate".equals(varName)) {
+ serviceRate = rate;
+ } else if ("requestRate".equals(varName)) {
+ requestRate = rate;
+ }
count = 0;
lastTime = curTime;
totalEvents = totalServiceTime = 0;
}
}
- public synchronized void recordRequestRate(int numEvents,
long time) {
- totalEvents += numEvents;
- cumulativeEvents += numEvents;
- totalServiceTime += time;
- requestRate = totalServiceTime / cumulativeEvents;
- }
-
@Override
public synchronized void reset() {
serviceRate = 0.0;
@@ -276,11 +278,11 @@
}
@Override
- public void recordServiceRate(int numEvents, long time) {
+ public void recordServiceRate(int numEvents, long time,
String varName) {
}
@Override
- public void recordRequestRate(int numEvents, long time) {
+ public void recordServiceRate(int numEvents, long time) {
}
@Override
Index: webapp/src/org/ofbiz/webapp/control/RequestHandler.java
===================================================================
--- webapp/src/org/ofbiz/webapp/control/RequestHandler.java (revision
1484069)
+++ webapp/src/org/ofbiz/webapp/control/RequestHandler.java (working
copy)
@@ -423,7 +423,7 @@
System.currentTimeMillis() -
eventStartTime, userLogin);
}
if (requestMap.metrics != null) {
- requestMap.metrics.recordRequestRate(numEvent,
System.currentTimeMillis() - eventStartTime);
+ requestMap.metrics.recordServiceRate(numEvent,
System.currentTimeMillis() - eventStartTime, "requetRate");
numEvent = 0;
}
Also I'd need to introduce a metricsSmoothingFactor properties in
serverstats.properties to avoid the burden of putting smoothing="1"
everywhere I'd need in controllers.
Anyway event with that I'd not have simple average calculation or
would need to have also a metricsEstimationSize properties in
serverstats.properties (set to a very high value in my case)
I think we can discuss changes without reverting all, can't we?
Jacques
-Adrian
On 5/17/2013 6:18 PM, Jacques Le Roux wrote:
From: "Adrian Crum" <adrian.c...@sandglass-software.com>
Agreed. You added something that is unnecessary. Please remove it.
I don't agree it's unnecessary, it does measure the request events
durations for chained request and that's quite useful.
Without this new column the measure for chained requests does not
correspond to the reality
You just need to add the average of chained requests measures of a
request to be persuaded.
So this new functionality completes the measures for chained requests.
It does not use moving average, that could be discussed, but is not
necessary IMO.
Jacques
Metrics.java does not need to be changed. If you want to change
what is
being measured, then apply Metrics java in a different way, but do
not
change it.
-Adrian
On 5/17/2013 5:45 PM, Jacques Le Roux wrote:
From: "Adrian Crum" <adrian.c...@sandglass-software.com>
What you believe users want and what the feature was designed to
do seem
to be entirely different.
The metric is intended to measure the entire request. If you want
something different, then feel free to modify your local copy.
I believe some users will be interested by what I added. I did
not remove anything!
Jacques
-Adrian
On 5/17/2013 5:25 PM, Jacques Le Roux wrote:
From: "Adrian Crum" <adrian.c...@sandglass-software.com>
The real request duration is meaningless. If a garbage
collection occurs
during the request, the measurement will be artificially high.
That's
the whole point of the smoothing - it factors out transient
performance
differences.
Please don't break the design. If you don't like the way it
works, then
create your own version by extending it.
To keep things clear. The curent design is good for services
but not for requests: it does not measure the time passed by
the event but the whole request. I believe much users would be
interested by the 1st.
Jacques
-Adrian
On 5/17/2013 4:41 PM, Jacques Le Roux wrote:
What I wanted to add by
Just thinking I did not use the
was that I did not use the moving average (I forgot to remove
this part after).
Actually I had no needs for it and it's already covered by
recordServiceRate() anyway.
The pb with current position of recordServiceRate() in
RequestHandler is it does not measure the real request event
duration. That's what I was interested by, and I believe much
users. Having a moving average is secondary for me.
Jacques
From: "Jacques Le Roux" <jacques.le.r...@les7arts.com>
Hi Adrian,
At r1483822 I have improved it a bit for requests. I was
more interested in chained ones, it works for all. It's a
new colums in Metrics page which measure the request event
duration. Just thinking I did not use the
I wonder if the use of recordServiceRate() in doRequest()
makes sense. Let me know if you think we should adapt it for
the threshold functionnality using the event duration of
it's it fine as is.
Also let me know if you want me to create the wiki page when
we will be done.
Cheers
Jacques
From: "Adrian Crum" <adrian.c...@sandglass-software.com>
Anything you can do to improve the implementation will be
appreciated.
Sorry I haven't had more time to document it.
-Adrian
On 4/19/2013 12:01 PM, Jacques Le Roux wrote:
OK, I guess the smoothing factor explains the tiny
differences for services
But for chained requests it makes no sense.
The metrics report ten more than in log.
I will have a look because it's a fine tool for chained
requests duration which are not availabe in the stats feature
Thanks
Jacques
Adrian Crum wrote:
Keep in mind the metrics provide a moving average. Also,
there is a
smoothing factor that can affect the value.
The smoothing factor is important. Let's say you are
using a monitoring
tool to monitor a particular request. If that request has
a very brief
increase in response time, you don't want to be alerted.
Instead, you
want to know when the increase in response time is
sustained over a
period of time. So, the smoothing factor helps prevent
"false positives"
in a way.
Another scenario: You have an eCommerce site and you put
a metric on the
store's landing page and you configure it with a
threshold value and an
alternate view - one that doesn't include a shopping
cart. Normally,
customers land on the normal page that includes a
shopping cart. But a
new promotion causes customers to swamp the server and
the shopping
experience degrades. The metric will smooth the response
time values so
the alternate (non-shopping cart) landing page is not
offered too soon.
But if the server is swamped for an extended period, then
new visitors
will get the alternate page and be able to browse the
site without being
able to order anything. As the request load decreases,
the metric will
smooth the response times so the alternate landing page
is not removed
too soon. When the landing page response time drops below
the configured
threshold, all visitors will receive the normal shopping
experience.
-Adrian
On 4/19/2013 10:53 AM, Jacques Le Roux wrote:
Actually r1361725 was also needed for services metrics
to work.
For services I get slightly higher results than in log.
It's more consistent than for chained requests which are
plain false.
I did not get a chance to look at code yet...
Jacques
Jacques Le Roux wrote:
From: "Jacques Le Roux" <jacques.le.r...@les7arts.com>
Thanks Adrian,
Yes I saw the threshold idea in Java comments
Looks like an interesting tool
BTW for the other statistics (I never used it), do you
know if chained requests are taken into account?
Hi Adrian,
I can answer now that chained requests are not taken
into account by the Statistics feature.
I also noticed something weird in Metrics feature. The
total duration of the chained requests and the duration
of the initial
calling request don't match And when I compare
durations in log with those in Metrics I find those in
metrics much higher in
some cases, and always higher.
Do I misinterpret them? Did you use/check this feature
for chained requests?
Maybe I should wait your wiki page?
Thanks
Jacques
Still, can you confirm only if 1361722 and 1361724 the
only commits? I guess there is no Jira?
Jacques
From: "Adrian Crum" <adrian.c...@sandglass-software.com>
I will create a Wiki page this weekend.
The metric name can be anything - the example has a
URL: prefix to help
distinguish request metrics from service metrics. You
can put anything
you want there.
The request-map can have an additional <response>
element - which will
direct the servlet to an alternate view if the metric
crosses a
threshold. I think the JavaDocs explain that.
The basic idea is to put metrics in places where you
anticipate heavy
loads or bottlenecks, then use a third-party
reporting/monitoring tool
to read the metrics.
-Adrian
On 4/11/2013 8:36 AM, Jacques Le Roux wrote:
Thanks Atul,
So you mean you simply need to put a token (like
webapp name) before the request name in this metric
name? Did you try it
with another request?
I see it works on trunk demo and OOTB locally. But I
can't get it to work on a custom app I have patched,
so maybe I miss
something I will wait Adrian's answer about that
Jacques
From: "Atul Vani" <atul.v...@hotwaxmedia.com>
I think that's just the name, to recognize it among
list of several
others, when there will be lots of them. A certain
convention is used to
specify that it is for URL or any service. Then
again same names request
mappings can exist in separate webapps, so a
leading appname, in this case
'webtools'. It can be changed to anything.
On Thu, 11 Apr 2013 00:47:26 +0530, Jacques Le Roux
<jacques.le.r...@les7arts.com> wrote:
Hi Adrian,
Let me know if you will, else I might do it...
BTW, are 1361722 and 1361724 the only commit? Is
there a Jira?
Could you explain just a bit more the request
syntax, sorry but the OOTB
example is a bit confusing.
+ <request-map uri="ViewMetrics">
+ <security https="true" auth="true"/>
+ <metric name="URL: webtools/ViewMetrics"
/><!-- Here for
demonstration -->
+ <response name="success" type="view"
value="ViewMetrics"/>
+ </request-map>
What would it be for another than ViewMetrics?
I mean why "URL: webtools/ViewMetrics"?
Thanks
Jacques
From: "Jacques Le Roux"
<jacques.le.r...@les7arts.com>
Great, thanks Adrian!
Maybe a wiki page using
http://markmail.org/message/x4lzvda66ju6gdg5
would help to remember the commands?
Jacques
From: "Adrian Crum"
<adrian.c...@sandglass-software.com>
On 4/7/2013 6:38 PM, Adrian Crum wrote:
On 4/7/2013 12:11 PM, Jacques Le Roux wrote:
Jacques Le Roux wrote:
From: "Adrian Crum"
<adrian.c...@sandglass-software.com>
Have you considered using the metrics feature?
Ha forgot to ask about it, now I see and
remember this thread and I
will digg in
http://markmail.org/message/x4lzvda66ju6gdg5
Of course, I'd not be against a brief
briefing, or a link to
explain.
What I mostly miss is where are the metrics in
webtools?
Oops, I forgot to commit that part. I will take
care of it.
Actually, I did commit it:
https://localhost:8443/webtools/control/ViewMetrics
-Adrian
--
Using Opera's revolutionary email client:
http://www.opera.com/mail/
