Skip to content

Commit

Permalink
[Infrastructure UI] Improve error handling metric index (#152840)
Browse files Browse the repository at this point in the history
## 📓 Summary

Closes #144882 

This PR updates the error handling in case the user configure a remote
index that was returning an error, giving more meaningful feedback to
the user.

It also includes a refactor for the `useSource` custom hook and removes
the usages for the legacy `useSourceViaHttp` custom hook.

## 🧪 Testing

- Navigate to the metrics setting page
- Set a wrongly formatted or non-existing remote cluster
- Verify to get a toast notification about the issue and to get notified
about the same issue when accessing inventory and hosts pages.

**Before**
<img width="1741" alt="before"
src="https://user-images.githubusercontent.com/34506779/223672635-abb3a7f3-e8a6-43b6-972c-813514d1b63a.png">

**After**


https://user-images.githubusercontent.com/34506779/223672676-017422fb-ccc7-456e-854c-ec79837da281.mp4

## 👣 Next step
As happens in the Data View stack management section, would be nice to
inform the user while he types the index pattern if no associated data
view matches the typed wildcard.
An additional value could be, in case the user wants to anyway set the
index pattern, to inform him with a callout message that it doesn't
match any data view, so that the issue cause is immediately clear when
no data are viewed on the metrics pages.

- [ ] #152885

---------

Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
  • Loading branch information
4 people authored Mar 23, 2023
1 parent 293dd26 commit ca8848e
Show file tree
Hide file tree
Showing 37 changed files with 530 additions and 535 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/infra/common/metrics_sources/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const metricsSourceConfigurationOriginRT = rt.keyof({

export const metricsSourceStatusRT = rt.strict({
metricIndicesExist: SourceStatusRuntimeType.props.metricIndicesExist,
remoteClustersExist: SourceStatusRuntimeType.props.metricIndicesExist,
indexFields: SourceStatusRuntimeType.props.indexFields,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ export type InfraSourceIndexField = rt.TypeOf<typeof SourceStatusFieldRuntimeTyp
export const SourceStatusRuntimeType = rt.type({
logIndicesExist: rt.boolean,
metricIndicesExist: rt.boolean,
remoteClustersExist: rt.boolean,
indexFields: rt.array(SourceStatusFieldRuntimeType),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import { SnapshotCustomMetricInput } from '../../../../common/http_api/snapshot_
import { AlertContextMeta, defaultExpression, ExpressionRow, Expressions } from './expression';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';

jest.mock('../../../containers/metrics_source/use_source_via_http', () => ({
useSourceViaHttp: () => ({
jest.mock('../../../containers/metrics_source/source', () => ({
withSourceProvider: () => jest.fn,
useSourceContext: () => ({
source: { id: 'default' },
createDerivedIndexPattern: () => ({ fields: [], title: 'metricbeat-*' }),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,11 @@ import {
SnapshotMetricTypeRT,
} from '../../../../common/inventory_models/types';
import { toMetricOpt } from '../../../../common/snapshot_metric_i18n';
import { DerivedIndexPattern } from '../../../containers/metrics_source';
import { useSourceViaHttp } from '../../../containers/metrics_source/use_source_via_http';
import { useKibanaContextForPlugin } from '../../../hooks/use_kibana';
import {
DerivedIndexPattern,
useSourceContext,
withSourceProvider,
} from '../../../containers/metrics_source';
import { InfraWaffleMapOptions } from '../../../lib/lib';
import { MetricsExplorerKueryBar } from '../../../pages/metrics/metrics_explorer/components/kuery_bar';
import { convertKueryToElasticSearchQuery } from '../../../utils/kuery';
Expand Down Expand Up @@ -104,13 +106,9 @@ export const defaultExpression = {
} as InventoryMetricConditions;

export const Expressions: React.FC<Props> = (props) => {
const { http, notifications } = useKibanaContextForPlugin().services;
const { setRuleParams, ruleParams, errors, metadata } = props;
const { source, createDerivedIndexPattern } = useSourceViaHttp({
sourceId: 'default',
fetch: http.fetch,
toastWarning: notifications.toasts.addWarning,
});
const { source, createDerivedIndexPattern } = useSourceContext();

const [timeSize, setTimeSize] = useState<number | undefined>(1);
const [timeUnit, setTimeUnit] = useState<TimeUnitChar>('m');

Expand Down Expand Up @@ -400,7 +398,7 @@ export const Expressions: React.FC<Props> = (props) => {

// required for dynamic import
// eslint-disable-next-line import/no-default-export
export default Expressions;
export default withSourceProvider<Props>(Expressions)('default');

interface ExpressionRowProps {
nodeType: InventoryItemType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ import { Expression, AlertContextMeta } from './expression';
import { act } from 'react-dom/test-utils';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';

jest.mock('../../../containers/metrics_source/use_source_via_http', () => ({
useSourceViaHttp: () => ({
jest.mock('../../../containers/metrics_source/source', () => ({
withSourceProvider: () => jest.fn,
useSourceContext: () => ({
source: { id: 'default' },
createDerivedIndexPattern: () => ({ fields: [], title: 'metricbeat-*' }),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@ import {
RuleTypeParamsExpressionProps,
WhenExpression,
} from '@kbn/triggers-actions-ui-plugin/public';
import { useSourceContext, withSourceProvider } from '../../../containers/metrics_source';
import { MetricAnomalyParams } from '../../../../common/alerting/metrics';
import { ANOMALY_THRESHOLD } from '../../../../common/infra_ml';
import { findInventoryModel } from '../../../../common/inventory_models';
import { InventoryItemType, SnapshotMetricType } from '../../../../common/inventory_models/types';
import { SubscriptionSplashPrompt } from '../../../components/subscription_splash_content';
import { useSourceViaHttp } from '../../../containers/metrics_source/use_source_via_http';
import { useInfraMLCapabilities } from '../../../containers/ml/infra_ml_capabilities';
import { useKibanaContextForPlugin } from '../../../hooks/use_kibana';
import { useActiveKibanaSpace } from '../../../hooks/use_kibana_space';
import { InfraWaffleMapOptions } from '../../../lib/lib';
import { InfluencerFilter } from './influencer_filter';
Expand Down Expand Up @@ -51,15 +50,10 @@ export const defaultExpression = {

export const Expression: React.FC<Props> = (props) => {
const { hasInfraMLCapabilities, isLoading: isLoadingMLCapabilities } = useInfraMLCapabilities();
const { http, notifications } = useKibanaContextForPlugin().services;
const { space } = useActiveKibanaSpace();

const { setRuleParams, ruleParams, ruleInterval, metadata } = props;
const { source, createDerivedIndexPattern } = useSourceViaHttp({
sourceId: 'default',
fetch: http.fetch,
toastWarning: notifications.toasts.addWarning,
});
const { source, createDerivedIndexPattern } = useSourceContext();

const derivedIndexPattern = useMemo(
() => createDerivedIndexPattern(),
Expand Down Expand Up @@ -247,7 +241,7 @@ export const Expression: React.FC<Props> = (props) => {

// required for dynamic import
// eslint-disable-next-line import/no-default-export
export default Expression;
export default withSourceProvider<Props>(Expression)('default');

const StyledExpressionRow = euiStyled(EuiFlexGroup)`
display: flex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@ import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
import { render } from '@testing-library/react';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { buildMetricThresholdRule } from '../mocks/metric_threshold_rule';
import AlertDetailsAppSection from './alert_details_app_section';
import { AlertDetailsAppSection } from './alert_details_app_section';

jest.mock('../../../hooks/use_kibana', () => ({
useKibanaContextForPlugin: () => ({
services: mockCoreMock.createStart(),
}),
}));

jest.mock('../../../containers/metrics_source/use_source_via_http', () => ({
useSourceViaHttp: () => ({
jest.mock('../../../containers/metrics_source/source', () => ({
withSourceProvider: () => jest.fn,
useSourceContext: () => ({
source: { id: 'default' },
createDerivedIndexPattern: () => ({ fields: [], title: 'metricbeat-*' }),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@
import { EuiFlexGroup, EuiFlexItem, EuiPanel } from '@elastic/eui';
import React, { useMemo } from 'react';
import { Rule } from '@kbn/alerting-plugin/common';
import { useSourceContext, withSourceProvider } from '../../../containers/metrics_source';
import { MetricThresholdRuleTypeParams } from '..';
import { generateUniqueKey } from '../lib/generate_unique_key';
import { MetricsExplorerChartType } from '../../../pages/metrics/metrics_explorer/hooks/use_metrics_explorer_options';
import { ExpressionChart } from './expression_chart';
import { useSourceViaHttp } from '../../../containers/metrics_source/use_source_via_http';
import { useKibanaContextForPlugin } from '../../../hooks/use_kibana';

// TODO Use a generic props for app sections https://github.com/elastic/kibana/issues/152690
interface AppSectionProps {
Expand All @@ -26,12 +25,8 @@ interface AppSectionProps {
}

export function AlertDetailsAppSection({ rule }: AppSectionProps) {
const { http, notifications } = useKibanaContextForPlugin().services;
const { source, createDerivedIndexPattern } = useSourceViaHttp({
sourceId: 'default',
fetch: http.fetch,
toastWarning: notifications.toasts.addWarning,
});
const { source, createDerivedIndexPattern } = useSourceContext();

const derivedIndexPattern = useMemo(
() => createDerivedIndexPattern(),
[createDerivedIndexPattern]
Expand All @@ -58,4 +53,4 @@ export function AlertDetailsAppSection({ rule }: AppSectionProps) {
}

// eslint-disable-next-line import/no-default-export
export default AlertDetailsAppSection;
export default withSourceProvider<AppSectionProps>(AlertDetailsAppSection)('default');
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import { MetricsExplorerMetric } from '../../../../common/http_api/metrics_explo
import { Expressions } from './expression';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';

jest.mock('../../../containers/metrics_source/use_source_via_http', () => ({
useSourceViaHttp: () => ({
jest.mock('../../../containers/metrics_source/source', () => ({
withSourceProvider: () => jest.fn,
useSourceContext: () => ({
source: { id: 'default' },
createDerivedIndexPattern: () => ({ fields: [], title: 'metricbeat-*' }),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import {
RuleTypeParamsExpressionProps,
} from '@kbn/triggers-actions-ui-plugin/public';
import { TimeUnitChar } from '@kbn/observability-plugin/common/utils/formatters/duration';
import { useSourceContext, withSourceProvider } from '../../../containers/metrics_source';
import { Aggregators, Comparator, QUERY_INVALID } from '../../../../common/alerting/metrics';
import { useSourceViaHttp } from '../../../containers/metrics_source/use_source_via_http';
import { useKibanaContextForPlugin } from '../../../hooks/use_kibana';
import { MetricsExplorerGroupBy } from '../../../pages/metrics/metrics_explorer/components/group_by';
import { MetricsExplorerKueryBar } from '../../../pages/metrics/metrics_explorer/components/kuery_bar';
Expand All @@ -56,12 +56,8 @@ export { defaultExpression };

export const Expressions: React.FC<Props> = (props) => {
const { setRuleParams, ruleParams, errors, metadata } = props;
const { http, notifications, docLinks } = useKibanaContextForPlugin().services;
const { source, createDerivedIndexPattern } = useSourceViaHttp({
sourceId: 'default',
fetch: http.fetch,
toastWarning: notifications.toasts.addWarning,
});
const { docLinks } = useKibanaContextForPlugin().services;
const { source, createDerivedIndexPattern } = useSourceContext();

const [timeSize, setTimeSize] = useState<number | undefined>(1);
const [timeUnit, setTimeUnit] = useState<TimeUnitChar | undefined>('m');
Expand Down Expand Up @@ -501,4 +497,4 @@ const docCountNoDataDisabledHelpText = i18n.translate(

// required for dynamic import
// eslint-disable-next-line import/no-default-export
export default Expressions;
export default withSourceProvider<Props>(Expressions)('default');
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import { CUSTOM_EQUATION } from '../i18n_strings';
interface Props {
expression: MetricExpression;
derivedIndexPattern: DataViewBase;
source: MetricsSourceConfiguration | null;
source?: MetricsSourceConfiguration;
filterQuery?: string;
groupBy?: string | string[];
chartType?: MetricsExplorerChartType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import { Comparator } from '../../../../common/alerting/metrics';
import { MetricExpression } from '../types';
import { ExpressionRow } from './expression_row';

jest.mock('../../../containers/metrics_source/use_source_via_http', () => ({
useSourceViaHttp: () => ({
jest.mock('../../../containers/metrics_source/source', () => ({
withSourceProvider: () => jest.fn,
useSourceContext: () => ({
source: { id: 'default' },
createDerivedIndexPattern: () => ({ fields: [], title: 'metricbeat-*' }),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { MetricExplorerCustomMetricAggregations } from '../../../../common/http_
export const useMetricsExplorerChartData = (
expression: MetricExpression,
derivedIndexPattern: DataViewBase,
source: MetricsSourceConfiguration | null,
source?: MetricsSourceConfiguration,
filterQuery?: string,
groupBy?: string | string[]
) => {
Expand Down
37 changes: 20 additions & 17 deletions x-pack/plugins/infra/public/apps/metrics_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { InfraClientStartDeps, InfraClientStartExports } from '../types';
import { RedirectWithQueryParams } from '../utils/redirect_with_query_params';
import { CommonInfraProviders, CoreProviders } from './common_providers';
import { prepareMountElement } from './common_styles';
import { SourceProvider } from '../containers/metrics_source';

export const renderApp = (
core: CoreStart,
Expand Down Expand Up @@ -70,23 +71,25 @@ const MetricsApp: React.FC<{
theme$={theme$}
triggersActionsUI={plugins.triggersActionsUi}
>
<Router history={history}>
<Switch>
<Route path="/link-to" component={LinkToMetricsPage} />
{uiCapabilities?.infrastructure?.show && (
<RedirectWithQueryParams from="/" exact={true} to="/inventory" />
)}
{uiCapabilities?.infrastructure?.show && (
<RedirectWithQueryParams from="/snapshot" exact={true} to="/inventory" />
)}
{uiCapabilities?.infrastructure?.show && (
<RedirectWithQueryParams from="/metrics-explorer" exact={true} to="/explorer" />
)}
{uiCapabilities?.infrastructure?.show && (
<Route path="/" component={InfrastructurePage} />
)}
</Switch>
</Router>
<SourceProvider sourceId="default">
<Router history={history}>
<Switch>
<Route path="/link-to" component={LinkToMetricsPage} />
{uiCapabilities?.infrastructure?.show && (
<RedirectWithQueryParams from="/" exact={true} to="/inventory" />
)}
{uiCapabilities?.infrastructure?.show && (
<RedirectWithQueryParams from="/snapshot" exact={true} to="/inventory" />
)}
{uiCapabilities?.infrastructure?.show && (
<RedirectWithQueryParams from="/metrics-explorer" exact={true} to="/explorer" />
)}
{uiCapabilities?.infrastructure?.show && (
<Route path="/" component={InfrastructurePage} />
)}
</Switch>
</Router>
</SourceProvider>
</CommonInfraProviders>
</CoreProviders>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
export * from './no_metric_indices';
export { NoData } from './no_data';
export { NoIndices } from './no_indices';
export { NoRemoteCluster } from './no_remote_cluster';
19 changes: 11 additions & 8 deletions x-pack/plugins/infra/public/components/empty_states/no_indices.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,30 @@
* 2.0.
*/

import { KibanaPageTemplate } from '@kbn/shared-ux-page-kibana-template';
import React from 'react';
import styled from '@emotion/styled';
import { EuiEmptyPromptProps } from '@elastic/eui';
import { KibanaPageTemplate } from '@kbn/shared-ux-page-kibana-template';
import { PageTemplate } from '../page_template';

interface NoIndicesProps {
message: string;
interface NoIndicesProps extends Omit<EuiEmptyPromptProps, 'body' | 'title'> {
body: string;
title: string;
actions: React.ReactNode;
'data-test-subj'?: string;
}

// Represents a fully constructed page, including page template.
export const NoIndices: React.FC<NoIndicesProps> = ({ actions, message, title, ...rest }) => {
export const NoIndices: React.FC<NoIndicesProps> = ({ body, title, ...rest }) => {
return (
<PageTemplate isEmptyState={true}>
<KibanaPageTemplate.EmptyPrompt
title={<h2>{title}</h2>}
body={<p>{message}</p>}
actions={actions}
body={<PreLineText>{body}</PreLineText>}
{...rest}
/>
</PageTemplate>
);
};

const PreLineText = styled.p`
white-space: pre-line;
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import { EuiButton } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { useLinkProps } from '@kbn/observability-plugin/public';
import { NoIndices } from './no_indices';

export const NoRemoteCluster = () => {
const settingLinkProps = useLinkProps({ app: 'metrics', pathname: '/settings' });

const goToSettings = (
<EuiButton data-test-subj="infraHostsPageGoToSettings" color="danger" {...settingLinkProps}>
{i18n.translate('xpack.infra.hostsPage.goToMetricsSettings', {
defaultMessage: 'Check settings',
})}
</EuiButton>
);

return (
<NoIndices
color="danger"
iconType="error"
titleSize="m"
title={i18n.translate('xpack.infra.sourceConfiguration.noRemoteClusterTitle', {
defaultMessage: "Couldn't connect to the remote cluster",
})}
body={i18n.translate('xpack.infra.sourceConfiguration.noRemoteClusterMessage', {
defaultMessage:
"We're unable to connect to the remote cluster, which is preventing us from retrieving the metrics and data you need.\nTo resolve this issue, please check your indices configuration and ensure that it's properly configured.",
})}
actions={[goToSettings]}
/>
);
};
6 changes: 5 additions & 1 deletion x-pack/plugins/infra/public/components/source_error_page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ export const SourceErrorPage: React.FunctionComponent<SourceErrorPageProps> = ({
defaultMessage="Failed to load data sources."
/>
}
detailedMessage={<code>{errorMessage}</code>}
detailedMessage={
<pre>
<code>{errorMessage}</code>
</pre>
}
retry={retry}
/>
);
Loading

0 comments on commit ca8848e

Please sign in to comment.