From 6628232433c0144f7564b22d8e6d2941425431ae Mon Sep 17 00:00:00 2001 From: Bena Kansara <69037875+benakansara@users.noreply.github.com> Date: Thu, 7 Dec 2023 13:05:01 +0530 Subject: [PATCH] Deprecate feature flag for Log threshold alert details page (#172554) Resolves https://github.com/elastic/kibana/issues/172379 - Deprecates following feature flag used for enabling/disabling Log threshold alert details page: ``` xpack.observability.unsafe.alertDetails.logs.enabled ``` - Removes usage of this flag from code. - Adding this flag in `kibana.yml` will generate following warning: ``` [WARN ][config.deprecation] You no longer need to configure "xpack.observability.unsafe.alertDetails.logs.enabled". ``` --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- x-pack/README.md | 6 ---- .../log_threshold/log_threshold_executor.ts | 31 ++----------------- .../public/application/application.test.tsx | 1 - .../alert_details/alert_details.test.tsx | 1 - .../public/pages/alerts/alerts.test.tsx | 1 - .../alerts/components/alert_actions.test.tsx | 1 - .../sections/apm/apm_section.test.tsx | 1 - .../pages/overview/overview.stories.tsx | 1 - .../public/pages/rules/rules.test.tsx | 1 - x-pack/plugins/observability/public/plugin.ts | 2 +- .../utils/is_alert_details_enabled.test.ts | 13 +------- .../public/utils/is_alert_details_enabled.ts | 6 ++-- .../kibana_react.storybook_decorator.tsx | 1 - .../public/utils/test_helper.tsx | 1 - x-pack/plugins/observability/server/index.ts | 4 +-- 15 files changed, 10 insertions(+), 61 deletions(-) diff --git a/x-pack/README.md b/x-pack/README.md index ea8777f46b143..255a4d3e2b6e8 100644 --- a/x-pack/README.md +++ b/x-pack/README.md @@ -14,12 +14,6 @@ xpack.observability.unsafe.alertDetails.metrics.enabled: true **[For Infrastructure rule types]** In Kibana configuration, will allow the user to navigate to the new Alert Details page, instead of the Alert Flyout when clicking on `View alert details` in the Alert table -```yaml -xpack.observability.unsafe.alertDetails.logs.enabled: true -``` - -**[For Logs threshold rule type]** In Kibana configuration, will allow the user to navigate to the new Alert Details page, instead of the Alert Flyout when clicking on `View alert details` in the Alert table - ```yaml xpack.observability.unsafe.alertDetails.uptime.enabled: true ``` diff --git a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts index e36ae54746b3d..517b020eaeace 100644 --- a/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts @@ -7,11 +7,7 @@ import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { i18n } from '@kbn/i18n'; -import { - AlertsLocatorParams, - getAlertDetailsUrl, - getAlertUrl, -} from '@kbn/observability-plugin/common'; +import { AlertsLocatorParams, getAlertDetailsUrl } from '@kbn/observability-plugin/common'; import { ALERT_CONTEXT, ALERT_EVALUATION_THRESHOLD, @@ -63,7 +59,6 @@ import { InfraBackendLibs } from '../../infra_types'; import { AdditionalContext, flattenAdditionalContext, - getAlertDetailsPageEnabledForApp, getContextForRecoveredAlerts, getGroupByObject, unflattenObject, @@ -138,7 +133,6 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) => getAlertByAlertUuid, } = services; const { basePath, alertsLocator } = libs; - const config = libs.getAlertDetailsConfig(); const alertFactory: LogThresholdAlertFactory = ( id, @@ -189,15 +183,7 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) => alert.scheduleActions(actionGroup, { ...sharedContext, ...context, - alertDetailsUrl: getAlertDetailsPageEnabledForApp(config, 'logs') - ? getAlertDetailsUrl(libs.basePath, spaceId, alertUuid) - : await getAlertUrl( - alertUuid, - spaceId, - indexedStartedAt, - libs.alertsLocator, - libs.basePath.publicBaseUrl - ), + alertDetailsUrl: getAlertDetailsUrl(libs.basePath, spaceId, alertUuid), }); }); } @@ -254,7 +240,6 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) => validatedParams, getAlertByAlertUuid, alertsLocator, - isAlertDetailsPageEnabled: getAlertDetailsPageEnabledForApp(config, 'logs'), }); } catch (e) { throw new Error(e); @@ -868,7 +853,6 @@ const processRecoveredAlerts = async ({ validatedParams, getAlertByAlertUuid, alertsLocator, - isAlertDetailsPageEnabled = false, }: { basePath: IBasePath; getAlertStartedDate: (alertId: string) => string | null; @@ -881,7 +865,6 @@ const processRecoveredAlerts = async ({ alertUuid: string ) => Promise | null> | null; alertsLocator?: LocatorPublic; - isAlertDetailsPageEnabled?: boolean; }) => { const groupByKeysObjectForRecovered = getGroupByObject( validatedParams.groupBy, @@ -898,15 +881,7 @@ const processRecoveredAlerts = async ({ const viewInAppUrl = addSpaceIdToPath(basePath.publicBaseUrl, spaceId, relativeViewInAppUrl); const baseContext = { - alertDetailsUrl: isAlertDetailsPageEnabled - ? getAlertDetailsUrl(basePath, spaceId, alertUuid) - : await getAlertUrl( - alertUuid, - spaceId, - indexedStartedAt, - alertsLocator, - basePath.publicBaseUrl - ), + alertDetailsUrl: getAlertDetailsUrl(basePath, spaceId, alertUuid), group: hasGroupBy(validatedParams) ? recoveredAlertId : null, groupByKeys: groupByKeysObjectForRecovered[recoveredAlertId], timestamp: startedAt.toISOString(), diff --git a/x-pack/plugins/observability/public/application/application.test.tsx b/x-pack/plugins/observability/public/application/application.test.tsx index eb7da89d3441d..3b385fee702f5 100644 --- a/x-pack/plugins/observability/public/application/application.test.tsx +++ b/x-pack/plugins/observability/public/application/application.test.tsx @@ -77,7 +77,6 @@ describe('renderApp', () => { const config = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, }, diff --git a/x-pack/plugins/observability/public/pages/alert_details/alert_details.test.tsx b/x-pack/plugins/observability/public/pages/alert_details/alert_details.test.tsx index 756f90ecd97cf..f2a7a4d72d66f 100644 --- a/x-pack/plugins/observability/public/pages/alert_details/alert_details.test.tsx +++ b/x-pack/plugins/observability/public/pages/alert_details/alert_details.test.tsx @@ -90,7 +90,6 @@ const params = { const config: Subset = { unsafe: { alertDetails: { - logs: { enabled: true }, metrics: { enabled: true }, uptime: { enabled: true }, }, diff --git a/x-pack/plugins/observability/public/pages/alerts/alerts.test.tsx b/x-pack/plugins/observability/public/pages/alerts/alerts.test.tsx index 247a700acfa6c..28b4c7b2a3d73 100644 --- a/x-pack/plugins/observability/public/pages/alerts/alerts.test.tsx +++ b/x-pack/plugins/observability/public/pages/alerts/alerts.test.tsx @@ -53,7 +53,6 @@ jest.spyOn(pluginContext, 'usePluginContext').mockImplementation(() => ({ slo: { enabled: false }, alertDetails: { apm: { enabled: false }, - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, observability: { enabled: false }, diff --git a/x-pack/plugins/observability/public/pages/alerts/components/alert_actions.test.tsx b/x-pack/plugins/observability/public/pages/alerts/components/alert_actions.test.tsx index cbdb9aa068dad..b8e216ded52fb 100644 --- a/x-pack/plugins/observability/public/pages/alerts/components/alert_actions.test.tsx +++ b/x-pack/plugins/observability/public/pages/alerts/components/alert_actions.test.tsx @@ -59,7 +59,6 @@ jest.mock('@kbn/triggers-actions-ui-plugin/public/common/lib/kibana/kibana_react const config = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, }, diff --git a/x-pack/plugins/observability/public/pages/overview/components/sections/apm/apm_section.test.tsx b/x-pack/plugins/observability/public/pages/overview/components/sections/apm/apm_section.test.tsx index 6da498042f8fb..23d3af1167b99 100644 --- a/x-pack/plugins/observability/public/pages/overview/components/sections/apm/apm_section.test.tsx +++ b/x-pack/plugins/observability/public/pages/overview/components/sections/apm/apm_section.test.tsx @@ -48,7 +48,6 @@ describe('APMSection', () => { const config = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, }, diff --git a/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx b/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx index d0937d1f2c72b..3114c084be289 100644 --- a/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx +++ b/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx @@ -81,7 +81,6 @@ const withCore = makeDecorator({ const config: ConfigSchema = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, observability: { enabled: false }, diff --git a/x-pack/plugins/observability/public/pages/rules/rules.test.tsx b/x-pack/plugins/observability/public/pages/rules/rules.test.tsx index 8be89be03d57b..3b36403c4e18f 100644 --- a/x-pack/plugins/observability/public/pages/rules/rules.test.tsx +++ b/x-pack/plugins/observability/public/pages/rules/rules.test.tsx @@ -40,7 +40,6 @@ jest.spyOn(pluginContext, 'usePluginContext').mockImplementation(() => ({ slo: { enabled: false }, alertDetails: { apm: { enabled: false }, - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, observability: { enabled: false }, diff --git a/x-pack/plugins/observability/public/plugin.ts b/x-pack/plugins/observability/public/plugin.ts index 12848e1bb4974..b6c17e0c3f914 100644 --- a/x-pack/plugins/observability/public/plugin.ts +++ b/x-pack/plugins/observability/public/plugin.ts @@ -94,7 +94,7 @@ export interface ConfigSchema { metrics: { enabled: boolean; }; - logs: { + logs?: { enabled: boolean; }; uptime: { diff --git a/x-pack/plugins/observability/public/utils/is_alert_details_enabled.test.ts b/x-pack/plugins/observability/public/utils/is_alert_details_enabled.test.ts index a39c4cd00b576..101e10a314451 100644 --- a/x-pack/plugins/observability/public/utils/is_alert_details_enabled.test.ts +++ b/x-pack/plugins/observability/public/utils/is_alert_details_enabled.test.ts @@ -31,7 +31,6 @@ import type { TopAlert } from '../typings/alerts'; const defaultConfig = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, }, @@ -63,15 +62,10 @@ describe('isAlertDetailsEnabled', () => { start: 1630587249674, lastUpdated: 1630588131750, } as unknown as TopAlert; - it('returns FALSE when logs: { enabled: false }', () => { - expect(isAlertDetailsEnabledPerApp(logsAlert, defaultConfig)).toBeFalsy(); - }); - - it('returns TRUE when logs: { enabled: true }', () => { + it('returns TRUE when rule type is logs.alert.document.count', () => { const updatedConfig = { unsafe: { alertDetails: { - logs: { enabled: true }, metrics: { enabled: false }, uptime: { enabled: false }, }, @@ -113,7 +107,6 @@ describe('isAlertDetailsEnabled', () => { const updatedConfig = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, }, @@ -159,7 +152,6 @@ describe('isAlertDetailsEnabled', () => { const updatedConfig = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: true }, uptime: { enabled: false }, }, @@ -201,7 +193,6 @@ describe('isAlertDetailsEnabled', () => { const updatedConfig = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: true }, }, @@ -243,7 +234,6 @@ describe('isAlertDetailsEnabled', () => { const updatedConfig = { unsafe: { alertDetails: { - logs: { enabled: true }, metrics: { enabled: true }, uptime: { enabled: true }, }, @@ -255,7 +245,6 @@ describe('isAlertDetailsEnabled', () => { const updatedConfig = { unsafe: { alertDetails: { - logs: { enabled: true }, metrics: { enabled: true }, uptime: { enabled: true }, }, diff --git a/x-pack/plugins/observability/public/utils/is_alert_details_enabled.ts b/x-pack/plugins/observability/public/utils/is_alert_details_enabled.ts index 15b9589be60f6..2b628f26ee794 100644 --- a/x-pack/plugins/observability/public/utils/is_alert_details_enabled.ts +++ b/x-pack/plugins/observability/public/utils/is_alert_details_enabled.ts @@ -9,12 +9,12 @@ import { ALERT_RULE_TYPE_ID } from '@kbn/rule-data-utils'; import type { ConfigSchema } from '../plugin'; import type { TopAlert } from '../typings/alerts'; -const ALLOWED_RULE_TYPES = ['apm.transaction_duration']; +const ALLOWED_RULE_TYPES = ['apm.transaction_duration', 'logs.alert.document.count']; const isUnsafeAlertDetailsFlag = ( subject: string -): subject is keyof ConfigSchema['unsafe']['alertDetails'] => - ['uptime', 'logs', 'metrics', 'observability'].includes(subject); +): subject is keyof Omit => + ['uptime', 'metrics', 'observability'].includes(subject); // We are mapping the ruleTypeId from the feature flag with the ruleTypeId from the alert // to know whether the feature flag is enabled or not. diff --git a/x-pack/plugins/observability/public/utils/kibana_react.storybook_decorator.tsx b/x-pack/plugins/observability/public/utils/kibana_react.storybook_decorator.tsx index d5714924bdc97..8b8f65e244ddf 100644 --- a/x-pack/plugins/observability/public/utils/kibana_react.storybook_decorator.tsx +++ b/x-pack/plugins/observability/public/utils/kibana_react.storybook_decorator.tsx @@ -28,7 +28,6 @@ export function KibanaReactStorybookDecorator(Story: ComponentType) { const config: ConfigSchema = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, observability: { enabled: false }, diff --git a/x-pack/plugins/observability/public/utils/test_helper.tsx b/x-pack/plugins/observability/public/utils/test_helper.tsx index 6c1610ac059c9..12fa672552db9 100644 --- a/x-pack/plugins/observability/public/utils/test_helper.tsx +++ b/x-pack/plugins/observability/public/utils/test_helper.tsx @@ -32,7 +32,6 @@ export const data = dataPluginMock.createStartContract(); const defaultConfig: ConfigSchema = { unsafe: { alertDetails: { - logs: { enabled: false }, metrics: { enabled: false }, uptime: { enabled: false }, observability: { enabled: false }, diff --git a/x-pack/plugins/observability/server/index.ts b/x-pack/plugins/observability/server/index.ts index ce2279db23203..da4797f42c43f 100644 --- a/x-pack/plugins/observability/server/index.ts +++ b/x-pack/plugins/observability/server/index.ts @@ -36,8 +36,7 @@ const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: false }), }), logs: schema.object({ - // Enable it by default: https://github.com/elastic/kibana/issues/159945 - enabled: schema.boolean({ defaultValue: true }), + enabled: schema.boolean({ defaultValue: false }), }), uptime: schema.object({ enabled: schema.boolean({ defaultValue: false }), @@ -71,6 +70,7 @@ export const config: PluginConfigDescriptor = { }, }, schema: configSchema, + deprecations: ({ unused }) => [unused('unsafe.alertDetails.logs.enabled', { level: 'warning' })], }; export type ObservabilityConfig = TypeOf;