Skip to content

Commit

Permalink
[Alerting]: harden APIs of built-in alert index-threshold (#60702)
Browse files Browse the repository at this point in the history
resolves #59889

The index threshold APIs - used by both the index threshold UI and the
alert executor - were returning errors (500's from http endpoints) when
getting errors from ES.

These have been changed so that the error is logged as a warning, and the
relevant API returns an "empty" result.

Another 500 response was found while experimenting with this.  Apparently
the date_range agg requires a date format to be passed in if the date format
in ES is not an ISO date.  The repro on this was to select the `.security`
alias (or it's index) within the index threshold alert UI, and then select
one of it's date fields.
  • Loading branch information
pmuellr authored Mar 20, 2020
1 parent 7983d1d commit 5efd59b
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export async function timeSeriesQuery(
dateAgg: {
date_range: {
field: timeField,
format: 'strict_date_time',
ranges: dateRangeInfo.dateRanges,
},
},
Expand Down Expand Up @@ -134,8 +135,8 @@ export async function timeSeriesQuery(
esResult = await callCluster('search', esQuery);
} catch (err) {
// console.log('time_series_query.ts error\n', JSON.stringify(err, null, 4));
logger.warn(`${logPrefix} error: ${JSON.stringify(err.message)}`);
throw new Error('error running search');
logger.warn(`${logPrefix} error: ${err.message}`);
return { results: [] };
}

// console.log('time_series_query.ts response\n', JSON.stringify(esResult, null, 4));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ export function createFieldsRoute(service: Service, router: IRouter, baseRoute:
try {
rawFields = await getRawFields(ctx.core.elasticsearch.dataClient, req.body.indexPatterns);
} catch (err) {
service.logger.debug(`route ${path} error: ${err.message}`);
return res.internalError({ body: 'error getting field data' });
const indexPatterns = req.body.indexPatterns.join(',');
service.logger.warn(
`route ${path} error getting fields from pattern "${indexPatterns}": ${err.message}`
);
return res.ok({ body: { fields: [] } });
}

const result = { fields: getFieldsFromRawFields(rawFields) };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,18 @@ export function createIndicesRoute(service: Service, router: IRouter, baseRoute:
try {
aliases = await getAliasesFromPattern(ctx.core.elasticsearch.dataClient, pattern);
} catch (err) {
service.logger.debug(`route ${path} error: ${err.message}`);
return res.internalError({ body: 'error getting alias data' });
service.logger.warn(
`route ${path} error getting aliases from pattern "${pattern}": ${err.message}`
);
}

let indices: string[] = [];
try {
indices = await getIndicesFromPattern(ctx.core.elasticsearch.dataClient, pattern);
} catch (err) {
service.logger.debug(`route ${path} error: ${err.message}`);
return res.internalError({ body: 'error getting index data' });
service.logger.warn(
`route ${path} error getting indices from pattern "${pattern}": ${err.message}`
);
}

const result = { indices: uniqueCombined(aliases, indices, MAX_INDICES) };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
} from 'kibana/server';

import { Service } from '../../../types';
import { TimeSeriesQuery, TimeSeriesQuerySchema, TimeSeriesResult } from '../lib/time_series_types';
import { TimeSeriesQuery, TimeSeriesQuerySchema } from '../lib/time_series_types';
export { TimeSeriesQuery, TimeSeriesResult } from '../lib/time_series_types';

export function createTimeSeriesQueryRoute(service: Service, router: IRouter, baseRoute: string) {
Expand All @@ -33,21 +33,15 @@ export function createTimeSeriesQueryRoute(service: Service, router: IRouter, ba
req: KibanaRequest<any, any, TimeSeriesQuery, any>,
res: KibanaResponseFactory
): Promise<IKibanaResponse> {
service.logger.debug(`route query_data request: ${JSON.stringify(req.body)}`);
service.logger.debug(`route ${path} request: ${JSON.stringify(req.body)}`);

let result: TimeSeriesResult;
try {
result = await service.indexThreshold.timeSeriesQuery({
logger: service.logger,
callCluster: ctx.core.elasticsearch.dataClient.callAsCurrentUser,
query: req.body,
});
} catch (err) {
service.logger.debug(`route query_data error: ${err.message}`);
return res.internalError({ body: 'error running time series query' });
}
const result = await service.indexThreshold.timeSeriesQuery({
logger: service.logger,
callCluster: ctx.core.elasticsearch.dataClient.callAsCurrentUser,
query: req.body,
});

service.logger.debug(`route query_data response: ${JSON.stringify(result)}`);
service.logger.debug(`route ${path} response: ${JSON.stringify(result)}`);
return res.ok({ body: result });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ export class ESTestIndexTool {
type: 'date',
format: 'strict_date_time',
},
date_epoch_millis: {
type: 'date',
format: 'epoch_millis',
},
testedValue: {
type: 'long',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,14 @@ export default function alertTests({ getService }: FtrProviderContext) {
// create some more documents in the first group
createEsDocumentsInGroups(1);

// this never fires because of bad fields error
await createAlert({
name: 'never fire',
timeField: 'source', // bad field for time
aggType: 'avg',
aggField: 'testedValue',
aggField: 'source', // bad field for agg
groupBy: 'all',
thresholdComparator: '<',
thresholdComparator: '>',
threshold: [0],
});

Expand Down Expand Up @@ -303,6 +305,7 @@ export default function alertTests({ getService }: FtrProviderContext) {
name: string;
aggType: string;
aggField?: string;
timeField?: string;
groupBy: 'all' | 'top';
termField?: string;
termSize?: number;
Expand Down Expand Up @@ -347,7 +350,7 @@ export default function alertTests({ getService }: FtrProviderContext) {
actions: [action],
params: {
index: ES_TEST_INDEX_NAME,
timeField: 'date',
timeField: params.timeField || 'date',
aggType: params.aggType,
aggField: params.aggField,
groupBy: params.groupBy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ async function createEsDocument(es: any, epochMillis: number, testedValue: numbe
source: DOCUMENT_SOURCE,
reference: DOCUMENT_REFERENCE,
date: new Date(epochMillis).toISOString(),
date_epoch_millis: epochMillis,
testedValue,
group,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ export default function fieldsEndpointTests({ getService }: FtrProviderContext)
expect(field.name).to.eql('updated_at');
expect(field.type).to.eql('date');
});

// TODO: the pattern '*a:b,c:d*' throws an exception in dev, but not ci!
it('should handle no_such_remote_cluster', async () => {
const result = await runQueryExpect({ indexPatterns: ['*a:b,c:d*'] }, 200);
expect(result.fields.length).to.be(0);
});
});

function getFieldNamed(fields: any[], fieldName: string): any | undefined {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ export default function indicesEndpointTests({ getService }: FtrProviderContext)
expect(result.indices).to.be.an('array');
expect(result.indices.includes('.kibana')).to.be(true);
});

// TODO: the pattern '*a:b,c:d*' throws an exception in dev, but not ci!
it('should handle no_such_remote_cluster', async () => {
const result = await runQueryExpect({ pattern: '*a:b,c:d*' }, 200);
expect(result.indices.length).to.be(0);
});
});

async function runQueryExpect(requestBody: any, status: number): Promise<any> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,32 @@ export default function timeSeriesQueryEndpointTests({ getService }: FtrProvider
};
expect(await runQueryExpect(query, 400)).eql(expected);
});

it('should handle epoch_millis time field', async () => {
const query = getQueryBody({
dateStart: START_DATE,
dateEnd: START_DATE,
timeField: 'date_epoch_millis',
});
const expected = {
results: [{ group: 'all documents', metrics: [[START_DATE, 6]] }],
};
expect(await runQueryExpect(query, 200)).eql(expected);
});

it('should handle ES errors', async () => {
const query = getQueryBody({
dateStart: START_DATE,
dateEnd: START_DATE,
timeField: 'source', // bad field for time
aggType: 'avg',
aggField: 'source', // bad field for agg
});
const expected = {
results: [],
};
expect(await runQueryExpect(query, 200)).eql(expected);
});
});

async function runQueryExpect(requestBody: TimeSeriesQuery, status: number): Promise<any> {
Expand Down

0 comments on commit 5efd59b

Please sign in to comment.