jmeter-commits 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: r1798048 - in /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend: ErrorMetric.java SamplerMetric.java influxdb/InfluxdbBackendListenerClient.java
Date Thu, 08 Jun 2017 19:40:06 GMT
Am 08.06.2017 um 13:53 schrieb mchassagneux@apache.org:
> Author: mchassagneux
> Date: Thu Jun  8 11:53:42 2017
> New Revision: 1798048
>
> URL: http://svn.apache.org/viewvc?rev=1798048&view=rev
> Log:
> InfluxdbBackendListener : add number of errors by response code and message for each
transaction
> Bugzilla Id: 61167
>
> Added:
>      jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/ErrorMetric.java
> Modified:
>      jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/SamplerMetric.java
>      jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/influxdb/InfluxdbBackendListenerClient.java
>
> Added: jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/ErrorMetric.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/ErrorMetric.java?rev=1798048&view=auto
> ==============================================================================
> --- jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/ErrorMetric.java
(added)
> +++ jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/ErrorMetric.java
Thu Jun  8 11:53:42 2017
> @@ -0,0 +1,82 @@
> +/*
> + * 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.jmeter.samplers.SampleResult;
> +
> +/**
> + * Object representing an error by a response code and response message
> + * @since 3.3
> + */
> +public class ErrorMetric {
> +
> +    private String responseCode = "";
> +    private String responseMessage = "";
> +
> +    public ErrorMetric() {
Place a comment inside to specify, why this is empty.
This and the other comments below are also reported by sonar :)
> +    }
> +
> +    public ErrorMetric(SampleResult result) {
> +        responseCode = result.getResponseCode();
> +        responseMessage = result.getResponseMessage();
> +    }
> +
> +    /**
> +     * @return the response code , '0' if he code is empty
> +     */
> +    public String getResponseCode() {
> +        if (responseCode.isEmpty()) {
> +            return "0";
> +        } else {
> +            return responseCode;
> +        }
> +    }
> +
> +    /**
> +     * @return the response message , 'none' if he code is empty
> +     */
> +    public String getResponseMessage() {
> +        if (responseMessage.isEmpty()) {
> +            return "None";
> +        } else {
> +            return responseMessage;
> +        }
> +    }
> +
> +    @Override
> +    public boolean equals(Object other) {
> +        if (!(other instanceof ErrorMetric)) {
> +            return false;
> +        }
> +
> +        ErrorMetric otherError = (ErrorMetric) other;
> +        if (getResponseCode().equalsIgnoreCase(otherError.getResponseCode())
> +                && getResponseMessage().equalsIgnoreCase(otherError.getResponseMessage()))
{
> +            return true;
> +        } else {
> +            return false;
> +        }

Return the value of the expression directly instead of if (expr) { 
return true; } else { return false; }

> +    }
> +
> +    @Override
> +    public int hashCode() {
> +        return getResponseCode().hashCode() + getResponseMessage().hashCode();
> +    }
> +
> +}
>
> 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=1798048&r1=1798047&r2=1798048&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
Thu Jun  8 11:53:42 2017
> @@ -21,6 +21,7 @@ package org.apache.jmeter.visualizers.ba
>   import java.util.ArrayList;
>   import java.util.Arrays;
>   import java.util.Collections;
> +import java.util.HashMap;
>   import java.util.List;
>   
>   import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
> @@ -61,6 +62,8 @@ public class SamplerMetric {
>       private int successes;
>       private int failures;
>       private int hits;
> +    private HashMap<ErrorMetric, Integer> errors = new HashMap<ErrorMetric,
Integer>();

Use the generalized Interface Map rather than specific implementation 
HashMap for instance variables.
Use the diamond "<>" operator on the right side instead of re-specifying 
the types.

> +
>       
>       /**
>        *
> @@ -95,7 +98,9 @@ public class SamplerMetric {
>               successes+=result.getSampleCount()-result.getErrorCount();
>           } else {
>               failures+=result.getErrorCount();
> -        }
> +            ErrorMetric error = new ErrorMetric(result);
> +            errors.put(error, errors.getOrDefault(error, 0) + result.getErrorCount()
);
> +        }
>           long time = result.getTime();
>           allResponsesStats.addValue(time);
>           pctResponseStats.addValue(time);
> @@ -140,6 +145,7 @@ public class SamplerMetric {
>           default:
>               // This cannot happen
>           }
> +        errors.clear();
>           successes = 0;
>           failures = 0;
>           hits = 0;
> @@ -302,4 +308,12 @@ public class SamplerMetric {
>       public int getHits() {
>           return hits;
>       }
> +
> +    /**
> +     * Returns details of errors occurs
> +     * @return errors
> +     */
> +    public HashMap<ErrorMetric, Integer> getErrors() {
public interfaces should return general interfaces rather than specific 
implementations. A Map would probably be better in this case.

> +        return errors;
> +    }
>   }
>
> Modified: jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/influxdb/InfluxdbBackendListenerClient.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/influxdb/InfluxdbBackendListenerClient.java?rev=1798048&r1=1798047&r2=1798048&view=diff
> ==============================================================================
> --- jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/influxdb/InfluxdbBackendListenerClient.java
(original)
> +++ jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/influxdb/InfluxdbBackendListenerClient.java
Thu Jun  8 11:53:42 2017
> @@ -62,8 +62,11 @@ public class InfluxdbBackendListenerClie
>   
>       private static final String TAG_TRANSACTION = ",transaction=";
>   
> -    private static final String TAG_STATUS = ",status=";
> +    // As influxdb can't rename tag for now, keep the old name for backward compatibility
> +    private static final String TAG_STATUS = ",statut=";

