Bug 58987 - Report/Dashboard: Improve error reporting
Summary: Report/Dashboard: Improve error reporting
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: 2.13
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-09 19:47 UTC by Sebb
Modified: 2016-02-27 09:20 UTC (History)
1 user (show)



Attachments
This parses OK (375 bytes, text/plain)
2016-02-09 20:01 UTC, Sebb
Details
This does not (369 bytes, text/plain)
2016-02-09 20:01 UTC, Sebb
Details
Sample log output (12.33 KB, text/plain)
2016-02-09 20:03 UTC, Sebb
Details
Copy of ok.jtl with last column not a number (375 bytes, text/plain)
2016-02-13 14:23 UTC, Sebb
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sebb 2016-02-09 19:47:20 UTC
I have two very similar-looking CSV files.
One is processed OK, the other generates lots of stack traces.

The difference is that one is missing a column, however this is extremely hard to determine this from the log file.

There are several issues here:

The error message should explain what the code was trying to do in terms of the field name(s) it is having problems with. The stack trace is not really helpful and should be dropped. It would probably help to print the error message(s) to standard out as well as the log file.

The sample number is shown as #0. It's not clear if that is supposed to be zero-based or if the count is not being incremented at all.

Since the reporter relies on being able to parse the CSV, and the CSV file format is not fixed, it seems to me that the solution is to insist on having the field names in the file. There's no way of knowing whether the CSV file corresponds with the current property settings - the most that one can hope to check is whether the number of columns in the CSV agrees with the settings.
The code does not even do that.
Comment 1 Philippe Mouawad 2016-02-09 19:50:00 UTC
Did you run your test on last nightly build ?
Can you provide a sample and the config of user.properties used in your test ?

