Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AO] Add alertDetailsUrl to infra rule types #157987

Merged
merged 18 commits into from
May 26, 2023
Merged

[AO] Add alertDetailsUrl to infra rule types #157987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5c3f200
Add alertDetailsUrl for metric threshold rule
maryam-saeidi May 17, 2023
e0af59f
Add alertDetailsUrl for inventory and log threshold, plus fixing test
maryam-saeidi May 22, 2023
2a59068
Merge branch 'main' into 156534-add-alertDetailsUrl-for-infra-rules
maryam-saeidi May 22, 2023
250d49b
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine May 22, 2023
c9bb075
Fix test
maryam-saeidi May 22, 2023
0ebeeae
Merge branch 'main' into 156534-add-alertDetailsUrl-for-infra-rules
maryam-saeidi May 22, 2023
43c360b
Fix type
maryam-saeidi May 22, 2023
0c4b23f
Merge branch 'main' into 156534-add-alertDetailsUrl-for-infra-rules
maryam-saeidi May 22, 2023
f73a510
Add rangeFrom to the alert URL
maryam-saeidi May 23, 2023
2e2cd15
Merge branch 'main' into 156534-add-alertDetailsUrl-for-infra-rules
maryam-saeidi May 23, 2023
04b47b2
Merge branch 'main' into 156534-add-alertDetailsUrl-for-infra-rules
maryam-saeidi May 24, 2023
43715ab
Always set alert start time in alertDetailsUrl
maryam-saeidi May 24, 2023
135bae0
Merge branch 'main' into 156534-add-alertDetailsUrl-for-infra-rules
maryam-saeidi May 24, 2023
77cf960
Update import
maryam-saeidi May 24, 2023
1eabdf8
Undo import update
maryam-saeidi May 24, 2023
86ba4b4
Merge branch 'main' into 156534-add-alertDetailsUrl-for-infra-rules
maryam-saeidi May 24, 2023
9a8529f
Merge branch 'main' into 156534-add-alertDetailsUrl-for-infra-rules
maryam-saeidi May 24, 2023
fadca68
Update CODEOWNER of x-pack/test/alerting_api_integration/observability
maryam-saeidi May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .buildkite/ftr_configs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ enabled:
- x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/config.ts
- x-pack/test/alerting_api_integration/spaces_only/tests/actions/config.ts
- x-pack/test/alerting_api_integration/spaces_only/tests/action_task_params/config.ts
- x-pack/test/alerting_api_integration/observability/config.ts
- x-pack/test/api_integration_basic/config.ts
- x-pack/test/api_integration/config_security_basic.ts
- x-pack/test/api_integration/config_security_trial.ts
Expand Down
1 change: 1 addition & 0 deletions .github/CODEOWNERS
View file
Open in desktop
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ x-pack/plugins/cloud_integrations/cloud_full_story/server/config.ts @elastic/kib

