Initial implementation of numeric report charts.

[martinpovolny]

Replacement for #3528 w/o use of "Consolidation"

Work towards numeric report charts: https://bugzilla.redhat.com/show_bug.cgi?id=1213294

[martinpovolny]

Average number of CPUs per provider

[martinpovolny]

Number of VMs per host

[dclarizio]

@martinpovolny This is looking pretty good . . . I did a report on disk space and noticed that maybe we could use the same formatting for the legend markers as we have in the report, as seen below:

Report sort criteria

Report chart options

Note the legends with all the zeroes and no commas

Report shows them as humanized, to MB, GB, TB, etc

Same in column view with vertical axis legends

This may be a jqPlots issue, but something to keep in mind for the future C3 charting is that with the flash charts, we could hover over a bar or chart item or node on a line chart and it would show a popup with the actual value. Would be a nice feature to keep.

Pie charts for values look good and they have hover text over the pie slices, but same note about formatting of the values shown

Pie chart using counts didn't fare as well, chart is blank.

Column chart based on counts results in a line chart (strange)

[dclarizio]

@martinpovolny Lots of feedback above, let's discuss where you're at tomorrow morning. Thx, Dan

[martinpovolny]

@dclarizio :

1. fixed the missing chart and the line chart -- seems chart type information was missing in the json
2. added a spec for 2 main cases of the count-based charts -- this shoul cover the case above
3. fixed some problem in the report editor when switching chart types

[dclarizio]

@martinpovolny more testing . . .

Summary tab set to counts with no aggregations set, Chart tab should not allow values. Currently lets you select values, but the data column has a null choice. If no aggregations are being done, it should show Counts as a string with no choice and, of course, not show the column drop down. Here are the screenshots:

Running the report with counts, not sure the output is correct. I can see some break lines in the report output and 1 of them lines up with the legend and has a value of 3 showing. The others, I think the name is too long so I can't see the count in the legend??? The chart shows percentages (which is probably ok), but hovering the mouse over the pie slices doesn't tell me which provider it is or the count, there is no popup. Basically, I can't tell if the chart matches the report.

[dclarizio]

@martinpovolny Simple test, ran the "Vendor and Guest OS" report with the stacked bar chart (so 3d charts by count) and the charts look identical, so it seems those are working as before. Thought I would post some good news as well. :)

Before:

After:

[dclarizio]

@martinpovolny More good news, testing of column charts with values. Only issue here is from before, Y axis labels need formatting. Would be nice to have hover over values, but we can wait for C3.

Chart configuration:

Chart/Report:

[dclarizio]

@martinpovolny Same issues as above for bar charts (X axis labels crazy) . . . they look pretty good tho:

[martinpovolny]

@dclarizio : Thinking about the use of MiqReport::Formatting for the formatting of the axis labels. The problem is that the ticks for numeric values are calculated by the javascript library. The same most likely holds for the flash charts.

For one we don't know the values for the ticks when calculation the chart. Then depending on type of chart we might have a problem with associating labels of the axis with the values in the chart. In jqplot you can have a formatter class for the ticks or you can use a special axis renderer for a couple of typical axis such as Date and Cathegory.

Seems to me that the proper way would be providing formatters based on column types similarly to what we have in db/fixtures/miq_report_formats.yml -- bytes, mHz, percent etc. No idea how to do this for the flash charts. Also might be work investigating what we will do if we use the C3 charts instead and do in in a general way. Definitely even though the labels are pretty ugly (I can rotate them at least to fit better) I would say that we need a separate issue and some research for this.

@himdel : do you know that ways one can use in the C3 to format axis values?

[himdel]

@martinpovolny sure, I think this should be useable :)

[martinpovolny]

@himdel : http://www.jqplot.com/docs/files/plugins/jqplot-canvasAxisTickRenderer-js.html#$.jqplot.CanvasAxisTickRenderer.formatter

so we need

1. some javascript code that would take understand how to format according to the db/fixtures/miq_report_formats.yml formats and
2. then we just pass in the format of the column that is being projected on the axis along with the other options as we generate the charts