Thanks
Comment 2 Sebb 2016-02-09 20:01:33 UTC
Created attachment 33540 [details]
This parses OK
Comment 3 Sebb 2016-02-09 20:01:58 UTC
Created attachment 33541 [details]
This does not
Comment 4 Sebb 2016-02-09 20:03:29 UTC
Created attachment 33542 [details]
Sample log output
Comment 5 Sebb 2016-02-09 20:04:25 UTC
I used current trunk freshly built. No changes to jmeter.properties
Comment 6 Philippe Mouawad 2016-02-09 20:07:43 UTC
Did you face the issue with a CSV generated with current default settings ?
Thanks
Comment 7 Sebb 2016-02-09 20:13:08 UTC
(In reply to Philippe Mouawad from comment #6)
> Did you face the issue with a CSV generated with current default settings ?
> Thanks

No, but that's not the issue.

The problem is that generating a report for an existing CSV relies on it having been created with the same properties. That is not guaranteed, especially if the defaults are changed. And users can change the values. And if the CSV was created by a GUI listener, it may not have the same settings.
Comment 8 Philippe Mouawad 2016-02-09 20:22:30 UTC
(In reply to Sebb from comment #7)
> (In reply to Philippe Mouawad from comment #6)
> > Did you face the issue with a CSV generated with current default settings ?
> > Thanks
> 
> No, but that's not the issue.
> 
> The problem is that generating a report for an existing CSV relies on it
> having been created with the same properties. That is not guaranteed,
> especially if the defaults are changed. And users can change the values. And
> if the CSV was created by a GUI listener, it may not have the same settings.

This would be nice to fix indeed but be aware that it is for example a limitation for example of JMeter-Plugins and I wonder if all current listeners in JMeter do not have this issue.
Comment 9 Sebb 2016-02-09 20:27:05 UTC
(In reply to Philippe Mouawad from comment #8)
> (In reply to Sebb from comment #7)
> > (In reply to Philippe Mouawad from comment #6)
> > > Did you face the issue with a CSV generated with current default settings ?
> > > Thanks
> > 
> > No, but that's not the issue.
> > 
> > The problem is that generating a report for an existing CSV relies on it
> > having been created with the same properties. That is not guaranteed,
> > especially if the defaults are changed. And users can change the values. And
> > if the CSV was created by a GUI listener, it may not have the same settings.
> 
> This would be nice to fix indeed but be aware that it is for example a
> limitation for example of JMeter-Plugins and I wonder if all current
> listeners in JMeter do not have this issue.

No idea what you mean. What limitation are you referring to?
Comment 10 Philippe Mouawad 2016-02-09 20:31:59 UTC
With jmeter-plugins, if you try to generate graphs from a CSV file generated with different jmeter properties, it will also fail.
You need to have the same properties that led to the CSV file.
Comment 11 Sebb 2016-02-09 22:44:54 UTC
(In reply to Philippe Mouawad from comment #10)
> With jmeter-plugins, if you try to generate graphs from a CSV file generated
> with different jmeter properties, it will also fail.
> You need to have the same properties that led to the CSV file.

I see. This is the same with JMeter's Listeners.

But that does not mean that nothing can or should be done to improve the behaviour of report dashboard. It's particularly important for this listener as it is new, and so users will want to apply it to existing CSV files.

So we should:
- log/print a warning if there are no field names
- if no field names, then check that the number fields agrees with the properties, and quit if there is a mismatch
- if there is a parsing error, report the record, field number, and expected field name to enable the problem to be debugged.


Some of these improvements can also be applied to the other JMeter listeners.
Comment 12 Philippe Mouawad 2016-02-10 22:04:53 UTC
Author: pmouawad
Date: Wed Feb 10 22:04:30 2016
New Revision: 1729747

URL: http://svn.apache.org/viewvc?rev=1729747&view=rev
Log:
Bug 58987 - Report/Dashboard: Improve error reporting
Bugzilla Id: 58987

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/report/core/CsvSampleReader.java
    jmeter/trunk/src/core/org/apache/jmeter/report/core/Sample.java
    jmeter/trunk/src/core/org/apache/jmeter/report/processor/FieldSampleComparator.java
    jmeter/trunk/xdocs/changes.xml
Comment 13 Philippe Mouawad 2016-02-10 22:05:08 UTC
@sebb, is it OK for you ?
Comment 14 Sebb 2016-02-11 00:05:16 UTC
The line number and field name are now reported. 
However the column number is not reported, and there are two sets of stack traces about the same error.

I would expect to see something like:

2016/02/10 23:53:55 ERROR - jmeter.report.dashboard.ReportGenerator: Error in sample at line: 1 converting field <n>: Latency to:long, fieldValue: ''

*without* the other 100 or so lines of stack trace.

Also, the code should check whether the column count is correct.
There's no point trying to parse the data if the count is wrong.
Comment 15 Philippe Mouawad 2016-02-13 13:54:23 UTC
(In reply to Sebb from comment #14)
> The line number and field name are now reported. 
> However the column number is not reported,
Done
 and there are two sets of stack
> traces about the same error.
Fixed
> 
> I would expect to see something like:
> 
> 2016/02/10 23:53:55 ERROR - jmeter.report.dashboard.ReportGenerator: Error
> in sample at line: 1 converting field <n>: Latency to:long, fieldValue: ''
> 
> *without* the other 100 or so lines of stack trace.
I think stacktrace is important for bug reporting.
> 
> Also, the code should check whether the column count is correct.
> There's no point trying to parse the data if the count is wrong.
Fixed
Comment 16 Philippe Mouawad 2016-02-13 13:54:33 UTC
Author: pmouawad
Date: Sat Feb 13 13:43:44 2016
New Revision: 1730205

URL: http://svn.apache.org/viewvc?rev=1730205&view=rev
Log:
Bug 58987 - Report/Dashboard: Improve error reporting
Avoid Log+Rethrow, only rethrow
Add column index in message
Bugzilla Id: 58987

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/JMeter.java
    jmeter/trunk/src/core/org/apache/jmeter/report/core/Sample.java
    jmeter/trunk/src/core/org/apache/jmeter/report/dashboard/ReportGenerator.java
    jmeter/trunk/src/core/org/apache/jmeter/report/processor/AbstractSampleConsumer.java
    jmeter/trunk/src/core/org/apache/jmeter/report/processor/CsvFileSampleSource.java
    
Author: pmouawad
Date: Sat Feb 13 13:53:46 2016
New Revision: 1730206

URL: http://svn.apache.org/viewvc?rev=1730206&view=rev
Log:
Bug 58987 - Report/Dashboard: Improve error reporting
Throw exception if mismatch between expected number of columns and columns in csv file
Bugzilla Id: 58987

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/report/core/CsvSampleReader.java
Comment 17 Sebb 2016-02-13 14:20:55 UTC
The file bad.jtl is handled fine.

However, If I run the following test:

./jmeter -n -g ok.jtl -J jmeter.save.saveservice.assertion_results_failure_message=false

I get the following console message:

An error occurred: Error while processing samples:Consumer failed with message :Consumer failed with message :Consumer failed with message :Error in sample at line:1 converting field:bytes at column:8 to:int, fieldValue:''

This is confusing.
Also I would expect to see a column mismatch error, not a parsing error.

Also the jmeter.log file is filled with unnecessary stack traces.

There is no point in providing stack traces here; it's likely to confuse users.
We just need to ensure that the error message is clear.
Comment 18 Sebb 2016-02-13 14:23:50 UTC
Created attachment 33550 [details]
Copy of ok.jtl with last column not a number

This should generate a parse error. 
It does, but the message is tricky to read because of all the unnecessary text:

An error occurred: Error while processing samples:Consumer failed with message :Consumer failed with message :Consumer failed with message :Consumer failed with message :Error in sample at line:1 converting field:Latency at column:12 to:long, fieldValue:'O'

Also there is no need for a stack trace.
Comment 19 Sebb 2016-02-13 17:22:45 UTC
URL: http://svn.apache.org/viewvc?rev=1730253&view=rev
Log:
Report/Dashboard: Improve error reporting
Throw exception if mismatch between expected number of columns and columns in csv file (don't allow missing columns)
Bugzilla Id: 58987

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/report/core/CsvSampleReader.java

URL: http://svn.apache.org/viewvc?rev=1730255&view=rev
Log:
Revert r1730205 as it results in duplicate information in the log file
Bugzilla Id: 58987

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/JMeter.java
Comment 20 Philippe Mouawad 2016-02-27 09:20:08 UTC
Improved also with commits:
http://svn.apache.org/viewvc?rev=1731954&view=rev
http://svn.apache.org/viewvc?rev=1731945&view=rev
http://svn.apache.org/viewvc?rev=1731944&view=rev

Closing. 
Feel free to reopen if not enough