Merge pull request #4833 from hashicorp/b-ui-gracefully-handle-stat-e…

…rrors

UI: Gracefully handle stat errors

ui/app/services/system.js

@@ -1,13 +1,9 @@
 import Service, { inject as service } from '@ember/service';
 import { computed } from '@ember/object';
-import { copy } from '@ember/object/internals';
 import PromiseObject from '../utils/classes/promise-object';
 import PromiseArray from '../utils/classes/promise-array';
 import { namespace } from '../adapters/application';
-
-// When the request isn't ok (e.g., forbidden) handle gracefully
-const jsonWithDefault = defaultResponse => res =>
-  res.ok ? res.json() : copy(defaultResponse, true);
+import jsonWithDefault from '../utils/json-with-default';
 
 export default Service.extend({
   token: service(),

ui/app/utils/classes/abstract-stats-tracker.js

@@ -1,12 +1,21 @@
+import Ember from 'ember';
 import Mixin from '@ember/object/mixin';
 import { assert } from '@ember/debug';
 import { task, timeout } from 'ember-concurrency';
+import jsonWithDefault from 'nomad-ui/utils/json-with-default';
 
 export default Mixin.create({
   url: '',
 
+  // The max number of data points tracked. Once the max is reached,
+  // data points at the head of the list are removed in favor of new
+  // data appended at the tail
   bufferSize: 500,
 
+  // The number of consecutive request failures that can occur before an
+  // empty frame is appended
+  maxFrameMisses: 5,
+
   fetch() {
     assert('StatsTrackers need a fetch method, which should have an interface like window.fetch');
   },
@@ -23,6 +32,25 @@ export default Mixin.create({
     );
   },
 
+  frameMisses: 0,
+
+  handleResponse(frame) {
+    if (frame.error) {
+      this.incrementProperty('frameMisses');
+      if (this.get('frameMisses') >= this.get('maxFrameMisses')) {
+        // Missing enough data consecutively is effectively a pause
+        this.pause();
+        this.set('frameMisses', 0);
+      }
+      return;
+    } else {
+      this.set('frameMisses', 0);
+
+      // Only append non-error frames
+      this.append(frame);
+    }
+  },
+
   // Uses EC as a form of debounce to prevent multiple
   // references to the same tracker from flooding the tracker,
   // but also avoiding the issue where different places where the
@@ -36,18 +64,18 @@ export default Mixin.create({
       assert('Url must be defined', url);
 
       yield this.get('fetch')(url)
-        .then(res => res.json())
-        .then(frame => this.append(frame));
+        .then(jsonWithDefault({ error: true }))
+        .then(frame => this.handleResponse(frame));
     } catch (error) {
       throw new Error(error);
     }
 
-    yield timeout(2000);
+    yield timeout(Ember.testing ? 0 : 2000);
   }).drop(),
 
   signalPause: task(function*() {
     // wait 2 seconds
-    yield timeout(2000);
+    yield timeout(Ember.testing ? 0 : 2000);
     // if no poll called in 2 seconds, pause
     this.pause();
   }).drop(),

ui/app/utils/json-with-default.js

@@ -0,0 +1,10 @@
+import { copy } from '@ember/object/internals';
+
+// Used with fetch.
+// Fetch only goes into the promise catch if there is a network error.
+// This means that handling a 4xx or 5xx error is the responsibility
+// of the developer.
+const jsonWithDefault = defaultResponse => res =>
+  res.ok ? res.json() : copy(defaultResponse, true);
+
+export default jsonWithDefault;

ui/mirage/config.js

@@ -307,7 +307,17 @@ export default function() {
   this.get('/client/fs/logs/:allocation_id', clientAllocationLog);
 
   this.get('/client/stats', function({ clientStats }, { queryParams }) {
-    return this.serialize(clientStats.find(queryParams.node_id));
+    const seed = Math.random();
+    if (seed > 0.8) {
+      const stats = clientStats.find(queryParams.node_id);
+      stats.update({
+        timestamp: Date.now() * 1000000,
+        CPUTicksConsumed: stats.CPUTicksConsumed + (Math.random() * 20 - 10),
+      });
+      return this.serialize(stats);
+    } else {
+      return new Response(500, {}, null);
+    }
   });
 
   // TODO: in the future, this hack may be replaceable with dynamic host name

ui/mirage/factories/client-stats.js

@@ -23,7 +23,7 @@ export default Factory.extend({
       })),
   ],
 
-  CPUTicksConsumed: 1000000,
+  CPUTicksConsumed: faker.random.number({ min: 100, max: 1000 }),
 
   diskStats: () => [
     Array(faker.random.number({ min: 1, max: 5 }))
@@ -46,6 +46,6 @@ export default Factory.extend({
     Used: 10000000000,
   }),
 
-  timestamp: 149000000000,
+  timestamp: Date.now() * 1000000,
   uptime: 193838,
 });

ui/mirage/factories/node.js

@@ -1,6 +1,6 @@
 import { Factory, faker, trait } from 'ember-cli-mirage';
 import { provide } from '../utils';
-import { DATACENTERS, HOSTS } from '../common';
+import { DATACENTERS, HOSTS, generateResources } from '../common';
 import moment from 'moment';
 
 const UUIDS = provide(100, faker.random.uuid.bind(faker.random));
@@ -65,6 +65,8 @@ export default Factory.extend({
 
   drivers: makeDrivers,
 
+  resources: generateResources,
+
   attributes() {
     // TODO add variability to these
     return {

ui/tests/unit/adapters/node-test.js

@@ -12,6 +12,8 @@ moduleForAdapter('node', 'Unit | Adapter | Node', {
     'model:node-driver',
     'model:node-event',
     'model:evaluation',
+    'model:resources',
+    'model:network',
     'model:job',
     'serializer:application',
     'serializer:node',
@@ -21,6 +23,7 @@ moduleForAdapter('node', 'Unit | Adapter | Node', {
     'service:watchList',
     'transform:fragment',
     'transform:fragment-array',
+    'transform:array',
   ],
   beforeEach() {
     this.server = startMirage();

ui/tests/unit/utils/allocation-stats-tracker-test.js

@@ -6,6 +6,7 @@ import sinon from 'sinon';
 import Pretender from 'pretender';
 import AllocationStatsTracker, { stats } from 'nomad-ui/utils/classes/allocation-stats-tracker';
 import fetch from 'nomad-ui/utils/fetch';
+import statsTrackerFrameMissingBehavior from './behaviors/stats-tracker-frame-missing';
 
 module('Unit | Util | AllocationStatsTracker');
 
@@ -453,3 +454,27 @@ test('changing the value of the allocationProp constructs a new AllocationStatsT
     'Changing the value of alloc results in creating a new AllocationStatsTracker instance'
   );
 });
+
+statsTrackerFrameMissingBehavior({
+  resourceName: 'allocation',
+  ResourceConstructor: MockAllocation,
+  TrackerConstructor: AllocationStatsTracker,
+  mockFrame,
+  compileResources(frame) {
+    const timestamp = makeDate(frame.Timestamp);
+    const cpu = frame.ResourceUsage.CpuStats.TotalTicks;
+    const memory = frame.ResourceUsage.MemoryStats.RSS;
+    return [
+      {
+        timestamp,
+        used: cpu,
+        percent: cpu / 200,
+      },
+      {
+        timestamp,
+        used: memory,
+        percent: memory / 1024 / 1024 / 512,
+      },
+    ];
+  },
+});

ui/tests/unit/utils/behaviors/stats-tracker-frame-missing.js

@@ -0,0 +1,97 @@
+import { resolve } from 'rsvp';
+import wait from 'ember-test-helpers/wait';
+import { test } from 'ember-qunit';
+import sinon from 'sinon';
+
+const MockResponse = json => ({
+  ok: true,
+  json() {
+    return resolve(json);
+  },
+});
+
+export default function statsTrackerFrameMissing({
+  resourceName,
+  TrackerConstructor,
+  ResourceConstructor,
+  mockFrame,
+  compileResources,
+}) {
+  test('a bad response from a fetch request is handled gracefully', function(assert) {
+    const frame = mockFrame(1);
+    const [compiledCPU, compiledMemory] = compileResources(frame);
+
+    let shouldFail = false;
+    const fetch = () => {
+      return resolve(shouldFail ? { ok: false } : new MockResponse(frame));
+    };
+
+    const resource = ResourceConstructor();
+    const tracker = TrackerConstructor.create({ fetch, [resourceName]: resource });
+
+    tracker.get('poll').perform();
+
+    return wait()
+      .then(() => {
+        assert.deepEqual(tracker.get('cpu'), [compiledCPU], 'One frame of cpu');
+        assert.deepEqual(tracker.get('memory'), [compiledMemory], 'One frame of memory');
+
+        shouldFail = true;
+        tracker.get('poll').perform();
+        return wait();
+      })
+      .then(() => {
+        assert.deepEqual(tracker.get('cpu'), [compiledCPU], 'Still one frame of cpu');
+        assert.deepEqual(tracker.get('memory'), [compiledMemory], 'Still one frame of memory');
+        assert.equal(tracker.get('frameMisses'), 1, 'Frame miss is tracked');
+
+        shouldFail = false;
+        tracker.get('poll').perform();
+        return wait();
+      })
+      .then(() => {
+        assert.deepEqual(tracker.get('cpu'), [compiledCPU, compiledCPU], 'Still one frame of cpu');
+        assert.deepEqual(
+          tracker.get('memory'),
+          [compiledMemory, compiledMemory],
+          'Still one frame of memory'
+        );
+        assert.equal(tracker.get('frameMisses'), 0, 'Frame misses is reset');
+      });
+  });
+
+  test('enough bad responses from fetch consecutively (as set by maxFrameMisses) results in a pause', function(assert) {
+    const fetch = () => {
+      return resolve({ ok: false });
+    };
+
+    const resource = ResourceConstructor();
+    const tracker = TrackerConstructor.create({
+      fetch,
+      [resourceName]: resource,
+      maxFrameMisses: 3,
+      pause: sinon.spy(),
+    });
+
+    tracker.get('poll').perform();
+    return wait()
+      .then(() => {
+        assert.equal(tracker.get('frameMisses'), 1, 'Tick misses');
+        assert.notOk(tracker.pause.called, 'Pause not called yet');
+
+        tracker.get('poll').perform();
+        return wait();
+      })
+      .then(() => {
+        assert.equal(tracker.get('frameMisses'), 2, 'Tick misses');
+        assert.notOk(tracker.pause.called, 'Pause still not called yet');
+
+        tracker.get('poll').perform();
+        return wait();
+      })
+      .then(() => {
+        assert.equal(tracker.get('frameMisses'), 0, 'Misses reset');
+        assert.ok(tracker.pause.called, 'Pause called now that frameMisses == maxFrameMisses');
+      });
+  });
+}

ui/tests/unit/utils/node-stats-tracker-test.js

@@ -6,6 +6,7 @@ import sinon from 'sinon';
 import Pretender from 'pretender';
 import NodeStatsTracker, { stats } from 'nomad-ui/utils/classes/node-stats-tracker';
 import fetch from 'nomad-ui/utils/fetch';
+import statsTrackerFrameMissingBehavior from './behaviors/stats-tracker-frame-missing';
 
 module('Unit | Util | NodeStatsTracker');
 
@@ -221,3 +222,25 @@ test('changing the value of the nodeProp constructs a new NodeStatsTracker', fun
     'Changing the value of the node results in creating a new NodeStatsTracker instance'
   );
 });
+
+statsTrackerFrameMissingBehavior({
+  resourceName: 'node',
+  ResourceConstructor: MockNode,
+  TrackerConstructor: NodeStatsTracker,
+  mockFrame,
+  compileResources(frame) {
+    const timestamp = makeDate(frame.Timestamp);
+    return [
+      {
+        timestamp,
+        used: frame.CPUTicksConsumed,
+        percent: frame.CPUTicksConsumed / 2000,
+      },
+      {
+        timestamp,
+        used: frame.Memory.Used,
+        percent: frame.Memory.Used / 1024 / 1024 / 4096,
+      },
+    ];
+  },
+});