@dclarizio : I'd probably create an issue for that

at least is seems that we can use most of the work we do on the charts for both jqplot and C3

[martinpovolny]

@himdel : do you know about some nice js library we could use? https://github.com/adamwdraper/Numeral-js maybe?

[himdel]

Uh, looking at the formats, angular actually has most of these as filters but it'd be rather hacky to use here I guess... I've never heard of Numeral.js but looks like it could be useful for the numeric formats. We already have moment.js so we can use that for dates/times, and possibly something like sprintf-js (any version) to cover the rest?

I can look more into it tomorrow .. sounds like simply reimplementing most of miq_report/formatting.rb in JS, right?

[martinpovolny]

@himdel : seems to me to be the best way to reimplement that in JS. In the reports we have way for the user to specify his/her format so we might actually end really reimplementing it so that we have the same format specification on both sides.

[martinpovolny]

@himdel : if you can look into it, I'd be glad. I can make the charts pass in the formating information for the right column, but If you could help with the javascript implementation of the formating function, it would be great help for me. Your experience with variety of javascript libraries would be usefull here ;-)

[himdel]

@martinpovolny sure, no problem :) I'm assuming we want to keep the same function names mentioned in :function :name, I'll put it under .. say, ManageIQ.charts.formatters or something like that, ok?

[himdel]

Support for strftime-like date format specifiers in format_date will depend on this moment-strftime PR. Is there any chance we may need other format specifiers not mentioned in the yaml?

[dclarizio]

@martinpovolny Minor detail in pie popup, not sure why it shows 2 values with : separator. Maybe legend has the value in the text, then it is adding the actual value again after the text in the popup.

[martinpovolny]

@dclarizio : thanks for detailed testing!

Now I:

1. fixed the field labels in editor
2. unified the formating of counts and value based 2dim charts

[dclarizio]

@martinpovolny looks like a relevant spec test failure on vmdb.

[dclarizio]

@martinpovolny Reran my saved chart report (with 2 series and non-stacked chart selected) with the new code, got this error again when chart is rendered for display (same as above with productization):

• Notes: this also occurs with stacked charts and no error when set to Count
  