# Response Ops team
/x-pack/test/alerting_api_integration/ @elastic/response-ops
/x-pack/test/alerting_api_integration/observability @elastic/actionable-observability
/x-pack/test/plugin_api_integration/test_suites/task_manager/ @elastic/response-ops
/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/ @elastic/response-ops
/docs/user/alerting/ @elastic/response-ops
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/infra/server/lib/adapters/framework/adapter_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { Lifecycle } from '@hapi/hapi';
import { SharePluginSetup } from '@kbn/share-plugin/server';
import { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server';
import { JsonArray, JsonValue } from '@kbn/utility-types';
import { RouteConfig, RouteMethod } from '@kbn/core/server';
Expand All @@ -31,6 +32,7 @@ export interface InfraServerPluginSetupDeps {
features: FeaturesPluginSetup;
ruleRegistry: RuleRegistryPluginSetupContract;
observability: ObservabilityPluginSetup;
share: SharePluginSetup;
spaces: SpacesPluginSetup;
usageCollection: UsageCollectionSetup;
visTypeTimeseries: VisTypeTimeseriesSetup;
Expand Down
34 changes: 24 additions & 10 deletions x-pack/plugins/infra/server/lib/alerting/common/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
* 2.0.
*/

import moment from 'moment';
import { AlertsLocatorParams } from '@kbn/observability-plugin/common';
import { LocatorPublic } from '@kbn/share-plugin/common';
import { isEmpty, isError } from 'lodash';
import { schema } from '@kbn/config-schema';
import { Logger, LogMeta } from '@kbn/logging';
import type { ElasticsearchClient, IBasePath } from '@kbn/core/server';
import { addSpaceIdToPath } from '@kbn/spaces-plugin/common';
import { ObservabilityConfig } from '@kbn/observability-plugin/server';
import { ALERT_RULE_PARAMETERS, TIMESTAMP } from '@kbn/rule-data-utils';
import {
ParsedTechnicalFields,
Expand Down Expand Up @@ -110,15 +112,6 @@ export const createScopedLogger = (
};
};

export const getAlertDetailsPageEnabledForApp = (
config: ObservabilityConfig['unsafe']['alertDetails'] | null,
appName: keyof ObservabilityConfig['unsafe']['alertDetails']
): boolean => {
if (!config) return false;

return config[appName].enabled;
};

export const getViewInInventoryAppUrl = ({
basePath,
criteria,
Expand Down Expand Up @@ -153,6 +146,27 @@ export const getViewInInventoryAppUrl = ({
export const getViewInMetricsAppUrl = (basePath: IBasePath, spaceId: string) =>
addSpaceIdToPath(basePath.publicBaseUrl, spaceId, LINK_TO_METRICS_EXPLORER);

export const getAlertUrl = async (
alertUuid: string | null,
spaceId: string,
startedAt: string,
alertsLocator?: LocatorPublic<AlertsLocatorParams>,
publicBaseUrl?: string
) => {
if (!publicBaseUrl || !alertsLocator || !alertUuid) return '';

const rangeFrom = moment(startedAt).subtract('5', 'minute').toISOString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to take the window / look-back into account. Or perhaps rule interval. And consider multiples of those - for example, the range should be 3 x window, or something. If the look-back was an hour, or a day, seeing 5m may not be useful.

We'll probably get additional requirements for this anyway, so deferring till then is probably fine.

Copy link
Member Author

@maryam-saeidi maryam-saeidi May 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I was also thinking about the same but wanted to have a smaller PR focused on generating the link for infra rules for now.
In another instance, I used 1/5 of the alert duration period before the start of the alert and 1/5 after the alert ended (in case it was recovered.)

There is room for improvement, and I created the following ticket to improve this time range:
#158480


return (
await alertsLocator.getLocation({
baseUrl: publicBaseUrl,
spaceId,
kuery: `kibana.alert.uuid: "${alertUuid}"`,
rangeFrom,
})
).path;
};

export const getAlertDetailsUrl = (
basePath: IBasePath,
spaceId: string,
Expand Down
38 changes: 28 additions & 10 deletions ...fra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
AdditionalContext,
createScopedLogger,
flattenAdditionalContext,
getAlertDetailsUrl,
getAlertUrl,
getContextForRecoveredAlerts,
getViewInInventoryAppUrl,
UNGROUPED_FACTORY_KEY,
Expand Down Expand Up @@ -130,12 +130,18 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
const actionGroupId = FIRED_ACTIONS.id; // Change this to an Error action group when able
const reason = buildInvalidQueryAlertReason(params.filterQueryText);
const alert = alertFactory(UNGROUPED_FACTORY_KEY, reason, actionGroupId);
const indexedStartedDate =
const indexedStartedAt =
getAlertStartedDate(UNGROUPED_FACTORY_KEY) ?? startedAt.toISOString();
const alertUuid = getAlertUuid(UNGROUPED_FACTORY_KEY);

alert.scheduleActions(actionGroupId, {
alertDetailsUrl: getAlertDetailsUrl(libs.basePath, spaceId, alertUuid),
alertDetailsUrl: await getAlertUrl(
alertUuid,
spaceId,
indexedStartedAt,
libs.alertsLocator,
libs.basePath.publicBaseUrl
),
alertState: stateToAlertMessage[AlertStates.ERROR],
group: UNGROUPED_FACTORY_KEY,
metric: mapToConditionsLookup(criteria, (c) => c.metric),
Expand All @@ -146,7 +152,7 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
basePath: libs.basePath,
criteria,
nodeType,
timestamp: indexedStartedDate,
timestamp: indexedStartedAt,
spaceId,
}),
});
Expand Down Expand Up @@ -256,13 +262,19 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
additionalContext,
evaluationValues
);
const indexedStartedDate = getAlertStartedDate(group) ?? startedAt.toISOString();
const indexedStartedAt = getAlertStartedDate(group) ?? startedAt.toISOString();
const alertUuid = getAlertUuid(group);

scheduledActionsCount++;

const context = {
alertDetailsUrl: getAlertDetailsUrl(libs.basePath, spaceId, alertUuid),
alertDetailsUrl: await getAlertUrl(
alertUuid,
spaceId,
indexedStartedAt,
libs.alertsLocator,
libs.basePath.publicBaseUrl
),
alertState: stateToAlertMessage[nextState],
group,
reason,
Expand All @@ -276,7 +288,7 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
basePath: libs.basePath,
criteria,
nodeType,
timestamp: indexedStartedDate,
timestamp: indexedStartedAt,
spaceId,
}),
...additionalContext,
Expand All @@ -290,14 +302,20 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =

for (const alert of recoveredAlerts) {
const recoveredAlertId = alert.getId();
const indexedStartedDate = getAlertStartedDate(recoveredAlertId) ?? startedAt.toISOString();
const indexedStartedAt = getAlertStartedDate(recoveredAlertId) ?? startedAt.toISOString();
const alertUuid = getAlertUuid(recoveredAlertId);
const alertHits = alertUuid ? await getAlertByAlertUuid(alertUuid) : undefined;
const additionalContext = getContextForRecoveredAlerts(alertHits);
const originalActionGroup = getOriginalActionGroup(alertHits);

alert.setContext({
alertDetailsUrl: getAlertDetailsUrl(libs.basePath, spaceId, alertUuid),
alertDetailsUrl: await getAlertUrl(
alertUuid,
spaceId,
indexedStartedAt,
libs.alertsLocator,
libs.basePath.publicBaseUrl
),
alertState: stateToAlertMessage[AlertStates.OK],
group: recoveredAlertId,
metric: mapToConditionsLookup(criteria, (c) => c.metric),
Expand All @@ -307,7 +325,7 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
basePath: libs.basePath,
criteria,
nodeType,
timestamp: indexedStartedDate,
timestamp: indexedStartedAt,
spaceId,
}),
originalAlertState: translateActionGroupToAlertState(originalActionGroup),
Expand Down
22 changes: 6 additions & 16 deletions .../lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_rule_type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,7 @@ import {
valueActionVariableDescription,
viewInAppUrlActionVariableDescription,
} from '../common/messages';
import {
getAlertDetailsPageEnabledForApp,
oneOfLiterals,
validateIsStringElasticsearchJSONFilter,
} from '../common/utils';
import { oneOfLiterals, validateIsStringElasticsearchJSONFilter } from '../common/utils';
import {
createInventoryMetricThresholdExecutor,
FIRED_ACTIONS,
Expand Down Expand Up @@ -86,8 +82,6 @@ export async function registerMetricInventoryThresholdRuleType(
alertingPlugin: PluginSetupContract,
libs: InfraBackendLibs
) {
const config = libs.getAlertDetailsConfig();

alertingPlugin.registerType({
id: METRIC_INVENTORY_THRESHOLD_ALERT_TYPE_ID,
name: i18n.translate('xpack.infra.metrics.inventory.alertName', {
Expand Down Expand Up @@ -118,15 +112,11 @@ export async function registerMetricInventoryThresholdRuleType(
context: [
{ name: 'group', description: groupActionVariableDescription },
{ name: 'alertState', description: alertStateActionVariableDescription },
...(getAlertDetailsPageEnabledForApp(config, 'metrics')
? [
{
name: 'alertDetailsUrl',
description: alertDetailUrlActionVariableDescription,
usesPublicBaseUrl: true,
},
]
: []),
{
name: 'alertDetailsUrl',
description: alertDetailUrlActionVariableDescription,
usesPublicBaseUrl: true,
},
{ name: 'reason', description: reasonActionVariableDescription },
{ name: 'timestamp', description: timestampActionVariableDescription },
{ name: 'value', description: valueActionVariableDescription },
Expand Down
28 changes: 23 additions & 5 deletions x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { i18n } from '@kbn/i18n';
import { AlertsLocatorParams } from '@kbn/observability-plugin/common';
import {
ALERT_CONTEXT,
ALERT_EVALUATION_THRESHOLD,
Expand All @@ -23,7 +24,9 @@ import {
RuleExecutorServices,
RuleTypeState,
} from '@kbn/alerting-plugin/server';
import { LocatorPublic } from '@kbn/share-plugin/common';
import { addSpaceIdToPath } from '@kbn/spaces-plugin/common';
maryam-saeidi marked this conversation as resolved.
Show resolved Hide resolved
import { asyncForEach } from '@kbn/std';

import { ParsedTechnicalFields } from '@kbn/rule-registry-plugin/common';
import { ParsedExperimentalFields } from '@kbn/rule-registry-plugin/common/parse_experimental_fields';
Expand Down Expand Up @@ -57,7 +60,7 @@ import { InfraBackendLibs } from '../../infra_types';
import {
AdditionalContext,
flattenAdditionalContext,
getAlertDetailsUrl,
getAlertUrl,
getContextForRecoveredAlerts,
getGroupByObject,
unflattenObject,
Expand Down Expand Up @@ -127,7 +130,7 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) =>
getAlertUuid,
getAlertByAlertUuid,
} = services;
const { basePath } = libs;
const { basePath, alertsLocator } = libs;

const alertFactory: LogThresholdAlertFactory = (
id,
Expand Down Expand Up @@ -168,7 +171,7 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) =>
viewInAppUrl,
};

actions.forEach((actionSet) => {
asyncForEach(actions, async (actionSet) => {
const { actionGroup, context } = actionSet;

const alertInstanceId = (context.group || id) as string;
Expand All @@ -178,7 +181,13 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) =>
alert.scheduleActions(actionGroup, {
...sharedContext,
...context,
alertDetailsUrl: getAlertDetailsUrl(libs.basePath, spaceId, alertUuid),
alertDetailsUrl: await getAlertUrl(
alertUuid,
spaceId,
indexedStartedAt,
libs.alertsLocator,
libs.basePath.publicBaseUrl
),
});
});
}
Expand Down Expand Up @@ -234,6 +243,7 @@ export const createLogThresholdExecutor = (libs: InfraBackendLibs) =>
startedAt,
validatedParams,
getAlertByAlertUuid,
alertsLocator,
});
} catch (e) {
throw new Error(e);
Expand Down Expand Up @@ -997,6 +1007,7 @@ const processRecoveredAlerts = async ({
startedAt,
validatedParams,
getAlertByAlertUuid,
alertsLocator,
}: {
basePath: IBasePath;
getAlertStartedDate: (alertId: string) => string | null;
Expand All @@ -1008,6 +1019,7 @@ const processRecoveredAlerts = async ({
getAlertByAlertUuid: (
alertUuid: string
) => Promise<Partial<ParsedTechnicalFields & ParsedExperimentalFields> | null> | null;
alertsLocator?: LocatorPublic<AlertsLocatorParams>;
}) => {
const groupByKeysObjectForRecovered = getGroupByObject(
validatedParams.groupBy,
Expand All @@ -1024,7 +1036,13 @@ const processRecoveredAlerts = async ({
const viewInAppUrl = addSpaceIdToPath(basePath.publicBaseUrl, spaceId, relativeViewInAppUrl);

const baseContext = {
alertDetailsUrl: getAlertDetailsUrl(basePath, spaceId, alertUuid),
alertDetailsUrl: await getAlertUrl(
alertUuid,
spaceId,
indexedStartedAt,
alertsLocator,
basePath.publicBaseUrl
),
group: hasGroupBy(validatedParams) ? recoveredAlertId : null,
groupByKeys: groupByKeysObjectForRecovered[recoveredAlertId],
timestamp: startedAt.toISOString(),
Expand Down
17 changes: 5 additions & 12 deletions x-pack/plugins/infra/server/lib/alerting/log_threshold/register_log_threshold_rule_type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
} from '../../../../common/alerting/logs/log_threshold';
import { InfraBackendLibs } from '../../infra_types';
import { decodeOrThrow } from '../../../../common/runtime_types';
import { getAlertDetailsPageEnabledForApp } from '../common/utils';
import {
alertDetailUrlActionVariableDescription,
groupByKeysActionVariableDescription,
Expand Down Expand Up @@ -110,8 +109,6 @@ export async function registerLogThresholdRuleType(
);
}

const config = libs.getAlertDetailsConfig();

alertingPlugin.registerType({
id: LOG_DOCUMENT_COUNT_RULE_TYPE_ID,
name: i18n.translate('xpack.infra.logs.alertName', {
Expand Down Expand Up @@ -144,15 +141,11 @@ export async function registerLogThresholdRuleType(
name: 'denominatorConditions',
description: denominatorConditionsActionVariableDescription,
},
...(getAlertDetailsPageEnabledForApp(config, 'logs')
? [
{
name: 'alertDetailsUrl',
description: alertDetailUrlActionVariableDescription,
usesPublicBaseUrl: true,
},
]
: []),
{
name: 'alertDetailsUrl',
description: alertDetailUrlActionVariableDescription,
usesPublicBaseUrl: true,
},
{
name: 'viewInAppUrl',
description: viewInAppUrlActionVariableDescription,
Expand Down
Loading