[Uptime] Fix Total Calculation for AllPings (#28224)

The all pings panel on the monitor page was incorrectly calculating the number of pings. This patch uses the ES search's hits.total for the value, whereas before we simply counted the returned array. For anything but the shortest histories this number would be wrong since it'd only count the number of items returned, by default 200.
This commit is contained in:
Andrew Cholakian 2019-01-10 12:26:38 -08:00 committed by GitHub
parent 91a4d0749e
commit b09d849d4c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 161 additions and 89 deletions

View file

@ -61,15 +61,7 @@
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": { "kind": "OBJECT", "name": "Ping", "ofType": null }
}
}
"ofType": { "kind": "OBJECT", "name": "PingResults", "ofType": null }
},
"isDeprecated": false,
"deprecationReason": null
@ -342,6 +334,49 @@
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "PingResults",
"description": "",
"fields": [
{
"name": "total",
"description": "",
"args": [],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": { "kind": "SCALAR", "name": "UnsignedInteger", "ofType": null }
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "pings",
"description": "",
"args": [],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": { "kind": "OBJECT", "name": "Ping", "ofType": null }
}
}
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
"interfaces": [],
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "Ping",

View file

@ -16,7 +16,7 @@ export type UnsignedInteger = any;
export interface Query {
/** Get a list of all recorded pings for all monitors */
allPings: Ping[];
allPings: PingResults;
/** Gets the number of documents in the target index */
getDocCount: DocCount;
@ -32,6 +32,12 @@ export interface Query {
getErrorsList?: (ErrorListItem | null)[] | null;
}
export interface PingResults {
total: UnsignedInteger;
pings: Ping[];
}
/** A request sent from a monitor to a host */
export interface Ping {
/** The timestamp of the ping's creation */

View file

@ -23,26 +23,29 @@ export const getPingsQuery = gql`
sort: $sort
size: $size
) {
timestamp
http {
response {
status_code
total
pings {
timestamp
http {
response {
status_code
}
}
}
error {
message
type
}
monitor {
duration {
us
error {
message
type
}
monitor {
duration {
us
}
id
ip
name
scheme
status
type
}
id
ip
name
scheme
status
type
}
}
}

View file

@ -105,7 +105,9 @@ export class Pings extends React.Component<PingListProps, PingListState> {
if (error) {
return `Error ${error.message}`;
}
const { allPings } = data;
const {
allPings: { total, pings },
} = data;
const columns = [
{
field: 'monitor.status',
@ -156,7 +158,7 @@ export class Pings extends React.Component<PingListProps, PingListState> {
),
},
];
const hasStatus = allPings.reduce(
const hasStatus = pings.reduce(
(hasHttpStatus: boolean, currentPing: Ping) =>
hasHttpStatus || get(currentPing, 'http.response.status_code'),
false
@ -173,7 +175,7 @@ export class Pings extends React.Component<PingListProps, PingListState> {
</EuiTitle>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiBadge color="primary">{allPings.length}</EuiBadge>
<EuiBadge color="primary">{total}</EuiBadge>
</EuiFlexItem>
</EuiFlexGroup>
<EuiPanel paddingSize="l">
@ -213,7 +215,7 @@ export class Pings extends React.Component<PingListProps, PingListState> {
</EuiFlexGroup>
<EuiInMemoryTable
columns={columns}
items={allPings}
items={pings}
pagination={{ initialPageSize: 10, pageSizeOptions: [5, 10, 20, 100] }}
sorting={true}
/>

View file

@ -6,7 +6,7 @@
import { UMPingSortDirectionArg } from '../../../common/domain_types';
import { UMResolver } from '../../../common/graphql/resolver_types';
import { DocCount, Ping } from '../../../common/graphql/types';
import { DocCount, PingResults } from '../../../common/graphql/types';
import { UMServerLibs } from '../../lib/lib';
import { UMContext } from '../types';
import { CreateUMGraphQLResolvers } from '../types';
@ -21,7 +21,7 @@ interface UMAllPingsArgs {
}
export type UMAllPingsResolver = UMResolver<
Ping[] | Promise<Ping[]>,
PingResults | Promise<PingResults>,
any,
UMAllPingsArgs,
UMContext
@ -30,7 +30,7 @@ export type UMAllPingsResolver = UMResolver<
export type UMGetDocCountResolver = UMResolver<DocCount | Promise<DocCount>, any, never, UMContext>;
export interface UMPingResolver {
allPings: () => Ping[];
allPings: () => PingResults;
getDocCount: () => number;
}
@ -47,7 +47,7 @@ export const createPingsResolvers: CreateUMGraphQLResolvers = (
resolver,
{ monitorId, sort, size, status, dateRangeStart, dateRangeEnd },
{ req }
): Promise<Ping[]> {
): Promise<PingResults> {
return libs.pings.getAll(req, dateRangeStart, dateRangeEnd, monitorId, status, sort, size);
},
async getDocCount(resolver, args, { req }): Promise<DocCount> {

View file

@ -11,6 +11,11 @@ export const pingsSchema = gql`
query: Query
}
type PingResults {
total: UnsignedInteger!
pings: [Ping!]!
}
type Query {
"Get a list of all recorded pings for all monitors"
allPings(
@ -20,7 +25,7 @@ export const pingsSchema = gql`
status: String
dateRangeStart: UnsignedInteger!
dateRangeEnd: UnsignedInteger!
): [Ping!]!
): PingResults!
"Gets the number of documents in the target index"
getDocCount: DocCount!

View file

@ -35,6 +35,9 @@ describe('ElasticsearchPingsAdapter class', () => {
];
mockEsResult = {
hits: {
total: {
value: mockHits.length,
},
hits: mockHits,
},
};
@ -69,11 +72,15 @@ describe('ElasticsearchPingsAdapter class', () => {
it('returns data in the appropriate shape', async () => {
const result = await adapter.getAll(serverRequest, 100, 200, undefined, undefined, 'asc', 12);
const count = 3;
expect(result).toHaveLength(3);
expect(result[0].timestamp).toBe('2018-10-30T18:51:59.792Z');
expect(result[1].timestamp).toBe('2018-10-30T18:53:59.792Z');
expect(result[2].timestamp).toBe('2018-10-30T18:55:59.792Z');
expect(result.total).toBe(count);
const pings = result.pings!;
expect(pings).toHaveLength(count);
expect(pings[0].timestamp).toBe('2018-10-30T18:51:59.792Z');
expect(pings[1].timestamp).toBe('2018-10-30T18:53:59.792Z');
expect(pings[2].timestamp).toBe('2018-10-30T18:55:59.792Z');
});
it('creates appropriate sort and size parameters', async () => {

View file

@ -5,7 +5,7 @@
*/
import { UMGqlRange, UMPingSortDirectionArg } from '../../../../common/domain_types';
import { DocCount, HistogramSeries, Ping } from '../../../../common/graphql/types';
import { DocCount, HistogramSeries, Ping, PingResults } from '../../../../common/graphql/types';
export interface UMPingsAdapter {
getAll(
@ -16,7 +16,7 @@ export interface UMPingsAdapter {
status?: string,
sort?: UMPingSortDirectionArg,
size?: number
): Promise<Ping[]>;
): Promise<PingResults>;
getLatestMonitorDocs(
request: any,

View file

@ -7,7 +7,7 @@
import { get } from 'lodash';
import { INDEX_NAMES } from '../../../../common/constants';
import { UMGqlRange, UMPingSortDirectionArg } from '../../../../common/domain_types';
import { DocCount, HistogramSeries, Ping } from '../../../../common/graphql/types';
import { DocCount, HistogramSeries, Ping, PingResults } from '../../../../common/graphql/types';
import { DatabaseAdapter } from '../database';
import { UMPingsAdapter } from './adapter_types';
@ -49,7 +49,7 @@ export class ElasticsearchPingsAdapter implements UMPingsAdapter {
status?: string,
sort?: UMPingSortDirectionArg,
size?: number
): Promise<Ping[]> {
): Promise<PingResults> {
const sortParam = sort ? { sort: [{ '@timestamp': { order: sort } }] } : undefined;
const sizeParam = size ? { size } : undefined;
const must: any[] = [];
@ -72,13 +72,20 @@ export class ElasticsearchPingsAdapter implements UMPingsAdapter {
},
};
const {
hits: { hits },
hits: { hits, total },
} = await this.database.search(request, params);
return hits.map(({ _source }: any) => {
const pings: Ping[] = hits.map(({ _source }: any) => {
const timestamp = _source['@timestamp'];
return { timestamp, ..._source };
});
const results: PingResults = {
total: total.value,
pings,
};
return results;
}
public async getLatestMonitorDocs(

View file

@ -6,7 +6,7 @@
import { take } from 'lodash';
import { UMPingSortDirectionArg } from '../../../../common/domain_types';
import { DocCount, HistogramSeries, Ping } from '../../../../common/graphql/types';
import { DocCount, HistogramSeries, Ping, PingResults } from '../../../../common/graphql/types';
import { UMPingsAdapter } from './adapter_types';
const sortPings = (sort: UMPingSortDirectionArg) =>
@ -29,16 +29,17 @@ export class MemoryPingsAdapter implements UMPingsAdapter {
status?: string,
sort?: UMPingSortDirectionArg,
size?: number
): Promise<Ping[]> {
): Promise<PingResults> {
let pings = this.pingsDB;
if (monitorId) {
pings = pings.filter(ping => ping.monitor && ping.monitor.id === monitorId);
}
if (sort) {
const sortedPings = pings.sort(sortPings(sort));
return take(sortedPings, size ? size : 10);
}
return take(pings, size ? size : 10);
size = size ? size : 10;
return {
total: size,
pings: take(sort ? pings.sort(sortPings(sort)) : pings, size),
};
}
// TODO implement

View file

@ -52,9 +52,10 @@ describe('Pings domain lib', () => {
'asc',
2
);
expect(apiResponse).toHaveLength(2);
testMonitorId('foo', apiResponse[0].monitor);
testMonitorId('baz', apiResponse[1].monitor);
expect(apiResponse.total).toBe(2);
expect(apiResponse.pings).toHaveLength(2);
testMonitorId('foo', apiResponse.pings[0].monitor);
testMonitorId('baz', apiResponse.pings[1].monitor);
});
it('should sort desc and take a range', async () => {
@ -67,9 +68,10 @@ describe('Pings domain lib', () => {
'desc',
2
);
expect(apiResponse).toHaveLength(2);
testMonitorId('bar', apiResponse[0].monitor);
testMonitorId('baz', apiResponse[1].monitor);
expect(apiResponse.total).toBe(2);
expect(apiResponse.pings).toHaveLength(2);
testMonitorId('bar', apiResponse.pings[0].monitor);
testMonitorId('baz', apiResponse.pings[1].monitor);
});
it('should take range without sort', async () => {
@ -82,9 +84,10 @@ describe('Pings domain lib', () => {
undefined,
2
);
expect(apiResponse).toHaveLength(2);
testMonitorId('foo', apiResponse[0].monitor);
testMonitorId('bar', apiResponse[1].monitor);
expect(apiResponse.total).toBe(2);
expect(apiResponse.pings).toHaveLength(2);
testMonitorId('foo', apiResponse.pings[0].monitor);
testMonitorId('bar', apiResponse.pings[1].monitor);
});
it('should sort without range', async () => {
@ -97,10 +100,11 @@ describe('Pings domain lib', () => {
'desc',
undefined
);
expect(apiResponse).toHaveLength(3);
testMonitorId('bar', apiResponse[0].monitor);
testMonitorId('baz', apiResponse[1].monitor);
testMonitorId('foo', apiResponse[2].monitor);
expect(apiResponse.total).toBe(10);
expect(apiResponse.pings).toHaveLength(3);
testMonitorId('bar', apiResponse.pings[0].monitor);
testMonitorId('baz', apiResponse.pings[1].monitor);
testMonitorId('foo', apiResponse.pings[2].monitor);
});
it('should return unsorted, with default size of 10', async () => {
@ -113,10 +117,11 @@ describe('Pings domain lib', () => {
undefined,
undefined
);
expect(apiResponse).toHaveLength(3);
testMonitorId('foo', apiResponse[0].monitor);
testMonitorId('bar', apiResponse[1].monitor);
testMonitorId('baz', apiResponse[2].monitor);
expect(apiResponse.total).toBe(10);
expect(apiResponse.pings).toHaveLength(3);
testMonitorId('foo', apiResponse.pings[0].monitor);
testMonitorId('bar', apiResponse.pings[1].monitor);
testMonitorId('baz', apiResponse.pings[2].monitor);
});
});
});

View file

@ -5,7 +5,7 @@
*/
import { UMGqlRange, UMPingSortDirectionArg } from '../../../common/domain_types';
import { DocCount, HistogramSeries, Ping } from '../../../common/graphql/types';
import { DocCount, HistogramSeries, Ping, PingResults } from '../../../common/graphql/types';
import { UMPingsAdapter } from '../adapters/pings';
export class UMPingsDomain {
@ -21,7 +21,7 @@ export class UMPingsDomain {
status?: string,
sort?: UMPingSortDirectionArg,
size?: number
): Promise<Ping[]> {
): Promise<PingResults> {
return this.adapter.getAll(
request,
dateRangeStart,

View file

@ -5,7 +5,7 @@
*/
import Joi from 'joi';
import { Ping } from '../../../common/graphql/types';
import { PingResults } from '../../../common/graphql/types';
import { UMServerLibs } from '../../lib/lib';
export const createGetAllRoute = (libs: UMServerLibs) => ({
@ -23,7 +23,7 @@ export const createGetAllRoute = (libs: UMServerLibs) => ({
}),
},
},
handler: async (request: any): Promise<Ping[]> => {
handler: async (request: any): Promise<PingResults> => {
const { size, sort, dateRangeStart, dateRangeEnd, monitorId, status } = request.query;
return await libs.pings.getAll(
request,

View file

@ -21,13 +21,12 @@ export default function ({ getService }) {
it('should get all pings stored in index', async () => {
const { body: apiResponse } = await supertest
.get(
`/api/uptime/pings?dateRangeStart=${dateRangeStart}&dateRangeEnd=${dateRangeEnd}`
)
.get(`/api/uptime/pings?dateRangeStart=${dateRangeStart}&dateRangeEnd=${dateRangeEnd}`)
.expect(200);
expect(apiResponse.length).to.be(2);
expect(apiResponse[0].monitor.id).to.be('http@https://www.github.com/');
expect(apiResponse.total).to.be(2);
expect(apiResponse.pings.length).to.be(2);
expect(apiResponse.pings[0].monitor.id).to.be('http@https://www.github.com/');
});
it('should sort pings according to timestamp', async () => {
@ -36,9 +35,11 @@ export default function ({ getService }) {
`/api/uptime/pings?sort=asc&dateRangeStart=${dateRangeStart}&dateRangeEnd=${dateRangeEnd}`
)
.expect(200);
expect(apiResponse.length).to.be(2);
expect(apiResponse[0].timestamp).to.be('2018-10-30T14:49:23.889Z');
expect(apiResponse[1].timestamp).to.be('2018-10-30T18:51:56.792Z');
expect(apiResponse.total).to.be(2);
expect(apiResponse.pings.length).to.be(2);
expect(apiResponse.pings[0].timestamp).to.be('2018-10-30T14:49:23.889Z');
expect(apiResponse.pings[1].timestamp).to.be('2018-10-30T18:51:56.792Z');
});
it('should return results of n length', async () => {
@ -48,20 +49,20 @@ export default function ({ getService }) {
)
.expect(200);
expect(apiResponse.length).to.be(1);
expect(apiResponse[0].monitor.id).to.be('http@https://www.github.com/');
expect(apiResponse.total).to.be(2);
expect(apiResponse.pings.length).to.be(1);
expect(apiResponse.pings[0].monitor.id).to.be('http@https://www.github.com/');
});
it('should miss pings outside of date range', async () => {
dateRangeStart = moment('2002-01-01').valueOf();
dateRangeEnd = moment('2002-01-02').valueOf();
const { body: apiResponse } = await supertest
.get(
`/api/uptime/pings?dateRangeStart=${dateRangeStart}&dateRangeEnd=${dateRangeEnd}`
)
.get(`/api/uptime/pings?dateRangeStart=${dateRangeStart}&dateRangeEnd=${dateRangeEnd}`)
.expect(200);
expect(apiResponse.length).to.be(0);
expect(apiResponse.total).to.be(0);
expect(apiResponse.pings.length).to.be(0);
});
});
}