[----] I, [2015-09-03T15:03:15.888138 #5574:ad9990]  INFO -- : Completed 200 OK in 500ms (Views: 0.3ms | ActiveRecord: 16.4ms)
[----] I, [2015-09-03T15:03:16.048822 #5574:ad9990]  INFO -- : Started GET "/report/render_chart?height=250&rand=558489395&width=350" for 127.0.0.1 at 2015-09-03 15:03:16 -0700
[----] I, [2015-09-03T15:03:16.109741 #5574:ad9990]  INFO -- : Processing by 
ReportController#render_chart as JSON
[----] I, [2015-09-03T15:03:16.109938 #5574:ad9990]  INFO -- :   Parameters: {"height"=>"250", "rand"=>"558489395", "width"=>"350"}
[----] F, [2015-09-03T15:03:16.274265 #5574:ad9990] FATAL -- : Error caught: [ArgumentError] comparison of String with nil failed
/home/dclarizio/dev/manageiq/lib/report_formatter/chart_common.rb:448:in `sort'
/home/dclarizio/dev/manageiq/lib/report_formatter/chart_common.rb:448:in `build_numeric_chart_grouped_2dim'
/home/dclarizio/dev/manageiq/lib/report_formatter/jqplot.rb:127:in `build_numeric_chart_grouped_2dim'
/home/dclarizio/dev/manageiq/lib/report_formatter/chart_common.rb:560:in `build_reporting_chart_numeric'

[dclarizio]

@martinpovolny Easy fix, remove (2D) from chart type names since there are no 3D charts:

[martinpovolny]

@dclarizio : on "Reran my saved chart report (with 2 series and non-stacked chart selected) with the new code, got this error again when chart is rendered for display (same as above with productization):"

Can you, please, either create the report again from scratch or send me also a screenshot of the "Summary page". I cannot reproduce the error.

Seems to me you must have only one sort criteria in the Summary (which the editor does not allow any more) -- so you have an invalid combination or I don't know. Just cannot create the situation on my box.

[dclarizio]

@martinpovolny Still seeing the exception when rendering the values chart, screenshots below. Also IRC'd you a link to my DB.

[martinpovolny]

@dclarizio : please, retest with the last patch. I have also modified the spec to fail w/o the change.

[miq-bot]

Checked commits martinpovolny/manageiq@1900db9~...97ceb99 with ruby 1.9.3, rubocop 0.33.0, and haml-lint 0.13.0
14 files checked, 86 offenses detected

app/controllers/report_controller/reports/editor.rb

• 🔴 Warn - Line 744, Col 35 - Lint/AssignmentInCondition - Assignment in condition - you probably meant to use ==.
• 🔶 - Line 156, Col 15 - Style/CommentIndentation - Incorrect indentation detected (column 14 instead of 6).
• 🔶 - Line 159, Col 34 - Style/SpaceAfterComma - Space missing after comma.
• 🔶 - Line 176, Col 15 - Style/CommentIndentation - Incorrect indentation detected (column 14 instead of 6).
• 🔶 - Line 179, Col 15 - Style/CommentIndentation - Incorrect indentation detected (column 14 instead of 6).
• 🔶 - Line 180, Col 26 - Style/SpaceAroundOperators - Operator || should be surrounded with a single space.
• 🔶 - Line 181, Col 11 - Style/MultilineOperationIndentation - Align the operands of a condition in an if statement spanning multiple lines.
• 🔶 - Line 181, Col 11 - Style/WordArray - Use %w or %W for array of words.
• 🔶 - Line 184, Col 9 - Style/NegatedIf - Favor unless over if for negative conditions.
• 🔶 - Line 193, Col 37 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 194, Col 7 - Style/NegatedIf - Favor unless over if for negative conditions.
• 🔶 - Line 195, Col 87 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 197, Col 56 - Style/ColonMethodCall - Do not use :: for method calls.
• 🔶 - Line 198, Col 56 - Style/ColonMethodCall - Do not use :: for method calls.
• 🔶 - Line 199, Col 65 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 201, Col 31 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 205, Col 13 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 221, Col 20 - Style/EmptyLiteral - Use array literal [] instead of Array.new.
• 🔶 - Line 229, Col 47 - Rails/TimeZone - Do not use Time.now without zone. Use one of Time.zone.now, Time.now.current, Time.now.in_time_zone, Time.now.utc, Time.now.getlocal, Time.now.iso8601, Time.now.jisx0301, Time.now.rfc3339, Time.now.to_i, Time.now.to_f instead.
• 🔶 - Line 231, Col 47 - Rails/TimeZone - Do not use Time.now without zone. Use one of Time.zone.now, Time.now.current, Time.now.in_time_zone, Time.now.utc, Time.now.getlocal, Time.now.iso8601, Time.now.jisx0301, Time.now.rfc3339, Time.now.to_i, Time.now.to_f instead.
• 🔶 - Line 236, Col 73 - Style/SpaceAroundOperators - Surrounding space missing for operator *.
• 🔶 - Line 248, Col 15 - Style/CommentIndentation - Incorrect indentation detected (column 14 instead of 6).
• 🔶 - Line 248, Col 15 - Style/LeadingCommentSpace - Missing space after #.
• 🔶 - Line 668, Col 36 - Style/SpaceAroundOperators - Operator ||= should be surrounded with a single space.
• 🔶 - Line 669, Col 36 - Style/SpaceAroundOperators - Operator ||= should be surrounded with a single space.
• 🔶 - Line 1131, Col 9 - Style/NegatedIf - Favor unless over if for negative conditions.
• 🔹 - Line 156, Col 121 - Metrics/LineLength - Line is too long. [137/120]
• 🔹 - Line 176, Col 121 - Metrics/LineLength - Line is too long. [141/120]
• 🔹 - Line 188, Col 121 - Metrics/LineLength - Line is too long. [121/120]
• 🔹 - Line 199, Col 121 - Metrics/LineLength - Line is too long. [122/120]

app/helpers/report_helper.rb

• 🔹 - Line 29, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for chart_fields_options is too high. [30.33/15]

app/models/miq_report/generator/aggregation.rb

• 🔶 - Line 6, Col 32 - Style/RedundantSelf - Redundant self detected.
• 🔶 - Line 6, Col 46 - Style/RedundantSelf - Redundant self detected.
• 🔶 - Line 7, Col 5 - Style/RedundantSelf - Redundant self detected.
• 🔶 - Line 7, Col 30 - Style/RedundantSelf - Redundant self detected.
• 🔶 - Line 7, Col 54 - Style/RedundantSelf - Redundant self detected.
• 🔶 - Line 7, Col 81 - Style/RedundantSelf - Redundant self detected.
• 🔹 - Line 2, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for build_subtotals is too high. [20.64/15]

app/views/report/_form_chart.html.haml

• 🔴 Warn - Line 23, Col - - Style/ExtraSpacing: Unnecessary spacing detected.
• 🔴 Warn - Line 23, Col - - Style/ExtraSpacing: Unnecessary spacing detected.
• 🔴 Warn - Line 23, Col - - Style/ExtraSpacing: Unnecessary spacing detected.
• 🔴 Warn - Line 23, Col - - Style/ExtraSpacing: Unnecessary spacing detected.
• 🔴 Warn - Line 38, Col - - Style/ExtraSpacing: Unnecessary spacing detected.
• 🔴 Warn - Line 38, Col - - Style/ExtraSpacing: Unnecessary spacing detected.
• 🔴 Warn - Line 38, Col - - Style/ExtraSpacing: Unnecessary spacing detected.
• 🔴 Warn - Line 38, Col - - Style/ExtraSpacing: Unnecessary spacing detected.

lib/charting/jqplot_charting.rb

• 🔶 - Line 43, Col 5 - Style/WordArray - Use %w or %W for array of words.
• 🔶 - Line 45, Col 5 - Style/WordArray - Use %w or %W for array of words.
• 🔶 - Line 47, Col 5 - Style/WordArray - Use %w or %W for array of words.
• 🔶 - Line 48, Col 5 - Style/WordArray - Use %w or %W for array of words.

lib/report_formatter/chart_common.rb

• 🔶 - Line 19, Col 11 - Style/MultilineOperationIndentation - Align the operands of a condition in an if statement spanning multiple lines.
• 🔹 - Line 141, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for build_performance_chart_pie is too high. [37.44/15]
• 🔹 - Line 168, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for build_planning_chart is too high. [72.78/15]
• 🔹 - Line 168, Col 5 - Metrics/CyclomaticComplexity - Cyclomatic complexity for build_planning_chart is too high. [8/6]
• 🔹 - Line 168, Col 5 - Metrics/MethodLength - Method has too many lines. [32/25]
• 🔹 - Line 168, Col 5 - Metrics/PerceivedComplexity - Perceived complexity for build_planning_chart is too high. [10/7]
• 🔹 - Line 366, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for build_numeric_chart_simple is too high. [46.84/15]
• 🔹 - Line 396, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for build_numeric_chart_grouped is too high. [53.72/15]
• 🔹 - Line 396, Col 5 - Metrics/MethodLength - Method has too many lines. [26/25]
• 🔹 - Line 434, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for build_numeric_chart_grouped_2dim is too high. [91.17/15]
• 🔹 - Line 434, Col 5 - Metrics/CyclomaticComplexity - Cyclomatic complexity for build_numeric_chart_grouped_2dim is too high. [11/6]
• 🔹 - Line 434, Col 5 - Metrics/MethodLength - Method has too many lines. [46/25]
• 🔹 - Line 434, Col 5 - Metrics/PerceivedComplexity - Perceived complexity for build_numeric_chart_grouped_2dim is too high. [11/7]

lib/report_formatter/jqplot.rb

• 🔶 - Line 137, Col 11 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• 🔶 - Line 137, Col 16 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 143, Col 11 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• 🔶 - Line 143, Col 23 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 152, Col 11 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• 🔶 - Line 152, Col 16 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔶 - Line 158, Col 11 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• 🔶 - Line 158, Col 23 - Style/ExtraSpacing - Unnecessary spacing detected.
• 🔹 - Line 85, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for axis_category_labels_ticks is too high. [25.2/15]
• 🔹 - Line 90, Col 121 - Metrics/LineLength - Line is too long. [124/120]
• 🔹 - Line 131, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for dim2_formating is too high. [26.31/15]
• 🔹 - Line 131, Col 5 - Metrics/MethodLength - Method has too many lines. [33/25]

spec/helpers/report_helper_spec.rb

• 🔶 - Line 9, Col 107 - Style/SpaceAroundOperators - Surrounding space missing for operator =>.
• 🔶 - Line 9, Col 119 - Style/SpaceAroundOperators - Surrounding space missing for operator =>.
• 🔶 - Line 11, Col 119 - Style/SpaceAroundOperators - Surrounding space missing for operator =>.
• 🔶 - Line 11, Col 144 - Style/SpaceAroundOperators - Surrounding space missing for operator =>.
• 🔹 - Line 9, Col 121 - Metrics/LineLength - Line is too long. [184/120]
• 🔹 - Line 11, Col 121 - Metrics/LineLength - Line is too long. [212/120]
• 🔹 - Line 23, Col 121 - Metrics/LineLength - Line is too long. [140/120]

spec/lib/report_formater/jqplot_formater_spec.rb

• 🔹 - Line 20, Col 121 - Metrics/LineLength - Line is too long. [126/120]
• 🔹 - Line 69, Col 121 - Metrics/LineLength - Line is too long. [123/120]
• 🔹 - Line 113, Col 121 - Metrics/LineLength - Line is too long. [130/120]
• 🔹 - Line 115, Col 121 - Metrics/LineLength - Line is too long. [135/120]

[dclarizio]

@martinpovolny Exception is gone upstream. When productized, I get the following when rendering the chart with values (counts renders fine). Should be able to reproduce this with the DB I sent you.

[----] I, [2015-09-04T13:21:45.238407 #7786:64f988]  INFO -- : Started GET "/report/render_chart?height=250&rand=144678747&width=350" for 127.0.0.1 at 2015-09-04 13:21:45 -0700
[----] I, [2015-09-04T13:21:45.294838 #7786:64f988]  INFO -- : Processing by ReportController#render_chart as HTML
[----] I, [2015-09-04T13:21:45.295038 #7786:64f988]  INFO -- :   Parameters: {"height"=>"250", "rand"=>"144678747", "width"=>"350"}
[----] F, [2015-09-04T13:21:45.453925 #7786:64f988] FATAL -- : Error caught: [NoMethodError] undefined method `match' for nil:NilClass
/home/dclarizio/dev/manageiq/lib/report_formatter/chart_common.rb:437:in `build_numeric_chart_grouped_2dim'
/home/dclarizio/dev/manageiq/lib/report_formatter/chart_common.rb:560:in `build_reporting_chart_numeric'
/home/dclarizio/dev/manageiq/lib/report_formatter/chart_common.rb:62:in `call'
/home/dclarizio/dev/manageiq/lib/report_formatter/chart_common.rb:62:in `build_document_body'

[martinpovolny]

@dclarizio : the above error seems as if you where loading a broken report from DB. That's a failure in parsing the saved chart column from the report definition.

Also I was not able to reproduce this even when I tried your database, opened "Dan's Chart Tests" and then tried to edit your report (changing Value based to Counts based and more).

Tried switching to product charts (should have no effect as the stacktrace leads to common code) and no change.

Clicked on all reports that have "Dan" in the name too...

[dclarizio]

@martinpovolny OK, so I created a chart report from scratch when productized and it worked just fine. I also copied the suspect chart and when I went to the summary screen, non of the aggregations were defined. Defined those and the charts worked there as well. Merging this now.

[chessbyte]

Should highligh be spelled highlight?