From a64554517868d1c686db0b26c3d86250ba59b817 Mon Sep 17 00:00:00 2001 From: Gerard Soldevila Date: Fri, 25 Mar 2022 15:28:55 +0100 Subject: [PATCH 1/9] Rewrite plugin_status logic limiting usage of Observables (reducing heap size) (#128324) * WIP * Fix behavior when no plugins are defined * Remove unused import, reduce debounce times * Fix startup behavior * Misc improvements following PR comments * Fix plugin_status UTs * Code cleanup + enhancements * Remove fixed FIXME --- .../server/status/cached_plugins_status.ts | 50 +++ src/core/server/status/plugins_status.test.ts | 41 +- src/core/server/status/plugins_status.ts | 390 +++++++++++++----- src/core/server/status/status_service.test.ts | 32 +- src/core/server/status/status_service.ts | 6 +- 5 files changed, 374 insertions(+), 145 deletions(-) create mode 100644 src/core/server/status/cached_plugins_status.ts diff --git a/src/core/server/status/cached_plugins_status.ts b/src/core/server/status/cached_plugins_status.ts new file mode 100644 index 00000000000000..fec9f51e63172e --- /dev/null +++ b/src/core/server/status/cached_plugins_status.ts @@ -0,0 +1,50 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { Observable } from 'rxjs'; + +import { type PluginName } from '../plugins'; +import { type ServiceStatus } from './types'; + +import { type Deps, PluginsStatusService as BasePluginsStatusService } from './plugins_status'; + +export class PluginsStatusService extends BasePluginsStatusService { + private all$?: Observable>; + private dependenciesStatuses$: Record>>; + private derivedStatuses$: Record>; + + constructor(deps: Deps) { + super(deps); + this.dependenciesStatuses$ = {}; + this.derivedStatuses$ = {}; + } + + public getAll$(): Observable> { + if (!this.all$) { + this.all$ = super.getAll$(); + } + + return this.all$; + } + + public getDependenciesStatus$(plugin: PluginName): Observable> { + if (!this.dependenciesStatuses$[plugin]) { + this.dependenciesStatuses$[plugin] = super.getDependenciesStatus$(plugin); + } + + return this.dependenciesStatuses$[plugin]; + } + + public getDerivedStatus$(plugin: PluginName): Observable { + if (!this.derivedStatuses$[plugin]) { + this.derivedStatuses$[plugin] = super.getDerivedStatus$(plugin); + } + + return this.derivedStatuses$[plugin]; + } +} diff --git a/src/core/server/status/plugins_status.test.ts b/src/core/server/status/plugins_status.test.ts index 0befbf63bd186d..c07624826ff830 100644 --- a/src/core/server/status/plugins_status.test.ts +++ b/src/core/server/status/plugins_status.test.ts @@ -10,7 +10,7 @@ import { PluginName } from '../plugins'; import { PluginsStatusService } from './plugins_status'; import { of, Observable, BehaviorSubject, ReplaySubject } from 'rxjs'; import { ServiceStatusLevels, CoreStatus, ServiceStatus } from './types'; -import { first } from 'rxjs/operators'; +import { first, skip } from 'rxjs/operators'; import { ServiceStatusLevelSnapshotSerializer } from './test_utils'; expect.addSnapshotSerializer(ServiceStatusLevelSnapshotSerializer); @@ -215,7 +215,7 @@ describe('PluginStatusService', () => { service.set('a', of({ level: ServiceStatusLevels.available, summary: 'a status' })); expect(await service.getAll$().pipe(first()).toPromise()).toEqual({ - a: { level: ServiceStatusLevels.available, summary: 'a status' }, // a is available depsite savedObjects being degraded + a: { level: ServiceStatusLevels.available, summary: 'a status' }, // a is available despite savedObjects being degraded b: { level: ServiceStatusLevels.degraded, summary: '1 service is degraded: savedObjects', @@ -239,6 +239,10 @@ describe('PluginStatusService', () => { const statusUpdates: Array> = []; const subscription = service .getAll$() + // If we subscribe to the $getAll() Observable BEFORE setting a custom status Observable + // for a given plugin ('a' in this test), then the first emission will happen + // right after core$ services Observable emits + .pipe(skip(1)) .subscribe((pluginStatuses) => statusUpdates.push(pluginStatuses)); service.set('a', of({ level: ServiceStatusLevels.degraded, summary: 'a degraded' })); @@ -261,6 +265,8 @@ describe('PluginStatusService', () => { const statusUpdates: Array> = []; const subscription = service .getAll$() + // the first emission happens right after core services emit (see explanation above) + .pipe(skip(1)) .subscribe((pluginStatuses) => statusUpdates.push(pluginStatuses)); const aStatus$ = new BehaviorSubject({ @@ -280,19 +286,21 @@ describe('PluginStatusService', () => { }); it('emits an unavailable status if first emission times out, then continues future emissions', async () => { - jest.useFakeTimers(); - const service = new PluginsStatusService({ - core$: coreAllAvailable$, - pluginDependencies: new Map([ - ['a', []], - ['b', ['a']], - ]), - }); + const service = new PluginsStatusService( + { + core$: coreAllAvailable$, + pluginDependencies: new Map([ + ['a', []], + ['b', ['a']], + ]), + }, + 10 // set a small timeout so that the registered status Observable for 'a' times out quickly + ); const pluginA$ = new ReplaySubject(1); service.set('a', pluginA$); - const firstEmission = service.getAll$().pipe(first()).toPromise(); - jest.runAllTimers(); + // the first emission happens right after core$ services emit + const firstEmission = service.getAll$().pipe(skip(1), first()).toPromise(); expect(await firstEmission).toEqual({ a: { level: ServiceStatusLevels.unavailable, summary: 'Status check timed out after 30s' }, @@ -308,16 +316,16 @@ describe('PluginStatusService', () => { pluginA$.next({ level: ServiceStatusLevels.available, summary: 'a available' }); const secondEmission = service.getAll$().pipe(first()).toPromise(); - jest.runAllTimers(); expect(await secondEmission).toEqual({ a: { level: ServiceStatusLevels.available, summary: 'a available' }, b: { level: ServiceStatusLevels.available, summary: 'All dependencies are available' }, }); - jest.useRealTimers(); }); }); describe('getDependenciesStatus$', () => { + const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + it('only includes dependencies of specified plugin', async () => { const service = new PluginsStatusService({ core$: coreAllAvailable$, @@ -357,7 +365,7 @@ describe('PluginStatusService', () => { it('debounces plugins custom status registration', async () => { const service = new PluginsStatusService({ - core$: coreAllAvailable$, + core$: coreOneCriticalOneDegraded$, pluginDependencies, }); const available: ServiceStatus = { @@ -375,8 +383,6 @@ describe('PluginStatusService', () => { expect(statusUpdates).toStrictEqual([]); - const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - // Waiting for the debounce timeout should cut a new update await delay(25); subscription.unsubscribe(); @@ -404,7 +410,6 @@ describe('PluginStatusService', () => { const subscription = service .getDependenciesStatus$('b') .subscribe((status) => statusUpdates.push(status)); - const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); pluginA$.next(degraded); pluginA$.next(available); diff --git a/src/core/server/status/plugins_status.ts b/src/core/server/status/plugins_status.ts index c4e8e7e3642482..8d042d4cba3f9f 100644 --- a/src/core/server/status/plugins_status.ts +++ b/src/core/server/status/plugins_status.ts @@ -5,166 +5,338 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ - -import { BehaviorSubject, Observable, combineLatest, of } from 'rxjs'; +import { BehaviorSubject, Observable, ReplaySubject, Subscription } from 'rxjs'; import { map, distinctUntilChanged, - switchMap, + filter, debounceTime, timeoutWith, startWith, } from 'rxjs/operators'; +import { sortBy } from 'lodash'; import { isDeepStrictEqual } from 'util'; -import { PluginName } from '../plugins'; -import { ServiceStatus, CoreStatus, ServiceStatusLevels } from './types'; +import { type PluginName } from '../plugins'; +import { type ServiceStatus, type CoreStatus, ServiceStatusLevels } from './types'; import { getSummaryStatus } from './get_summary_status'; const STATUS_TIMEOUT_MS = 30 * 1000; // 30 seconds -interface Deps { +const defaultStatus: ServiceStatus = { + level: ServiceStatusLevels.unavailable, + summary: `Status check timed out after ${STATUS_TIMEOUT_MS / 1000}s`, +}; + +export interface Deps { core$: Observable; pluginDependencies: ReadonlyMap; } +interface PluginData { + [name: PluginName]: { + name: PluginName; + depth: number; // depth of this plugin in the dependency tree (root plugins will have depth = 1) + dependencies: PluginName[]; + reverseDependencies: PluginName[]; + reportedStatus?: ServiceStatus; + derivedStatus: ServiceStatus; + }; +} +interface PluginStatus { + [name: PluginName]: ServiceStatus; +} + +interface ReportedStatusSubscriptions { + [name: PluginName]: Subscription; +} + export class PluginsStatusService { - private readonly pluginStatuses = new Map>(); - private readonly derivedStatuses = new Map>(); - private readonly dependenciesStatuses = new Map< - PluginName, - Observable> - >(); - private allPluginsStatuses?: Observable>; - - private readonly update$ = new BehaviorSubject(true); - private readonly defaultInheritedStatus$: Observable; + private coreStatus: CoreStatus = { elasticsearch: defaultStatus, savedObjects: defaultStatus }; + private pluginData: PluginData; + private rootPlugins: PluginName[]; // root plugins are those that do not have any dependencies + private orderedPluginNames: PluginName[]; + private pluginData$ = new ReplaySubject(1); + private pluginStatus: PluginStatus = {}; + private pluginStatus$ = new BehaviorSubject(this.pluginStatus); + private reportedStatusSubscriptions: ReportedStatusSubscriptions = {}; + private isReportingStatus: Record = {}; private newRegistrationsAllowed = true; + private coreSubscription: Subscription; - constructor(private readonly deps: Deps) { - this.defaultInheritedStatus$ = this.deps.core$.pipe( - map((coreStatus) => { - return getSummaryStatus(Object.entries(coreStatus), { - allAvailableSummary: `All dependencies are available`, - }); - }) - ); + constructor(deps: Deps, private readonly statusTimeoutMs: number = STATUS_TIMEOUT_MS) { + this.pluginData = this.initPluginData(deps.pluginDependencies); + this.rootPlugins = this.getRootPlugins(); + this.orderedPluginNames = this.getOrderedPluginNames(); + + this.coreSubscription = deps.core$ + .pipe(debounceTime(10)) + .subscribe((coreStatus: CoreStatus) => this.updateCoreAndPluginStatuses(coreStatus)); } + /** + * Register a status Observable for a specific plugin + * @param {PluginName} plugin The name of the plugin + * @param {Observable} status$ An external Observable that must be trusted as the source of truth for the status of the plugin + * @throws An error if the status registrations are not allowed + */ public set(plugin: PluginName, status$: Observable) { if (!this.newRegistrationsAllowed) { throw new Error( `Custom statuses cannot be registered after setup, plugin [${plugin}] attempted` ); } - this.pluginStatuses.set(plugin, status$); - this.update$.next(true); // trigger all existing Observables to update from the new source Observable + + this.isReportingStatus[plugin] = true; + // unsubscribe from any previous subscriptions. Ideally plugins should register a status Observable only once + this.reportedStatusSubscriptions[plugin]?.unsubscribe(); + + // delete any derived statuses calculated before the custom status Observable was registered + delete this.pluginStatus[plugin]; + + this.reportedStatusSubscriptions[plugin] = status$ + // Set a timeout for externally-defined status Observables + .pipe(timeoutWith(this.statusTimeoutMs, status$.pipe(startWith(defaultStatus)))) + .subscribe((status) => this.updatePluginReportedStatus(plugin, status)); } + /** + * Prevent plugins from registering status Observables + */ public blockNewRegistrations() { this.newRegistrationsAllowed = false; } + /** + * Obtain an Observable of the status of all the plugins + * @returns {Observable>} An Observable that will yield the current status of all plugins + */ public getAll$(): Observable> { - if (!this.allPluginsStatuses) { - this.allPluginsStatuses = this.getPluginStatuses$([...this.deps.pluginDependencies.keys()]); - } - return this.allPluginsStatuses; + return this.pluginStatus$.asObservable().pipe( + // do not emit until we have a status for all plugins + filter((all) => Object.keys(all).length === this.orderedPluginNames.length), + distinctUntilChanged>(isDeepStrictEqual) + ); } + /** + * Obtain an Observable of the status of the dependencies of the given plugin + * @param {PluginName} plugin the name of the plugin whose dependencies' status must be retreived + * @returns {Observable>} An Observable that will yield the current status of the plugin's dependencies + */ public getDependenciesStatus$(plugin: PluginName): Observable> { - const dependencies = this.deps.pluginDependencies.get(plugin); - if (!dependencies) { - throw new Error(`Unknown plugin: ${plugin}`); - } - if (!this.dependenciesStatuses.has(plugin)) { - this.dependenciesStatuses.set( - plugin, - this.getPluginStatuses$(dependencies).pipe( - // Prevent many emissions at once from dependency status resolution from making this too noisy - debounceTime(25) - ) - ); - } - return this.dependenciesStatuses.get(plugin)!; + const directDependencies = this.pluginData[plugin].dependencies; + + return this.getAll$().pipe( + map((allStatus) => { + const dependenciesStatus: Record = {}; + directDependencies.forEach((dep) => (dependenciesStatus[dep] = allStatus[dep])); + return dependenciesStatus; + }), + debounceTime(10) + ); } + /** + * Obtain an Observable of the derived status of the given plugin + * @param {PluginName} plugin the name of the plugin whose derived status must be retrieved + * @returns {Observable} An Observable that will yield the derived status of the plugin + */ public getDerivedStatus$(plugin: PluginName): Observable { - if (!this.derivedStatuses.has(plugin)) { - this.derivedStatuses.set( - plugin, - this.update$.pipe( - debounceTime(25), // Avoid calling the plugin's custom status logic for every plugin that depends on it. - switchMap(() => { - // Only go up the dependency tree if any of this plugin's dependencies have a custom status - // Helps eliminate memory overhead of creating thousands of Observables unnecessarily. - if (this.anyCustomStatuses(plugin)) { - return combineLatest([this.deps.core$, this.getDependenciesStatus$(plugin)]).pipe( - map(([coreStatus, pluginStatuses]) => { - return getSummaryStatus( - [...Object.entries(coreStatus), ...Object.entries(pluginStatuses)], - { - allAvailableSummary: `All dependencies are available`, - } - ); - }) - ); - } else { - return this.defaultInheritedStatus$; - } - }) - ) - ); - } - return this.derivedStatuses.get(plugin)!; + return this.pluginData$.asObservable().pipe( + map((pluginData) => pluginData[plugin]?.derivedStatus), + filter((status: ServiceStatus | undefined): status is ServiceStatus => !!status), + distinctUntilChanged(isDeepStrictEqual) + ); } - private getPluginStatuses$(plugins: PluginName[]): Observable> { - if (plugins.length === 0) { - return of({}); + /** + * Hook to be called at the stop lifecycle event + */ + public stop() { + // Cancel all active subscriptions + this.coreSubscription.unsubscribe(); + Object.values(this.reportedStatusSubscriptions).forEach((subscription) => { + subscription.unsubscribe(); + }); + } + + /** + * Initialize a convenience data structure + * that maintain up-to-date information about the plugins and their statuses + * @param {ReadonlyMap} pluginDependencies Information about the different plugins and their dependencies + * @returns {PluginData} + */ + private initPluginData(pluginDependencies: ReadonlyMap): PluginData { + const pluginData: PluginData = {}; + + if (pluginDependencies) { + pluginDependencies.forEach((dependencies, name) => { + pluginData[name] = { + name, + depth: 0, + dependencies, + reverseDependencies: [], + derivedStatus: defaultStatus, + }; + }); + + pluginDependencies.forEach((dependencies, name) => { + dependencies.forEach((dependency) => { + pluginData[dependency].reverseDependencies.push(name); + }); + }); } - return this.update$.pipe( - switchMap(() => { - const pluginStatuses = plugins - .map((depName) => { - const pluginStatus = this.pluginStatuses.get(depName) - ? this.pluginStatuses.get(depName)!.pipe( - timeoutWith( - STATUS_TIMEOUT_MS, - this.pluginStatuses.get(depName)!.pipe( - startWith({ - level: ServiceStatusLevels.unavailable, - summary: `Status check timed out after ${STATUS_TIMEOUT_MS / 1000}s`, - }) - ) - ) - ) - : this.getDerivedStatus$(depName); - return [depName, pluginStatus] as [PluginName, Observable]; - }) - .map(([pName, status$]) => - status$.pipe(map((status) => [pName, status] as [PluginName, ServiceStatus])) - ); - - return combineLatest(pluginStatuses).pipe( - map((statuses) => Object.fromEntries(statuses)), - distinctUntilChanged>(isDeepStrictEqual) - ); - }) + return pluginData; + } + + /** + * Create a list with all the root plugins. + * Root plugins are all those plugins that do not have any dependency. + * @returns {PluginName[]} a list with all the root plugins present in the provided deps + */ + private getRootPlugins(): PluginName[] { + return Object.keys(this.pluginData).filter( + (plugin) => this.pluginData[plugin].dependencies.length === 0 ); } /** - * Determines whether or not this plugin or any plugin in it's dependency tree have a custom status registered. + * Obtain a list of plugins names, ordered by depth. + * @see {calculateDepthRecursive} + * @returns {PluginName[]} a list of plugins, ordered by depth + name + */ + private getOrderedPluginNames(): PluginName[] { + this.rootPlugins.forEach((plugin) => { + this.calculateDepthRecursive(plugin, 1); + }); + + return sortBy(Object.values(this.pluginData), ['depth', 'name']).map(({ name }) => name); + } + + /** + * Calculate the depth of the given plugin, knowing that it's has at least the specified depth + * The depth of a plugin is determined by how many levels of dependencies the plugin has above it. + * We define root plugins as depth = 1, plugins that only depend on root plugins will have depth = 2 + * and so on so forth + * @param {PluginName} plugin the name of the plugin whose depth must be calculated + * @param {number} depth the minimum depth that we know for sure this plugin has + */ + private calculateDepthRecursive(plugin: PluginName, depth: number): void { + const pluginData = this.pluginData[plugin]; + pluginData.depth = Math.max(pluginData.depth, depth); + const newDepth = depth + 1; + pluginData.reverseDependencies.forEach((revDep) => + this.calculateDepthRecursive(revDep, newDepth) + ); + } + + /** + * Updates the core services statuses and plugins' statuses + * according to the latest status reported by core services. + * @param {CoreStatus} coreStatus the latest status of core services + */ + private updateCoreAndPluginStatuses(coreStatus: CoreStatus): void { + this.coreStatus = coreStatus!; + const derivedStatus = getSummaryStatus(Object.entries(this.coreStatus), { + allAvailableSummary: `All dependencies are available`, + }); + + this.rootPlugins.forEach((plugin) => { + this.pluginData[plugin].derivedStatus = derivedStatus; + if (!this.isReportingStatus[plugin]) { + // this root plugin has NOT registered any status Observable. Thus, its status is derived from core + this.pluginStatus[plugin] = derivedStatus; + } + }); + + this.updatePluginsStatuses(this.rootPlugins); + } + + /** + * Determine the derived statuses of the specified plugins and their dependencies, + * updating them on the pluginData structure + * Optionally, if the plugins have not registered a custom status Observable, update their "current" status as well. + * @param {PluginName[]} plugins The names of the plugins to be updated + */ + private updatePluginsStatuses(plugins: PluginName[]): void { + const toCheck = new Set(plugins); + + // Note that we are updating the plugins in an ordered fashion. + // This way, when updating plugin X (at depth = N), + // all of its dependencies (at depth < N) have already been updated + for (let i = 0; i < this.orderedPluginNames.length; ++i) { + const current = this.orderedPluginNames[i]; + if (toCheck.has(current)) { + // update the current plugin status + this.updatePluginStatus(current); + // flag all its reverse dependencies to be checked + // TODO flag them only IF the status of this plugin has changed, seems to break some tests + this.pluginData[current].reverseDependencies.forEach((revDep) => toCheck.add(revDep)); + } + } + + this.pluginData$.next(this.pluginData); + this.pluginStatus$.next({ ...this.pluginStatus }); + } + + /** + * Determine the derived status of the specified plugin and update it on the pluginData structure + * Optionally, if the plugin has not registered a custom status Observable, update its "current" status as well + * @param {PluginName} plugin The name of the plugin to be updated */ - private anyCustomStatuses(plugin: PluginName): boolean { - if (this.pluginStatuses.get(plugin)) { - return true; + private updatePluginStatus(plugin: PluginName): void { + const newStatus = this.determinePluginStatus(plugin); + this.pluginData[plugin].derivedStatus = newStatus; + + if (!this.isReportingStatus[plugin]) { + // this plugin has NOT registered any status Observable. + // Thus, its status is derived from its dependencies + core + this.pluginStatus[plugin] = newStatus; } + } + + /** + * Deterime the current plugin status, taking into account its reported status, its derived status + * and the status of the core services + * @param {PluginName} plugin the name of the plugin whose status must be determined + * @returns {ServiceStatus} The status of the plugin + */ + private determinePluginStatus(plugin: PluginName): ServiceStatus { + const coreStatus: Array<[PluginName, ServiceStatus]> = Object.entries(this.coreStatus); + const newLocal = this.pluginData[plugin]; + + let depsStatus: Array<[PluginName, ServiceStatus]> = []; - return this.deps.pluginDependencies - .get(plugin)! - .reduce((acc, depName) => acc || this.anyCustomStatuses(depName), false as boolean); + if (Object.keys(this.isReportingStatus).length) { + // if at least one plugin has registered a status Observable... take into account plugin dependencies + depsStatus = newLocal.dependencies.map((dependency) => [ + dependency, + this.pluginData[dependency].reportedStatus || this.pluginData[dependency].derivedStatus, + ]); + } + + const newStatus = getSummaryStatus([...coreStatus, ...depsStatus], { + allAvailableSummary: `All dependencies are available`, + }); + + return newStatus; + } + + /** + * Updates the reported status for the given plugin, along with the status of its dependencies tree. + * @param {PluginName} plugin The name of the plugin whose reported status must be updated + * @param {ServiceStatus} reportedStatus The newly reported status for that plugin + */ + private updatePluginReportedStatus(plugin: PluginName, reportedStatus: ServiceStatus): void { + const previousReportedLevel = this.pluginData[plugin].reportedStatus?.level; + + this.pluginData[plugin].reportedStatus = reportedStatus; + this.pluginStatus[plugin] = reportedStatus; + + if (reportedStatus.level !== previousReportedLevel) { + this.updatePluginsStatuses([plugin]); + } } } diff --git a/src/core/server/status/status_service.test.ts b/src/core/server/status/status_service.test.ts index dfd0ff9a7e1034..262667fddf26a3 100644 --- a/src/core/server/status/status_service.test.ts +++ b/src/core/server/status/status_service.test.ts @@ -239,20 +239,20 @@ describe('StatusService', () => { // Wait for timers to ensure that duplicate events are still filtered out regardless of debouncing. elasticsearch$.next(available); - await delay(500); + await delay(100); elasticsearch$.next(available); - await delay(500); + await delay(100); elasticsearch$.next({ level: ServiceStatusLevels.available, summary: `Wow another summary`, }); - await delay(500); + await delay(100); savedObjects$.next(degraded); - await delay(500); + await delay(100); savedObjects$.next(available); - await delay(500); + await delay(100); savedObjects$.next(available); - await delay(500); + await delay(100); subscription.unsubscribe(); expect(statusUpdates).toMatchInlineSnapshot(` @@ -300,9 +300,9 @@ describe('StatusService', () => { savedObjects$.next(available); savedObjects$.next(degraded); // Waiting for the debounce timeout should cut a new update - await delay(500); + await delay(100); savedObjects$.next(available); - await delay(500); + await delay(100); subscription.unsubscribe(); expect(statusUpdates).toMatchInlineSnapshot(` @@ -410,20 +410,20 @@ describe('StatusService', () => { // Wait for timers to ensure that duplicate events are still filtered out regardless of debouncing. elasticsearch$.next(available); - await delay(500); + await delay(100); elasticsearch$.next(available); - await delay(500); + await delay(100); elasticsearch$.next({ level: ServiceStatusLevels.available, summary: `Wow another summary`, }); - await delay(500); + await delay(100); savedObjects$.next(degraded); - await delay(500); + await delay(100); savedObjects$.next(available); - await delay(500); + await delay(100); savedObjects$.next(available); - await delay(500); + await delay(100); subscription.unsubscribe(); expect(statusUpdates).toMatchInlineSnapshot(` @@ -471,9 +471,9 @@ describe('StatusService', () => { savedObjects$.next(available); savedObjects$.next(degraded); // Waiting for the debounce timeout should cut a new update - await delay(500); + await delay(100); savedObjects$.next(available); - await delay(500); + await delay(100); subscription.unsubscribe(); expect(statusUpdates).toMatchInlineSnapshot(` diff --git a/src/core/server/status/status_service.ts b/src/core/server/status/status_service.ts index 63a1b02d5b2e7c..a5b5f0a37397ac 100644 --- a/src/core/server/status/status_service.ts +++ b/src/core/server/status/status_service.ts @@ -25,7 +25,7 @@ import type { InternalCoreUsageDataSetup } from '../core_usage_data'; import { config, StatusConfigType } from './status_config'; import { ServiceStatus, CoreStatus, InternalStatusServiceSetup } from './types'; import { getSummaryStatus } from './get_summary_status'; -import { PluginsStatusService } from './plugins_status'; +import { PluginsStatusService } from './cached_plugins_status'; import { getOverallStatusChanges } from './log_overall_status'; interface StatusLogMeta extends LogMeta { @@ -71,7 +71,7 @@ export class StatusService implements CoreService { this.overall$ = combineLatest([core$, this.pluginsStatus.getAll$()]).pipe( // Prevent many emissions at once from dependency status resolution from making this too noisy - debounceTime(500), + debounceTime(80), map(([coreStatus, pluginsStatus]) => { const summary = getSummaryStatus([ ...Object.entries(coreStatus), @@ -174,6 +174,8 @@ export class StatusService implements CoreService { this.subscriptions.forEach((subscription) => { subscription.unsubscribe(); }); + + this.pluginsStatus?.stop(); this.subscriptions = []; } From 32ac1a5355593be82eb0d31cf7e3cb9d20e56965 Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Fri, 25 Mar 2022 11:30:59 -0400 Subject: [PATCH 2/9] [responseOps] add snoozing MVP in alerting framework (#127694) resolves https://github.com/elastic/kibana/issues/126512 Changes the calculation of rule-level muting to take into account snoozeEndTime. If muteAll is true, the rule is considered snoozing forever, regardless of the setting of snoozeEndTime. If muteAll is false, snoozeEndTime determines whether the rule is snoozed. If snoozeEndTime is null, the rule is not snoozed. Otherwise, snoozeEndTime is a Date, and if it's >= than Date.now(), the rule is snoozed. Otherwise, the rule is not snoozed. --- .../server/rules_client/rules_client.ts | 4 + .../alerting/server/task_runner/fixtures.ts | 1 + .../server/task_runner/task_runner.test.ts | 66 ++++++++- .../server/task_runner/task_runner.ts | 20 ++- .../spaces_only/tests/alerting/index.ts | 1 + .../spaces_only/tests/alerting/snooze.ts | 132 ++++++++++++++++-- 6 files changed, 207 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts index 9642db04e504a5..4dc1ab6ce11226 100644 --- a/x-pack/plugins/alerting/server/rules_client/rules_client.ts +++ b/x-pack/plugins/alerting/server/rules_client/rules_client.ts @@ -2116,12 +2116,15 @@ export class RulesClient { executionStatus, schedule, actions, + snoozeEndTime, ...partialRawRule }: Partial, references: SavedObjectReference[] | undefined, includeLegacyId: boolean = false, excludeFromPublicApi: boolean = false ): PartialRule | PartialRuleWithLegacyId { + const snoozeEndTimeDate = snoozeEndTime != null ? new Date(snoozeEndTime) : snoozeEndTime; + const includeSnoozeEndTime = snoozeEndTimeDate !== undefined && !excludeFromPublicApi; const rule = { id, notifyWhen, @@ -2131,6 +2134,7 @@ export class RulesClient { schedule: schedule as IntervalSchedule, actions: actions ? this.injectReferencesIntoActions(id, actions, references || []) : [], params: this.injectReferencesIntoParams(id, ruleType, params, references || []) as Params, + ...(includeSnoozeEndTime ? { snoozeEndTime: snoozeEndTimeDate } : {}), ...(updatedAt ? { updatedAt: new Date(updatedAt) } : {}), ...(createdAt ? { createdAt: new Date(createdAt) } : {}), ...(scheduledTaskId ? { scheduledTaskId } : {}), diff --git a/x-pack/plugins/alerting/server/task_runner/fixtures.ts b/x-pack/plugins/alerting/server/task_runner/fixtures.ts index 3ba21c0de092cc..4e38be4834c860 100644 --- a/x-pack/plugins/alerting/server/task_runner/fixtures.ts +++ b/x-pack/plugins/alerting/server/task_runner/fixtures.ts @@ -22,6 +22,7 @@ export const RULE_TYPE_ID = 'test'; export const DATE_1969 = '1969-12-31T00:00:00.000Z'; export const DATE_1970 = '1970-01-01T00:00:00.000Z'; export const DATE_1970_5_MIN = '1969-12-31T23:55:00.000Z'; +export const DATE_9999 = '9999-12-31T12:34:56.789Z'; export const MOCK_DURATION = 86400000000000; export const SAVED_OBJECT = { diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts index e3e4f3045dd8f2..2227a34dfa1112 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.test.ts @@ -65,6 +65,7 @@ import { DATE_1969, DATE_1970, DATE_1970_5_MIN, + DATE_9999, } from './fixtures'; import { EVENT_LOG_ACTIONS } from '../plugin'; import { IN_MEMORY_METRICS } from '../monitoring'; @@ -414,7 +415,7 @@ describe('Task Runner', () => { ); expect(logger.debug).nthCalledWith( 3, - `no scheduling of actions for rule test:1: '${RULE_NAME}': rule is muted.` + `no scheduling of actions for rule test:1: '${RULE_NAME}': rule is snoozed.` ); expect(logger.debug).nthCalledWith( 4, @@ -468,6 +469,69 @@ describe('Task Runner', () => { expect(mockUsageCounter.incrementCounter).not.toHaveBeenCalled(); }); + type SnoozeTestParams = [ + muteAll: boolean, + snoozeEndTime: string | undefined | null, + shouldBeSnoozed: boolean + ]; + + const snoozeTestParams: SnoozeTestParams[] = [ + [false, null, false], + [false, undefined, false], + [false, DATE_1970, false], + [false, DATE_9999, true], + [true, null, true], + [true, undefined, true], + [true, DATE_1970, true], + [true, DATE_9999, true], + ]; + + test.each(snoozeTestParams)( + 'snoozing works as expected with muteAll: %s; snoozeEndTime: %s', + async (muteAll, snoozeEndTime, shouldBeSnoozed) => { + taskRunnerFactoryInitializerParams.actionsPlugin.isActionTypeEnabled.mockReturnValue(true); + taskRunnerFactoryInitializerParams.actionsPlugin.isActionExecutable.mockReturnValue(true); + ruleType.executor.mockImplementation( + async ({ + services: executorServices, + }: AlertExecutorOptions< + AlertTypeParams, + AlertTypeState, + AlertInstanceState, + AlertInstanceContext, + string + >) => { + executorServices.alertFactory.create('1').scheduleActions('default'); + } + ); + const taskRunner = new TaskRunner( + ruleType, + mockedTaskInstance, + taskRunnerFactoryInitializerParams, + inMemoryMetrics + ); + rulesClient.get.mockResolvedValue({ + ...mockedRuleTypeSavedObject, + muteAll, + snoozeEndTime: snoozeEndTime != null ? new Date(snoozeEndTime) : snoozeEndTime, + }); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue(SAVED_OBJECT); + await taskRunner.run(); + + const expectedExecutions = shouldBeSnoozed ? 0 : 1; + expect(actionsClient.enqueueExecution).toHaveBeenCalledTimes(expectedExecutions); + expect(actionsClient.ephemeralEnqueuedExecution).toHaveBeenCalledTimes(0); + + const logger = taskRunnerFactoryInitializerParams.logger; + const expectedMessage = `no scheduling of actions for rule test:1: '${RULE_NAME}': rule is snoozed.`; + if (expectedExecutions) { + expect(logger.debug).not.toHaveBeenCalledWith(expectedMessage); + } else { + expect(logger.debug).toHaveBeenCalledWith(expectedMessage); + } + } + ); + test.each(ephemeralTestParams)( 'skips firing actions for active alert if alert is muted %s', async (nameExtension, customTaskRunnerFactoryInitializerParams, enqueueFunction) => { diff --git a/x-pack/plugins/alerting/server/task_runner/task_runner.ts b/x-pack/plugins/alerting/server/task_runner/task_runner.ts index b3dacd053b6326..b7980f612e7b9a 100644 --- a/x-pack/plugins/alerting/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerting/server/task_runner/task_runner.ts @@ -252,6 +252,18 @@ export class TaskRunner< } } + private isRuleSnoozed(rule: SanitizedAlert): boolean { + if (rule.muteAll) { + return true; + } + + if (rule.snoozeEndTime == null) { + return false; + } + + return Date.now() < rule.snoozeEndTime.getTime(); + } + private shouldLogAndScheduleActionsForAlerts() { // if execution hasn't been cancelled, return true if (!this.cancelled) { @@ -312,7 +324,6 @@ export class TaskRunner< schedule, throttle, notifyWhen, - muteAll, mutedInstanceIds, name, tags, @@ -484,7 +495,8 @@ export class TaskRunner< triggeredActionsStatus: ActionsCompletion.COMPLETE, }; - if (!muteAll && this.shouldLogAndScheduleActionsForAlerts()) { + const ruleIsSnoozed = this.isRuleSnoozed(rule); + if (!ruleIsSnoozed && this.shouldLogAndScheduleActionsForAlerts()) { const mutedAlertIdsSet = new Set(mutedInstanceIds); const alertsWithExecutableActions = Object.entries(alertsWithScheduledActions).filter( @@ -535,8 +547,8 @@ export class TaskRunner< alertExecutionStore, }); } else { - if (muteAll) { - this.logger.debug(`no scheduling of actions for rule ${ruleLabel}: rule is muted.`); + if (ruleIsSnoozed) { + this.logger.debug(`no scheduling of actions for rule ${ruleLabel}: rule is snoozed.`); } if (!this.shouldLogAndScheduleActionsForAlerts()) { this.logger.debug( diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts index 823de1aa798c18..3007e373951565 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/index.ts @@ -44,6 +44,7 @@ export default function alertingTests({ loadTestFile, getService }: FtrProviderC loadTestFile(require.resolve('./notify_when')); loadTestFile(require.resolve('./ephemeral')); loadTestFile(require.resolve('./event_log_alerts')); + loadTestFile(require.resolve('./snooze')); loadTestFile(require.resolve('./scheduled_task_id')); // Do not place test files here, due to https://github.com/elastic/kibana/issues/123059 diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/snooze.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/snooze.ts index bb3e0cea469e49..5be5b59a152480 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/snooze.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/snooze.ts @@ -14,6 +14,7 @@ import { getUrlPrefix, getTestRuleData, ObjectRemover, + getEventLog, } from '../../../common/lib'; const FUTURE_SNOOZE_TIME = '9999-12-31T06:00:00.000Z'; @@ -22,6 +23,8 @@ const FUTURE_SNOOZE_TIME = '9999-12-31T06:00:00.000Z'; export default function createSnoozeRuleTests({ getService }: FtrProviderContext) { const supertest = getService('supertest'); const supertestWithoutAuth = getService('supertestWithoutAuth'); + const log = getService('log'); + const retry = getService('retry'); describe('snooze', () => { const objectRemover = new ObjectRemover(supertest); @@ -32,7 +35,7 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext it('should handle snooze rule request appropriately', async () => { const { body: createdAction } = await supertest - .post(`${getUrlPrefix(Spaces.space1.id)}}/api/actions/connector`) + .post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`) .set('kbn-xsrf', 'foo') .send({ name: 'MY action', @@ -41,8 +44,9 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext secrets: {}, }) .expect(200); + objectRemover.add(Spaces.space1.id, createdAction.id, 'action', 'actions'); - const { body: createdAlert } = await supertest + const { body: createdRule } = await supertest .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) .set('kbn-xsrf', 'foo') .send( @@ -58,16 +62,16 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext }) ) .expect(200); - objectRemover.add(Spaces.space1.id, createdAlert.id, 'rule', 'alerting'); + objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting'); const response = await alertUtils - .getSnoozeRequest(createdAlert.id) + .getSnoozeRequest(createdRule.id) .send({ snooze_end_time: FUTURE_SNOOZE_TIME }); expect(response.statusCode).to.eql(204); expect(response.body).to.eql(''); const { body: updatedAlert } = await supertestWithoutAuth - .get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdAlert.id}`) + .get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdRule.id}`) .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.snooze_end_time).to.eql(FUTURE_SNOOZE_TIME); @@ -77,13 +81,13 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext supertest, spaceId: Spaces.space1.id, type: 'alert', - id: createdAlert.id, + id: createdRule.id, }); }); it('should handle snooze rule request appropriately when snoozeEndTime is -1', async () => { const { body: createdAction } = await supertest - .post(`${getUrlPrefix(Spaces.space1.id)}}/api/actions/connector`) + .post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`) .set('kbn-xsrf', 'foo') .send({ name: 'MY action', @@ -92,8 +96,9 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext secrets: {}, }) .expect(200); + objectRemover.add(Spaces.space1.id, createdAction.id, 'action', 'actions'); - const { body: createdAlert } = await supertest + const { body: createdRule } = await supertest .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) .set('kbn-xsrf', 'foo') .send( @@ -109,16 +114,16 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext }) ) .expect(200); - objectRemover.add(Spaces.space1.id, createdAlert.id, 'rule', 'alerting'); + objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting'); const response = await alertUtils - .getSnoozeRequest(createdAlert.id) + .getSnoozeRequest(createdRule.id) .send({ snooze_end_time: -1 }); expect(response.statusCode).to.eql(204); expect(response.body).to.eql(''); const { body: updatedAlert } = await supertestWithoutAuth - .get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdAlert.id}`) + .get(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdRule.id}`) .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.snooze_end_time).to.eql(null); @@ -128,8 +133,111 @@ export default function createSnoozeRuleTests({ getService }: FtrProviderContext supertest, spaceId: Spaces.space1.id, type: 'alert', - id: createdAlert.id, + id: createdRule.id, }); }); + + it('should not trigger actions when snoozed', async () => { + const { body: createdAction, status: connStatus } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'MY action', + connector_type_id: 'test.noop', + config: {}, + secrets: {}, + }); + expect(connStatus).to.be(200); + objectRemover.add(Spaces.space1.id, createdAction.id, 'action', 'actions'); + + log.info('creating rule'); + const { body: createdRule, status: ruleStatus } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send( + getTestRuleData({ + name: 'should not trigger actions when snoozed', + rule_type_id: 'test.patternFiring', + schedule: { interval: '1s' }, + throttle: null, + notify_when: 'onActiveAlert', + params: { + pattern: { instance: arrayOfTrues(100) }, + }, + actions: [ + { + id: createdAction.id, + group: 'default', + params: {}, + }, + ], + }) + ); + expect(ruleStatus).to.be(200); + objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting'); + + // wait for an action to be triggered + log.info('wait for rule to trigger an action'); + await getRuleEvents(createdRule.id); + + log.info('start snoozing'); + const snoozeSeconds = 10; + const snoozeEndDate = new Date(Date.now() + 1000 * snoozeSeconds); + await alertUtils + .getSnoozeRequest(createdRule.id) + .send({ snooze_end_time: snoozeEndDate.toISOString() }); + + // could be an action execution while calling snooze, so set snooze start + // to a value that we know it will be in effect (after this call) + const snoozeStartDate = new Date(); + + // wait for 4 triggered actions - in case some fired before snooze went into effect + log.info('wait for snoozing to end'); + const ruleEvents = await getRuleEvents(createdRule.id, 4); + const snoozeStart = snoozeStartDate.valueOf(); + const snoozeEnd = snoozeStartDate.valueOf(); + let actionsBefore = 0; + let actionsDuring = 0; + let actionsAfter = 0; + + for (const event of ruleEvents) { + const timestamp = event?.['@timestamp']; + if (!timestamp) continue; + + const time = new Date(timestamp).valueOf(); + if (time < snoozeStart) { + actionsBefore++; + } else if (time > snoozeEnd) { + actionsAfter++; + } else { + actionsDuring++; + } + } + + expect(actionsBefore).to.be.greaterThan(0, 'no actions triggered before snooze'); + expect(actionsAfter).to.be.greaterThan(0, 'no actions triggered after snooze'); + expect(actionsDuring).to.be(0); + }); }); + + async function getRuleEvents(id: string, minActions: number = 1) { + return await retry.try(async () => { + return await getEventLog({ + getService, + spaceId: Spaces.space1.id, + type: 'alert', + id, + provider: 'alerting', + actions: new Map([['execute-action', { gte: minActions }]]), + }); + }); + } +} + +function arrayOfTrues(length: number) { + const result = []; + for (let i = 0; i < length; i++) { + result.push(true); + } + return result; } From 2c4c2ac664ef4d5330861a50a32e2a269fd87d0b Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Fri, 25 Mar 2022 14:23:32 -0400 Subject: [PATCH 3/9] skip failing suite (#128564) --- x-pack/test/functional/apps/lens/show_underlying_data.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/functional/apps/lens/show_underlying_data.ts b/x-pack/test/functional/apps/lens/show_underlying_data.ts index cbe6820ccef4d8..85d0a238832a68 100644 --- a/x-pack/test/functional/apps/lens/show_underlying_data.ts +++ b/x-pack/test/functional/apps/lens/show_underlying_data.ts @@ -16,7 +16,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const find = getService('find'); const browser = getService('browser'); - describe('show underlying data', () => { + // Failing: See https://github.com/elastic/kibana/issues/128564 + describe.skip('show underlying data', () => { it('should show the open button for a compatible saved visualization', async () => { await PageObjects.visualize.gotoVisualizationLandingPage(); await listingTable.searchForItemWithName('lnsXYvis'); From 3dc0e5810dcbb80457acff420e25c5a8d1722e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Marcondes?= <55978943+cauemarcondes@users.noreply.github.com> Date: Fri, 25 Mar 2022 14:42:02 -0400 Subject: [PATCH 4/9] [APM] Service overview only shows top 5 transaction groups (#128287) * fix transaction table * fixing i18n * PR comments * fixing types Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../app/transaction_overview/index.tsx | 1 + .../routing/service_detail/index.tsx | 46 ++++-- .../shared/transactions_table/get_columns.tsx | 12 +- .../shared/transactions_table/index.tsx | 136 +++++++++++++----- .../translations/translations/ja-JP.json | 2 - .../translations/translations/zh-CN.json | 2 - 6 files changed, 136 insertions(+), 63 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx b/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx index 68315fc3b2b021..bfe73e4ab0869e 100644 --- a/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx +++ b/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx @@ -81,6 +81,7 @@ export function TransactionOverview() { kuery={kuery} start={start} end={end} + saveTableOptionsToUrl /> diff --git a/x-pack/plugins/apm/public/components/routing/service_detail/index.tsx b/x-pack/plugins/apm/public/components/routing/service_detail/index.tsx index a4c2b84d57b355..acc231615d42f1 100644 --- a/x-pack/plugins/apm/public/components/routing/service_detail/index.tsx +++ b/x-pack/plugins/apm/public/components/routing/service_detail/index.tsx @@ -8,7 +8,7 @@ import * as t from 'io-ts'; import { i18n } from '@kbn/i18n'; import React from 'react'; import { Outlet } from '@kbn/typed-react-router-config'; -import { toBooleanRt } from '@kbn/io-ts-utils'; +import { toBooleanRt, toNumberRt } from '@kbn/io-ts-utils'; import { comparisonTypeRt } from '../../../../common/runtime_types/comparison_type_rt'; import { ENVIRONMENT_ALL } from '../../../../common/environment_filter_values'; import { environmentRt } from '../../../../common/environment_rt'; @@ -99,17 +99,27 @@ export const serviceDetail = { }, }, children: { - '/services/{serviceName}/overview': page({ - element: , - tab: 'overview', - title: i18n.translate('xpack.apm.views.overview.title', { - defaultMessage: 'Overview', + '/services/{serviceName}/overview': { + ...page({ + element: , + tab: 'overview', + title: i18n.translate('xpack.apm.views.overview.title', { + defaultMessage: 'Overview', + }), + searchBarOptions: { + showTransactionTypeSelector: true, + showTimeComparison: true, + }, }), - searchBarOptions: { - showTransactionTypeSelector: true, - showTimeComparison: true, - }, - }), + params: t.partial({ + query: t.partial({ + page: toNumberRt, + pageSize: toNumberRt, + sortField: t.string, + sortDirection: t.union([t.literal('asc'), t.literal('desc')]), + }), + }), + }, '/services/{serviceName}/transactions': { ...page({ tab: 'transactions', @@ -122,6 +132,14 @@ export const serviceDetail = { showTimeComparison: true, }, }), + params: t.partial({ + query: t.partial({ + page: toNumberRt, + pageSize: toNumberRt, + sortField: t.string, + sortDirection: t.union([t.literal('asc'), t.literal('desc')]), + }), + }), children: { '/services/{serviceName}/transactions/view': { element: , @@ -167,10 +185,10 @@ export const serviceDetail = { }), params: t.partial({ query: t.partial({ - sortDirection: t.string, + page: toNumberRt, + pageSize: toNumberRt, sortField: t.string, - pageSize: t.string, - page: t.string, + sortDirection: t.union([t.literal('asc'), t.literal('desc')]), }), }), children: { diff --git a/x-pack/plugins/apm/public/components/shared/transactions_table/get_columns.tsx b/x-pack/plugins/apm/public/components/shared/transactions_table/get_columns.tsx index 054514f430a077..da7aa46cab1541 100644 --- a/x-pack/plugins/apm/public/components/shared/transactions_table/get_columns.tsx +++ b/x-pack/plugins/apm/public/components/shared/transactions_table/get_columns.tsx @@ -6,6 +6,7 @@ */ import { + EuiBasicTableColumn, EuiFlexGroup, EuiFlexItem, EuiIcon, @@ -23,16 +24,15 @@ import { asTransactionRate, } from '../../../../common/utils/formatters'; import { APIReturnType } from '../../../services/rest/create_call_apm_api'; +import { + ChartType, + getTimeSeriesColor, +} from '../charts/helper/get_timeseries_color'; import { ImpactBar } from '../impact_bar'; import { TransactionDetailLink } from '../links/apm/transaction_detail_link'; import { ListMetric } from '../list_metric'; -import { ITableColumn } from '../managed_table'; import { TruncateWithTooltip } from '../truncate_with_tooltip'; import { getLatencyColumnLabel } from './get_latency_column_label'; -import { - ChartType, - getTimeSeriesColor, -} from '../charts/helper/get_timeseries_color'; type TransactionGroupMainStatistics = APIReturnType<'GET /internal/apm/services/{serviceName}/transactions/groups/main_statistics'>; @@ -59,7 +59,7 @@ export function getColumns({ comparisonEnabled?: boolean; shouldShowSparkPlots?: boolean; comparisonType?: TimeRangeComparisonType; -}): Array> { +}): Array> { return [ { field: 'name', diff --git a/x-pack/plugins/apm/public/components/shared/transactions_table/index.tsx b/x-pack/plugins/apm/public/components/shared/transactions_table/index.tsx index 66f068f6cb05c0..149e7350cc36c0 100644 --- a/x-pack/plugins/apm/public/components/shared/transactions_table/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/transactions_table/index.tsx @@ -5,14 +5,20 @@ * 2.0. */ -import { EuiFlexGroup, EuiFlexItem, EuiTitle } from '@elastic/eui'; +import { + EuiBasicTable, + EuiFlexGroup, + EuiFlexItem, + EuiTitle, +} from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { orderBy } from 'lodash'; -import React, { useState } from 'react'; +import React, { useMemo, useState } from 'react'; import uuid from 'uuid'; import { EuiCallOut } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n-react'; import { EuiCode } from '@elastic/eui'; +import { useHistory } from 'react-router-dom'; import { APIReturnType } from '../../../services/rest/create_call_apm_api'; import { useApmServiceContext } from '../../../context/apm_service/use_apm_service_context'; import { FETCH_STATUS, useFetcher } from '../../../hooks/use_fetcher'; @@ -22,9 +28,9 @@ import { OverviewTableContainer } from '../overview_table_container'; import { getColumns } from './get_columns'; import { ElasticDocsLink } from '../links/elastic_docs_link'; import { useBreakpoints } from '../../../hooks/use_breakpoints'; -import { ManagedTable } from '../managed_table'; import { useAnyOfApmParams } from '../../../hooks/use_apm_params'; import { LatencyAggregationType } from '../../../../common/latency_aggregation_types'; +import { fromQuery, toQuery } from '../links/url_helpers'; type ApiResponse = APIReturnType<'GET /internal/apm/services/{serviceName}/transactions/groups/main_statistics'>; @@ -64,6 +70,7 @@ interface Props { kuery: string; start: string; end: string; + saveTableOptionsToUrl?: boolean; } export function TransactionsTable({ @@ -77,32 +84,45 @@ export function TransactionsTable({ kuery, start, end, + saveTableOptionsToUrl = false, }: Props) { - const [tableOptions] = useState<{ - pageIndex: number; - sort: { - direction: SortDirection; - field: SortField; - }; + const history = useHistory(); + + const { + query: { + comparisonEnabled, + comparisonType, + latencyAggregationType, + page: urlPage = 0, + pageSize: urlPageSize = numberOfTransactionsPerPage, + sortField: urlSortField = 'impact', + sortDirection: urlSortDirection = 'desc', + }, + } = useAnyOfApmParams( + '/services/{serviceName}/transactions', + '/services/{serviceName}/overview' + ); + + const [tableOptions, setTableOptions] = useState<{ + page: { index: number; size: number }; + sort: { direction: SortDirection; field: SortField }; }>({ - pageIndex: 0, - sort: DEFAULT_SORT, + page: { index: urlPage, size: urlPageSize }, + sort: { + field: urlSortField as SortField, + direction: urlSortDirection as SortDirection, + }, }); // SparkPlots should be hidden if we're in two-column view and size XL (1200px) const { isXl } = useBreakpoints(); const shouldShowSparkPlots = isSingleColumn || !isXl; - const { pageIndex, sort } = tableOptions; + const { page, sort } = tableOptions; const { direction, field } = sort; + const { index, size } = page; const { transactionType, serviceName } = useApmServiceContext(); - const { - query: { comparisonEnabled, comparisonType, latencyAggregationType }, - } = useAnyOfApmParams( - '/services/{serviceName}/transactions', - '/services/{serviceName}/overview' - ); const { comparisonStart, comparisonEnd } = getTimeRangeComparison({ start, @@ -137,10 +157,7 @@ export function TransactionsTable({ response.transactionGroups, field, direction - ).slice( - pageIndex * numberOfTransactionsPerPage, - (pageIndex + 1) * numberOfTransactionsPerPage - ); + ).slice(index * size, (index + 1) * size); return { // Everytime the main statistics is refetched, updates the requestId making the detailed API to be refetched. @@ -162,7 +179,8 @@ export function TransactionsTable({ end, transactionType, latencyAggregationType, - pageIndex, + index, + size, direction, field, // not used, but needed to trigger an update when comparisonType is changed either manually by user or when time range is changed @@ -240,6 +258,21 @@ export function TransactionsTable({ const isLoading = status === FETCH_STATUS.LOADING; const isNotInitiated = status === FETCH_STATUS.NOT_INITIATED; + const pagination = useMemo( + () => ({ + pageIndex: index, + pageSize: size, + totalItemCount: transactionGroupsTotalItems, + showPerPageOptions, + }), + [index, size, transactionGroupsTotalItems, showPerPageOptions] + ); + + const sorting = useMemo( + () => ({ sort: { field, direction } }), + [field, direction] + ); + return ( - { + setTableOptions({ + page: { + index: newTableOptions.page?.index ?? 0, + size: + newTableOptions.page?.size ?? numberOfTransactionsPerPage, + }, + sort: newTableOptions.sort + ? { + field: newTableOptions.sort.field as SortField, + direction: newTableOptions.sort.direction, + } + : DEFAULT_SORT, + }); + if (saveTableOptionsToUrl) { + history.push({ + ...history.location, + search: fromQuery({ + ...toQuery(history.location.search), + page: newTableOptions.page?.index, + pageSize: newTableOptions.page?.size, + sortField: newTableOptions.sort?.field, + sortDirection: newTableOptions.sort?.direction, + }), + }); + } + }} /> diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index a8c561b138927d..0719666625dc74 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -7648,8 +7648,6 @@ "xpack.apm.transactionsTable.cardinalityWarning.docsLink": "詳細はドキュメントをご覧ください", "xpack.apm.transactionsTable.cardinalityWarning.title": "このビューには、報告されたトランザクションのサブセットが表示されます。", "xpack.apm.transactionsTable.linkText": "トランザクションを表示", - "xpack.apm.transactionsTable.loading": "読み込み中...", - "xpack.apm.transactionsTable.noResults": "トランザクショングループが見つかりません", "xpack.apm.transactionsTable.title": "トランザクション", "xpack.apm.transactionTypesSelectCustomOptionText": "新しいトランザクションタイプとして\\{searchValue\\}を追加", "xpack.apm.transactionTypesSelectPlaceholder": "トランザクションタイプを選択", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 6b2fa0b53fae2f..55651c16b66e81 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -7666,8 +7666,6 @@ "xpack.apm.transactionsTable.cardinalityWarning.docsLink": "在文档中了解详情", "xpack.apm.transactionsTable.cardinalityWarning.title": "此视图显示已报告事务的子集。", "xpack.apm.transactionsTable.linkText": "查看事务", - "xpack.apm.transactionsTable.loading": "正在加载……", - "xpack.apm.transactionsTable.noResults": "未找到事务组", "xpack.apm.transactionsTable.title": "事务", "xpack.apm.transactionTypesSelectCustomOptionText": "将 \\{searchValue\\} 添加为新事务类型", "xpack.apm.transactionTypesSelectPlaceholder": "选择事务类型", From f5229950bb7a358b4fe15aeb420ff7ba38c75f18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Fri, 25 Mar 2022 21:08:14 +0100 Subject: [PATCH 5/9] Ignore `jest.config.js` in coverage (#128537) --- packages/kbn-test/jest-preset.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kbn-test/jest-preset.js b/packages/kbn-test/jest-preset.js index ba515865e53230..fd14c973683f7d 100644 --- a/packages/kbn-test/jest-preset.js +++ b/packages/kbn-test/jest-preset.js @@ -16,7 +16,7 @@ module.exports = { coverageDirectory: '/target/kibana-coverage/jest', // An array of regexp pattern strings used to skip coverage collection - coveragePathIgnorePatterns: ['/node_modules/', '.*\\.d\\.ts'], + coveragePathIgnorePatterns: ['/node_modules/', '.*\\.d\\.ts', 'jest\\.config\\.js'], // A list of reporter names that Jest uses when writing coverage reports coverageReporters: !!process.env.CODE_COVERAGE From 805d7cf87db6a3e3095126af0b434a3d876e7533 Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Sun, 27 Mar 2022 12:38:53 -0500 Subject: [PATCH 6/9] skip flaky suite. #127076 --- .../server/integration_tests/cloud_preconfiguration.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/fleet/server/integration_tests/cloud_preconfiguration.test.ts b/x-pack/plugins/fleet/server/integration_tests/cloud_preconfiguration.test.ts index f3a4e045d042dc..b55a95aec61030 100644 --- a/x-pack/plugins/fleet/server/integration_tests/cloud_preconfiguration.test.ts +++ b/x-pack/plugins/fleet/server/integration_tests/cloud_preconfiguration.test.ts @@ -18,7 +18,8 @@ import { CLOUD_KIBANA_CONFIG } from './fixtures/cloud_kibana_config'; const logFilePath = Path.join(__dirname, 'logs.log'); -describe('Fleet preconfiguration reset', () => { +// FLAKY: https://github.com/elastic/kibana/issues/127076 +describe.skip('Fleet preconfiguration reset', () => { let esServer: kbnTestServer.TestElasticsearchUtils; let kbnServer: kbnTestServer.TestKibanaUtils; From 48783a08372a120ed8688d41d431dd363174965c Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Sun, 27 Mar 2022 12:47:25 -0500 Subject: [PATCH 7/9] skip flaky suite. #128558 --- test/functional/apps/management/_index_pattern_filter.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/apps/management/_index_pattern_filter.ts b/test/functional/apps/management/_index_pattern_filter.ts index afa64c474d39d4..732065282d5463 100644 --- a/test/functional/apps/management/_index_pattern_filter.ts +++ b/test/functional/apps/management/_index_pattern_filter.ts @@ -15,7 +15,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const PageObjects = getPageObjects(['settings']); const esArchiver = getService('esArchiver'); - describe('index pattern filter', function describeIndexTests() { + // FLAKY: https://github.com/elastic/kibana/issues/128558 + describe.skip('index pattern filter', function describeIndexTests() { before(async function () { await esArchiver.emptyKibanaIndex(); await kibanaServer.uiSettings.replace({}); From ca81ce870710c1ec31f2bb6736ce426be95b90b4 Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Sun, 27 Mar 2022 12:51:42 -0500 Subject: [PATCH 8/9] skip flaky suite --- .../spaces_only/tests/alerting/get_execution_log.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_execution_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_execution_log.ts index 55d4a72643c86a..5b2ba00ace7f85 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_execution_log.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_execution_log.ts @@ -26,7 +26,8 @@ export default function createGetExecutionLogTests({ getService }: FtrProviderCo const dateStart = new Date(Date.now() - 600000).toISOString(); - describe('getExecutionLog', () => { + // FLAKY: https://github.com/elastic/kibana/issues/128225 + describe.skip('getExecutionLog', () => { const objectRemover = new ObjectRemover(supertest); beforeEach(async () => { From c9aad65b6746fc2f27ad1d718df35b07022732a4 Mon Sep 17 00:00:00 2001 From: Ersin Erdal <92688503+ersin-erdal@users.noreply.github.com> Date: Mon, 28 Mar 2022 08:36:30 +0200 Subject: [PATCH 9/9] [Alerting] Remove defaultRuleTaskTimeout and set ruleType specific timeout from kibana.yml (#128294) Read ruleTaskTimeout from kibana.yml --- docs/settings/alert-action-settings.asciidoc | 26 +++++--- .../resources/base/bin/kibana-docker | 2 + x-pack/plugins/alerting/server/config.test.ts | 1 - x-pack/plugins/alerting/server/config.ts | 1 - x-pack/plugins/alerting/server/index.ts | 7 +++ .../server/lib/get_rule_task_timeout.test.ts | 61 +++++++++++++++++++ .../server/lib/get_rule_task_timeout.ts | 35 +++++++++++ x-pack/plugins/alerting/server/plugin.test.ts | 5 +- x-pack/plugins/alerting/server/plugin.ts | 30 ++++----- .../task_runner/create_execution_handler.ts | 5 +- 10 files changed, 146 insertions(+), 27 deletions(-) create mode 100644 x-pack/plugins/alerting/server/lib/get_rule_task_timeout.test.ts create mode 100644 x-pack/plugins/alerting/server/lib/get_rule_task_timeout.ts diff --git a/docs/settings/alert-action-settings.asciidoc b/docs/settings/alert-action-settings.asciidoc index 8f365381f1b8ee..51fa0b71f96011 100644 --- a/docs/settings/alert-action-settings.asciidoc +++ b/docs/settings/alert-action-settings.asciidoc @@ -185,13 +185,6 @@ For example, `20m`, `24h`, `7d`, `1w`. Default: `60s`. `xpack.alerting.maxEphemeralActionsPerAlert`:: Sets the number of actions that will be executed ephemerally. To use this, enable ephemeral tasks in task manager first with <> -`xpack.alerting.defaultRuleTaskTimeout`:: -Specifies the default timeout for the all rule types tasks. The time is formatted as: -+ -`[ms,s,m,h,d,w,M,Y]` -+ -For example, `20m`, `24h`, `7d`, `1w`. Default: `5m`. - `xpack.alerting.cancelAlertsOnRuleTimeout`:: Specifies whether to skip writing alerts and scheduling actions if rule execution is cancelled due to timeout. Default: `true`. This setting can be overridden by individual rule types. @@ -207,3 +200,22 @@ Specifies the behavior when a new or changed rule has a schedule interval less t `xpack.alerting.rules.execution.actions.max`:: Specifies the maximum number of actions that a rule can trigger each time detection checks run. + +`xpack.alerting.rules.execution.timeout`:: +Specifies the default timeout for tasks associated with all types of rules. The time is formatted as: ++ +`[ms,s,m,h,d,w,M,Y]` ++ +For example, `20m`, `24h`, `7d`, `1w`. Default: `5m`. + +`xpack.alerting.rules.execution.ruleTypeOverrides`:: +Overrides the configs under `xpack.alerting.rules.execution` for the rule type with the given ID. List the rule identifier and its settings in an array of objects. ++ +For example: +``` +xpack.alerting.rules.execution: + timeout: '5m' + ruleTypeOverrides: + - id: '.index-threshold' + timeout: '15m' +``` \ No newline at end of file diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker index 83a542c93d12b9..191f53208df721 100755 --- a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker +++ b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker @@ -199,6 +199,8 @@ kibana_vars=( xpack.alerting.invalidateApiKeysTask.interval xpack.alerting.invalidateApiKeysTask.removalDelay xpack.alerting.defaultRuleTaskTimeout + xpack.alerting.rules.execution.timeout + xpack.alerting.rules.execution.ruleTypeOverrides xpack.alerting.cancelAlertsOnRuleTimeout xpack.alerting.rules.minimumScheduleInterval.value xpack.alerting.rules.minimumScheduleInterval.enforce diff --git a/x-pack/plugins/alerting/server/config.test.ts b/x-pack/plugins/alerting/server/config.test.ts index cdb12b6755adaf..b08145219c24b6 100644 --- a/x-pack/plugins/alerting/server/config.test.ts +++ b/x-pack/plugins/alerting/server/config.test.ts @@ -13,7 +13,6 @@ describe('config validation', () => { expect(configSchema.validate(config)).toMatchInlineSnapshot(` Object { "cancelAlertsOnRuleTimeout": true, - "defaultRuleTaskTimeout": "5m", "healthCheck": Object { "interval": "60m", }, diff --git a/x-pack/plugins/alerting/server/config.ts b/x-pack/plugins/alerting/server/config.ts index d126fc4295050e..c727c137aa9a24 100644 --- a/x-pack/plugins/alerting/server/config.ts +++ b/x-pack/plugins/alerting/server/config.ts @@ -42,7 +42,6 @@ export const configSchema = schema.object({ maxEphemeralActionsPerAlert: schema.number({ defaultValue: DEFAULT_MAX_EPHEMERAL_ACTIONS_PER_ALERT, }), - defaultRuleTaskTimeout: schema.string({ validate: validateDurationSchema, defaultValue: '5m' }), cancelAlertsOnRuleTimeout: schema.boolean({ defaultValue: true }), rules: rulesSchema, }); diff --git a/x-pack/plugins/alerting/server/index.ts b/x-pack/plugins/alerting/server/index.ts index 49b65c678aa1f8..b44df6c3d1c862 100644 --- a/x-pack/plugins/alerting/server/index.ts +++ b/x-pack/plugins/alerting/server/index.ts @@ -60,5 +60,12 @@ export const config: PluginConfigDescriptor = { 'xpack.alerting.invalidateApiKeysTask.removalDelay', { level: 'warning' } ), + renameFromRoot( + 'xpack.alerting.defaultRuleTaskTimeout', + 'xpack.alerting.rules.execution.timeout', + { + level: 'warning', + } + ), ], }; diff --git a/x-pack/plugins/alerting/server/lib/get_rule_task_timeout.test.ts b/x-pack/plugins/alerting/server/lib/get_rule_task_timeout.test.ts new file mode 100644 index 00000000000000..6a41d5068682de --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/get_rule_task_timeout.test.ts @@ -0,0 +1,61 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { getRuleTaskTimeout } from './get_rule_task_timeout'; +import { RulesConfig } from '../config'; + +const ruleTypeId = 'test-rule-type-id'; +const config = { + minimumScheduleInterval: { + value: '2m', + enforce: false, + }, + execution: { + timeout: '1m', + actions: { max: 1000 }, + }, +} as RulesConfig; + +const configWithRuleType = { + ...config, + execution: { + ...config.execution, + ruleTypeOverrides: [ + { + id: ruleTypeId, + timeout: '10m', + }, + ], + }, +}; + +const configWithoutTimeout = { + ...config, + execution: { + actions: { max: 1000 }, + }, +}; + +describe('get rule task timeout', () => { + test('returns the rule type specific timeout', () => { + expect(getRuleTaskTimeout({ config: configWithRuleType, ruleTypeId })).toBe('10m'); + }); + + test('returns the timeout that applies all the rule types', () => { + expect(getRuleTaskTimeout({ config, ruleTypeId })).toBe('1m'); + }); + + test('returns the timeout passed by the plugin', () => { + expect( + getRuleTaskTimeout({ config: configWithoutTimeout, ruleTaskTimeout: '20m', ruleTypeId }) + ).toBe('20m'); + }); + + test('returns the default timeout', () => { + expect(getRuleTaskTimeout({ config: configWithoutTimeout, ruleTypeId })).toBe('5m'); + }); +}); diff --git a/x-pack/plugins/alerting/server/lib/get_rule_task_timeout.ts b/x-pack/plugins/alerting/server/lib/get_rule_task_timeout.ts new file mode 100644 index 00000000000000..abb6c6e9717118 --- /dev/null +++ b/x-pack/plugins/alerting/server/lib/get_rule_task_timeout.ts @@ -0,0 +1,35 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { RulesConfig } from '../config'; + +const DEFAULT_EXECUTION_TIMEOUT = '5m'; + +export const getRuleTaskTimeout = ({ + config, + ruleTaskTimeout, + ruleTypeId, +}: { + config: RulesConfig; + ruleTaskTimeout?: string; + ruleTypeId: string; +}): string => { + const ruleTypeConfig = config.execution.ruleTypeOverrides?.find( + (ruleType) => ruleTypeId === ruleType.id + ); + + // First, rule type specific timeout config (ruleTypeOverrides) is applied if it's set in kibana.yml + // if not, then timeout for all the rule types is applied if it's set in kibana.yml + // if not, ruleTaskTimeout is applied that is passed from the rule type registering plugin + // if none of above is set, DEFAULT_EXECUTION_TIMEOUT is applied + return ( + ruleTypeConfig?.timeout || + config.execution.timeout || + ruleTaskTimeout || + DEFAULT_EXECUTION_TIMEOUT + ); +}; diff --git a/x-pack/plugins/alerting/server/plugin.test.ts b/x-pack/plugins/alerting/server/plugin.test.ts index 5a93d389cb73d1..450c177d724730 100644 --- a/x-pack/plugins/alerting/server/plugin.test.ts +++ b/x-pack/plugins/alerting/server/plugin.test.ts @@ -30,7 +30,6 @@ const generateAlertingConfig = (): AlertingConfig => ({ removalDelay: '1h', }, maxEphemeralActionsPerAlert: 10, - defaultRuleTaskTimeout: '5m', cancelAlertsOnRuleTimeout: true, rules: { minimumScheduleInterval: { value: '1m', enforce: false }, @@ -180,6 +179,10 @@ describe('Alerting Plugin', () => { describe('registerType()', () => { let setup: PluginSetupContract; beforeEach(async () => { + const context = coreMock.createPluginInitializerContext( + generateAlertingConfig() + ); + plugin = new AlertingPlugin(context); setup = await plugin.setup(setupMocks, mockPlugins); }); diff --git a/x-pack/plugins/alerting/server/plugin.ts b/x-pack/plugins/alerting/server/plugin.ts index de9524d69a84e6..8fa394445fe500 100644 --- a/x-pack/plugins/alerting/server/plugin.ts +++ b/x-pack/plugins/alerting/server/plugin.ts @@ -66,6 +66,7 @@ import { getSecurityHealth, SecurityHealth } from './lib/get_security_health'; import { MonitoringCollectionSetup } from '../../monitoring_collection/server'; import { registerNodeCollector, registerClusterCollector, InMemoryMetrics } from './monitoring'; import { getExecutionConfigForRuleType } from './lib/get_rules_config'; +import { getRuleTaskTimeout } from './lib/get_rule_task_timeout'; export const EVENT_LOG_PROVIDER = 'alerting'; export const EVENT_LOG_ACTIONS = { @@ -282,15 +283,13 @@ export class AlertingPlugin { encryptedSavedObjects: plugins.encryptedSavedObjects, }); - const alertingConfig: AlertingConfig = this.config; - return { - registerType< - Params extends AlertTypeParams = AlertTypeParams, - ExtractedParams extends AlertTypeParams = AlertTypeParams, - State extends AlertTypeState = AlertTypeState, - InstanceState extends AlertInstanceState = AlertInstanceState, - InstanceContext extends AlertInstanceContext = AlertInstanceContext, + registerType: < + Params extends AlertTypeParams = never, + ExtractedParams extends AlertTypeParams = never, + State extends AlertTypeState = never, + InstanceState extends AlertInstanceState = never, + InstanceContext extends AlertInstanceContext = never, ActionGroupIds extends string = never, RecoveryActionGroupId extends string = never >( @@ -303,18 +302,21 @@ export class AlertingPlugin { ActionGroupIds, RecoveryActionGroupId > - ) { + ) => { if (!(ruleType.minimumLicenseRequired in LICENSE_TYPE)) { throw new Error(`"${ruleType.minimumLicenseRequired}" is not a valid license type`); } ruleType.config = getExecutionConfigForRuleType({ - config: alertingConfig.rules, + config: this.config.rules, + ruleTypeId: ruleType.id, + }); + ruleType.ruleTaskTimeout = getRuleTaskTimeout({ + config: this.config.rules, + ruleTaskTimeout: ruleType.ruleTaskTimeout, ruleTypeId: ruleType.id, }); - ruleType.ruleTaskTimeout = - ruleType.ruleTaskTimeout ?? alertingConfig.defaultRuleTaskTimeout; ruleType.cancelAlertsOnRuleTimeout = - ruleType.cancelAlertsOnRuleTimeout ?? alertingConfig.cancelAlertsOnRuleTimeout; + ruleType.cancelAlertsOnRuleTimeout ?? this.config.cancelAlertsOnRuleTimeout; ruleType.doesSetRecoveryContext = ruleType.doesSetRecoveryContext ?? false; ruleTypeRegistry.register(ruleType); }, @@ -329,7 +331,7 @@ export class AlertingPlugin { ); }, getConfig: () => { - return pick(alertingConfig.rules, 'minimumScheduleInterval'); + return pick(this.config.rules, 'minimumScheduleInterval'); }, }; } diff --git a/x-pack/plugins/alerting/server/task_runner/create_execution_handler.ts b/x-pack/plugins/alerting/server/task_runner/create_execution_handler.ts index 279afee0e42c60..253099957f8d33 100644 --- a/x-pack/plugins/alerting/server/task_runner/create_execution_handler.ts +++ b/x-pack/plugins/alerting/server/task_runner/create_execution_handler.ts @@ -132,6 +132,8 @@ export function createExecutionHandler< continue; } + alertExecutionStore.numberOfTriggeredActions++; + const namespace = spaceId === 'default' ? {} : { namespace: spaceId }; const enqueueOptions = { @@ -165,12 +167,9 @@ export function createExecutionHandler< if (isEphemeralTaskRejectedDueToCapacityError(err)) { await actionsClient.enqueueExecution(enqueueOptions); } - } finally { - alertExecutionStore.numberOfTriggeredActions++; } } else { await actionsClient.enqueueExecution(enqueueOptions); - alertExecutionStore.numberOfTriggeredActions++; } const event = createAlertEventLogRecordObject({