Skip to content

Commit

Permalink
[maps] display incomplete results warning in layer legend (elastic#17…
Browse files Browse the repository at this point in the history
…1144)

Closes elastic#170653
Closes elastic#170654

PR updates Maps to display incomplete result warnings in layer legend
(instead of displaying toast)

### Test setup
1. In console, run:
    ```
    PUT geo1
    {}

    PUT geo1/_mapping
    {
      "properties": {
        "location": {
          "type": "geo_point"
        }
      }
    }

    PUT geo1/_doc/1
    {
      "location": "25,25"
    }

    PUT geo2
    {}

    PUT geo2/_mapping
    {
      "properties": {
        "location": {
          "type": "geo_point"
        }
      }
    }

    PUT geo2/_doc/1
    {
      "location": "35,35"
    }
    ```
2. Create `geo*` data view

### Test vector tile request warning
"View details" button for vector tile requests is out of scope for this
PR. Vector tile requests use _mvt API instead of _search API. As such,
vector tile requests do not use search source or request inspector.

1. create new map, add documents layer from `geo*` data view.
2. add filter
    ```
    {
      "error_query": {
        "indices": [
          {
            "error_type": "exception",
            "message": "local shard failure message 123",
            "name": "geo2"
          }
        ]
      }
    }
    ```
<img width="400" alt="Screenshot 2023-11-13 at 2 08 06 PM"
src="https://github.com/elastic/kibana/assets/373691/8b608400-79d0-4800-9980-5af76b507a43">

### Test geojson incomplete results warning with single request
1. create new map, add documents layer from `geo*` data view.
2. Set scaling to "Limit results to 10000"
3. add filter
    ```
    {
      "error_query": {
        "indices": [
          {
            "error_type": "exception",
            "message": "local shard failure message 123",
            "name": "geo2"
          }
        ]
      }
    }
    ```
<img width="400" alt="Screenshot 2023-11-13 at 2 11 48 PM"
src="https://github.com/elastic/kibana/assets/373691/e1b1de01-1db7-40ad-b221-29f42baf5735">

### Test geojson incomplete results warning with multiple requests
1. create new map, add documents layer from `geo*` data view.
2. Set scaling to "Show clusters when results exceed 10000"
3. add filter
    ```
    {
      "error_query": {
        "indices": [
          {
            "error_type": "exception",
            "message": "local shard failure message 123",
            "name": "geo2"
          }
        ]
      }
    }
    ```
<img width="400" alt="Screenshot 2023-11-13 at 2 12 57 PM"
src="https://github.com/elastic/kibana/assets/373691/27beffc4-1dba-4ec4-90f8-e92002f8a63a">

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
nreese and kibanamachine authored Nov 16, 2023
1 parent c3ac89c commit 0b24e40
Show file tree
Hide file tree
Showing 42 changed files with 690 additions and 400 deletions.
4 changes: 4 additions & 0 deletions packages/kbn-search-response-warnings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@
export type { SearchResponseWarning, WarningHandlerCallback } from './src/types';

export {
getWarningsDescription,
getWarningsTitle,
SearchResponseWarningsBadge,
SearchResponseWarningsBadgePopoverContent,
SearchResponseWarningsCallout,
SearchResponseWarningsEmptyPrompt,
ViewDetailsPopover,
} from './src/components/search_response_warnings';

export { extractWarnings } from './src/extract_warnings';
export { handleWarnings } from './src/handle_warnings';
export { hasUnsupportedDownsampledAggregationFailure } from './src/has_unsupported_downsampled_aggregation_failure';
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
* Side Public License, v 1.
*/

export { getWarningsDescription, getWarningsTitle } from './i18n_utils';
export { SearchResponseWarningsBadge } from './badge';
export { SearchResponseWarningsBadgePopoverContent } from './badge_popover_content';
export { SearchResponseWarningsCallout } from './callout';
export { SearchResponseWarningsEmptyPrompt } from './empty_prompt';
export { ViewDetailsPopover } from './view_details_popover';
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { KibanaExecutionContext } from '@kbn/core/public';
import type { Query } from '@kbn/data-plugin/common';
import type { Filter } from '@kbn/es-query';
import type { TimeRange } from '@kbn/es-query';
import type { SearchResponseWarning } from '@kbn/search-response-warnings';
import { MapExtent } from './map_descriptor';

