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.

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
Changes from 1 commit
Commits
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
Next Next commit
[RAC] Rename occurrences of alert_type to rule_type in Infra
ersin-erdal committed Dec 6, 2021
commit cc173ab01f19a904cf79cbcfb86cbda11343e613
6 changes: 3 additions & 3 deletions x-pack/plugins/infra/public/alerting/inventory/index.ts
Original file line number Diff line number Diff line change
@@ -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', {
Original file line number Diff line number Diff line change
@@ -5,5 +5,5 @@
* 2.0.
*/

export * from './log_threshold_alert_type';
export * from './log_threshold_rule_type';
export { AlertDropdown } from './components/alert_dropdown';
Original file line number Diff line number Diff line change
@@ -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', {
9 changes: 4 additions & 5 deletions x-pack/plugins/infra/public/alerting/metric_anomaly/index.ts
Original file line number Diff line number Diff line change
@@ -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', {
Original file line number Diff line number Diff line change
@@ -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', {
14 changes: 6 additions & 8 deletions x-pack/plugins/infra/public/plugin.ts
Original file line number Diff line number Diff line change
@@ -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',
2 changes: 1 addition & 1 deletion x-pack/plugins/infra/server/lib/alerting/index.ts
Original file line number Diff line number Diff line change
@@ -5,4 +5,4 @@
* 2.0.
*/

export { registerAlertTypes } from './register_alert_types';
export { registerRuleTypes } from './register_rule_types';
Original file line number Diff line number Diff line change
@@ -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';
Original file line number Diff line number Diff line change
@@ -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';
@@ -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

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: {
@@ -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,
@@ -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
Original file line number Diff line number Diff line change
@@ -58,7 +58,7 @@ const condition = schema.object({
),
});

export async function registerMetricInventoryThresholdAlertType(
export async function registerMetricInventoryThresholdRuleType(
alertingPlugin: PluginSetupContract,
libs: InfraBackendLibs
) {
Original file line number Diff line number Diff line change
@@ -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';

@@ -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]],
@@ -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: {
@@ -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]],
@@ -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: {
@@ -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: {
Loading