Is this a JMeter mistake? If so, are there so many data sets already, 
that we can't change it anymore?

>       private static final String TAG_APPLICATION = ",application=";
> +    private static final String TAG_RESPONSE_CODE = ",responseCode=";
> +    private static final String TAG_RESPONSE_MESSAGE = ",responseMessage=";
>   
>       private static final String METRIC_COUNT = "count=";
>       private static final String METRIC_COUNT_ERROR = "countError=";
> @@ -172,8 +175,24 @@ public class InfluxdbBackendListenerClie
>           // FOR KO STATUS
>           addMetric(transaction, metric.getFailures(), true, TAG_KO, metric.getKoMean(),
metric.getKoMinTime(),
>                   metric.getKoMaxTime(), koPercentiles.values(), metric::getKoPercentile);
> +
> +        metric.getErrors().forEach((error, count) -> addErrorMetric(transaction,
error.getResponseCode(),
> +                    error.getResponseMessage(), count));
>       }
>   
> +    private void addErrorMetric(String transaction, String responseCode, String responseMessage,
long count) {
> +        if (count > 0) {
> +            StringBuilder tag = new StringBuilder(70);
> +            tag.append(TAG_APPLICATION).append(application);
> +            tag.append(TAG_TRANSACTION).append(transaction);
> +            tag.append(TAG_RESPONSE_CODE).append(AbstractInfluxdbMetricsSender.tagToStringValue(responseCode));
> +            tag.append(TAG_RESPONSE_MESSAGE).append(AbstractInfluxdbMetricsSender.tagToStringValue(responseMessage));
> +
> +            StringBuilder field = new StringBuilder(30);
> +            field.append(METRIC_COUNT).append(count);

I asked about the usage of StringBuilder vs String just a few days ago 
in another thread. Do you really think this is more readable than 
METRIC_COUNT + count? Same for the longer builder usage for tag above
> +            influxdbMetricsManager.addMetric(measurement, tag.toString(), field.toString());
> +        }
> +    }
>   
>       private void addMetric(String transaction, int count, boolean includeResponseCode,
>               String statut, double mean, double minTime, double maxTime,
>
>

I am missing test classes for ErrorMetric and if possible the newly 
added code in the other classes.

Regards,

  Felix


Mime
View raw message