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

[RAC] Rename occurrences of alert_type to rule_type in Infra #120455

Merged
merged 3 commits into from
Dec 7, 2021
Merged

[RAC] Rename occurrences of alert_type to rule_type in Infra #120455

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc173ab
[RAC] Rename occurrences of alert_type to rule_type in Infra
ersin-erdal Dec 6, 2021
7b696cf
[RAC] Rename missing RuleTypeState params
ersin-erdal Dec 6, 2021
ae56798
[RAC] Rename missing RuleTypeState params
ersin-erdal Dec 6, 2021
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
6 changes: 3 additions & 3 deletions x-pack/plugins/infra/public/alerting/inventory/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ import {

import { ObservabilityRuleTypeModel } from '../../../../observability/public';

import { AlertTypeParams } from '../../../../alerting/common';
import { AlertTypeParams as RuleTypeParams } from '../../../../alerting/common';
Copy link
Contributor

Choose a reason for hiding this comment

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

And this is just until things change on the alerting side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ys exactly. We also talked with them, they will use these names too, so we (or they) can just remove " .... as" part once they did the refactoring on their side.

import { validateMetricThreshold } from './components/validation';
import { formatReason } from './rule_data_formatters';

interface InventoryMetricAlertTypeParams extends AlertTypeParams {
interface InventoryMetricRuleTypeParams extends RuleTypeParams {
criteria: InventoryMetricConditions[];
}

export function createInventoryMetricAlertType(): ObservabilityRuleTypeModel<InventoryMetricAlertTypeParams> {
export function createInventoryMetricRuleType(): ObservabilityRuleTypeModel<InventoryMetricRuleTypeParams> {
return {
id: METRIC_INVENTORY_THRESHOLD_ALERT_TYPE_ID,
description: i18n.translate('xpack.infra.metrics.inventory.alertFlyout.alertDescription', {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/infra/public/alerting/log_threshold/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
* 2.0.
*/

export * from './log_threshold_alert_type';
export * from './log_threshold_rule_type';
export { AlertDropdown } from './components/alert_dropdown';
4 changes: 2 additions & 2 deletions ...log_threshold/log_threshold_alert_type.ts → .../log_threshold/log_threshold_rule_type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import { ObservabilityRuleTypeModel } from '../../../../observability/public';
import {
LOG_DOCUMENT_COUNT_ALERT_TYPE_ID,
ersin-erdal marked this conversation as resolved.
Show resolved Hide resolved
PartialAlertParams,
ersin-erdal marked this conversation as resolved.
Show resolved Hide resolved
} from '../../../common/alerting/logs/log_threshold/types';
} from '../../../common/alerting/logs/log_threshold';
import { formatRuleData } from './rule_data_formatters';
import { validateExpression } from './validation';

export function createLogThresholdAlertType(): ObservabilityRuleTypeModel<PartialAlertParams> {
export function createLogThresholdRuleType(): ObservabilityRuleTypeModel<PartialAlertParams> {
return {
id: LOG_DOCUMENT_COUNT_ALERT_TYPE_ID,
description: i18n.translate('xpack.infra.logs.alertFlyout.alertDescription', {
Expand Down
9 changes: 4 additions & 5 deletions x-pack/plugins/infra/public/alerting/metric_anomaly/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,15 @@
import { i18n } from '@kbn/i18n';
import React from 'react';
import { METRIC_ANOMALY_ALERT_TYPE_ID } from '../../../common/alerting/metrics';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { AlertTypeModel } from '../../../../triggers_actions_ui/public/types';
import { AlertTypeParams } from '../../../../alerting/common';
import { AlertTypeModel } from '../../../../triggers_actions_ui/public';
Copy link
Contributor

Choose a reason for hiding this comment

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

import { AlertTypeModel as RuleTypeModel } from '../../../../triggers_actions_ui/public';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if we should rename this now? Because we will refactor triggers_actions_ui too, then we can tackle these imports by searching by the usages.

import { AlertTypeParams as RuleTypeParams } from '../../../../alerting/common';
import { validateMetricAnomaly } from './components/validation';

interface MetricAnomalyAlertTypeParams extends AlertTypeParams {
interface MetricAnomalyRuleTypeParams extends RuleTypeParams {
hasInfraMLCapabilities: boolean;
}

export function createMetricAnomalyAlertType(): AlertTypeModel<MetricAnomalyAlertTypeParams> {
export function createMetricAnomalyRuleType(): AlertTypeModel<MetricAnomalyRuleTypeParams> {
return {
id: METRIC_ANOMALY_ALERT_TYPE_ID,
description: i18n.translate('xpack.infra.metrics.anomaly.alertFlyout.alertDescription', {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/infra/public/alerting/metric_threshold/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@ import React from 'react';
import { ObservabilityRuleTypeModel } from '../../../../observability/public';
import { validateMetricThreshold } from './components/validation';
import { formatReason } from './rule_data_formatters';
import { AlertTypeParams } from '../../../../alerting/common';
import { AlertTypeParams as RuleTypeParams } from '../../../../alerting/common';
import {
MetricExpressionParams,
METRIC_THRESHOLD_ALERT_TYPE_ID,
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
} from '../../../server/lib/alerting/metric_threshold/types';

interface MetricThresholdAlertTypeParams extends AlertTypeParams {
interface MetricThresholdRuleTypeParams extends RuleTypeParams {
criteria: MetricExpressionParams[];
}

export function createMetricThresholdAlertType(): ObservabilityRuleTypeModel<MetricThresholdAlertTypeParams> {
export function createMetricThresholdRuleType(): ObservabilityRuleTypeModel<MetricThresholdRuleTypeParams> {
return {
id: METRIC_THRESHOLD_ALERT_TYPE_ID,
description: i18n.translate('xpack.infra.metrics.alertFlyout.alertDescription', {
Expand Down
14 changes: 6 additions & 8 deletions x-pack/plugins/infra/public/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,17 @@ export class Plugin implements InfraClientPluginClass {
registerFeatures(pluginsSetup.home);
}

const { createInventoryMetricAlertType } = await import('./alerting/inventory');
const { createLogThresholdAlertType } = await import('./alerting/log_threshold');
const { createMetricThresholdAlertType } = await import('./alerting/metric_threshold');
const { createInventoryMetricRuleType } = await import('./alerting/inventory');
const { createLogThresholdRuleType } = await import('./alerting/log_threshold');
const { createMetricThresholdRuleType } = await import('./alerting/metric_threshold');

pluginsSetup.observability.observabilityRuleTypeRegistry.register(
createInventoryMetricAlertType()
createInventoryMetricRuleType()
);

pluginsSetup.observability.observabilityRuleTypeRegistry.register(createLogThresholdRuleType());
pluginsSetup.observability.observabilityRuleTypeRegistry.register(
createLogThresholdAlertType()
);
pluginsSetup.observability.observabilityRuleTypeRegistry.register(
createMetricThresholdAlertType()
createMetricThresholdRuleType()
);
pluginsSetup.observability.dashboard.register({
appName: 'infra_logs',
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/infra/server/lib/alerting/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
* 2.0.
*/

export { registerAlertTypes } from './register_alert_types';
export { registerRuleTypes } from './register_rule_types';
12 changes: 6 additions & 6 deletions x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/evaluate_condition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@
import { mapValues, last, first } from 'lodash';
import moment from 'moment';
import { ElasticsearchClient } from 'kibana/server';
import { SnapshotCustomMetricInput } from '../../../../common/http_api/snapshot_api';
import {
isTooManyBucketsPreviewException,
TOO_MANY_BUCKETS_PREVIEW_EXCEPTION,
} from '../../../../common/alerting/metrics';
import {
InfraDatabaseSearchResponse,
CallWithRequestParams,
} from '../../adapters/framework/adapter_types';
import { InfraDatabaseSearchResponse, CallWithRequestParams } from '../../adapters/framework';
import { Comparator, InventoryMetricConditions } from './types';
import { InventoryItemType, SnapshotMetricType } from '../../../../common/inventory_models/types';
import { InfraTimerangeInput, SnapshotRequest } from '../../../../common/http_api/snapshot_api';
import {
InfraTimerangeInput,
SnapshotRequest,
SnapshotCustomMetricInput,
} from '../../../../common/http_api';
import { InfraSource } from '../../sources';
import { UNGROUPED_FACTORY_KEY } from '../common/utils';
import { getNodes } from '../../../routes/snapshot/lib/get_nodes';
Expand Down
41 changes: 22 additions & 19 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 @@ -15,11 +15,14 @@ import { AlertStates } from './types';
import {
ActionGroupIdsOf,
ActionGroup,
AlertInstanceContext,
AlertInstanceState,
AlertInstanceContext as AlertContext,
AlertInstanceState as AlertState,
RecoveredActionGroup,
} from '../../../../../alerting/common';
import { AlertInstance, AlertTypeState } from '../../../../../alerting/server';
import {
AlertInstance as Alert,
AlertTypeState as RuleTypeState,
} from '../../../../../alerting/server';
import { SnapshotMetricType } from '../../../../common/inventory_models/types';
import { InfraBackendLibs } from '../../infra_types';
import { METRIC_FORMATTERS } from '../../../../common/formatters/snapshot_metric_formats';
Expand All @@ -39,34 +42,34 @@ type InventoryMetricThresholdAllowedActionGroups = ActionGroupIdsOf<
typeof FIRED_ACTIONS | typeof WARNING_ACTIONS
>;

export type InventoryMetricThresholdAlertTypeState = AlertTypeState; // no specific state used
export type InventoryMetricThresholdAlertInstanceState = AlertInstanceState; // no specific state used
export type InventoryMetricThresholdAlertInstanceContext = AlertInstanceContext; // no specific instance context used
export type InventoryMetricThresholdRuleState = RuleTypeState; // no specific state used
ersin-erdal marked this conversation as resolved.
Show resolved Hide resolved
export type InventoryMetricThresholdAlertState = AlertState; // no specific state used
export type InventoryMetricThresholdAlertContext = AlertContext; // no specific instance context used

type InventoryMetricThresholdAlertInstance = AlertInstance<
InventoryMetricThresholdAlertInstanceState,
InventoryMetricThresholdAlertInstanceContext,
type InventoryMetricThresholdAlert = Alert<
InventoryMetricThresholdAlertState,
InventoryMetricThresholdAlertContext,
InventoryMetricThresholdAllowedActionGroups
>;
type InventoryMetricThresholdAlertInstanceFactory = (
type InventoryMetricThresholdAlertFactory = (
id: string,
reason: string,
threshold?: number | undefined,
value?: number | undefined
) => InventoryMetricThresholdAlertInstance;
) => InventoryMetricThresholdAlert;

export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =>
libs.metricsRules.createLifecycleRuleExecutor<
InventoryMetricThresholdParams & Record<string, unknown>,
InventoryMetricThresholdAlertTypeState,
InventoryMetricThresholdAlertInstanceState,
InventoryMetricThresholdAlertInstanceContext,
InventoryMetricThresholdRuleState,
ersin-erdal marked this conversation as resolved.
Show resolved Hide resolved
InventoryMetricThresholdAlertState,
InventoryMetricThresholdAlertContext,
InventoryMetricThresholdAllowedActionGroups
>(async ({ services, params }) => {
const { criteria, filterQuery, sourceId, nodeType, alertOnNoData } = params;
if (criteria.length === 0) throw new Error('Cannot execute an alert with 0 conditions');
const { alertWithLifecycle, savedObjectsClient } = services;
const alertInstanceFactory: InventoryMetricThresholdAlertInstanceFactory = (id, reason) =>
Copy link
Contributor

@mgiota mgiota Dec 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The task_runner of the alerting framework exposes an alertInstanceFactory method https://github.com/elastic/kibana/blob/main/x-pack/plugins/alerting/server/task_runner/task_runner.ts#L321

This alertInstanceFactory variable is a local variable in this file, so I don't think we have a problem here. I make the comment just to double check if everything works as expected (I wait for yarn start to complete to double check)

const alertFactory: InventoryMetricThresholdAlertFactory = (id, reason) =>
alertWithLifecycle({
id,
fields: {
Expand All @@ -82,8 +85,8 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
} catch (e) {
const actionGroupId = FIRED_ACTIONS.id; // Change this to an Error action group when able
const reason = buildInvalidQueryAlertReason(params.filterQueryText);
const alertInstance = alertInstanceFactory('*', reason);
alertInstance.scheduleActions(actionGroupId, {
const alert = alertFactory('*', reason);
alert.scheduleActions(actionGroupId, {
group: '*',
alertState: stateToAlertMessage[AlertStates.ERROR],
reason,
Expand Down Expand Up @@ -191,8 +194,8 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) =
? WARNING_ACTIONS.id
: FIRED_ACTIONS.id;

const alertInstance = alertInstanceFactory(`${group}`, reason);
alertInstance.scheduleActions(
const alert = alertFactory(`${group}`, reason);
alert.scheduleActions(
/**
* TODO: We're lying to the compiler here as explicitly calling `scheduleActions` on
* the RecoveredActionGroup isn't allowed
Expand Down
2 changes: 1 addition & 1 deletion ..._inventory_metric_threshold_alert_type.ts → ...r_inventory_metric_threshold_rule_type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const condition = schema.object({
),
});

export async function registerMetricInventoryThresholdAlertType(
export async function registerMetricInventoryThresholdRuleType(
alertingPlugin: PluginSetupContract,
libs: InfraBackendLibs
) {
Expand Down
24 changes: 12 additions & 12 deletions x-pack/plugins/infra/server/lib/alerting/log_threshold/log_threshold_executor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
Criterion,
UngroupedSearchQueryResponse,
GroupedSearchQueryResponse,
} from '../../../../common/alerting/logs/log_threshold/types';
} from '../../../../common/alerting/logs/log_threshold';
import { alertsMock } from '../../../../../alerting/server/mocks';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';

Expand Down Expand Up @@ -407,7 +407,7 @@ describe('Log threshold executor', () => {
describe('Results processors', () => {
describe('Can process ungrouped results', () => {
test('It handles the ALERT state correctly', () => {
const alertInstanceUpdaterMock = jest.fn();
const alertUpdaterMock = jest.fn();
const alertParams = {
...baseAlertParams,
criteria: [positiveCriteria[0]],
Expand All @@ -423,12 +423,12 @@ describe('Log threshold executor', () => {
results,
alertParams,
alertsMock.createAlertInstanceFactory,
alertInstanceUpdaterMock
alertUpdaterMock
);
// First call, second argument
expect(alertInstanceUpdaterMock.mock.calls[0][1]).toBe(AlertStates.ALERT);
expect(alertUpdaterMock.mock.calls[0][1]).toBe(AlertStates.ALERT);
// First call, third argument
expect(alertInstanceUpdaterMock.mock.calls[0][2]).toEqual([
expect(alertUpdaterMock.mock.calls[0][2]).toEqual([
{
actionGroup: 'logs.threshold.fired',
context: {
Expand All @@ -444,7 +444,7 @@ describe('Log threshold executor', () => {

describe('Can process grouped results', () => {
test('It handles the ALERT state correctly', () => {
const alertInstanceUpdaterMock = jest.fn();
const alertUpdaterMock = jest.fn();
const alertParams = {
...baseAlertParams,
criteria: [positiveCriteria[0]],
Expand Down Expand Up @@ -487,13 +487,13 @@ describe('Log threshold executor', () => {
results,
alertParams,
alertsMock.createAlertInstanceFactory,
alertInstanceUpdaterMock
alertUpdaterMock
);
expect(alertInstanceUpdaterMock.mock.calls.length).toBe(2);
expect(alertUpdaterMock.mock.calls.length).toBe(2);
// First call, second argument
expect(alertInstanceUpdaterMock.mock.calls[0][1]).toBe(AlertStates.ALERT);
expect(alertUpdaterMock.mock.calls[0][1]).toBe(AlertStates.ALERT);
// First call, third argument
expect(alertInstanceUpdaterMock.mock.calls[0][2]).toEqual([
expect(alertUpdaterMock.mock.calls[0][2]).toEqual([
{
actionGroup: 'logs.threshold.fired',
context: {
Expand All @@ -506,9 +506,9 @@ describe('Log threshold executor', () => {
]);

// Second call, second argument
expect(alertInstanceUpdaterMock.mock.calls[1][1]).toBe(AlertStates.ALERT);
expect(alertUpdaterMock.mock.calls[1][1]).toBe(AlertStates.ALERT);
// Second call, third argument
expect(alertInstanceUpdaterMock.mock.calls[1][2]).toEqual([
expect(alertUpdaterMock.mock.calls[1][2]).toEqual([
{
actionGroup: 'logs.threshold.fired',
context: {
Expand Down
Loading