Deprecate feature flag for Log threshold alert details page (#172554)

Resolves #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 <[email protected]>
elastic · Dec 7, 2023 · 6628232 · 6628232
1 parent 7ea0b23
commit 6628232
Show file tree

Hide file tree

Showing 15 changed files with 10 additions and 61 deletions.
diff --git 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
@@ -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<Partial<ParsedTechnicalFields & ParsedExperimentalFields> | null> | null;
   alertsLocator?: LocatorPublic<AlertsLocatorParams>;
-  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
@@ -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
@@ -90,7 +90,6 @@ const params = {
 const config: Subset<ConfigSchema> = {
   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
@@ -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
@@ -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/.../plugins/observability/public/pages/overview/components/sections/apm/apm_section.test.tsx b/.../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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<ConfigSchema['unsafe']['alertDetails'], 'logs'> =>
+  ['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
@@ -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
@@ -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
@@ -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<typeof configSchema>;