From ea4c0d5c4a3b92b5d6485d45e5f8fac450dba677 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Mon, 9 Sep 2019 14:08:23 -0700 Subject: [PATCH] [APM] Fix indefinite loading state in agent settings for unauthorized user roles (#44970) * - handle unauthorized error to return empty list of agent settings - display to user when a failure occurred in settings initialization - fix incorrect settings link path * - moved agent config index creation to the plugin setup step - make failure text reflect a general failure to load settings * - Rename Settings/SettingsList -> Settings/index - Add Settings/SettingsList to render only the list/messaging - Mention permissions issue in failure message --- .../app/Main/route_config/index.tsx | 4 +- .../Settings/AddSettings/AddSettingFlyout.tsx | 2 +- .../AddSettings/AddSettingFlyoutBody.tsx | 2 +- .../components/app/Settings/SettingsList.tsx | 231 ++++-------------- .../public/components/app/Settings/index.tsx | 183 ++++++++++++++ .../shared/Links/apm/SettingsLink.tsx | 2 +- .../create_agent_config_index.ts | 7 +- .../plugins/apm/server/new-platform/plugin.ts | 3 +- .../plugins/apm/server/routes/settings.ts | 13 +- 9 files changed, 251 insertions(+), 196 deletions(-) create mode 100644 x-pack/legacy/plugins/apm/public/components/app/Settings/index.tsx diff --git a/x-pack/legacy/plugins/apm/public/components/app/Main/route_config/index.tsx b/x-pack/legacy/plugins/apm/public/components/app/Main/route_config/index.tsx index fe208f08ac4ec..3bba5ebe8faab 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/Main/route_config/index.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/Main/route_config/index.tsx @@ -13,7 +13,7 @@ import { TransactionDetails } from '../../TransactionDetails'; import { Home } from '../../Home'; import { BreadcrumbRoute } from '../ProvideBreadcrumbs'; import { RouteName } from './route_names'; -import { SettingsList } from '../../Settings/SettingsList'; +import { Settings } from '../../Settings'; import { toQuery } from '../../../shared/Links/url_helpers'; interface RouteParams { @@ -60,7 +60,7 @@ export const routes: BreadcrumbRoute[] = [ { exact: true, path: '/settings', - component: SettingsList, + component: Settings, breadcrumb: i18n.translate('xpack.apm.breadcrumb.listSettingsTitle', { defaultMessage: 'Settings' }), diff --git a/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx b/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx index 52813b0577bbe..220d78c007d71 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx @@ -24,10 +24,10 @@ import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { transactionSampleRateRt } from '../../../../../common/runtime_types/transaction_sample_rate_rt'; import { AddSettingFlyoutBody } from './AddSettingFlyoutBody'; -import { Config } from '../SettingsList'; import { useFetcher } from '../../../../hooks/useFetcher'; import { ENVIRONMENT_NOT_DEFINED } from '../../../../../common/environment_filter_values'; import { callApmApi } from '../../../../services/rest/callApmApi'; +import { Config } from '..'; interface Props { onClose: () => void; diff --git a/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx b/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx index 0d92020856c24..83ffd44bace2a 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx @@ -18,9 +18,9 @@ import { import { i18n } from '@kbn/i18n'; import { isEmpty } from 'lodash'; import { FETCH_STATUS } from '../../../../hooks/useFetcher'; -import { Config } from '../SettingsList'; import { ENVIRONMENT_NOT_DEFINED } from '../../../../../common/environment_filter_values'; import { SelectWithPlaceholder } from '../../../shared/SelectWithPlaceholder'; +import { Config } from '..'; const selectPlaceholderLabel = `- ${i18n.translate( 'xpack.apm.settings.agentConf.flyOut.selectPlaceholder', diff --git a/x-pack/legacy/plugins/apm/public/components/app/Settings/SettingsList.tsx b/x-pack/legacy/plugins/apm/public/components/app/Settings/SettingsList.tsx index bc773288b9269..00fcd09cdcf30 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/Settings/SettingsList.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/Settings/SettingsList.tsx @@ -4,46 +4,29 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useState } from 'react'; +import React from 'react'; import moment from 'moment'; import { i18n } from '@kbn/i18n'; -import { FormattedMessage } from '@kbn/i18n/react'; -import { - EuiTitle, - EuiFlexGroup, - EuiFlexItem, - EuiButtonEmpty, - EuiPanel, - EuiBetaBadge, - EuiSpacer, - EuiCallOut, - EuiEmptyPrompt, - EuiButton, - EuiLink -} from '@elastic/eui'; +import { EuiEmptyPrompt, EuiButton, EuiButtonEmpty } from '@elastic/eui'; import { isEmpty } from 'lodash'; -import { useFetcher } from '../../../hooks/useFetcher'; +import { FETCH_STATUS } from '../../../hooks/useFetcher'; import { ITableColumn, ManagedTable } from '../../shared/ManagedTable'; -import { AgentConfigurationListAPIResponse } from '../../../../server/lib/settings/agent_configuration/list_configurations'; -import { AddSettingsFlyout } from './AddSettings/AddSettingFlyout'; import { LoadingStatePrompt } from '../../shared/LoadingStatePrompt'; -import { callApmApi } from '../../../services/rest/callApmApi'; -import { HomeLink } from '../../shared/Links/apm/HomeLink'; - -export type Config = AgentConfigurationListAPIResponse[0]; - -export function SettingsList() { - const { data = [], status, refresh } = useFetcher( - () => - callApmApi({ - pathname: `/api/apm/settings/agent-configuration` - }), - [] - ); - const [selectedConfig, setSelectedConfig] = useState(null); - const [isFlyoutOpen, setIsFlyoutOpen] = useState(false); +import { AgentConfigurationListAPIResponse } from '../../../../server/lib/settings/agent_configuration/list_configurations'; +import { Config } from '.'; - const COLUMNS: Array> = [ +export function SettingsList({ + status, + data, + setIsFlyoutOpen, + setSelectedConfig +}: { + status: FETCH_STATUS; + data: AgentConfigurationListAPIResponse; + setIsFlyoutOpen: React.Dispatch>; + setSelectedConfig: React.Dispatch>; +}) { + const columns: Array> = [ { field: 'service.name', name: i18n.translate( @@ -127,15 +110,6 @@ export function SettingsList() { } ]; - const RETURN_TO_OVERVIEW_LINK_LABEL = i18n.translate( - 'xpack.apm.settings.agentConf.returnToOverviewLinkLabel', - { - defaultMessage: 'Return to overview' - } - ); - - const hasConfigurations = !isEmpty(data); - const emptyStatePrompt = ( ); - return ( - <> - { - setSelectedConfig(null); - setIsFlyoutOpen(false); - }} - onSubmit={() => { - setSelectedConfig(null); - setIsFlyoutOpen(false); - refresh(); - }} - /> - - - - -

- {i18n.translate('xpack.apm.settings.agentConf.pageTitle', { - defaultMessage: 'Settings' - })} -

- - - - - - {RETURN_TO_OVERVIEW_LINK_LABEL} - - - - - - - - - - - -

- {i18n.translate( - 'xpack.apm.settings.agentConf.configurationsPanelTitle', - { - defaultMessage: 'Configurations' - } - )} -

- - - - - - {hasConfigurations ? ( - - - - setIsFlyoutOpen(true)} - > - {i18n.translate( - 'xpack.apm.settings.agentConf.createConfigButtonLabel', - { - defaultMessage: 'Create configuration' - } - )} - - - - - ) : null} - - - - - + const failurePrompt = ( +

- - {i18n.translate( - 'xpack.apm.settings.agentConf.agentConfigDocsLinkLabel', - { defaultMessage: 'Learn more in our docs.' } - )} - - ) - }} - /> + {i18n.translate( + 'xpack.apm.settings.agentConf.configTable.failurePromptText', + { + defaultMessage: + 'The list of agent configurations could not be fetched. Your user may not have the sufficient permissions.' + } + )}

