jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maxime Chassagneux <mchassagn...@apache.org>
Subject Re: svn commit: r1798048 - in /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend: ErrorMetric.java SamplerMetric.java influxdb/InfluxdbBackendListenerClient.java
Date Fri, 09 Jun 2017 10:41:20 GMT
2017-06-08 21:40 GMT+02:00 Felix Schumacher <
felix.schumacher@internetallee.de>:

> 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/b
>> ackend/ErrorMetric.java
>> Modified:
>>      jmeter/trunk/src/components/org/apache/jmeter/visualizers/b
>> ackend/SamplerMetric.java
>>      jmeter/trunk/src/components/org/apache/jmeter/visualizers/b
>> ackend/influxdb/InfluxdbBackendListenerClient.java
>>
>> Added: jmeter/trunk/src/components/org/apache/jmeter/visualizers/ba
>> ckend/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 :)


Done


>
> +    }
>> +
>> +    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.getResponseCo
>> de())
>> +                && getResponseMessage().equalsIgn
>> oreCase(otherError.getResponseMessage())) {
>> +            return true;
>> +        } else {
>> +            return false;
>> +        }
>>
>
> Return the value of the expression directly instead of if (expr) { return
> true; } else { return false; }


Done

>


> +    }
>> +
>> +    @Override
>> +    public int hashCode() {
>> +        return getResponseCode().hashCode() +
>> getResponseMessage().hashCode();
>> +    }
>> +
>> +}
>>
>> Modified: jmeter/trunk/src/components/org/apache/jmeter/visualizers/ba
>> ckend/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.DescriptiveStatist
>> ics;
>> @@ -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.


Done

>
>
> +
>>             /**
>>        *
>> @@ -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.


Done

>
>
> +        return errors;
>> +    }
>>   }
>>
>> Modified: jmeter/trunk/src/components/org/apache/jmeter/visualizers/ba
>> ckend/influxdb/InfluxdbBackendListenerClient.java
>> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/org
>> /apache/jmeter/visualizers/backend/influxdb/InfluxdbBackendL
>> istenerClient.java?rev=1798048&r1=1798047&r2=1798048&view=diff
>> ============================================================
>> ==================
>> --- jmeter/trunk/src/components/org/apache/jmeter/visualizers/ba
>> ckend/influxdb/InfluxdbBackendListenerClient.java (original)
>> +++ jmeter/trunk/src/components/org/apache/jmeter/visualizers/ba
>> ckend/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?


It's typo error ( fr vs en name )
The problem is since JMeter 3.2 was released we can't know how many run was
affected by this typo.
Just for me, it's about 40Go of influxdb data, about 600 tests.
If you change the name of a tag here, you create a new one and keep the old
one.
And you have to change all yours queries to check two tags
For me it's better to wait that influxdb implement a rename tag function.


>
>
>       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(AbstractInfluxdbMetrics
>> Sender.tagToStringValue(responseCode));
>> +            tag.append(TAG_RESPONSE_MESSAGE).append(AbstractInfluxdbMetr
>> icsSender.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.
>

Done too

>
> Regards,
>
>  Felix
>
>
Thanks for all your remark !

Max

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message