jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Schumacher <felix.schumac...@internetallee.de>
Subject Re: svn commit: r1642603 - in /jmeter/trunk: src/components/org/apache/jmeter/visualizers/backend/ src/components/org/apache/jmeter/visualizers/backend/graphite/ xdocs/usermanual/
Date Sun, 30 Nov 2014 21:09:33 GMT
Hi Philippe,

a few comments inline.

Am 30.11.2014 um 21:29 schrieb pmouawad@apache.org:
> 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?
> +    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.
>   
>       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.
>       
>   
>       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?
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.
Do we want to catch any exception here, or just those that get 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>
>
>


Mime
View raw message