Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Container dashboard utilization metrics - fix hourly metrics #5232

Merged
merged 4 commits into from
Feb 11, 2019
Merged

Container dashboard utilization metrics - fix hourly metrics #5232

merged 4 commits into from
Feb 11, 2019

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Feb 8, 2019

Compute > Containers > Providers > pick a provider, dashboard view

good


When there is daily utilization data, there is no bug.

When no daily data is available, we get console errors:

TypeError: Cannot set property 'tooltipFn' of undefined
in app/assets/javascripts/controllers/ems_container_dashboard/utilization_trend_chart_controller.js:70

TypeError: Cannot read property 'layout' of undefined
in app/assets/javascripts/components/pf_charts/trends-chart.component.js:27

When no hourly data is available, the fallback to realtime or empty (... || realtime_utilization || empty_utilization_trend_data) would never get called, because hourly_utilization always returned data.


=> made sure hourly returns no data when empty, allowing the realtime fallback
=> fix the JS error by setting the tooltipFn on the right metricsDataStruct object.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1667986

@himdel
Copy link
Contributor Author

himdel commented Feb 8, 2019

Testing:

(In app/services/container_dashboard_service.rb, line ~237)

   def ems_utilization
-    daily_utilization || hourly_utilization || realtime_utilization || empty_utilization_trend_data
+    {"interval_name":"hourly","xy_data":{"cpu":{"used":1,"total":20,"xData":["2019-01-29T00:00:00.000Z","2019-01-29T01:00:00.000Z","2019-01-29T02:00:00.000Z","2019-01-29T03:00:00.000Z","2019-01-29T04:00:00.000Z","2019-01-29T05:00:00.000Z","2019-01-29T06:00:00.000Z","2019-01-29T07:00:00.000Z","2019-01-29T08:00:00.000Z","2019-01-29T09:00:00.000Z","2019-01-29T10:00:00.000Z","2019-01-29T11:00:00.000Z","2019-01-29T12:00:00.000Z","2019-01-29T13:00:00.000Z","2019-01-29T14:00:00.000Z"],"yData":[1.21,1.21,1.21,1.24,1.2,1.21,1.21,1.21,1.22,1.21,1.21,1.21,1.2,1.21,1.2]},"memory":{"used":17,"total":70,"xData":["2019-01-29T00:00:00.000Z","2019-01-29T01:00:00.000Z","2019-01-29T02:00:00.000Z","2019-01-29T03:00:00.000Z","2019-01-29T04:00:00.000Z","2019-01-29T05:00:00.000Z","2019-01-29T06:00:00.000Z","2019-01-29T07:00:00.000Z","2019-01-29T08:00:00.000Z","2019-01-29T09:00:00.000Z","2019-01-29T10:00:00.000Z","2019-01-29T11:00:00.000Z","2019-01-29T12:00:00.000Z","2019-01-29T13:00:00.000Z","2019-01-29T14:00:00.000Z"],"yData":[17.2,17.08,17.08,17.01,17.15,17.02,17.18,17.1,17.23,17.12,17.29,17.18,17.17,17.0,17.18]}}}
   end
 
   def network_metrics

Or, if you have data, try removing daily_utilization, then daily & hourly , etc. until only empty remains. None should give errors.

(If you don't have data, #519 (comment) might help.)

…realtime

`vm.cpuUsageSparklineConfig` was replaced by `chartsMixin.cpuUsageSparklineConfig` (and same for memory),
which the existing code is already using to create the `metricsDataStruct` object

So just changing the tooltip in the existing structure, when needed.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1667986
@himdel
Copy link
Contributor Author

himdel commented Feb 8, 2019

gaprindashvili/no because #2499 is also gaprindashvili/no

@himdel
Copy link
Contributor Author

himdel commented Feb 8, 2019

@miq-bot add_label wip

still TODO

daily:

angular.js:14961 TypeError: Cannot read property 'chartId' of undefined
    at lineChartController.vm.updateAll (line-chart.component.self-cbdb72da01a9e802cd650e3c3210738799a6bfabc36f49b924fe55dc53dbf9ee.js?body=1:28)
    at lineChartController.vm.$onChanges (line-chart.component.self-cbdb72da01a9e802cd650e3c3210738799a6bfabc36f49b924fe55dc53dbf9ee.js?body=1:108)
    at angular.js:10022
    at forEach (angular.js:426)
    at nodeLinkFn (angular.js:10018)
    at angular.js:10410
    at processQueue (angular.js:17330)
    at angular.js:17378
    at Scope.$digest (angular.js:18515)
    at Scope.$apply (angular.js:18903)

EDIT: fixed 👇

@miq-bot miq-bot changed the title Container dashboard utilization metrics - fix hourly metrics [WIP] Container dashboard utilization metrics - fix hourly metrics Feb 8, 2019
@miq-bot miq-bot added the wip label Feb 8, 2019
The pfLineChart component does deal with the possibility that `vm.chartData.dataAvailable` is false, and shows an empty chart instead.

However, before getting there, it fails, because there is no `vm.config` object when no data is available.

All of updateAll only deals with the situation where there is data available, so just bailing early when it isn't.
@himdel
Copy link
Contributor Author

himdel commented Feb 8, 2019

@miq-bot remove_label wip

When no data is available, we get {data: {dataAvailable: false}} instead of {data: ..., config: ...}.
Which means the pf-line-chart component got passed undefined config, and failed even before checking that dataAvailable is false and doing nothing.

@himdel himdel removed the wip label Feb 8, 2019
@himdel himdel changed the title [WIP] Container dashboard utilization metrics - fix hourly metrics Container dashboard utilization metrics - fix hourly metrics Feb 8, 2019
@mzazrivec mzazrivec self-assigned this Feb 11, 2019
@mzazrivec mzazrivec added this to the Sprint 105 Ending Feb 18, 2019 milestone Feb 11, 2019
@mzazrivec mzazrivec merged commit 2e05444 into ManageIQ:master Feb 11, 2019
@himdel himdel deleted the container-dashboard-bz1667986 branch February 11, 2019 08:56
simaishi pushed a commit that referenced this pull request Feb 11, 2019
Container dashboard utilization metrics - fix hourly metrics

(cherry picked from commit 2e05444)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1674585
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 44db7ff6240ee0abf3e89526ad3728bc3794868a
Author: Milan Zázrivec <[email protected]>
Date:   Mon Feb 11 09:30:07 2019 +0100

    Merge pull request #5232 from himdel/container-dashboard-bz1667986
    
    Container dashboard utilization metrics - fix hourly metrics
    
    (cherry picked from commit 2e054446e05694ddb51484e13a3bc60e1d6d95b9)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1674585

@himdel
Copy link
Contributor Author

himdel commented Mar 1, 2019

Also fixes https://bugzilla.redhat.com/show_bug.cgi?id=1684226

EDIT: nope, there's #5310 for that one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants