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.

Sign up for GitHub

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

[server/status] implement generic status tracking #7459

Merged
merged 7 commits into from
Jun 15, 2016
Merged

[server/status] implement generic status tracking #7459

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e6a8da0
[server/status] implement generic status tracking
Jun 15, 2016
ef9eba5
[status] update functional tests to use testSubject selector
Jun 15, 2016
fe67508
[server/status] cleanup services table on status page
Jun 15, 2016
8cb1694
[statusPage] remove "service" naming
Jun 15, 2016
10be384
[statusPage] remove status grouping, fix some classnames
Jun 15, 2016
1504a9e
[server/status] remove PluginStatus class
Jun 15, 2016
e232e07
[statusPage] update functional test
Jun 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 29 additions & 9 deletions src/plugins/status_page/public/status_page.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,47 @@ <h1>
</div>
</div>

<div class="row plugin_status_wrapper">
<h3>Installed Plugins</h3>
<div class="row status_statuses_wrapper">
<h3>Status Breakdown</h3>

<div ng-if="!ui.statuses && ui.loading" class="loading_statuses">
<span class="spinner"></span>
</div>

<h4 ng-if="!ui.statuses && !ui.loading" class="missing_statuses">
No plugin status information available
No status information available
</h4>

<table class="plugin_status_breakdown" ng-if="ui.statuses">
<table class="status_statuses_breakdown" ng-if="ui.statuses.generic">
<tr class="row">
<th class="col-xs-2">Name</th>
<th class="col-xs-4">ID</th>
<th class="col-xs-8">Status</th>
</tr>
<tr
ng-repeat="status in ui.statuses.generic"
class="status_row status_state_default status_state_{{status.state}} row">

<td class="col-xs-4 status_name">{{status.id}}</td>
<td class="col-xs-8 status_message">
<i class="fa status_state_color status_state_icon" />
{{status.message}}
</td>
</tr>
</table>
<table class="status_statuses_breakdown" data-test-subj="pluginStatusBreakdown" ng-if="ui.statuses.plugins">
<tr class="row">
<th class="col-xs-2">Plugin</th>
<th class="col-xs-2">Version</th>
<th class="col-xs-8">Status</th>
</tr>
<tr ng-repeat="status in ui.statuses" class="status_row plugin_state_default plugin_state_{{status.state}} row">
<td class="col-xs-2 status_name">{{status.name}}</td>
<td class="col-xs-2 status_version">{{status.version}}</td>
<tr
ng-repeat="status in ui.statuses.plugins"
class="status_row status_state_default status_state_{{status.state}} row">

<td class="col-xs-2 status_name">{{status.plugin.id}}</td>
<td class="col-xs-2 status_version">{{status.plugin.version}}</td>
<td class="col-xs-8 status_message">
<i class="fa plugin_state_color plugin_state_icon" />
<i class="fa status_state_color status_state_icon" />
{{status.message}}
</td>
</tr>
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/status_page/public/status_page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ const chrome = require('ui/chrome')

const data = resp.data;
ui.metrics = data.metrics;
ui.statuses = data.status.statuses;
ui.name = data.name;

ui.statuses = _.groupBy(data.status.statuses, s => s.plugin ? 'plugins' : 'generic');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check for s.plugin here seems to break abstraction. What do you think of the Status class having an isPluginStatus method that abstracts away this check, to with similar methods like createForPlugin and getForPluginId and then using that method here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be up for this, but this logic is all front-end, where it's just processing a list of JSON objects

Copy link
Contributor

@ycombinator ycombinator Jun 15, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is moot now, based on the discussion in https://github.com/elastic/kibana/pull/7459/files#r67094458

if (!_.size(ui.statuses)) ui.statuses = null;

const overall = data.status.overall;
if (!ui.serverState || (ui.serverState !== overall.state)) {
ui.serverState = overall.state;
Expand Down
47 changes: 24 additions & 23 deletions src/plugins/status_page/public/status_page.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
@status-metric-border: #aaa;
@status-metric-title-color: #666;

@status-plugins-bg: #fff;
@status-plugins-border: #bbb;
@status-plugins-headings-color: #666;
@status-statuses-bg: #fff;
@status-statuses-border: #bbb;
@status-statuses-headings-color: #666;

@status-default: #7c7c7c;
@status-green: #94c63d;
Expand Down Expand Up @@ -58,13 +58,13 @@
}
}

