Remove feature flag for APM Alert Details page (#150475)

## Summary

This PR removes the
`xpack.observability.unsafe.alertDetails.apm.enabled` feature flag to
enable the Alert Detail page for the APM Latency Threshold Rule. I've
also removed the`alertDetailsAppSection` options along with the
`alertDetailsUrl` action context variable for the remaining APM rules
since the APM Alert Detail page should only be available for the APM
Latency Threshold rule.

I also changed`isAlertDetailsEnabledPerApp()` method to allow the
`apm.transaction_duration` rule type to work but not `apm.error_rate`,
`apm.transaction_error_rate`, or `apm.anomaly` rule types. This change
was necessary because the granularity of the feature flag was set to the
solution level and didn't allow to set the flag per rule type.

### Testing

To test this PR you will need to have some APM data in your cluster.
1. Create an APM Latency Threshold Rule with a low threshold so it will
trigger
2. Create an APM Error Count Rule with a low threshold so it will
trigger
3. Click on the "Action Menu" (the ellipses) in the alert (in the Alert
Table) for the latency threshold alert
4. Click on the "View alert details" menu item for the latency threshold
alert
5. Verify it takes you to the new "alert details" page for APM
6. Repeat steps 3 & 4 for the error count alert BUT it should open the
"alert flyout" instead.

---------

Co-authored-by: Faisal Kanout <faisal.kanout@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker

@@ -300,7 +300,6 @@ kibana_vars=(
     xpack.ingestManager.fleet.tlsCheckDisabled
     xpack.ingestManager.registryUrl
     xpack.observability.annotations.index
-    xpack.observability.unsafe.alertDetails.apm.enabled
     xpack.observability.unsafe.alertDetails.metrics.enabled
     xpack.observability.unsafe.alertDetails.logs.enabled
     xpack.observability.unsafe.alertDetails.uptime.enabled

test/plugin_functional/test_suites/core_plugins/rendering.ts

@@ -229,7 +229,6 @@ export default function ({ getService }: PluginFunctionalProviderContext) {
         'xpack.upgrade_assistant.featureSet.mlSnapshots (boolean)',
         'xpack.upgrade_assistant.featureSet.reindexCorrectiveActions (boolean)',
         'xpack.upgrade_assistant.ui.enabled (boolean)',
-        'xpack.observability.unsafe.alertDetails.apm.enabled (boolean)',
         'xpack.observability.unsafe.alertDetails.metrics.enabled (boolean)',
         'xpack.observability.unsafe.alertDetails.logs.enabled (boolean)',
         'xpack.observability.unsafe.alertDetails.uptime.enabled (boolean)',

x-pack/README.md

@@ -8,12 +8,6 @@ from files dual-licensed under the Server Side Public License and the Elastic Li
 
 If you have:
 
-```yaml
-xpack.observability.unsafe.alertDetails.apm.enabled: true
-```
-
-**[For APM 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.metrics.enabled: true
 ```

x-pack/plugins/apm/public/components/alerting/rule_types/register_apm_rule_types.ts

@@ -53,9 +53,6 @@ export function registerApmRuleTypes(
     validate: () => ({
       errors: [],
     }),
-    alertDetailsAppSection: lazy(
-      () => import('../ui_components/alert_details_app_section')
-    ),
     requiresAppContext: false,
     defaultActionMessage: errorCountMessage,
   });
@@ -69,7 +66,7 @@ export function registerApmRuleTypes(
           'Alert when the latency of a specific transaction type in a service exceeds a defined threshold.',
       }
     ),
-    format: ({ fields, formatters: { asDuration } }) => {
+    format: ({ fields }) => {
       return {
         reason: fields[ALERT_REASON]!,
         link: getAlertUrlTransaction(
@@ -106,7 +103,7 @@ export function registerApmRuleTypes(
           'Alert when the rate of transaction errors in a service exceeds a defined threshold.',
       }
     ),
-    format: ({ fields, formatters: { asPercent } }) => ({
+    format: ({ fields }) => ({
       reason: fields[ALERT_REASON]!,
       link: getAlertUrlTransaction(
         // TODO:fix SERVICE_NAME when we move it to initializeIndex
@@ -125,9 +122,6 @@ export function registerApmRuleTypes(
     validate: () => ({
       errors: [],
     }),
-    alertDetailsAppSection: lazy(
-      () => import('../ui_components/alert_details_app_section')
-    ),
     requiresAppContext: false,
     defaultActionMessage: transactionErrorRateMessage,
   });
@@ -157,9 +151,6 @@ export function registerApmRuleTypes(
     validate: () => ({
       errors: [],
     }),
-    alertDetailsAppSection: lazy(
-      () => import('../ui_components/alert_details_app_section')
-    ),
     requiresAppContext: false,
     defaultActionMessage: anomalyMessage,
   });

x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.test.ts

@@ -194,9 +194,6 @@ describe('Transaction duration anomaly alert', () => {
       );
 
       expect(scheduleActions).toHaveBeenCalledWith('threshold_met', {
-        alertDetailsUrl: expect.stringContaining(
-          'http://localhost:5601/eyr/app/observability/alerts/'
-        ),
         serviceName: 'foo',
         transactionType: 'type-foo',
         environment: 'development',

x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts

@@ -8,7 +8,6 @@ import { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/typesWith
 import { KibanaRequest } from '@kbn/core/server';
 import datemath from '@kbn/datemath';
 import type { ESSearchResponse } from '@kbn/es-types';
-import { getAlertDetailsUrl } from '@kbn/infra-plugin/server/lib/alerting/common/utils';
 import { ProcessorEvent } from '@kbn/observability-plugin/common';
 import { termQuery } from '@kbn/observability-plugin/server';
 import {
@@ -60,7 +59,6 @@ export function registerAnomalyRuleType({
   config$,
   logger,
   ml,
-  observability,
   ruleDataClient,
 }: RegisterRuleDependencies) {
   const createLifecycleRuleType = createLifecycleRuleTypeFactory({
@@ -77,9 +75,6 @@ export function registerAnomalyRuleType({
       validate: { params: anomalyParamsSchema },
       actionVariables: {
         context: [
-          ...(observability.getAlertDetailsConfig()?.apm.enabled
-            ? [apmActionVariables.alertDetailsUrl]
-            : []),
           apmActionVariables.environment,
           apmActionVariables.reason,
           apmActionVariables.serviceName,
@@ -97,8 +92,7 @@ export function registerAnomalyRuleType({
           return { state: {} };
         }
 
-        const { savedObjectsClient, scopedClusterClient, getAlertUuid } =
-          services;
+        const { savedObjectsClient, scopedClusterClient } = services;
 
         const ruleParams = params;
         const request = {} as KibanaRequest;
@@ -288,14 +282,6 @@ export function registerAnomalyRuleType({
             relativeViewInAppUrl
           );
 
-          const alertUuid = getAlertUuid(id);
-
-          const alertDetailsUrl = getAlertDetailsUrl(
-            basePath,
-            spaceId,
-            alertUuid
-          );
-
           services
             .alertWithLifecycle({
               id,
@@ -312,7 +298,6 @@ export function registerAnomalyRuleType({
               },
             })
             .scheduleActions(ruleTypeConfig.defaultActionGroupId, {
-              alertDetailsUrl,
               environment: getEnvironmentLabel(environment),
               reason: reasonMessage,
               serviceName,

x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.test.ts

@@ -138,9 +138,6 @@ describe('Error count alert', () => {
     expect(scheduleActions).toHaveBeenCalledTimes(3);
 
     expect(scheduleActions).toHaveBeenCalledWith('threshold_met', {
-      alertDetailsUrl: expect.stringContaining(
-        'http://localhost:5601/eyr/app/observability/alerts/'
-      ),
       serviceName: 'foo',
       environment: 'env-foo',
       threshold: 2,
@@ -151,9 +148,6 @@ describe('Error count alert', () => {
         'http://localhost:5601/eyr/app/apm/services/foo/errors?environment=env-foo',
     });
     expect(scheduleActions).toHaveBeenCalledWith('threshold_met', {
-      alertDetailsUrl: expect.stringContaining(
-        'http://localhost:5601/eyr/app/observability/alerts/'
-      ),
       serviceName: 'foo',
       environment: 'env-foo-2',
       threshold: 2,
@@ -164,9 +158,6 @@ describe('Error count alert', () => {
         'http://localhost:5601/eyr/app/apm/services/foo/errors?environment=env-foo-2',
     });
     expect(scheduleActions).toHaveBeenCalledWith('threshold_met', {
-      alertDetailsUrl: expect.stringContaining(
-        'http://localhost:5601/eyr/app/observability/alerts/'
-      ),
       serviceName: 'bar',
       environment: 'env-bar',
       reason: 'Error count is 3 in the last 5 mins for bar. Alert when > 2.',

x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts

@@ -5,19 +5,18 @@
  * 2.0.
  */
 
-import { getAlertDetailsUrl } from '@kbn/infra-plugin/server/lib/alerting/common/utils';
 import {
   formatDurationFromTimeUnitChar,
   ProcessorEvent,
   TimeUnitChar,
 } from '@kbn/observability-plugin/common';
-import { termQuery } from '@kbn/observability-plugin/server';
 import {
   ALERT_EVALUATION_THRESHOLD,
   ALERT_EVALUATION_VALUE,
   ALERT_REASON,
 } from '@kbn/rule-data-utils';
 import { createLifecycleRuleTypeFactory } from '@kbn/rule-registry-plugin/server';
+import { termQuery } from '@kbn/observability-plugin/server';
 import { addSpaceIdToPath } from '@kbn/spaces-plugin/common';
 import { firstValueFrom } from 'rxjs';
 import {
@@ -58,7 +57,6 @@ export function registerErrorCountRuleType({
   basePath,
   config$,
   logger,
-  observability,
   ruleDataClient,
 }: RegisterRuleDependencies) {
   const createLifecycleRuleType = createLifecycleRuleTypeFactory({
@@ -75,9 +73,6 @@ export function registerErrorCountRuleType({
       validate: { params: errorCountParamsSchema },
       actionVariables: {
         context: [
-          ...(observability.getAlertDetailsConfig()?.apm.enabled
-            ? [apmActionVariables.alertDetailsUrl]
-            : []),
           apmActionVariables.environment,
           apmActionVariables.interval,
           apmActionVariables.reason,
@@ -93,8 +88,7 @@ export function registerErrorCountRuleType({
       executor: async ({ params: ruleParams, services, spaceId }) => {
         const config = await firstValueFrom(config$);
 
-        const { getAlertUuid, savedObjectsClient, scopedClusterClient } =
-          services;
+        const { savedObjectsClient, scopedClusterClient } = services;
 
         const indices = await getApmIndices({
           config,
@@ -187,13 +181,6 @@ export function registerErrorCountRuleType({
               relativeViewInAppUrl
             );
 
-            const alertUuid = getAlertUuid(id);
-            const alertDetailsUrl = getAlertDetailsUrl(
-              basePath,
-              spaceId,
-              alertUuid
-            );
-
             services
               .alertWithLifecycle({
                 id,
@@ -208,7 +195,6 @@ export function registerErrorCountRuleType({
                 },
               })
               .scheduleActions(ruleTypeConfig.defaultActionGroupId, {
-                alertDetailsUrl,
                 environment: getEnvironmentLabel(environment),
                 interval: formatDurationFromTimeUnitChar(
                   ruleParams.windowSize,

x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts

@@ -73,7 +73,6 @@ export function registerTransactionDurationRuleType({
   ruleDataClient,
   config$,
   logger,
-  observability,
   basePath,
 }: RegisterRuleDependencies) {
   const createLifecycleRuleType = createLifecycleRuleTypeFactory({
@@ -89,9 +88,7 @@ export function registerTransactionDurationRuleType({
     validate: { params: transactionDurationParamsSchema },
     actionVariables: {
       context: [
-        ...(observability.getAlertDetailsConfig()?.apm.enabled
-          ? [apmActionVariables.alertDetailsUrl]
-          : []),
+        apmActionVariables.alertDetailsUrl,
         apmActionVariables.environment,
         apmActionVariables.interval,
         apmActionVariables.reason,

x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.test.ts

@@ -121,9 +121,6 @@ describe('Transaction error rate alert', () => {
     );
 
     expect(scheduleActions).toHaveBeenCalledWith('threshold_met', {
-      alertDetailsUrl: expect.stringContaining(
-        'http://localhost:5601/eyr/app/observability/alerts/'
-      ),
       serviceName: 'foo',
       transactionType: 'type-foo',
       environment: 'env-foo',

x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts

@@ -5,7 +5,6 @@
  * 2.0.
  */
 
-import { getAlertDetailsUrl } from '@kbn/infra-plugin/server/lib/alerting/common/utils';
 import {
   formatDurationFromTimeUnitChar,
   ProcessorEvent,
@@ -67,7 +66,6 @@ export function registerTransactionErrorRateRuleType({
   basePath,
   config$,
   logger,
-  observability,
   ruleDataClient,
 }: RegisterRuleDependencies) {
   const createLifecycleRuleType = createLifecycleRuleTypeFactory({
@@ -84,9 +82,6 @@ export function registerTransactionErrorRateRuleType({
       validate: { params: transactionErrorRateParamsSchema },
       actionVariables: {
         context: [
-          ...(observability.getAlertDetailsConfig()?.apm.enabled
-            ? [apmActionVariables.alertDetailsUrl]
-            : []),
           apmActionVariables.environment,
           apmActionVariables.interval,
           apmActionVariables.reason,
@@ -103,8 +98,7 @@ export function registerTransactionErrorRateRuleType({
       executor: async ({ services, spaceId, params: ruleParams }) => {
         const config = await firstValueFrom(config$);
 
-        const { getAlertUuid, savedObjectsClient, scopedClusterClient } =
-          services;
+        const { savedObjectsClient, scopedClusterClient } = services;
 
         const indices = await getApmIndices({
           config,
@@ -246,14 +240,6 @@ export function registerTransactionErrorRateRuleType({
             .filter((name) => name)
             .join('_');
 
-          const alertUuid = getAlertUuid(id);
-
-          const alertDetailsUrl = getAlertDetailsUrl(
-            basePath,
-            spaceId,
-            alertUuid
-          );
-
           const relativeViewInAppUrl = getAlertUrlTransaction(
             serviceName,
             getEnvironmentEsField(environment)?.[SERVICE_ENVIRONMENT],
@@ -281,7 +267,6 @@ export function registerTransactionErrorRateRuleType({
               },
             })
             .scheduleActions(ruleTypeConfig.defaultActionGroupId, {
-              alertDetailsUrl,
               environment: getEnvironmentLabel(environment),
               interval: formatDurationFromTimeUnitChar(
                 ruleParams.windowSize,

x-pack/plugins/observability/public/application/application.test.tsx

@@ -69,7 +69,6 @@ describe('renderApp', () => {
     const config = {
       unsafe: {
         alertDetails: {
-          apm: { enabled: false },
           logs: { enabled: false },
           metrics: { enabled: false },
           uptime: { enabled: false },

x-pack/plugins/observability/public/components/app/section/apm/index.test.tsx

@@ -48,7 +48,6 @@ describe('APMSection', () => {
     const config = {
       unsafe: {
         alertDetails: {
-          apm: { enabled: false },
           logs: { enabled: false },
           metrics: { enabled: false },
           uptime: { enabled: false },

x-pack/plugins/observability/public/pages/alert_details/alert_details.test.tsx

@@ -98,7 +98,6 @@ const params = {
 const config: Subset<ConfigSchema> = {
   unsafe: {
     alertDetails: {
-      apm: { enabled: true },
       logs: { enabled: true },
       metrics: { enabled: true },
       uptime: { enabled: true },

x-pack/plugins/observability/public/pages/alerts/components/alert_actions.test.tsx

@@ -32,7 +32,6 @@ jest.mock('../../../hooks/use_get_user_cases_permissions', () => ({
 const config = {
   unsafe: {
     alertDetails: {
-      apm: { enabled: false },
       logs: { enabled: false },
       metrics: { enabled: false },
       uptime: { enabled: false },

x-pack/plugins/observability/public/pages/overview/overview.stories.tsx

@@ -78,7 +78,6 @@ const withCore = makeDecorator({
     const config: ConfigSchema = {
       unsafe: {
         alertDetails: {
-          apm: { enabled: false },
           logs: { enabled: false },
           metrics: { enabled: false },
           uptime: { enabled: false },

x-pack/plugins/observability/public/plugin.ts

@@ -63,9 +63,6 @@ import { registerObservabilityRuleTypes } from './rules/register_observability_r
 export interface ConfigSchema {
   unsafe: {
     alertDetails: {
-      apm: {
-        enabled: boolean;
-      };
       metrics: {
         enabled: boolean;
       };

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: {
-      apm: { enabled: false },
       logs: { enabled: false },
       metrics: { enabled: false },
       uptime: { enabled: false },
@@ -72,7 +71,6 @@ describe('isAlertDetailsEnabled', () => {
       const updatedConfig = {
         unsafe: {
           alertDetails: {
-            apm: { enabled: false },
             logs: { enabled: true },
             metrics: { enabled: false },
             uptime: { enabled: false },
@@ -107,22 +105,25 @@ describe('isAlertDetailsEnabled', () => {
       start: 1630587249674,
       lastUpdated: 1630588131750,
     } as unknown as TopAlert;
-    it('returns FALSE when apm: { enabled: false }', () => {
+    it('returns FALSE when the rule type IS NOT apm.transaction_duration', () => {
       expect(isAlertDetailsEnabledPerApp(APMAlert, defaultConfig)).toBeFalsy();
     });
 
-    it('returns TRUE when apm: { enabled: true }', () => {
+    it('returns TRUE when rule type is apm.transaction_duration', () => {
       const updatedConfig = {
         unsafe: {
           alertDetails: {
-            apm: { enabled: true },
             logs: { enabled: false },
             metrics: { enabled: false },
             uptime: { enabled: false },
           },
         },
       } as ConfigSchema;
-      expect(isAlertDetailsEnabledPerApp(APMAlert, updatedConfig)).toBeTruthy();
+      const apmTransactionDurationAlert = {
+        ...APMAlert,
+        fields: { ...APMAlert.fields, [ALERT_RULE_TYPE_ID]: 'apm.transaction_duration' },
+      };
+      expect(isAlertDetailsEnabledPerApp(apmTransactionDurationAlert, updatedConfig)).toBeTruthy();
     });
   });
   describe('Metrics alert', () => {
@@ -158,7 +159,6 @@ describe('isAlertDetailsEnabled', () => {
       const updatedConfig = {
         unsafe: {
           alertDetails: {
-            apm: { enabled: false },
             logs: { enabled: false },
             metrics: { enabled: true },
             uptime: { enabled: false },
@@ -201,7 +201,6 @@ describe('isAlertDetailsEnabled', () => {
       const updatedConfig = {
         unsafe: {
           alertDetails: {
-            apm: { enabled: false },
             logs: { enabled: false },
             metrics: { enabled: false },
             uptime: { enabled: true },
@@ -244,7 +243,6 @@ describe('isAlertDetailsEnabled', () => {
       const updatedConfig = {
         unsafe: {
           alertDetails: {
-            apm: { enabled: true },
             logs: { enabled: true },
             metrics: { enabled: true },
             uptime: { enabled: true },
@@ -257,7 +255,6 @@ describe('isAlertDetailsEnabled', () => {
       const updatedConfig = {
         unsafe: {
           alertDetails: {
-            apm: { enabled: true },
             logs: { enabled: true },
             metrics: { enabled: true },
             uptime: { enabled: true },

x-pack/plugins/observability/public/utils/is_alert_details_enabled.ts

@@ -9,21 +9,39 @@ 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 isUnsafeAlertDetailsFlag = (
+  subject: string
+): subject is keyof ConfigSchema['unsafe']['alertDetails'] =>
+  ['uptime', 'logs', 'metrics'].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.
 export const isAlertDetailsEnabledPerApp = (
   alert: TopAlert | null,
   config: ConfigSchema | null
 ): boolean => {
-  if (!alert || !config) return false;
-
+  if (!alert) return false;
   const ruleTypeId = alert.fields[ALERT_RULE_TYPE_ID];
 
+  // The feature flags for alertDetails are not specific enough so we need to check
+  // the specific rule types to see if they should be blocked or not.
+  if (ALLOWED_RULE_TYPES.includes(ruleTypeId)) {
+    return true;
+  }
+
+  if (!config) return false;
+
+  // Since we are moving away from feature flags, this code will eventually be removed.
   const appNameFromAlertRuleType =
     ruleTypeId === 'xpack.uptime.alerts.monitorStatus' ||
     ruleTypeId === 'xpack.uptime.alerts.tlsCertificate'
       ? 'uptime'
-      : (ruleTypeId.split('.')[0] as keyof ConfigSchema['unsafe']['alertDetails']);
+      : ruleTypeId.split('.')[0];
+  if (isUnsafeAlertDetailsFlag(appNameFromAlertRuleType)) {
+    return config.unsafe?.alertDetails[appNameFromAlertRuleType]?.enabled;
+  }
 
-  return config.unsafe?.alertDetails[appNameFromAlertRuleType]?.enabled;
+  return false;
 };

x-pack/plugins/observability/public/utils/kibana_react.storybook_decorator.tsx

@@ -25,7 +25,6 @@ export function KibanaReactStorybookDecorator(Story: ComponentType) {
   const config: ConfigSchema = {
     unsafe: {
       alertDetails: {
-        apm: { enabled: false },
         logs: { enabled: false },
         metrics: { enabled: false },
         uptime: { enabled: false },

x-pack/plugins/observability/public/utils/test_helper.tsx

@@ -32,7 +32,6 @@ export const data = dataPluginMock.createStartContract();
 const defaultConfig: ConfigSchema = {
   unsafe: {
     alertDetails: {
-      apm: { enabled: false },
       logs: { enabled: false },
       metrics: { enabled: false },
       uptime: { enabled: false },

x-pack/plugins/observability/server/index.ts

@@ -30,9 +30,6 @@ const configSchema = schema.object({
   }),
   unsafe: schema.object({
     alertDetails: schema.object({
-      apm: schema.object({
-        enabled: schema.boolean({ defaultValue: false }),
-      }),
       metrics: schema.object({
         enabled: schema.boolean({ defaultValue: false }),
       }),