Bug 1388493 - Better handling of metrics errors

If the first request for metrics fails, show an empty state error message.
Otherwise show an alert if more than one consecutive request fails. The
alert has a retry link.

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

Author: spadgett

app/scripts/directives/deploymentMetrics.js

@@ -17,7 +17,8 @@ angular.module('openshiftConsole')
         // in case pods is empty.
         containers: '=',
         // Optional: set to 'compact' to show smaller charts (for the overview)
-        profile: '@'
+        profile: '@',
+        alerts: '=?'
       },
       templateUrl: function(elem, attrs) {
         if (attrs.profile === 'compact') {
@@ -93,6 +94,9 @@ angular.module('openshiftConsole')
         scope.loaded = false;
         scope.noData = true;
 
+        // Track the number of consecutive failures.
+        var failureCount = 0;
+
         // Get the URL to show in error messages.
         MetricsService.getMetricsURL().then(function(url) {
           scope.metricsURL = url;
@@ -311,7 +315,8 @@ angular.module('openshiftConsole')
             return;
           }
 
-          scope.loaded = true;
+          // Reset the number of failures on a successful request.
+          failureCount = 0;
 
           // Show an average instead of a multiline chart when there are many pods.
           scope.showAverage = _.size(scope.pods) > 5 || compact;
@@ -392,6 +397,50 @@ angular.module('openshiftConsole')
           return config;
         }
 
+        // If the first request for metrics fails, show an empty state error message.
+        // Otherwise show an alert if more than one consecutive request fails.
+        function metricsFailed(response) {
+          if (destroyed) {
+            return;
+          }
+
+          failureCount++;
+          if (scope.noData) {
+            // Show an empty state message if the first request for data fails.
+            scope.metricsError = {
+              status:  _.get(response, 'status', 0),
+              details: _.get(response, 'data.errorMsg') ||
+                       _.get(response, 'statusText') ||
+                       "Status code " + _.get(response, 'status', 0)
+            };
+            return;
+          }
+
+          // If this is the first failure and a previous request succeeded, wait and try again.
+          if (failureCount < 2) {
+            return;
+          }
+
+          // Show an alert if we've failed more than once.
+          // Use scope.$id in the alert ID so that it is unique on pages that
+          // use the directive multiple times like monitoring.
+          var alertID = 'metrics-failed-' + scope.uniqueID;
+          scope.alerts[alertID] = {
+            type: 'error',
+            message: 'An error occurred updating metrics.',
+            links: [{
+              href: '',
+              label: 'Retry',
+              onClick: function() {
+                delete scope.alerts[alertID];
+                // Reset failure count to 1 to trigger a retry.
+                failureCount = 1;
+                update();
+              }
+            }]
+          };
+        }
+
         // Make sure there are no errors or missing data before updating.
         function canUpdate() {
           var noPods = _.isEmpty(scope.pods);
@@ -400,7 +449,7 @@ angular.module('openshiftConsole')
             scope.loaded = true;
             return false;
           }
-          return !scope.metricsError;
+          return !scope.metricsError && failureCount < 2;
         }
 
         function updateData(metricType, podName, podData) {
@@ -419,23 +468,17 @@ angular.module('openshiftConsole')
           _.set(data, [metricType, podName], updated);
         }
 
-        function handleError(response) {
-          scope.loaded = true;
-          scope.metricsError = {
-            status:  _.get(response, 'status', 0),
-            details: _.get(response, 'data.errorMsg') ||
-                     _.get(response, 'statusText') ||
-                     "Status code " + _.get(response, 'status', 0)
-          };
-        }
-
         function update() {
           if (paused || !canUpdate()) {
             return;
           }
           lastUpdated = Date.now();
           var config = getConfig();
-          MetricsService.getPodMetrics(config).then(processData, handleError);
+          MetricsService.getPodMetrics(config).then(processData, metricsFailed).finally(function() {
+            // Even on errors mark metrics as loaded to replace the
+            // "Loading..." message with "No metrics to display."
+            scope.loaded = true;
+          });
         }
 
         // Updates immediately and then on options changes.

app/scripts/directives/podMetrics.js

@@ -16,7 +16,8 @@ angular.module('openshiftConsole')
         pod: '=',
         sparklineWidth: '=?',
         sparklineHeight: '=?',
-        includedMetrics: '=?' // defaults to ["cpu", "memory", "network"]
+        includedMetrics: '=?', // defaults to ["cpu", "memory", "network"]
+        alerts: '=?'
       },
       templateUrl: 'views/directives/pod-metrics.html',
       link: function(scope) {
@@ -27,6 +28,7 @@ angular.module('openshiftConsole')
         var getCPULimit = $parse('resources.limits.cpu');
 
         var updateInterval = 60 * 1000; // 60 seconds
+
         // Number of data points to display on the chart.
         var numDataPoints = 30;
 
@@ -382,9 +384,61 @@ angular.module('openshiftConsole')
           return null;
         }
 