export type Timeslice = {
Expand Down Expand Up @@ -91,6 +92,7 @@ export type VectorTileLayerMeta = {
export type DataRequestMeta = {
// request stop time in milliseconds since epoch
requestStopTime?: number;
warnings?: SearchResponseWarning[];
} & Partial<
SourceRequestMeta &
VectorSourceRequestMeta &
Expand Down
75 changes: 68 additions & 7 deletions x-pack/plugins/maps/public/classes/layers/layer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@
import { i18n } from '@kbn/i18n';
import type { Map as MbMap } from '@kbn/mapbox-gl';
import type { Query } from '@kbn/es-query';
import {
getWarningsTitle,
type SearchResponseWarning,
ViewDetailsPopover,
} from '@kbn/search-response-warnings';
import _ from 'lodash';
import React, { ReactElement, ReactNode } from 'react';
import { EuiIcon } from '@elastic/eui';
import { v4 as uuidv4 } from 'uuid';
import { FeatureCollection } from 'geojson';
import { DataRequest } from '../util/data_request';
import { hasIncompleteResults } from '../util/tile_meta_feature_utils';
import {
LAYER_TYPE,
MAX_ZOOM,
Expand All @@ -41,9 +47,16 @@ import { LICENSED_FEATURES } from '../../licensed_features';
import { IESSource } from '../sources/es_source';
import { TileErrorsList } from './tile_errors_list';

export interface LayerError {
export const INCOMPLETE_RESULTS_WARNING = i18n.translate(
'xpack.maps.layer.incompleteResultsWarning',
{
defaultMessage: `Layer had issues returning data and results might be incomplete.`,
}
);

export interface LayerMessage {
title: string;
error: ReactNode;
body: ReactNode;
}

export interface ILayer {
Expand Down Expand Up @@ -77,7 +90,9 @@ export interface ILayer {
isLayerLoading(zoom: number): boolean;
isFilteredByGlobalTime(): Promise<boolean>;
hasErrors(): boolean;
getErrors(): LayerError[];
getErrors(): LayerMessage[];
hasWarnings(): boolean;
getWarnings(): LayerMessage[];

/*
* ILayer.getMbLayerIds returns a list of all mapbox layers assoicated with this layer.
Expand Down Expand Up @@ -405,14 +420,14 @@ export class AbstractLayer implements ILayer {
});
}

getErrors(): LayerError[] {
const errors: LayerError[] = [];
getErrors(): LayerMessage[] {
const errors: LayerMessage[] = [];

const sourceError = this.getSourceDataRequest()?.renderError();
if (sourceError) {
errors.push({
title: this._getSourceErrorTitle(),
error: sourceError,
body: sourceError,
});
}

Expand All @@ -421,13 +436,59 @@ export class AbstractLayer implements ILayer {
title: i18n.translate('xpack.maps.layer.tileErrorTitle', {
defaultMessage: `An error occurred when loading layer tiles`,
}),
error: <TileErrorsList tileErrors={this._descriptor.__tileErrors} />,
body: <TileErrorsList tileErrors={this._descriptor.__tileErrors} />,
});
}

return errors;
}

hasWarnings(): boolean {
const hasDataRequestWarnings = this._dataRequests.some((dataRequest) => {
const dataRequestMeta = dataRequest.getMeta();
return dataRequestMeta?.warnings?.length;
});

if (hasDataRequestWarnings) {
return true;
}

return this._isTiled() ? this._getTileMetaFeatures().some(hasIncompleteResults) : false;
}

getWarnings(): LayerMessage[] {
const warningMessages: LayerMessage[] = [];

const dataRequestWarnings: SearchResponseWarning[] = [];
this._dataRequests.forEach((dataRequest) => {
const dataRequestMeta = dataRequest.getMeta();
if (dataRequestMeta?.warnings?.length) {
dataRequestWarnings.push(...dataRequestMeta.warnings);
}
});

if (dataRequestWarnings.length) {
warningMessages.push({
title: getWarningsTitle(dataRequestWarnings),
body: (
<>
{INCOMPLETE_RESULTS_WARNING}{' '}
<ViewDetailsPopover displayAsLink={true} warnings={dataRequestWarnings} />
</>
),
});
}

if (this._isTiled() && this._getTileMetaFeatures().some(hasIncompleteResults)) {
warningMessages.push({
title: '',
body: INCOMPLETE_RESULTS_WARNING,
});
}

return warningMessages;
}

async syncData(syncContext: DataRequestContext) {
// no-op by default
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
import { ISource, SourceEditorArgs } from '../../sources/source';
import { type DataRequestContext } from '../../../actions';
import { getLayersExtent } from '../../../actions/get_layers_extent';
import { ILayer, LayerIcon, LayerError } from '../layer';
import { ILayer, LayerIcon, LayerMessage } from '../layer';
import { IStyle } from '../../styles/style';
import { LICENSED_FEATURES } from '../../../licensed_features';

Expand Down Expand Up @@ -299,14 +299,33 @@ export class LayerGroup implements ILayer {
});
}

getErrors(): LayerError[] {
getErrors(): LayerMessage[] {
return this.hasErrors()
? [
{
title: i18n.translate('xpack.maps.layerGroup.childrenErrorMessage', {
defaultMessage: `An error occurred when loading nested layers`,
}),
error: '',
body: '',
},
]
: [];
}

hasWarnings(): boolean {
return this._children.some((child) => {
return child.hasWarnings();
});
}

getWarnings(): LayerMessage[] {
return this.hasWarnings()
? [
{
title: i18n.translate('xpack.maps.layerGroup.incompleteResultsWarning', {
defaultMessage: `Nested layer(s) had issues returning data and results might be incomplete.`,
}),
body: '',
},
]
: [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { i18n } from '@kbn/i18n';
import type { SearchResponseWarning } from '@kbn/search-response-warnings';
import { IVectorLayer } from '../vector_layer';
import { GeoJsonVectorLayer } from '../geojson_vector_layer';
import { IVectorStyle, VectorStyle } from '../../../styles/vector/vector_style';
Expand Down Expand Up @@ -183,7 +184,7 @@ export class BlendedVectorLayer extends GeoJsonVectorLayer implements IVectorLay
return layerDescriptor;
}

private readonly _isClustered: boolean;
private _isClustered: boolean;
private readonly _clusterSource: ESGeoGridSource;
private readonly _clusterStyle: VectorStyle;
private readonly _documentSource: ESSearchSource;
Expand Down Expand Up @@ -313,21 +314,34 @@ export class BlendedVectorLayer extends GeoJsonVectorLayer implements IVectorLay
let isSyncClustered;
try {
syncContext.startLoading(dataRequestId, requestToken, requestMeta);
const warnings: SearchResponseWarning[] = [];
isSyncClustered = !(await this._documentSource.canLoadAllDocuments(
await this.getDisplayName(this._documentSource),
requestMeta,
syncContext.registerCancelCallback.bind(null, requestToken)
syncContext.registerCancelCallback.bind(null, requestToken),
syncContext.inspectorAdapters,
(warning) => {
warnings.push(warning);
}
));
syncContext.stopLoading(dataRequestId, requestToken, { isSyncClustered }, requestMeta);
syncContext.stopLoading(
dataRequestId,
requestToken,
{ isSyncClustered },
{ ...requestMeta, warnings }
);
} catch (error) {
if (!(error instanceof DataRequestAbortError) || !isSearchSourceAbortError(error)) {
syncContext.onLoadError(dataRequestId, requestToken, error);
}
return;
}
if (isSyncClustered) {
this._isClustered = true;
activeSource = this._clusterSource;
activeStyle = this._clusterStyle;
} else {
this._isClustered = false;
activeSource = this._documentSource;
activeStyle = this._documentStyle;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { DataRequestContext } from '../../../../actions';
import { IVectorStyle, VectorStyle } from '../../../styles/vector/vector_style';
import { ISource } from '../../../sources/source';
import { IVectorSource } from '../../../sources/vector_source';
import { AbstractLayer, LayerError, LayerIcon } from '../../layer';
import { AbstractLayer, LayerMessage, LayerIcon } from '../../layer';
import {
AbstractVectorLayer,
noResultsIcon,
Expand Down Expand Up @@ -158,7 +158,7 @@ export class GeoJsonVectorLayer extends AbstractVectorLayer {
);
}

getErrors(): LayerError[] {
getErrors(): LayerMessage[] {
const errors = super.getErrors();

this.getValidJoins().forEach((join) => {
Expand All @@ -168,7 +168,7 @@ export class GeoJsonVectorLayer extends AbstractVectorLayer {
title: i18n.translate('xpack.maps.geojsonVectorLayer.joinErrorTitle', {
defaultMessage: `An error occurred when adding join metrics to layer features`,
}),
error: joinDescriptor.error,
body: joinDescriptor.error,
});
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ const mockVectorSource = {
},
} as unknown as IVectorSource;
const innerJoin = new InnerJoin(joinDescriptor, mockVectorSource);
const propertiesMap = new Map<string, Record<string | number, unknown>>();
propertiesMap.set('alpha', { [COUNT_PROPERTY_NAME]: 1 });
const joinMetrics = new Map<string, Record<string | number, unknown>>();
joinMetrics.set('alpha', { [COUNT_PROPERTY_NAME]: 1 });

test('should skip join when no state has changed', async () => {
const updateSourceData = sinon.spy();
Expand Down Expand Up @@ -170,7 +170,7 @@ test('should call updateSourceData with feature collection with updated feature
dataHasChanged: false,
join: innerJoin,
joinIndex: 0,
propertiesMap,
joinMetrics,
},
],
updateSourceData,
Expand Down Expand Up @@ -277,7 +277,7 @@ test('should call updateSourceData when no results returned from terms aggregati
dataHasChanged: true,
join: innerJoin,
joinIndex: 0,
propertiesMap: new Map<string, Record<string | number, unknown>>(),
joinMetrics: new Map<string, Record<string | number, unknown>>(),
},
],
updateSourceData,
Expand Down Expand Up @@ -321,8 +321,8 @@ test('should call onJoinError when there are no matching features', async () =>
const setJoinError = sinon.spy();

// instead of returning military alphabet like "alpha" or "bravo", mismatched key returns numbers, like '1'
const propertiesMapFromMismatchedKey = new Map<string, Record<string | number, unknown>>();
propertiesMapFromMismatchedKey.set('1', { [COUNT_PROPERTY_NAME]: 1 });
const joinMetricsFromMismatchedKey = new Map<string, Record<string | number, unknown>>();
joinMetricsFromMismatchedKey.set('1', { [COUNT_PROPERTY_NAME]: 1 });

await performInnerJoins(
{
Expand All @@ -334,7 +334,7 @@ test('should call onJoinError when there are no matching features', async () =>
dataHasChanged: true,
join: innerJoin,
joinIndex: 0,
propertiesMap: propertiesMapFromMismatchedKey,
joinMetrics: joinMetricsFromMismatchedKey,
},
],
updateSourceData,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ export async function performInnerJoins(
if (joinKey !== null) {
joinStatus.keys.push(joinKey);
}
const canJoinOnCurrent = joinState.propertiesMap
? innerJoin.joinPropertiesToFeature(feature, joinState.propertiesMap)
const canJoinOnCurrent = joinState.joinMetrics
? innerJoin.joinPropertiesToFeature(feature, joinState.joinMetrics)
: false;
if (canJoinOnCurrent && !joinStatus.joinedWithAtLeastOneFeature) {
joinStatus.joinedWithAtLeastOneFeature = true;
Expand Down Expand Up @@ -108,8 +108,7 @@ async function getJoinError(joinStatus: {
return;
}

const hasTerms =
joinStatus.joinState.propertiesMap && joinStatus.joinState.propertiesMap.size > 0;
const hasTerms = joinStatus.joinState.joinMetrics && joinStatus.joinState.joinMetrics.size > 0;

if (!hasTerms || joinStatus.joinedWithAtLeastOneFeature) {
return;
Expand All @@ -129,9 +128,7 @@ async function getJoinError(joinStatus: {
leftFieldName,
leftFieldValues: prettyPrintArray(joinStatus.keys),
rightFieldName,
rightFieldValues: prettyPrintArray(
Array.from(joinStatus.joinState.propertiesMap!.keys())
),
rightFieldValues: prettyPrintArray(Array.from(joinStatus.joinState.joinMetrics!.keys())),
},
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ export interface JoinState {
dataHasChanged: boolean;
join: InnerJoin;
joinIndex: number;
propertiesMap?: PropertiesMap;
joinMetrics?: PropertiesMap;
}
Loading

0 comments on commit 0b24e40

Please sign in to comment.