Skip to content

Commit

Permalink
Add rollupSearchStrategy and SearchError (#20505)
Browse files Browse the repository at this point in the history
* Add /api/rollup/search endpoint and rollupSearchStrategy.
* Add SearchError for surfacing courier search errors.
- Use toastNotifications to surface errors in visualization editor.
- Add call-out react directive and use it to surface rollup errors in the visualization editor sidebar.
- Temporarily assign timezone and interval values from rollup job to the search to avoid errors.
  • Loading branch information
cjcenizal committed Sep 4, 2018
1 parent c13b13f commit 3090d57
Show file tree
Hide file tree
Showing 16 changed files with 289 additions and 19 deletions.
11 changes: 9 additions & 2 deletions src/ui/public/courier/fetch/fetch_now.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ export function FetchNowProvider(Private, Promise) {

return searchRequest.retry();
}))
.catch(error => fatalError(error, 'Courier fetch'));
.catch(error => {
// If any errors occur after the search requests have resolved, then we kill Kibana.
fatalError(error, 'Courier fetch');
});
}

function fetchSearchResults(searchRequests) {
Expand All @@ -70,7 +73,11 @@ export function FetchNowProvider(Private, Promise) {
return startRequests(searchRequests)
.then(function () {
replaceAbortedRequests();
return callClient(searchRequests);
return callClient(searchRequests)
.catch(() => {
// Silently swallow errors that result from search requests so the consumer can surface
// them as notifications instead of courier forcing fatal errors.
});
})
.then(function (responses) {
replaceAbortedRequests();
Expand Down
2 changes: 2 additions & 0 deletions src/ui/public/courier/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export {
} from './search_source';

export {
addSearchStrategy,
hasSearchStategyForIndexPattern,
isDefaultTypeIndexPattern,
SearchError,
} from './search_strategy';
23 changes: 20 additions & 3 deletions src/ui/public/courier/search_strategy/default_search_strategy.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import { addSearchStrategy } from './search_strategy_registry';
import { isDefaultTypeIndexPattern } from './is_default_type_index_pattern';
import { SearchError } from './search_error';

function getAllFetchParams(searchRequests, Promise) {
return Promise.map(searchRequests, (searchRequest) => {
Expand Down Expand Up @@ -71,14 +72,30 @@ export const defaultSearchStrategy = {
const searching = es.msearch({ body: serializedFetchParams });

return {
// Unwrap the responses object returned by the es client.
searching: searching.then(({ responses }) => responses),
// Munge data into shape expected by consumer.
searching: new Promise((resolve, reject) => {
// Unwrap the responses object returned by the ES client.
searching.then(({ responses }) => {
resolve(responses);
}).catch(error => {
// Format ES client error as a SearchError.
const { statusCode, displayName, message, path } = error;

const searchError = new SearchError({
status: statusCode,
title: displayName,
message,
path,
});

reject(searchError);
});
}),
abort: searching.abort,
failedSearchRequests,
};
},

// Accept multiple criteria for determining viability to be as flexible as possible.
isViable: (indexPattern) => {
if (!indexPattern) {
return false;
Expand Down
2 changes: 2 additions & 0 deletions src/ui/public/courier/search_strategy/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ export {
} from './search_strategy_registry';

export { isDefaultTypeIndexPattern } from './is_default_type_index_pattern';

export { SearchError } from './search_error';
34 changes: 34 additions & 0 deletions src/ui/public/courier/search_strategy/search_error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export class SearchError extends Error {
constructor({ status, title, message, path }) {
super(message);
this.name = 'SearchError';
this.status = status;
this.title = title;
this.message = message;
this.path = path;
Error.captureStackTrace(this, SearchError);

// Babel doesn't support traditional `extends` syntax for built-in classes.
// https://babeljs.io/docs/en/caveats/#classes
Object.setPrototypeOf(this, SearchError.prototype);
}
}
13 changes: 3 additions & 10 deletions src/ui/public/courier/search_strategy/search_strategy_registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

const searchStrategies = [];

const addSearchStrategy = searchStrategy => {
export const addSearchStrategy = searchStrategy => {
if (searchStrategies.includes(searchStrategy)) {
return;
}
Expand Down Expand Up @@ -47,12 +47,11 @@ const getSearchStrategy = indexPattern => {
* We use an array of objects to preserve the order of the search requests, which we use to
* deterministically associate each response with the originating request.
*/
const assignSearchRequestsToSearchStrategies = searchRequests => {
export const assignSearchRequestsToSearchStrategies = searchRequests => {
const searchStrategiesWithRequests = [];
const searchStrategyById = {};

searchRequests.forEach(searchRequest => {

const indexPattern = searchRequest.source.getField('index');
const matchingSearchStrategy = getSearchStrategy(indexPattern);

Expand All @@ -76,12 +75,6 @@ const assignSearchRequestsToSearchStrategies = searchRequests => {
return searchStrategiesWithRequests;
};

const hasSearchStategyForIndexPattern = indexPattern => {
export const hasSearchStategyForIndexPattern = indexPattern => {
return Boolean(getSearchStrategy(indexPattern));
};

export {
assignSearchRequestsToSearchStrategies,
addSearchStrategy,
hasSearchStategyForIndexPattern,
};
9 changes: 9 additions & 0 deletions src/ui/public/vis/editors/default/sidebar.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@
</div>

<nav class="navbar navbar-default subnav">
<div class="visEditorSidebarError" ng-if="sidebar.persistentError">
<call-out
color="'danger'"
iconType="'alert'"
title="sidebar.persistentError"
size="'s'"
/>
</div>

<div class="container-fluid">

<!-- tabs -->
Expand Down
20 changes: 18 additions & 2 deletions src/ui/public/vis/editors/default/sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@ import './vis_options';
import { uiModules } from '../../../modules';
import sidebarTemplate from './sidebar.html';

import './sidebar.less';

uiModules
.get('app/visualize')
.directive('visEditorSidebar', function () {


return {
restrict: 'E',
template: sidebarTemplate,
scope: true,
controllerAs: 'sidebar',
controller: function ($scope) {
this.persistentError = undefined;

$scope.$watch('vis.type', (visType) => {
if (visType) {
Expand All @@ -49,6 +50,21 @@ uiModules
this.section = this.section || (this.showData ? 'data' : _.get(visType, 'editorConfig.optionTabs[0].name'));
}
});

// TODO: This doesn't update when requestError is set.
$scope.$watch('vis.requestError', (requestError) => {
if (requestError && requestError.messsage) {
const { message } = requestError;
const isRollupError = message.includes('rollup');

if (isRollupError) {
this.persistentError = message;
return;
}
}

this.persistentError = undefined;
});
}
};
});
3 changes: 3 additions & 0 deletions src/ui/public/vis/editors/default/sidebar.less
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.visEditorSidebarError {
padding: 8px;
}
8 changes: 6 additions & 2 deletions x-pack/plugins/rollup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
import { resolve } from 'path';
import { PLUGIN } from './common/constants';
import { registerLicenseChecker } from './server/lib/register_license_checker';
import { registerIndicesRoute, registerFieldsForWildcardRoute } from './server/routes/api';
import { registerIndicesRoute, registerFieldsForWildcardRoute, registerSearchRoute } from './server/routes/api';

export function rollup(kibana) {
export function rollup(kibana) {
return new kibana.Plugin({
id: PLUGIN.ID,
publicDir: resolve(__dirname, 'public'),
Expand All @@ -22,11 +22,15 @@ export function rollup(kibana) {
visualize: [
'plugins/rollup/visualize',
],
search: [
'plugins/rollup/search',
],
},
init: function (server) {
registerLicenseChecker(server);
registerIndicesRoute(server);
registerFieldsForWildcardRoute(server);
registerSearchRoute(server);
}
});
}
7 changes: 7 additions & 0 deletions x-pack/plugins/rollup/public/search/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import './register';
10 changes: 10 additions & 0 deletions x-pack/plugins/rollup/public/search/register.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { addSearchStrategy } from 'ui/courier';
import { rollupSearchStrategy } from './rollup_search_strategy';

addSearchStrategy(rollupSearchStrategy);
114 changes: 114 additions & 0 deletions x-pack/plugins/rollup/public/search/rollup_search_strategy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { kfetchAbortable } from 'ui/kfetch';
import { SearchError } from 'ui/courier';

export const rollupSearchStrategy = {
id: 'rollup',

search: async ({ searchRequests, Promise }) => {
// TODO: Batch together requests to hit a bulk rollup search endpoint.
const searchRequest = searchRequests[0];
const {
index: { title: indexPattern },
body: {
size,
aggs,
},
} = await searchRequest.getFetchParams();

function findDateHistogram(aggs) {
if (Array.isArray(aggs)) {
for (let i = 0; i < aggs.length; i++) {
const dateHistogram = findDateHistogram(aggs[i]);

if (dateHistogram) {
return dateHistogram;
}
}
} else if (typeof aggs === 'object') {
const aggNames = Object.keys(aggs);
const aggsList = aggNames.map(aggName => aggs[aggName]);

if (aggsList.includes('date_histogram')) {
return aggs;
}

return findDateHistogram(aggsList);
}
}

// TODO: Temporarily automatically assign same timezone and interval as what's defined by
// the rollup job. This should be done by the visualization itself.
const searchableAggs = JSON.parse(searchRequest.source.getField('index').originalBody.typeMeta);
const { time_zone: timeZone, interval } = findDateHistogram(searchableAggs);

Object.keys(aggs).forEach(aggName => {
const subAggs = aggs[aggName];

Object.keys(subAggs).forEach(subAggName => {
if (subAggName === 'date_histogram') {
const subAgg = subAggs[subAggName];
subAgg.time_zone = timeZone;
subAgg.interval = interval;
}
});
});

const index = indexPattern;
const query = {
'size': size,
'aggregations': aggs,
};

const {
fetching,
abort,
} = kfetchAbortable({
pathname: '../api/rollup/search',
method: 'POST',
body: JSON.stringify({ index, query }),
});

// TODO: Implement this. Search requests which can't be sent.
const failedSearchRequests = [];

return {
// Munge data into shape expected by consumer.
searching: new Promise((resolve, reject) => {
fetching.then(result => {
resolve([ result ]);
}).catch(error => {
const {
body: { statusText, error: title, message },
res: { url },
} = error;

// Format kfetch error as a SearchError.
const searchError = new SearchError({
status: statusText,
title,
message,
path: url,
});

reject(searchError);
});
}),
abort,
failedSearchRequests,
};
},

isViable: (indexPattern) => {
if (!indexPattern) {
return false;
}

return indexPattern.type === 'rollup';
},
};
15 changes: 15 additions & 0 deletions x-pack/plugins/rollup/server/client/elasticsearch_rollup.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,20 @@ export const elasticsearchJsPlugin = (Client, config, components) => {
],
method: 'GET'
});

rollup.search = ca({
urls: [
{
fmt: '/<%=index%>/_rollup_search',
req: {
index: {
type: 'string'
}
}
}
],
needBody: true,
method: 'POST'
});
};

Loading

0 comments on commit 3090d57

Please sign in to comment.