// plugin status table section
.plugin_status_wrapper {
// status status table section
.status_statuses_wrapper {
margin-top: 25px;
margin-left: -5px;
margin-right: -5px;
border-top:2px solid;
background-color: @status-plugins-bg;
background-color: @status-statuses-bg;
padding: 10px;

h3 {
Expand All @@ -78,24 +78,25 @@
text-align: center;
}

.plugin_status_breakdown {
.status_statuses_breakdown {
margin-left: 0;
margin-right: 0;
margin-bottom: 30px;

.status_row {
height:30px;
line-height:30px;
border-bottom:1px solid;
border-bottom-color: @status-plugins-border;
border-bottom-color: @status-statuses-border;
}

th {
color:@status-plugins-headings-color;
color:@status-statuses-headings-color;
font-weight: normal;
height:25px;
line-height:25px;
border-bottom:1px solid;
border-bottom-color: @status-plugins-border;
border-bottom-color: @status-statuses-border;
}

.status_name {
Expand All @@ -111,13 +112,13 @@
}
}

//plugin state
.plugin_state(@color, @icon) {
.plugin_state_color {
//status state
.status_state(@color, @icon) {
.status_state_color {
color: @color;
}

.plugin_state_icon:before {
.status_state_icon:before {
content: @icon;
}

Expand All @@ -130,20 +131,20 @@
}
}

.plugin_state_default {
.plugin_state(@status-default, @icon-default);
.status_state_default {
.status_state(@status-default, @icon-default);
}

.plugin_state_green {
.plugin_state(@status-green, @icon-green);
.status_state_green {
.status_state(@status-green, @icon-green);
}

.plugin_state_yellow {
.plugin_state(@status-yellow, @icon-yellow);
.status_state_yellow {
.status_state(@status-yellow, @icon-yellow);
}

.plugin_state_red {
.plugin_state(@status-red, @icon-red);
.status_state_red {
.status_state(@status-red, @icon-red);
}

//server state
Expand All @@ -156,7 +157,7 @@
content: @icon;
}

.plugin_status_wrapper {
.status_statuses_wrapper {
border-top-color: @color;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/plugins/plugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ module.exports = class Plugin {
server.exposeStaticDir(`/plugins/${id}/{path*}`, this.publicDir);
}

this.status = kbnServer.status.create(this);
this.status = kbnServer.status.createForPlugin(this);
server.expose('status', this.status);

return await attempt(this.externalInit, [server, options], this);
Expand Down
85 changes: 58 additions & 27 deletions src/server/status/__tests__/server_status.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import _ from 'lodash';
import { find } from 'lodash';
import expect from 'expect.js';
import sinon from 'sinon';

Expand All @@ -17,31 +17,63 @@ describe('ServerStatus class', function () {
serverStatus = new ServerStatus(server);
});

describe('#create(plugin)', function () {
describe('#create(id)', () => {
it('should create a new plugin with an id', () => {
const status = serverStatus.create('someid');
expect(status).to.be.a(Status);
});
});

describe('#createForPlugin(plugin)', function () {
it('should create a new status by plugin', function () {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);
expect(status).to.be.a(Status);
});
});

describe('#get(name)', function () {
it('exposes plugins by its id/name', function () {
let status = serverStatus.create(plugin);
expect(serverStatus.get('name')).to.be(status);
describe('#get(id)', () => {
it('exposes statuses by their id', () => {
const status = serverStatus.create('statusid');
expect(serverStatus.get('statusid')).to.be(status);
});

it('does not get the status for a plugin', () => {
serverStatus.createForPlugin(plugin);
expect(serverStatus.get(plugin)).to.be(undefined);
});
});

describe('#getForPluginId(plugin)', function () {
it('exposes plugin status for the plugin', function () {
let status = serverStatus.createForPlugin(plugin);
expect(serverStatus.getForPluginId(plugin.id)).to.be(status);
});

it('does not get plain statuses by their id', function () {
serverStatus.create('someid');
expect(serverStatus.getForPluginId('someid')).to.be(undefined);
});
});

describe('#getState(id)', function () {
it('should expose the state of a status by id', function () {
let status = serverStatus.create('someid');
status.green();
expect(serverStatus.getState('someid')).to.be('green');
});
});

describe('#getState(name)', function () {
it('should expose the state of the plugin by name', function () {
let status = serverStatus.create(plugin);
describe('#getStateForPluginId(plugin)', function () {
it('should expose the state of a plugin by id', function () {
let status = serverStatus.createForPlugin(plugin);
status.green();
expect(serverStatus.getState('name')).to.be('green');
expect(serverStatus.getStateForPluginId(plugin.id)).to.be('green');
});
});

describe('#overall()', function () {
it('considers each status to produce a summary', function () {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);

expect(serverStatus.overall().state).to.be('uninitialized');

Expand Down Expand Up @@ -69,25 +101,24 @@ describe('ServerStatus class', function () {
it('serializes to overall status and individuals', function () {
const pluginOne = {id: 'one', version: '1.0.0'};
const pluginTwo = {id: 'two', version: '2.0.0'};
const pluginThree = {id: 'three', version: '3.0.0'};

let one = serverStatus.create(pluginOne);
let two = serverStatus.create(pluginTwo);
let three = serverStatus.create(pluginThree);
let service = serverStatus.create('some service');
let p1 = serverStatus.createForPlugin(pluginOne);
let p2 = serverStatus.createForPlugin(pluginTwo);

one.green();
two.yellow();
three.red();
service.green();
p1.yellow();
p2.red();

let obj = JSON.parse(JSON.stringify(serverStatus));
expect(obj).to.have.property('overall');
expect(obj.overall.state).to.eql(serverStatus.overall().state);
expect(obj.statuses).to.have.length(3);
let json = JSON.parse(JSON.stringify(serverStatus));
expect(json).to.have.property('overall');
expect(json.overall.state).to.eql(serverStatus.overall().state);
expect(json.statuses).to.have.length(3);

let outs = _.indexBy(obj.statuses, 'name');
expect(outs.one).to.have.property('state', 'green');
expect(outs.two).to.have.property('state', 'yellow');
expect(outs.three).to.have.property('state', 'red');
const out = status => find(json.statuses, { id: status.id });
expect(out(service)).to.have.property('state', 'green');
expect(out(p1)).to.have.property('state', 'yellow');
expect(out(p2)).to.have.property('state', 'red');
});
});

Expand Down
22 changes: 11 additions & 11 deletions src/server/status/__tests__/status.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ describe('Status class', function () {
});

it('should have an "uninitialized" state initially', function () {
expect(serverStatus.create(plugin)).to.have.property('state', 'uninitialized');
expect(serverStatus.createForPlugin(plugin)).to.have.property('state', 'uninitialized');
});

it('emits change when the status is set', function (done) {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);

status.once('change', function (prev, prevMsg) {
expect(status.state).to.be('green');
Expand All @@ -42,7 +42,7 @@ describe('Status class', function () {
});

it('should only trigger the change listener when something changes', function () {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);
let stub = sinon.stub();
status.on('change', stub);
status.green('Ready');
Expand All @@ -52,18 +52,18 @@ describe('Status class', function () {
});

it('should create a JSON representation of the status', function () {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);
status.green('Ready');

let json = status.toJSON();
expect(json.name).to.eql(plugin.id);
expect(json.version).to.eql(plugin.version);
expect(json.id).to.eql(status.id);
expect(json.plugin).to.eql({ id: plugin.id, version: plugin.version });
expect(json.state).to.eql('green');
expect(json.message).to.eql('Ready');
});

it('should call on handler if status is already matched', function (done) {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);
let msg = 'Test Ready';
status.green(msg);

Expand All @@ -77,7 +77,7 @@ describe('Status class', function () {
});

it('should call once handler if status is already matched', function (done) {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);
let msg = 'Test Ready';
status.green(msg);

Expand All @@ -92,15 +92,15 @@ describe('Status class', function () {

function testState(color) {
it(`should change the state to ${color} when #${color}() is called`, function () {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);
let message = 'testing ' + color;
status[color](message);
expect(status).to.have.property('state', color);
expect(status).to.have.property('message', message);
});

it(`should trigger the "change" listner when #${color}() is called`, function (done) {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);
let message = 'testing ' + color;
status.on('change', function (prev, prevMsg) {
expect(status.state).to.be(color);
Expand All @@ -114,7 +114,7 @@ describe('Status class', function () {
});

it(`should trigger the "${color}" listner when #${color}() is called`, function (done) {
let status = serverStatus.create(plugin);
let status = serverStatus.createForPlugin(plugin);
let message = 'testing ' + color;
status.on(color, function (prev, prevMsg) {
expect(status.state).to.be(color);
Expand Down
Loading