+        // Track the number of consecutive failures.
+        var failureCount = 0;
+
+        function metricsSucceeded() {
+          // Reset the number of failures on a successful request.
+          failureCount = 0;
+        }
+
+        // If the first request for metrics fails, show an empty state error message.
+        // Otherwise show an alert if more than one consecutive request fails.
+        function metricsFailed(response) {
+          if (destroyed) {
+            return;
+          }
+
+          failureCount++;
+          if (scope.noData) {
+            // Show an empty state message if the first request for data fails.
+            scope.metricsError = {
+              status:  _.get(response, 'status', 0),
+              details: _.get(response, 'data.errorMsg') ||
+                       _.get(response, 'statusText') ||
+                       "Status code " + _.get(response, 'status', 0)
+            };
+            return;
+          }
+
+          // If this is the first failure and a previous request succeeded, wait and try again.
+          if (failureCount < 2) {
+            return;
+          }
+
+          // Show an alert if we've failed more than once.
+          // Use scope.$id in the alert ID so that it is unique on pages that
+          // use the directive multiple times like monitoring.
+          var alertID = 'metrics-failed-' + scope.uniqueID;
+          scope.alerts[alertID] = {
+            type: 'error',
+            message: 'An error occurred updating metrics for pod ' + _.get(scope, 'pod.metadata.name', '<unknown>') + '.',
+            links: [{
+              href: '',
+              label: 'Retry',
+              onClick: function() {
+                delete scope.alerts[alertID];
+                // Reset failure count to 1 to trigger a retry.
+                failureCount = 1;
+                update();
+              }
+            }]
+          };
+        }
+
         // Make sure there are no errors or missing data before updating.
         function canUpdate() {
-          if (scope.metricsError) {
+          if (scope.metricsError || failureCount > 1) {
             return false;
           }
 
@@ -419,8 +473,9 @@ angular.module('openshiftConsole')
           // time. This prevents an issue where the donut chart shows 0 for
           // current usage if the client clock is ahead of the server clock.
           var start = getStartTime();
+          var allPromises = [];
           angular.forEach(scope.metrics, function(metric) {
-            var promises = [];
+            var datasetPromises = [];
 
             // On metrics that require more than one set of data (e.g. network
             // incoming and outgoing traffic) we perform one request for each,
@@ -434,52 +489,40 @@ angular.module('openshiftConsole')
               if (!config) {
                 return;
               }
-              promises.push(MetricsService.get(config));
+              var promise = MetricsService.get(config);
+              datasetPromises.push(promise);
             });
 
+            allPromises = allPromises.concat(datasetPromises);
+
             // Collect all promises from every metric requested into one, so we
             // have all data the chart wants at the time of the chart creation
             // (or timeout updates, etc).
-            $q.all(promises).then(
-              // success
-              function(responses) {
-                if (destroyed) {
-                  return;
-                }
-
-                angular.forEach(responses, function(response) {
-                  if (!response) {
-                    return;
-                  }
+            $q.all(datasetPromises).then(function(responses) {
+              if (destroyed) {
+                return;
+              }
 
-                  var dataset = _.find(metric.datasets, {
-                    id: response.metricID
-                  });
-                  updateData(dataset, response);
-                });
-                updateChart(metric);
-              },
-              // failure
-              function(responses) {
-                if (destroyed) {
+              angular.forEach(responses, function(response) {
+                if (!response) {
                   return;
                 }
 
-                angular.forEach(responses, function(response) {
-                  scope.metricsError = {
-                    status:  _.get(response, 'status', 0),
-                    details: _.get(response, 'data.errorMsg') ||
-                             _.get(response, 'statusText') ||
-                             "Status code " + _.get(response, 'status', 0)
-                  };
+                var dataset = _.find(metric.datasets, {
+                  id: response.metricID
                 });
-              }
-            ).finally(function() {
-              // Even on errors mark metrics as loaded to replace the
-              // "Loading..." message with "No metrics to display."
-              scope.loaded = true;
+                updateData(dataset, response);
+              });
+              updateChart(metric);
             });
           });
+
+          // Handle failures when any request fails.
+          $q.all(allPromises).then(metricsSucceeded, metricsFailed).finally(function() {
+            // Even on errors mark metrics as loaded to replace the
+            // "Loading..." message with "No metrics to display."
+            scope.loaded = true;
+          });
         }
 
         // Updates immediately and then on options changes.

app/views/browse/pod.html

@@ -76,7 +76,8 @@ <h3>Container {{container.name}} Environment Variables</h3>
                          we don't update in the background. -->
                     <pod-metrics
                       ng-if="selectedTab.metrics"
-                      pod="pod">
+                      pod="pod"
+                      alerts="alerts">
                     </pod-metrics>
                   </uib-tab>
 

app/views/browse/replica-set.html

@@ -124,7 +124,8 @@ <h3>Container {{container.name}} Environment Variables</h3>
                          we don't update in the background. -->
                     <deployment-metrics
                         ng-if="selectedTab.metrics && podsForDeployment"
-                        pods="podsForDeployment" containers="replicaSet.spec.template.spec.containers">
+                        pods="podsForDeployment" containers="replicaSet.spec.template.spec.containers"
+                        alerts="alerts">
                     </deployment-metrics>
                   </uib-tab>
                   <uib-tab ng-if="deploymentConfigName && logOptions.version && ('deploymentconfigs/log' | canI : 'get')" active="selectedTab.logs">

app/views/monitoring.html

@@ -207,7 +207,8 @@ <h2>Deployments</h2>
                       <div class="mar-top-lg" ng-if="metricsAvailable">
                         <deployment-metrics
                             pods="podsByOwnerUID[replicationController.metadata.uid]"
-                            containers="replicationController.spec.template.spec.containers">
+                            containers="replicationController.spec.template.spec.containers"
+                            alerts="alerts">
                         </deployment-metrics>
                       </div>
                     </div>
@@ -257,7 +258,8 @@ <h2>Deployments</h2>
                       <div class="mar-top-lg" ng-if="metricsAvailable">
                         <deployment-metrics
                             pods="podsByOwnerUID[replicaSet.metadata.uid]"
-                            containers="replicaSet.spec.template.spec.containers">
+                            containers="replicaSet.spec.template.spec.containers"
+                            alerts="alerts">
                         </deployment-metrics>
                       </div>
                     </div>
@@ -351,7 +353,7 @@ <h2>Pods</h2>
                       </log-viewer>
                       <!-- Until the metrics directive pulls the metrics from the time range of the life of the pod, hide the metrics for old stuff -->
                       <div class="mar-top-lg" ng-if="metricsAvailable">
-                        <pod-metrics pod="pod"></pod-metrics>
+                        <pod-metrics pod="pod" alerts="alerts"></pod-metrics>
                       </div>
                     </div>
                   </div>