From ead2f975a99c8354c60f903ed9784f07824c529f Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 5 Jul 2018 12:24:35 -0700 Subject: [PATCH 1/4] Add SearchStrategyRegistry and defaultSearchStrategy to support existing search behavior, and integrate it with CallClient. Fix typo. --- src/ui/public/courier/courier.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ui/public/courier/courier.js b/src/ui/public/courier/courier.js index a969f09ad6481..66d3c40659913 100644 --- a/src/ui/public/courier/courier.js +++ b/src/ui/public/courier/courier.js @@ -30,6 +30,9 @@ import '../promises'; import { searchRequestQueue } from './search_request_queue'; import { FetchSoonProvider } from './fetch'; import { SearchPollProvider } from './search_poll'; +import { addSearchStrategy, defaultSearchStrategy } from './search_strategy'; + +addSearchStrategy(defaultSearchStrategy); uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => { const fetchSoon = Private(FetchSoonProvider); From 92fae6831664aa395087ed158d7fcafe96d78dde Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 19 Jun 2018 11:35:13 -0700 Subject: [PATCH 2/4] Add /api/rollup/search endpoint. --- x-pack/plugins/rollup/index.js | 3 +- .../server/client/elasticsearch_rollup.js | 15 ++++++++ .../plugins/rollup/server/routes/api/index.js | 1 + .../rollup/server/routes/api/search.js | 36 +++++++++++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/rollup/server/routes/api/search.js diff --git a/x-pack/plugins/rollup/index.js b/x-pack/plugins/rollup/index.js index 42879f503b445..c347fb20ec76e 100644 --- a/x-pack/plugins/rollup/index.js +++ b/x-pack/plugins/rollup/index.js @@ -7,7 +7,7 @@ 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) { return new kibana.Plugin({ @@ -27,6 +27,7 @@ export function rollup(kibana) { registerLicenseChecker(server); registerIndicesRoute(server); registerFieldsForWildcardRoute(server); + registerSearchRoute(server); } }); } diff --git a/x-pack/plugins/rollup/server/client/elasticsearch_rollup.js b/x-pack/plugins/rollup/server/client/elasticsearch_rollup.js index 22148a0b112b1..10399861cad64 100644 --- a/x-pack/plugins/rollup/server/client/elasticsearch_rollup.js +++ b/x-pack/plugins/rollup/server/client/elasticsearch_rollup.js @@ -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' + }); }; diff --git a/x-pack/plugins/rollup/server/routes/api/index.js b/x-pack/plugins/rollup/server/routes/api/index.js index 5e9e9644d422e..0bf361d6c6399 100644 --- a/x-pack/plugins/rollup/server/routes/api/index.js +++ b/x-pack/plugins/rollup/server/routes/api/index.js @@ -6,3 +6,4 @@ export { registerIndicesRoute } from './indices'; export { registerFieldsForWildcardRoute } from './index_patterns'; +export { registerSearchRoute } from './search'; diff --git a/x-pack/plugins/rollup/server/routes/api/search.js b/x-pack/plugins/rollup/server/routes/api/search.js new file mode 100644 index 0000000000000..a77040daad78a --- /dev/null +++ b/x-pack/plugins/rollup/server/routes/api/search.js @@ -0,0 +1,36 @@ +/* +* 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 { callWithRequestFactory } from '../../lib/call_with_request_factory'; +import { isEsErrorFactory } from '../../lib/is_es_error_factory'; +import { wrapEsError, wrapUnknownError } from '../../lib/error_wrappers'; + +export function registerSearchRoute(server) { + const isEsError = isEsErrorFactory(server); + + server.route({ + path: '/api/rollup/search', + method: 'POST', + handler: async (request, reply) => { + const { index, query } = request.payload; + const callWithRequest = callWithRequestFactory(server, request); + + try { + const results = await callWithRequest('rollup.search', { + index, + body: query, + }); + + reply(results); + } catch(err) { + if (isEsError(err)) { + return reply(wrapEsError(err)); + } + + reply(wrapUnknownError(err)); + } + }, + }); +} From de444d590192e06e9970894f96379f7ad8914849 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 5 Jul 2018 19:53:03 -0700 Subject: [PATCH 3/4] Add rollupSearchStrategy. - Add logic to rollupSearchStrategy for extracting criteria for the rollup search. --- src/ui/public/courier/courier.js | 3 - .../search_strategy_registry.js | 12 +--- x-pack/plugins/rollup/index.js | 3 + x-pack/plugins/rollup/public/search/index.js | 7 +++ .../plugins/rollup/public/search/register.js | 10 ++++ .../public/search/rollup_search_strategy.js | 56 +++++++++++++++++++ 6 files changed, 79 insertions(+), 12 deletions(-) create mode 100644 x-pack/plugins/rollup/public/search/index.js create mode 100644 x-pack/plugins/rollup/public/search/register.js create mode 100644 x-pack/plugins/rollup/public/search/rollup_search_strategy.js diff --git a/src/ui/public/courier/courier.js b/src/ui/public/courier/courier.js index 66d3c40659913..a969f09ad6481 100644 --- a/src/ui/public/courier/courier.js +++ b/src/ui/public/courier/courier.js @@ -30,9 +30,6 @@ import '../promises'; import { searchRequestQueue } from './search_request_queue'; import { FetchSoonProvider } from './fetch'; import { SearchPollProvider } from './search_poll'; -import { addSearchStrategy, defaultSearchStrategy } from './search_strategy'; - -addSearchStrategy(defaultSearchStrategy); uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => { const fetchSoon = Private(FetchSoonProvider); diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js index a0341932565da..d6df4bea2d741 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js @@ -19,7 +19,7 @@ const searchStrategies = []; -const addSearchStrategy = searchStrategy => { +export const addSearchStrategy = searchStrategy => { if (searchStrategies.includes(searchStrategy)) { return; } @@ -47,7 +47,7 @@ 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 = {}; @@ -76,12 +76,6 @@ const assignSearchRequestsToSearchStrategies = searchRequests => { return searchStrategiesWithRequests; }; -const hasSearchStategyForIndexPattern = indexPattern => { +export const hasSearchStategyForIndexPattern = indexPattern => { return Boolean(getSearchStrategy(indexPattern)); }; - -export { - assignSearchRequestsToSearchStrategies, - addSearchStrategy, - hasSearchStategyForIndexPattern, -}; diff --git a/x-pack/plugins/rollup/index.js b/x-pack/plugins/rollup/index.js index c347fb20ec76e..8c3c949ed3137 100644 --- a/x-pack/plugins/rollup/index.js +++ b/x-pack/plugins/rollup/index.js @@ -22,6 +22,9 @@ export function rollup(kibana) { visualize: [ 'plugins/rollup/visualize', ], + search: [ + 'plugins/rollup/search', + ], }, init: function (server) { registerLicenseChecker(server); diff --git a/x-pack/plugins/rollup/public/search/index.js b/x-pack/plugins/rollup/public/search/index.js new file mode 100644 index 0000000000000..4d2947eb36f46 --- /dev/null +++ b/x-pack/plugins/rollup/public/search/index.js @@ -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'; diff --git a/x-pack/plugins/rollup/public/search/register.js b/x-pack/plugins/rollup/public/search/register.js new file mode 100644 index 0000000000000..9f0da80f14245 --- /dev/null +++ b/x-pack/plugins/rollup/public/search/register.js @@ -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); diff --git a/x-pack/plugins/rollup/public/search/rollup_search_strategy.js b/x-pack/plugins/rollup/public/search/rollup_search_strategy.js new file mode 100644 index 0000000000000..55c405593a8a5 --- /dev/null +++ b/x-pack/plugins/rollup/public/search/rollup_search_strategy.js @@ -0,0 +1,56 @@ +/* +* 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. +*/ + +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(); + + const index = indexPattern; + const query = { + 'size': size, + 'aggregations': aggs, + }; + + // TODO: Handle errors gracefully and surface them to the user. + return new Promise((resolve, reject) => { + fetch('../api/rollup/search', { + method: 'post', + body: JSON.stringify({ index, query }), + headers: { + accept: 'application/json', + 'content-type': 'application/json', + 'kbn-xsrf': 'kibana', + }, + credentials: 'same-origin' + }).then(response => { + return response.json(); + }).then(data => { + // Munge data into shape expected by consumer. + resolve({ + took: data.took, + responses: [ data ], + }); + }).catch(error => { + return reject(error); + }); + }); + }, + + isValidForSearchRequest: searchRequest => { + const indexPattern = searchRequest.source.getField('index'); + return indexPattern.type === 'rollup'; + }, +}; From 43f59ddcf4d51304afeeba2dc9ef45e822db749f Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 6 Jul 2018 11:42:22 -0700 Subject: [PATCH 4/4] 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.. --- src/ui/public/courier/fetch/fetch_now.js | 12 +- src/ui/public/courier/index.js | 2 + .../default_search_strategy.js | 23 +++- .../public/courier/search_strategy/index.js | 2 + .../courier/search_strategy/search_error.js | 34 ++++++ .../search_strategy_registry.js | 1 - .../public/vis/editors/default/sidebar.html | 9 ++ src/ui/public/vis/editors/default/sidebar.js | 20 +++- .../public/vis/editors/default/sidebar.less | 3 + .../visualize/loader/visualize_data_loader.js | 40 +++++-- x-pack/plugins/rollup/index.js | 2 +- .../public/search/rollup_search_strategy.js | 106 ++++++++++++++---- 12 files changed, 210 insertions(+), 44 deletions(-) create mode 100644 src/ui/public/courier/search_strategy/search_error.js create mode 100644 src/ui/public/vis/editors/default/sidebar.less diff --git a/src/ui/public/courier/fetch/fetch_now.js b/src/ui/public/courier/fetch/fetch_now.js index 73a670100cece..7bb3890fbf95e 100644 --- a/src/ui/public/courier/fetch/fetch_now.js +++ b/src/ui/public/courier/fetch/fetch_now.js @@ -22,7 +22,6 @@ import { CallClientProvider } from './call_client'; import { CallResponseHandlersProvider } from './call_response_handlers'; import { ContinueIncompleteProvider } from './continue_incomplete'; import { RequestStatus } from './req_status'; -import { location } from './notifier'; /** * Fetch now provider should be used if you want the results searched and returned immediately. @@ -53,7 +52,10 @@ export function FetchNowProvider(Private, Promise) { return searchRequest.retry(); })) - .catch(error => fatalError(error, location)); + .catch(error => { + // If any errors occur after the search requests have resolved, then we kill Kibana. + fatalError(error, 'Courier fetch'); + }); } function fetchSearchResults(searchRequests) { @@ -71,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(); diff --git a/src/ui/public/courier/index.js b/src/ui/public/courier/index.js index d466b2e080e3f..583172f1731cd 100644 --- a/src/ui/public/courier/index.js +++ b/src/ui/public/courier/index.js @@ -30,6 +30,8 @@ export { } from './search_source'; export { + addSearchStrategy, hasSearchStategyForIndexPattern, isDefaultTypeIndexPattern, + SearchError, } from './search_strategy'; diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index 05fc45c5c67d1..f5bd529b35c0b 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -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) => { @@ -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; diff --git a/src/ui/public/courier/search_strategy/index.js b/src/ui/public/courier/search_strategy/index.js index 41991b63f950d..102610538df98 100644 --- a/src/ui/public/courier/search_strategy/index.js +++ b/src/ui/public/courier/search_strategy/index.js @@ -24,3 +24,5 @@ export { } from './search_strategy_registry'; export { isDefaultTypeIndexPattern } from './is_default_type_index_pattern'; + +export { SearchError } from './search_error'; diff --git a/src/ui/public/courier/search_strategy/search_error.js b/src/ui/public/courier/search_strategy/search_error.js new file mode 100644 index 0000000000000..fecafb9b6fc13 --- /dev/null +++ b/src/ui/public/courier/search_strategy/search_error.js @@ -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); + } +} diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js index d6df4bea2d741..79e959b2a80ad 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js @@ -52,7 +52,6 @@ export const assignSearchRequestsToSearchStrategies = searchRequests => { const searchStrategyById = {}; searchRequests.forEach(searchRequest => { - const indexPattern = searchRequest.source.getField('index'); const matchingSearchStrategy = getSearchStrategy(indexPattern); diff --git a/src/ui/public/vis/editors/default/sidebar.html b/src/ui/public/vis/editors/default/sidebar.html index f0cff4acd142f..43ddf5e1aff21 100644 --- a/src/ui/public/vis/editors/default/sidebar.html +++ b/src/ui/public/vis/editors/default/sidebar.html @@ -19,6 +19,15 @@