On Sunday, November 30, 2014, Felix Schumacher < [email protected]> wrote:
> Hi Philippe, > > a few comments inline. > > Am 30.11.2014 um 21:29 schrieb [email protected]: > >> Author: pmouawad >> Date: Sun Nov 30 20:29:49 2014 >> New Revision: 1642603 >> >> URL: http://svn.apache.org/r1642603 >> Log: >> Bug 57246 - BackendListener : Create a Graphite implementation >> Make reported percentiles configurable >> Compute min and max within sliding window >> Move threads (Users) computing to UserMetric >> Bugzilla Id: 57246 >> >> Added: >> >> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/UserMetric.java >> (with props) >> Modified: >> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/ >> AbstractBackendListenerClient.java >> jmeter/trunk/src/components/org/apache/jmeter/visualizers/ >> backend/SamplerMetric.java >> jmeter/trunk/src/components/org/apache/jmeter/visualizers/ >> backend/graphite/GraphiteBackendListenerClient.java >> jmeter/trunk/xdocs/usermanual/component_reference.xml >> >> Modified: jmeter/trunk/src/components/org/apache/jmeter/visualizers/ >> backend/AbstractBackendListenerClient.java >> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/ >> org/apache/jmeter/visualizers/backend/AbstractBackendListenerClient. >> java?rev=1642603&r1=1642602&r2=1642603&view=diff >> ============================================================ >> ================== >> --- jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/ >> AbstractBackendListenerClient.java (original) >> +++ jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/ >> AbstractBackendListenerClient.java Sun Nov 30 20:29:49 2014 >> @@ -53,6 +53,7 @@ import org.apache.log.Logger; >> public abstract class AbstractBackendListenerClient implements >> BackendListenerClient { >> private static final Logger LOGGER = LoggingManager. >> getLoggerForClass(); >> + private UserMetric userMetrics = new UserMetric(); >> private ConcurrentHashMap<String, SamplerMetric> >> metricsPerSampler = new ConcurrentHashMap<String, SamplerMetric>(); >> @@ -117,4 +118,10 @@ public abstract class AbstractBackendLis >> return metricsPerSampler; >> } >> + /** >> + * @return UserMetric >> + */ >> + protected UserMetric getUserMetrics() { >> + return userMetrics; >> + } >> } >> >> Modified: jmeter/trunk/src/components/org/apache/jmeter/visualizers/ >> backend/SamplerMetric.java >> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/ >> org/apache/jmeter/visualizers/backend/SamplerMetric.java? >> rev=1642603&r1=1642602&r2=1642603&view=diff >> ============================================================ >> ================== >> --- >> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/SamplerMetric.java >> (original) >> +++ >> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/SamplerMetric.java >> Sun Nov 30 20:29:49 2014 >> @@ -20,20 +20,19 @@ package org.apache.jmeter.visualizers.ba >> import org.apache.commons.math3.stat.descriptive. >> DescriptiveStatistics; >> import org.apache.jmeter.samplers.SampleResult; >> +import org.apache.jmeter.util.JMeterUtils; >> /** >> * Sampler metric >> * @since 2.13 >> */ >> public class SamplerMetric { >> - // Limit to sliding window of 100 values >> - private DescriptiveStatistics stats = new DescriptiveStatistics(100); >> - private int success; >> - private int failure; >> - private long maxTime=0L; >> - private long minTime=Long.MAX_VALUE; >> - private int maxActiveThreads = 0; >> - private int minActiveThreads = Integer.MAX_VALUE; >> + private static final int SLIDING_WINDOW_SIZE = >> JMeterUtils.getPropDefault("backend_metrics_window", 100); //$NON-NLS-1$ >> + >> + // Limit to sliding window of SLIDING_WINDOW_SIZE values >> + private DescriptiveStatistics responsesStats = new >> DescriptiveStatistics(SLIDING_WINDOW_SIZE); >> + private int successes; >> > I would have named it successCount and failureCount and shouldn't they > also be windowed, if all other values are? There are windowed through the call of resetForTimenterval no ? > + private int failures; >> /** >> * >> */ >> @@ -46,20 +45,15 @@ public class SamplerMetric { >> */ >> public synchronized void add(SampleResult result) { >> if(result.isSuccessful()) { >> - success++; >> + successes++; >> } else { >> - failure++; >> + failures++; >> } >> long time = result.getTime(); >> - int activeThreads = result.getAllThreads(); >> - maxTime = Math.max(time, maxTime); >> - minTime = Math.min(time, minTime); >> - maxActiveThreads = Math.max(maxActiveThreads, activeThreads); >> - minActiveThreads = Math.min(minActiveThreads, activeThreads); >> if(result.isSuccessful()) { >> // Should we also compute KO , all response time ? >> // only take successful requests for time computing >> - stats.addValue(time); >> + responsesStats.addValue(time); >> } >> } >> @@ -67,14 +61,10 @@ public class SamplerMetric { >> * Reset metric except for percentile related data >> */ >> public synchronized void resetForTimeInterval() { >> - // We don't clear stats as it will slide as per my understanding >> of >> + // We don't clear responsesStats nor usersStats as it will slide >> as per my understanding of >> // http://commons.apache.org/proper/commons-math/userguide/ >> stat.html >> - success = 0; >> - failure = 0; >> - maxTime=0L; >> - minTime=Long.MAX_VALUE; >> - maxActiveThreads = 0; >> - minActiveThreads = Integer.MAX_VALUE; >> + successes = 0; >> + failures = 0; >> } >> /** >> @@ -83,7 +73,7 @@ public class SamplerMetric { >> * @return The arithmetic mean of the stored values >> */ >> public double getMean() { >> - return stats.getMean(); >> + return responsesStats.getMean(); >> } >> /** >> @@ -95,7 +85,7 @@ public class SamplerMetric { >> * values. >> */ >> public double getPercentile(double percentile) { >> - return stats.getPercentile(percentile); >> + return responsesStats.getPercentile(percentile); >> } >> /** >> @@ -104,7 +94,7 @@ public class SamplerMetric { >> * @return number of total requests >> */ >> public int getTotal() { >> - return success+failure; >> + return successes+failures; >> } >> /** >> @@ -112,8 +102,8 @@ public class SamplerMetric { >> * >> * @return number of successful requests >> */ >> - public int getSuccess() { >> - return success; >> + public int getSuccesses() { >> + return successes; >> } >> /** >> @@ -121,43 +111,27 @@ public class SamplerMetric { >> * >> * @return number of failed requests >> */ >> - public int getFailure() { >> - return failure; >> + public int getFailures() { >> + return failures; >> } >> /** >> - * Get the maximal elapsed time for all requests >> + * Get the maximal elapsed time for requests within sliding window >> * >> * @return the maximal elapsed time, or <code>0</code> if no >> requests have >> * been added yet >> */ >> - public long getMaxTime() { >> - return maxTime; >> + public double getMaxTime() { >> + return responsesStats.getMax(); >> } >> /** >> - * Get the minimal elapsed time for all requests >> + * Get the minimal elapsed time for requests within sliding window >> * >> * @return the minTime, or {@link Long#MAX_VALUE} if no requests >> have been >> * added yet >> */ >> - public long getMinTime() { >> - return minTime; >> - } >> - >> - /** >> - * @return the max number of active threads for this test run, or >> - * <code>0</code> if no samples have been added yet >> - */ >> - public int getMaxActiveThreads() { >> - return maxActiveThreads; >> - } >> - >> - /** >> - * @return the min number of active threads for this test run, or >> - * {@link Integer#MAX_VALUE} if no samples have been added >> yet >> - */ >> - public int getMinActiveThreads() { >> - return minActiveThreads; >> + public double getMinTime() { >> + return responsesStats.getMin(); >> } >> } >> >> Added: jmeter/trunk/src/components/org/apache/jmeter/visualizers/ >> backend/UserMetric.java >> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/ >> org/apache/jmeter/visualizers/backend/UserMetric.java?rev= >> 1642603&view=auto >> ============================================================ >> ================== >> --- >> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/UserMetric.java >> (added) >> +++ >> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/UserMetric.java >> Sun Nov 30 20:29:49 2014 >> @@ -0,0 +1,78 @@ >> +/* >> + * Licensed to the Apache Software Foundation (ASF) under one or more >> + * contributor license agreements. See the NOTICE file distributed with >> + * this work for additional information regarding copyright ownership. >> + * The ASF licenses this file to You under the Apache License, Version >> 2.0 >> + * (the "License"); you may not use this file except in compliance with >> + * the License. You may obtain a copy of the License at >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, software >> + * distributed under the License is distributed on an "AS IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> implied. >> + * See the License for the specific language governing permissions and >> + * limitations under the License. >> + * >> + */ >> + >> +package org.apache.jmeter.visualizers.backend; >> + >> +import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics; >> +import org.apache.jmeter.samplers.SampleResult; >> +import org.apache.jmeter.util.JMeterUtils; >> + >> +/** >> + * User metric >> + * @since 2.13 >> + */ >> +public class UserMetric { >> + private static final int SLIDING_WINDOW_SIZE = >> JMeterUtils.getPropDefault("backend_metrics_window", 100); //$NON-NLS-1$ >> + >> + // Limit to sliding window of SLIDING_WINDOW_SIZE values >> + private DescriptiveStatistics usersStats = new >> DescriptiveStatistics(SLIDING_WINDOW_SIZE); >> + /** >> + * >> + */ >> + public UserMetric() { >> + } >> + >> + /** >> + * Add a {@link SampleResult} to be used in the statistics >> + * @param result {@link SampleResult} to be used >> + */ >> + public synchronized void add(SampleResult result) { >> + usersStats.addValue(result.getAllThreads()); >> + } >> + >> + /** >> + * Reset metric except for percentile related data >> + */ >> + public synchronized void resetForTimeInterval() { >> + // NOOP >> + } >> + >> + /** >> + * @return the max number of active threads for this test run >> + * using a sliding window of SLIDING_WINDOW_SIZE >> + */ >> + public int getMaxActiveThreads() { >> + return (int) usersStats.getMin(); >> + } >> + >> + /** >> + * @return the mean number of active threads for this test run >> + * using a sliding window of SLIDING_WINDOW_SIZE >> + */ >> + public int getMeanActiveThreads() { >> + return (int) usersStats.getMean(); >> + } >> + >> + /** >> + * @return the min number of active threads for this test run >> + * using a sliding window of SLIDING_WINDOW_SIZE >> + */ >> + public int getMinActiveThreads() { >> + return (int) usersStats.getMax(); >> + } >> +} >> >> Propchange: jmeter/trunk/src/components/org/apache/jmeter/visualizers/ >> backend/UserMetric.java >> ------------------------------------------------------------ >> ------------------ >> svn:mime-type = text/plain >> >> Modified: jmeter/trunk/src/components/org/apache/jmeter/visualizers/ >> backend/graphite/GraphiteBackendListenerClient.java >> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/ >> org/apache/jmeter/visualizers/backend/graphite/ >> GraphiteBackendListenerClient.java?rev=1642603&r1=1642602& >> r2=1642603&view=diff >> ============================================================ >> ================== >> --- jmeter/trunk/src/components/org/apache/jmeter/visualizers/ >> backend/graphite/GraphiteBackendListenerClient.java (original) >> +++ jmeter/trunk/src/components/org/apache/jmeter/visualizers/ >> backend/graphite/GraphiteBackendListenerClient.java Sun Nov 30 20:29:49 >> 2014 >> @@ -18,6 +18,8 @@ >> package org.apache.jmeter.visualizers.backend.graphite; >> +import java.text.DecimalFormat; >> +import java.util.HashMap; >> import java.util.HashSet; >> import java.util.List; >> import java.util.Map; >> @@ -26,6 +28,7 @@ import java.util.concurrent.Executors; >> import java.util.concurrent.ScheduledExecutorService; >> import java.util.concurrent.TimeUnit; >> +import org.apache.commons.lang3.StringUtils; >> import org.apache.jmeter.config.Arguments; >> import org.apache.jmeter.samplers.SampleResult; >> import org.apache.jmeter.threads.JMeterContextService; >> @@ -48,7 +51,9 @@ public class GraphiteBackendListenerClie >> private static final Logger LOGGER = LoggingManager. >> getLoggerForClass(); >> private static final String DEFAULT_METRICS_PREFIX = "jmeter."; >> //$NON-NLS-1$ >> private static final String CUMULATED_METRICS = "__cumulated__"; >> //$NON-NLS-1$ >> - private static final String METRIC_ACTIVE_THREADS = "activeThreads"; >> //$NON-NLS-1$ >> + private static final String METRIC_MAX_ACTIVE_THREADS = >> "maxActiveThreads"; //$NON-NLS-1$ >> + private static final String METRIC_MIN_ACTIVE_THREADS = >> "minActiveThreads"; //$NON-NLS-1$ >> + private static final String METRIC_MEAN_ACTIVE_THREADS = >> "meanActiveThreads"; //$NON-NLS-1$ >> private static final String METRIC_STARTED_THREADS = >> "startedThreads"; //$NON-NLS-1$ >> private static final String METRIC_STOPPED_THREADS = >> "stoppedThreads"; //$NON-NLS-1$ >> private static final String METRIC_FAILED_REQUESTS = "failure"; >> //$NON-NLS-1$ >> @@ -56,10 +61,10 @@ public class GraphiteBackendListenerClie >> private static final String METRIC_TOTAL_REQUESTS = "total"; >> //$NON-NLS-1$ >> private static final String METRIC_MIN_RESPONSE_TIME = "min"; >> //$NON-NLS-1$ >> private static final String METRIC_MAX_RESPONSE_TIME = "max"; >> //$NON-NLS-1$ >> - private static final String METRIC_PERCENTILE90_RESPONSE_TIME = >> "percentile90"; //$NON-NLS-1$ >> - private static final String METRIC_PERCENTILE95_RESPONSE_TIME = >> "percentile95"; //$NON-NLS-1$ >> + private static final String METRIC_PERCENTILE_PREFIX = "percentile"; >> //$NON-NLS-1$ >> private static final long ONE_SECOND = 1L; >> private static final int MAX_POOL_SIZE = 1; >> + private static final String DEFAULT_PERCENTILES = "90;95;99"; >> > In the documentation below you state, that this property will be split > upon comma. I think the semicolon here is a better choice. Good catch, I think also samplelist should be split around semi colon also. > private String graphiteHost; >> private int graphitePort; >> @@ -67,6 +72,7 @@ public class GraphiteBackendListenerClie >> private String rootMetricsPrefix; >> private String samplersList = ""; //$NON-NLS-1$ >> private Set<String> samplersToFilter; >> + private HashMap<String, Float> percentiles; >> > I would just declare Map<String, Float> instead of the specific HashMap. ok > private GraphiteMetricsSender pickleMetricsManager; >> @@ -93,7 +99,9 @@ public class GraphiteBackendListenerClie >> } >> ThreadCounts tc = JMeterContextService. >> getThreadCounts(); >> - pickleMetricsManager.addMetric(timestamp, >> CUMULATED_CONTEXT_NAME, METRIC_ACTIVE_THREADS, Integer.toString(tc. >> activeThreads)); >> + pickleMetricsManager.addMetric(timestamp, >> CUMULATED_CONTEXT_NAME, METRIC_MIN_ACTIVE_THREADS, Integer.toString( >> getUserMetrics().getMaxActiveThreads())); >> + pickleMetricsManager.addMetric(timestamp, >> CUMULATED_CONTEXT_NAME, METRIC_MAX_ACTIVE_THREADS, Integer.toString( >> getUserMetrics().getMinActiveThreads())); >> + pickleMetricsManager.addMetric(timestamp, >> CUMULATED_CONTEXT_NAME, METRIC_MEAN_ACTIVE_THREADS, Integer.toString( >> getUserMetrics().getMeanActiveThreads())); >> pickleMetricsManager.addMetric(timestamp, >> CUMULATED_CONTEXT_NAME, METRIC_STARTED_THREADS, Integer.toString(tc. >> startedThreads)); >> pickleMetricsManager.addMetric(timestamp, >> CUMULATED_CONTEXT_NAME, METRIC_STOPPED_THREADS, Integer.toString(tc. >> finishedThreads)); >> @@ -107,14 +115,16 @@ public class GraphiteBackendListenerClie >> * @param metric >> */ >> private void addMetrics(long timestamp, String contextName, >> SamplerMetric metric) { >> - pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_FAILED_REQUESTS, Integer.toString(metric.getFailure())); >> - pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_SUCCESSFUL_REQUESTS, Integer.toString(metric.getSuccess())); >> + pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_FAILED_REQUESTS, Integer.toString(metric.getFailures())); >> + pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_SUCCESSFUL_REQUESTS, Integer.toString(metric.getSuccesses())); >> pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_TOTAL_REQUESTS, Integer.toString(metric.getTotal())); >> - pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_MIN_RESPONSE_TIME, Long.toString(metric.getMinTime())); >> - pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_MAX_RESPONSE_TIME, Long.toString(metric.getMaxTime())); >> - // TODO Make this customizable >> - pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_PERCENTILE90_RESPONSE_TIME, Double.toString(metric. >> getPercentile(90))); >> - pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_PERCENTILE95_RESPONSE_TIME, Double.toString(metric. >> getPercentile(95))); >> + pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_MIN_RESPONSE_TIME, Double.toString(metric.getMinTime())); >> + pickleMetricsManager.addMetric(timestamp, contextName, >> METRIC_MAX_RESPONSE_TIME, Double.toString(metric.getMaxTime())); >> + for (Map.Entry<String, Float> entry : percentiles.entrySet()) { >> + pickleMetricsManager.addMetric(timestamp, contextName, >> + entry.getKey(), >> + Double.toString(metric. >> getPercentile(entry.getValue().floatValue()))); >> + } >> } >> /** >> @@ -135,6 +145,7 @@ public class GraphiteBackendListenerClie >> public void handleSampleResults(List<SampleResult> sampleResults, >> BackendListenerContext context) { >> for (SampleResult sampleResult : sampleResults) { >> + getUserMetrics().add(sampleResult); >> if(!summaryOnly && samplersToFilter.contains( >> sampleResult.getSampleLabel())) { >> SamplerMetric samplerMetric = >> getSamplerMetric(sampleResult.getSampleLabel()); >> samplerMetric.add(sampleResult); >> @@ -153,6 +164,22 @@ public class GraphiteBackendListenerClie >> summaryOnly = context.getBooleanParameter("summaryOnly", true); >> samplersList = context.getParameter("samplersList", ""); >> rootMetricsPrefix = context.getParameter("rootMetricsPrefix", >> DEFAULT_METRICS_PREFIX); >> + String percentilesAsString = context.getParameter("percentiles", >> DEFAULT_METRICS_PREFIX); >> + String[] percentilesStringArray = percentilesAsString.split(";") >> ; >> + percentiles = new HashMap<String, Float>(percentilesStringArray. >> length); >> + DecimalFormat format = new DecimalFormat("0.##"); >> + for (int i = 0; i < percentilesStringArray.length; i++) { >> > Any reason for not using for(String percentile: > percentilesAsString.split(";")) { instead? Because at first I used an array then I changed to map and didn't update code :) > Below in the documentation you state, that the property will be split upon > a comma (","). > >> + if(!StringUtils.isEmpty(percentilesStringArray[i].trim())) { >> + try { >> + Float percentileValue = Float.parseFloat( >> percentilesStringArray[i].trim()); >> + percentiles.put( >> + METRIC_PERCENTILE_PREFIX+ >> AbstractGraphiteMetricsSender.sanitizeString(format.format( >> percentileValue)), >> + percentileValue); >> + } catch(Exception e) { >> + LOGGER.error("Error parsing percentile:'"+ >> percentilesStringArray[i]+"'"); >> > You could include the exception in the log message. yes > Do we want to catch any exception here, or just those that get thrown by > parseFloat? thrown by parseFloat > + } >> + } >> + } >> Class<?> clazz = Class.forName(graphiteMetricsSenderClass); >> this.pickleMetricsManager = (GraphiteMetricsSender) >> clazz.newInstance(); >> pickleMetricsManager.setup(graphiteHost, graphitePort, >> rootMetricsPrefix); >> @@ -189,6 +216,7 @@ public class GraphiteBackendListenerClie >> arguments.addArgument("rootMetricsPrefix", >> DEFAULT_METRICS_PREFIX); >> arguments.addArgument("summaryOnly", "true"); >> arguments.addArgument("samplersList", ""); >> + arguments.addArgument("percentiles", DEFAULT_PERCENTILES); >> return arguments; >> } >> } >> >> Modified: jmeter/trunk/xdocs/usermanual/component_reference.xml >> URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/ >> component_reference.xml?rev=1642603&r1=1642602&r2=1642603&view=diff >> ============================================================ >> ================== >> --- jmeter/trunk/xdocs/usermanual/component_reference.xml (original) >> +++ jmeter/trunk/xdocs/usermanual/component_reference.xml Sun Nov 30 >> 20:29:49 2014 >> @@ -3402,6 +3402,19 @@ By default, a Graphite implementation is >> <property name="Async Queue size" required="Yes">Size of the queue >> that holds the SampleResults while they are processed >> asynchronously.</property> >> <property name="Parameters" required="Yes">Parameters of the >> BackendListenerClient implementation.</property> >> </properties> >> + >> + >> + <p>The following parameters apply to the <b> >> GraphiteBackendListenerClient</b> implementation:</p> >> + >> + <properties> >> + <property name="graphiteMetricsSender" required="Yes">org.apache. >> jmeter.visualizers.backend.graphite.TextGraphiteMetricsSender or >> org.apache.jmeter.visualizers.backend.graphite. >> PickleGraphiteMetricsSender</property> >> + <property name="graphiteHost" required="Yes">Graphite or >> InfluxDB or CollectD server port</property> >> + <property name="graphitePort" required="Yes">Graphite or >> InfluxDB or CollectD server port, defaults to 2003. Note >> PickleGraphiteMetricsSender can only talk to Graphite server.</property> >> + <property name="rootMetricsPrefix" required="Yes">Prefix of >> metrics sent to backend. Defaults to ""jmeter."</property> >> + <property name="summaryOnly" required="Yes">Only send a summary >> with no detail. Defaults to true.</property> >> + <property name="samplersList" required="Yes">Comma separated >> list of samplers for which you want to report metrics to backend.</property> >> > You are using a semicolon to separate the property above. > > Regards > Felix > > + <property name="percentiles" required="Yes">The percentiles you >> want to send to backend. List must be semicolon separated.</property> >> + </properties> >> </component> >> <a href="#">^</a> >> >> >> > -- Cordialement. Philippe Mouawad.
