Skip to content

Commit

Permalink
[8.12] Deprecate feature flag for Log threshold alert details page (e…
Browse files Browse the repository at this point in the history
…lastic#172554) (elastic#172775)

# Backport

This will backport the following commits from `main` to `8.12`:
- [Deprecate feature flag for Log threshold alert details page
(elastic#172554)](elastic#172554)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Bena
Kansara","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-12-07T07:35:01Z","message":"Deprecate
feature flag for Log threshold alert details page (elastic#172554)\n\nResolves
https://github.com/elastic/kibana/issues/172379\r\n\r\n- Deprecates
following feature flag used for enabling/disabling Log\r\nthreshold
alert details
page:\r\n```\r\nxpack.observability.unsafe.alertDetails.logs.enabled\r\n```\r\n-
Removes usage of this flag from code.\r\n- Adding this flag in
`kibana.yml` will generate following warning:\r\n```\r\n[WARN
][config.deprecation] You no longer need to configure
\"xpack.observability.unsafe.alertDetails.logs.enabled\".\r\n```\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"6628232433c0144f7564b22d8e6d2941425431ae","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v8.12.0","Team:obs-ux-management","v8.13.0"],"number":172554,"url":"https://github.com/elastic/kibana/pull/172554","mergeCommit":{"message":"Deprecate
feature flag for Log threshold alert details page (elastic#172554)\n\nResolves
https://github.com/elastic/kibana/issues/172379\r\n\r\n- Deprecates
following feature flag used for enabling/disabling Log\r\nthreshold
alert details
page:\r\n```\r\nxpack.observability.unsafe.alertDetails.logs.enabled\r\n```\r\n-
Removes usage of this flag from code.\r\n- Adding this flag in
`kibana.yml` will generate following warning:\r\n```\r\n[WARN
][config.deprecation] You no longer need to configure
\"xpack.observability.unsafe.alertDetails.logs.enabled\".\r\n```\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"6628232433c0144f7564b22d8e6d2941425431ae"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/172554","number":172554,"mergeCommit":{"message":"Deprecate
feature flag for Log threshold alert details page (elastic#172554)\n\nResolves
https://github.com/elastic/kibana/issues/172379\r\n\r\n- Deprecates
following feature flag used for enabling/disabling Log\r\nthreshold
alert details
page:\r\n```\r\nxpack.observability.unsafe.alertDetails.logs.enabled\r\n```\r\n-
Removes usage of this flag from code.\r\n- Adding this flag in
`kibana.yml` will generate following warning:\r\n```\r\n[WARN
][config.deprecation] You no longer need to configure
\"xpack.observability.unsafe.alertDetails.logs.enabled\".\r\n```\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"6628232433c0144f7564b22d8e6d2941425431ae"}}]}]
BACKPORT-->

Co-authored-by: Bena Kansara <[email protected]>
  • Loading branch information
kibanamachine and benakansara authored Dec 7, 2023
1 parent f40e47c commit 2a4a765
Show file tree
Hide file tree
Showing 15 changed files with 10 additions and 61 deletions.
6 changes: 0 additions & 6 deletions x-pack/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -63,7 +59,6 @@ import { InfraBackendLibs } from '../../infra_types';
import {
AdditionalContext,
flattenAdditionalContext,
getAlertDetailsPageEnabledForApp,
getContextForRecoveredAlerts,
getGroupByObject,
unflattenObject,
Expand Down Expand Up @@ -138,7 +133,6 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) =>
getAlertByAlertUuid,
} = services;
const { basePath, alertsLocator } = libs;
const config = libs.getAlertDetailsConfig();

const alertFactory: LogThresholdAlertFactory = (
id,
Expand Down Expand Up @@ -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),
});
});
}
Expand Down Expand Up @@ -254,7 +240,6 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) =>
validatedParams,
getAlertByAlertUuid,
alertsLocator,
isAlertDetailsPageEnabled: getAlertDetailsPageEnabledForApp(config, 'logs'),
});
} catch (e) {
throw new Error(e);
Expand Down Expand Up @@ -868,7 +853,6 @@ const processRecoveredAlerts = async ({
validatedParams,
getAlertByAlertUuid,
alertsLocator,
isAlertDetailsPageEnabled = false,
}: {
basePath: IBasePath;
getAlertStartedDate: (alertId: string) => string | null;
Expand All @@ -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,
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ describe('renderApp', () => {
const config = {
unsafe: {
alertDetails: {
logs: { enabled: false },
metrics: { enabled: false },
uptime: { enabled: false },
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ const params = {
const config: Subset<ConfigSchema> = {
unsafe: {
alertDetails: {
logs: { enabled: true },
metrics: { enabled: true },
uptime: { enabled: true },
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ describe('APMSection', () => {
const config = {
unsafe: {
alertDetails: {
logs: { enabled: false },
metrics: { enabled: false },
uptime: { enabled: false },
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ const withCore = makeDecorator({
const config: ConfigSchema = {
unsafe: {
alertDetails: {
logs: { enabled: false },
metrics: { enabled: false },
uptime: { enabled: false },
observability: { enabled: false },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/observability/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export interface ConfigSchema {
metrics: {
enabled: boolean;
};
logs: {
logs?: {
enabled: boolean;
};
uptime: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import type { TopAlert } from '../typings/alerts';
const defaultConfig = {
unsafe: {
alertDetails: {
logs: { enabled: false },
metrics: { enabled: false },
uptime: { enabled: false },
},
Expand Down Expand Up @@ -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 },
},
Expand Down Expand Up @@ -113,7 +107,6 @@ describe('isAlertDetailsEnabled', () => {
const updatedConfig = {
unsafe: {
alertDetails: {
logs: { enabled: false },
metrics: { enabled: false },
uptime: { enabled: false },
},
Expand Down Expand Up @@ -159,7 +152,6 @@ describe('isAlertDetailsEnabled', () => {
const updatedConfig = {
unsafe: {
alertDetails: {
logs: { enabled: false },
metrics: { enabled: true },
uptime: { enabled: false },
},
Expand Down Expand Up @@ -201,7 +193,6 @@ describe('isAlertDetailsEnabled', () => {
const updatedConfig = {
unsafe: {
alertDetails: {
logs: { enabled: false },
metrics: { enabled: false },
uptime: { enabled: true },
},
Expand Down Expand Up @@ -243,7 +234,6 @@ describe('isAlertDetailsEnabled', () => {
const updatedConfig = {
unsafe: {
alertDetails: {
logs: { enabled: true },
metrics: { enabled: true },
uptime: { enabled: true },
},
Expand All @@ -255,7 +245,6 @@ describe('isAlertDetailsEnabled', () => {
const updatedConfig = {
unsafe: {
alertDetails: {
logs: { enabled: true },
metrics: { enabled: true },
uptime: { enabled: true },
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/observability/public/utils/test_helper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/observability/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
Expand Down Expand Up @@ -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>;
Expand Down

0 comments on commit 2a4a765

Please sign in to comment.