- - - - - {status === 'success' && !hasConfigurations ? ( - emptyStatePrompt - ) : ( - } - columns={COLUMNS} - items={data} - initialSortField="service.name" - initialSortDirection="asc" - initialPageSize={50} - /> - )} - - + + } + /> ); + const hasConfigurations = !isEmpty(data); + + if (status === 'failure') { + return failurePrompt; + } + if (status === 'success') { + if (hasConfigurations) { + return ( + } + columns={columns} + items={data} + initialSortField="service.name" + initialSortDirection="asc" + initialPageSize={50} + /> + ); + } else { + return emptyStatePrompt; + } + } + return null; } diff --git a/x-pack/legacy/plugins/apm/public/components/app/Settings/index.tsx b/x-pack/legacy/plugins/apm/public/components/app/Settings/index.tsx new file mode 100644 index 0000000000000..0dc7574d34a8d --- /dev/null +++ b/x-pack/legacy/plugins/apm/public/components/app/Settings/index.tsx @@ -0,0 +1,183 @@ +/* + * 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 React, { useState } from 'react'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { + EuiTitle, + EuiFlexGroup, + EuiFlexItem, + EuiButtonEmpty, + EuiPanel, + EuiBetaBadge, + EuiSpacer, + EuiCallOut, + EuiButton, + EuiLink +} from '@elastic/eui'; +import { isEmpty } from 'lodash'; +import { useFetcher } from '../../../hooks/useFetcher'; +import { AgentConfigurationListAPIResponse } from '../../../../server/lib/settings/agent_configuration/list_configurations'; +import { AddSettingsFlyout } from './AddSettings/AddSettingFlyout'; +import { callApmApi } from '../../../services/rest/callApmApi'; +import { HomeLink } from '../../shared/Links/apm/HomeLink'; +import { SettingsList } from './SettingsList'; + +export type Config = AgentConfigurationListAPIResponse[0]; + +export function Settings() { + const { data = [], status, refresh } = useFetcher( + () => + callApmApi({ + pathname: `/api/apm/settings/agent-configuration` + }), + [] + ); + const [selectedConfig, setSelectedConfig] = useState(null); + const [isFlyoutOpen, setIsFlyoutOpen] = useState(false); + + const RETURN_TO_OVERVIEW_LINK_LABEL = i18n.translate( + 'xpack.apm.settings.agentConf.returnToOverviewLinkLabel', + { + defaultMessage: 'Return to overview' + } + ); + + const hasConfigurations = !isEmpty(data); + + return ( + <> + { + setSelectedConfig(null); + setIsFlyoutOpen(false); + }} + onSubmit={() => { + setSelectedConfig(null); + setIsFlyoutOpen(false); + refresh(); + }} + /> + + + + +

+ {i18n.translate('xpack.apm.settings.agentConf.pageTitle', { + defaultMessage: 'Settings' + })} +

+ + + + + + {RETURN_TO_OVERVIEW_LINK_LABEL} + + + + + + + + + + + +

+ {i18n.translate( + 'xpack.apm.settings.agentConf.configurationsPanelTitle', + { + defaultMessage: 'Configurations' + } + )} +

+ + + + + + {hasConfigurations ? ( + + + + setIsFlyoutOpen(true)} + > + {i18n.translate( + 'xpack.apm.settings.agentConf.createConfigButtonLabel', + { + defaultMessage: 'Create configuration' + } + )} + + + + + ) : null} + + + + + +

+ + {i18n.translate( + 'xpack.apm.settings.agentConf.agentConfigDocsLinkLabel', + { defaultMessage: 'Learn more in our docs.' } + )} + + ) + }} + /> +

+ + + + + + + + ); +} diff --git a/x-pack/legacy/plugins/apm/public/components/shared/Links/apm/SettingsLink.tsx b/x-pack/legacy/plugins/apm/public/components/shared/Links/apm/SettingsLink.tsx index 8c2b44cf41b82..853972f4df402 100644 --- a/x-pack/legacy/plugins/apm/public/components/shared/Links/apm/SettingsLink.tsx +++ b/x-pack/legacy/plugins/apm/public/components/shared/Links/apm/SettingsLink.tsx @@ -7,7 +7,7 @@ import React from 'react'; import { APMLink, APMLinkExtendProps } from './APMLink'; const SettingsLink = (props: APMLinkExtendProps) => { - return ; + return ; }; export { SettingsLink }; diff --git a/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/create_agent_config_index.ts b/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/create_agent_config_index.ts index 5759b38b71d07..1189892bd4888 100644 --- a/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/create_agent_config_index.ts +++ b/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/create_agent_config_index.ts @@ -4,10 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Server } from 'hapi'; +import { InternalCoreSetup } from 'src/core/server'; import Boom from 'boom'; -export async function createApmAgentConfigurationIndex(server: Server) { +export async function createApmAgentConfigurationIndex( + core: InternalCoreSetup +) { + const { server } = core.http; const index = server .config() .get('apm_oss.apmAgentConfigurationIndex'); diff --git a/x-pack/legacy/plugins/apm/server/new-platform/plugin.ts b/x-pack/legacy/plugins/apm/server/new-platform/plugin.ts index 89411dbabc493..0458c8e4fedf0 100644 --- a/x-pack/legacy/plugins/apm/server/new-platform/plugin.ts +++ b/x-pack/legacy/plugins/apm/server/new-platform/plugin.ts @@ -7,12 +7,13 @@ import { InternalCoreSetup } from 'src/core/server'; import { makeApmUsageCollector } from '../lib/apm_telemetry'; import { CoreSetupWithUsageCollector } from '../lib/apm_telemetry/make_apm_usage_collector'; +import { createApmAgentConfigurationIndex } from '../lib/settings/agent_configuration/create_agent_config_index'; import { createApmApi } from '../routes/create_apm_api'; export class Plugin { public setup(core: InternalCoreSetup) { createApmApi().init(core); - + createApmAgentConfigurationIndex(core); makeApmUsageCollector(core as CoreSetupWithUsageCollector); } } diff --git a/x-pack/legacy/plugins/apm/server/routes/settings.ts b/x-pack/legacy/plugins/apm/server/routes/settings.ts index d5557a39c1f70..142756137647f 100644 --- a/x-pack/legacy/plugins/apm/server/routes/settings.ts +++ b/x-pack/legacy/plugins/apm/server/routes/settings.ts @@ -13,7 +13,6 @@ import { searchConfigurations } from '../lib/settings/agent_configuration/search import { listConfigurations } from '../lib/settings/agent_configuration/list_configurations'; import { getEnvironments } from '../lib/settings/agent_configuration/get_environments'; import { deleteConfiguration } from '../lib/settings/agent_configuration/delete_configuration'; -import { createApmAgentConfigurationIndex } from '../lib/settings/agent_configuration/create_agent_config_index'; import { createRoute } from './create_route'; import { transactionSampleRateRt } from '../../common/runtime_types/transaction_sample_rate_rt'; @@ -21,12 +20,8 @@ import { transactionSampleRateRt } from '../../common/runtime_types/transaction_ export const agentConfigurationRoute = createRoute(core => ({ path: '/api/apm/settings/agent-configuration', handler: async req => { - await createApmAgentConfigurationIndex(core.http.server); - const setup = await setupRequest(req); - return await listConfigurations({ - setup - }); + return await listConfigurations({ setup }); } })); @@ -141,11 +136,7 @@ export const agentConfigurationSearchRoute = createRoute(core => ({ }) }, handler: async (req, { body }, h) => { - const [setup] = await Promise.all([ - setupRequest(req), - createApmAgentConfigurationIndex(core.http.server) - ]); - + const setup = await setupRequest(req); const config = await searchConfigurations({ serviceName: body.service.name, environment: body.service.environment,