Skip to content

Commit

Permalink
[Lens][TSVB][Agg based] TSDB warning handling support for Lens, Agg b…
Browse files Browse the repository at this point in the history
…ased and TSVB (elastic#136833)

* start integrating TSDB into Lens

* current state

* make new type filter work

* fix translations

* handle warnings

* clean up and make warnings better

* split function list for referenced fields too and extend list of unsupported functions

* improve warnings

* fix a bug

* add todo for utc timezone

* adding TSDB support to DataViews

* adding test

* date_histogram: force fixed interval for tsdb rollups

* fixing types

* fixing types

* fixing types

* fixing types

* handling edge cases

* removing undefined entries from FieldSpec

* renaming .meta property

* enforce UTC timezone

* isRolledUpField helper

* updating snapshot

* adjust fixed interval and utc timezone for timelion and tsvb

* fixed interval mapping

* fixing test

* add showWarning to search service

* add comments

* add unit tests

* test foo

* cleanup

* add s to property name in test

* comments for api items

* fixing test

* use the warnings when calling showWarnings

* change showWarning to just show a single warning

* put handleWarnings on the request adapter

* comment

* simplify 1

* fix lens unit test

* fix PR

* remove duplicate identifier

* remove underscoring for unused variables

* revert inspector changes, extract the response warnings in the search service

* fix bug

* remove console.log

* re-apply typescript fixes to app test code

* declutter

* add test, improve comments

* fix some unexported public api items

* include rawResponse in the warning structure

* fix lint

* tweak clean up example app

* SearchResponseWarnings and SearchResponseWarningNotification

* fix export bug

* not include shardStats if there are no warnings

* Update src/plugins/data/common/search/types.ts

* fix duplicated entries

* remove empty line

* simplify SearchResponseWarnings interface

* remove array copying

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* comments for api_docs

* simplify per feedback

* Pass callback to handleResponse in showWarnings

* export more public types

* update example to make possible to show shard failure

* pr cleanup

* eslint fix

* allow example app to not show default warnings

* move extractWarning and related types to inspector plugin

* wip functional test of example app

* fix test references

* finish functional test

* relocate extractWarnings back to search/fetch

* fix test

* remove need for isTimeout, isShardFailure

* ts fix

* improve test

* Change showWarnings to accept the RequestAdapter

* use showWarnings in vis_types/timeseries

* more tests

* use handle_warning name

* fix ts

* add reason field to SearchResponseWarning

* fix component snapshot

* update comments

* test cleanup

* fix test

* ensure notification appears only once

* fix and cleanup

* fix ts

* fix response.json bug

* use top-level type, and lower-level reason.type

* cleanup

* change warning structure

* change warning infrastructure

* fix

* allows to disable automatic handling of shard warnings on expressions

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* fix tests

* fix tsconfig json

* allows to disable automatic handling of shard warnings on expressions

* fix tests

* allows to disable automatic handling of shard warnings on expressions

* disable warnings the right way

* fix stuff and clean up

* random cleanup

* fix test

* remove unused function

* review comments

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* style warnings and switch to real error message

Co-authored-by: ppisljar <[email protected]>
Co-authored-by: Timothy Sullivan <[email protected]>
Co-authored-by: Tim Sullivan <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
5 people authored and guskovaue committed Sep 12, 2022
1 parent 8dbdf9a commit 3f69e0c
Show file tree
Hide file tree
Showing 27 changed files with 285 additions and 33 deletions.
1 change: 1 addition & 0 deletions src/plugins/charts/public/static/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export { EmptyPlaceholder } from './empty_placeholder';

export { useCommonChartStyles } from './common_chart_styles';
export * from './endzones';
export * from './warnings';

/**
* The Lazily-loaded `ColorPicker` component. Consumers should use `React.Suspense` or
Expand Down
52 changes: 52 additions & 0 deletions src/plugins/charts/public/static/components/warnings.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { EuiButtonEmpty, EuiHorizontalRule, EuiPopover, EuiText } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { euiThemeVars } from '@kbn/ui-theme';
import React, { useState } from 'react';

export function Warnings({ warnings }: { warnings: React.ReactNode[] }) {
const [open, setOpen] = useState(false);
if (warnings.length === 0) return null;
return (
<>
<EuiPopover
isOpen={open}
panelPaddingSize="none"
closePopover={() => setOpen(false)}
button={
<EuiButtonEmpty color="warning" iconType="alert" onClick={() => setOpen(!open)} size="xs">
{i18n.translate('charts.warning.warningLabel', {
defaultMessage:
'{numberWarnings, number} {numberWarnings, plural, one {warning} other {warnings}}',
values: {
numberWarnings: warnings.length,
},
})}
</EuiButtonEmpty>
}
>
<div style={{ maxWidth: 512 }}>
{warnings.map((w, i) => (
<React.Fragment key={i}>
<div
css={{
padding: euiThemeVars.euiSizeS,
}}
>
<EuiText size="s">{w}</EuiText>
</div>
{i < warnings.length - 1 && <EuiHorizontalRule margin="none" />}
</React.Fragment>
))}
</div>
</EuiPopover>
</>
);
}
5 changes: 3 additions & 2 deletions src/plugins/vis_types/timeseries/public/metrics_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import {
extractIndexPatternValues,
isStringTypeIndexPattern,
} from '../common/index_patterns_utils';
import { TSVB_DEFAULT_COLOR } from '../common/constants';
import { TSVB_DEFAULT_COLOR, UI_SETTINGS } from '../common/constants';
import { toExpressionAst } from './to_ast';
import { getDataViewsStart } from './services';
import { getDataViewsStart, getUISettings } from './services';
import type { TimeseriesVisDefaultParams, TimeseriesVisParams } from './types';
import type { IndexPatternValue, Panel } from '../common/types';
import { convertTSVBtoLensConfiguration } from './convert_to_lens';
Expand Down Expand Up @@ -176,5 +176,6 @@ export const metricsVisDefinition: VisTypeDefinition<
requests: new RequestAdapter(),
}),
requiresSearch: true,
suppressWarnings: () => !getUISettings().get(UI_SETTINGS.ALLOW_CHECKING_FOR_FAILED_SHARDS),
getUsedIndexPattern: getUsedIndexPatterns,
};
10 changes: 1 addition & 9 deletions src/plugins/vis_types/timeseries/public/request_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { Adapters } from '@kbn/inspector-plugin/common';
import { KibanaContext } from '@kbn/data-plugin/public';
import { getTimezone } from './application/lib/get_timezone';
import { getUISettings, getDataStart, getCoreStart } from './services';
import { ROUTES, UI_SETTINGS } from '../common/constants';
import { ROUTES } from '../common/constants';

import type { TimeseriesVisParams } from './types';
import type { TimeseriesVisData } from '../common/types';
Expand Down Expand Up @@ -84,14 +84,6 @@ export const metricsRequestHandler = async ({
?.start(query.label ?? key, { searchSessionId })
.json(query.body)
.ok({ time: query.time, json: { rawResponse: query.response } });

if (
query.response &&
inspectorAdapters?.requests &&
config.get(UI_SETTINGS.ALLOW_CHECKING_FOR_FAILED_SHARDS)
) {
data.search.showWarnings(inspectorAdapters.requests);
}
});

return visData;
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/visualizations/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"ui": true,
"requiredPlugins": [
"data",
"charts",
"expressions",
"fieldFormats",
"uiActions",
Expand All @@ -19,7 +20,7 @@
"dataViewEditor"
],
"optionalPlugins": ["home", "share", "spaces", "savedObjectsTaggingOss"],
"requiredBundles": ["kibanaUtils", "savedSearch", "kibanaReact"],
"requiredBundles": ["kibanaUtils", "savedSearch", "kibanaReact", "charts"],
"extraPublicDirs": ["common/constants", "common/utils", "common/expression_functions"],
"owner": {
"name": "Vis Editors",
Expand Down
7 changes: 7 additions & 0 deletions src/plugins/visualizations/public/embeddable/embeddables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@
}
}

.visPanel__warnings {
position: absolute;
z-index: 2;
right: $euiSizeM;
bottom: $euiSizeM;
}

// OPTIONS MENU

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type { ErrorLike } from '@kbn/expressions-plugin/common';
import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { TimefilterContract } from '@kbn/data-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
import { Warnings } from '@kbn/charts-plugin/public';
import {
Adapters,
AttributeService,
Expand Down Expand Up @@ -115,6 +116,7 @@ export class VisualizeEmbeddable
private expression?: ExpressionAstExpression;
private vis: Vis;
private domNode: any;
private warningDomNode: any;
public readonly type = VISUALIZE_EMBEDDABLE_TYPE;
private abortController?: AbortController;
private readonly deps: VisualizeEmbeddableFactoryDeps;
Expand Down Expand Up @@ -332,6 +334,31 @@ export class VisualizeEmbeddable
return dirty;
}

private handleWarnings() {
const warnings: React.ReactNode[] = [];
if (this.getInspectorAdapters()?.requests) {
this.deps
.start()
.plugins.data.search.showWarnings(this.getInspectorAdapters()!.requests!, (warning) => {
if (
warning.type === 'shard_failure' &&
warning.reason.type === 'unsupported_aggregation_on_downsampled_index'
) {
warnings.push(warning.reason.reason || warning.message);
return true;
}
if (this.vis.type.suppressWarnings?.()) {
// if the vis type wishes to supress all warnings, return true so the default logic won't pick it up
return true;
}
});
}

if (this.warningDomNode) {
render(<Warnings warnings={warnings || []} />, this.warningDomNode);
}
}

// this is a hack to make editor still work, will be removed once we clean up editor
// @ts-ignore
hasInspector = () => Boolean(this.getInspectorAdapters());
Expand All @@ -347,6 +374,7 @@ export class VisualizeEmbeddable
};

onContainerData = () => {
this.handleWarnings();
this.updateOutput({
...this.getOutput(),
loading: false,
Expand Down Expand Up @@ -386,6 +414,11 @@ export class VisualizeEmbeddable
div.className = `visualize panel-content panel-content--fullWidth`;
domNode.appendChild(div);

const warningDiv = document.createElement('div');
warningDiv.className = 'visPanel__warnings';
domNode.appendChild(warningDiv);
this.warningDomNode = warningDiv;

this.domNode = div;
super.render(this.domNode);

Expand Down Expand Up @@ -532,6 +565,7 @@ export class VisualizeEmbeddable
timeRange: this.timeRange,
query: this.input.query,
filters: this.input.filters,
disableShardWarnings: true,
},
variables: {
embeddableTitle: this.getTitle(),
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/visualizations/public/vis_types/base_vis_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export class BaseVisType<TVisParams = VisParams> {
public readonly editorConfig;
public hidden;
public readonly requiresSearch;
public readonly suppressWarnings;
public readonly hasPartialRows;
public readonly hierarchicalData;
public readonly setup;
Expand All @@ -64,6 +65,7 @@ export class BaseVisType<TVisParams = VisParams> {
this.title = opts.title;
this.icon = opts.icon;
this.image = opts.image;
this.suppressWarnings = opts.suppressWarnings;
this.visConfig = defaultsDeep({}, opts.visConfig, { defaults: {} });
this.editorConfig = defaultsDeep({}, opts.editorConfig, { collections: {} });
this.options = defaultsDeep({}, opts.options, defaultOptions);
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/visualizations/public/vis_types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ export interface VisTypeDefinition<TVisParams> {
* It sets the vis type on a deprecated mode when is true
*/
readonly isDeprecated?: boolean;
/**
* If returns true, no warning toasts will be shown
*/
readonly suppressWarnings?: () => boolean;
/**
* Describes the experience group that the visualization belongs.
* It can be on tools, aggregation based or promoted group.
Expand Down
1 change: 1 addition & 0 deletions src/plugins/visualizations/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
],
"references": [
{ "path": "../../core/tsconfig.json" },
{ "path": "../charts/tsconfig.json" },
{ "path": "../data/tsconfig.json" },
{ "path": "../data_views/tsconfig.json" },
{ "path": "../expressions/tsconfig.json" },
Expand Down
2 changes: 1 addition & 1 deletion test/examples/search/warnings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const testRollupIndex = 'sample-01-rollup';
const testRollupField = 'kubernetes.container.memory.usage.bytes';
const toastsSelector = '[data-test-subj=globalToastList] [data-test-subj=euiToastHeader]';
const shardFailureType = 'unsupported_aggregation_on_rollup_index';
const shardFailureType = 'unsupported_aggregation_on_downsampled_index';
const shardFailureReason = `Field [${testRollupField}] of type [aggregate_metric_double] is not supported for aggregation [percentiles]`;

const getTestJson = async (tabTestSubj: string, codeTestSubj: string) => {
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/lens/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
"kibanaUtils",
"kibanaReact",
"embeddable",
"fieldFormats"
"fieldFormats",
"charts"
],
"owner": {
"name": "Vis Editors",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
@include euiYScroll;
max-height: $euiSize * 20;
width: $euiSize * 16;
overflow-wrap: break-word;
}

.lnsWorkspaceWarningList__item {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,10 @@ describe('workspace_panel', () => {

expect(mounted.lensStore.dispatch).toHaveBeenCalledWith({
type: 'lens/onActiveDataChange',
payload: tablesData,
payload: {
activeData: tablesData,
requestWarnings: [],
},
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,22 +217,34 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({
if (renderDeps.current) {
const [defaultLayerId] = Object.keys(renderDeps.current.datasourceLayers);

const requestWarnings: string[] = [];
const datasource = Object.values(renderDeps.current?.datasourceMap)[0];
const datasourceState = Object.values(renderDeps.current?.datasourceStates)[0].state;
if (adapters?.requests) {
plugins.data.search.showWarnings(adapters.requests, (warning) => {
const warningMessage = datasource.getSearchWarningMessages?.(datasourceState, warning);

requestWarnings.push(...(warningMessage || []));
if (warningMessage && warningMessage.length) return true;
});
}
if (adapters && adapters.tables) {
dispatchLens(
onActiveDataChange(
Object.entries(adapters.tables?.tables).reduce<Record<string, Datatable>>(
onActiveDataChange({
activeData: Object.entries(adapters.tables?.tables).reduce<Record<string, Datatable>>(
(acc, [key, value], index, tables) => ({
...acc,
[tables.length === 1 ? defaultLayerId : key]: value,
}),
{}
)
)
),
requestWarnings,
})
);
}
}
},
[dispatchLens]
[dispatchLens, plugins.data.search]
);

const shouldApplyExpression = autoApplyEnabled || !initialRenderComplete.current || triggerApply;
Expand Down Expand Up @@ -606,6 +618,7 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({
datasourceMap={datasourceMap}
visualizationMap={visualizationMap}
isFullscreen={isFullscreen}
lensInspector={lensInspector}
>
{renderWorkspace()}
</WorkspacePanelWrapper>
Expand Down Expand Up @@ -656,6 +669,7 @@ export const VisualizationWrapper = ({
to: context.dateRange.toDate,
},
filters: context.filters,
disableShardWarnings: true,
}),
[context]
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
selectTriggerApplyChanges,
} from '../../../state_management';
import { enableAutoApply, setChangesApplied } from '../../../state_management/lens_slice';
import { LensInspector } from '../../../lens_inspector_service';

describe('workspace_panel_wrapper', () => {
let mockVisualization: jest.Mocked<Visualization>;
Expand All @@ -39,6 +40,7 @@ describe('workspace_panel_wrapper', () => {
datasourceMap={{}}
datasourceStates={{}}
isFullscreen={false}
lensInspector={{} as unknown as LensInspector}
>
<MyChild />
</WorkspacePanelWrapper>
Expand All @@ -60,6 +62,7 @@ describe('workspace_panel_wrapper', () => {
datasourceMap={{}}
datasourceStates={{}}
isFullscreen={false}
lensInspector={{} as unknown as LensInspector}
/>
);

Expand Down Expand Up @@ -114,6 +117,7 @@ describe('workspace_panel_wrapper', () => {
datasourceMap={{}}
datasourceStates={{}}
isFullscreen={false}
lensInspector={{} as unknown as LensInspector}
>
<div />
</WorkspacePanelWrapper>
Expand Down
Loading

0 comments on commit 3f69e0c

Please sign in to comment.