From ff92ae1e4722defe24eace5d7232c4ccc3c51a34 Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Wed, 6 Oct 2021 13:49:46 +0200 Subject: [PATCH 1/2] add `coreOverall$` to internal status contract (#113729) * add coreOverall$ to internal status contract * add unit tests * re-patch flaky tests * add and improve tests --- .../routes/integration_tests/status.test.ts | 78 +++++++- src/core/server/status/routes/status.ts | 11 +- src/core/server/status/status_service.mock.ts | 1 + src/core/server/status/status_service.test.ts | 176 ++++++++++++++++++ src/core/server/status/status_service.ts | 32 +++- src/core/server/status/types.ts | 5 + .../http/platform/status.ts | 5 + 7 files changed, 294 insertions(+), 14 deletions(-) diff --git a/src/core/server/status/routes/integration_tests/status.test.ts b/src/core/server/status/routes/integration_tests/status.test.ts index 645ce0b241612..9c0096764079e 100644 --- a/src/core/server/status/routes/integration_tests/status.test.ts +++ b/src/core/server/status/routes/integration_tests/status.test.ts @@ -18,19 +18,29 @@ import { MetricsServiceSetup } from '../../../metrics'; import { HttpService, InternalHttpServiceSetup } from '../../../http'; import { registerStatusRoute } from '../status'; -import { ServiceStatus, ServiceStatusLevels } from '../../types'; +import { ServiceStatus, ServiceStatusLevels, ServiceStatusLevel } from '../../types'; import { statusServiceMock } from '../../status_service.mock'; import { executionContextServiceMock } from '../../../execution_context/execution_context_service.mock'; import { contextServiceMock } from '../../../context/context_service.mock'; const coreId = Symbol('core'); +const createServiceStatus = ( + level: ServiceStatusLevel = ServiceStatusLevels.available +): ServiceStatus => ({ + level, + summary: 'status summary', +}); + describe('GET /api/status', () => { let server: HttpService; let httpSetup: InternalHttpServiceSetup; let metrics: jest.Mocked; - const setupServer = async ({ allowAnonymous = true }: { allowAnonymous?: boolean } = {}) => { + const setupServer = async ({ + allowAnonymous = true, + coreOverall, + }: { allowAnonymous?: boolean; coreOverall?: ServiceStatus } = {}) => { const coreContext = createCoreContext({ coreId }); const contextService = new ContextService(coreContext); @@ -42,7 +52,12 @@ describe('GET /api/status', () => { }); metrics = metricsServiceMock.createSetupContract(); - const status = statusServiceMock.createSetupContract(); + + const status = statusServiceMock.createInternalSetupContract(); + if (coreOverall) { + status.coreOverall$ = new BehaviorSubject(coreOverall); + } + const pluginsStatus$ = new BehaviorSubject>({ a: { level: ServiceStatusLevels.available, summary: 'a is available' }, b: { level: ServiceStatusLevels.degraded, summary: 'b is degraded' }, @@ -68,6 +83,7 @@ describe('GET /api/status', () => { metrics, status: { overall$: status.overall$, + coreOverall$: status.coreOverall$, core$: status.core$, plugins$: pluginsStatus$, }, @@ -312,4 +328,60 @@ describe('GET /api/status', () => { }); }); }); + + describe('status level and http response code', () => { + describe('using standard format', () => { + it('respond with a 200 when core.overall.status is available', async () => { + await setupServer({ + coreOverall: createServiceStatus(ServiceStatusLevels.available), + }); + await supertest(httpSetup.server.listener).get('/api/status?v8format=true').expect(200); + }); + it('respond with a 200 when core.overall.status is degraded', async () => { + await setupServer({ + coreOverall: createServiceStatus(ServiceStatusLevels.degraded), + }); + await supertest(httpSetup.server.listener).get('/api/status?v8format=true').expect(200); + }); + it('respond with a 503 when core.overall.status is unavailable', async () => { + await setupServer({ + coreOverall: createServiceStatus(ServiceStatusLevels.unavailable), + }); + await supertest(httpSetup.server.listener).get('/api/status?v8format=true').expect(503); + }); + it('respond with a 503 when core.overall.status is critical', async () => { + await setupServer({ + coreOverall: createServiceStatus(ServiceStatusLevels.critical), + }); + await supertest(httpSetup.server.listener).get('/api/status?v8format=true').expect(503); + }); + }); + + describe('using legacy format', () => { + it('respond with a 200 when core.overall.status is available', async () => { + await setupServer({ + coreOverall: createServiceStatus(ServiceStatusLevels.available), + }); + await supertest(httpSetup.server.listener).get('/api/status?v7format=true').expect(200); + }); + it('respond with a 200 when core.overall.status is degraded', async () => { + await setupServer({ + coreOverall: createServiceStatus(ServiceStatusLevels.degraded), + }); + await supertest(httpSetup.server.listener).get('/api/status?v7format=true').expect(200); + }); + it('respond with a 503 when core.overall.status is unavailable', async () => { + await setupServer({ + coreOverall: createServiceStatus(ServiceStatusLevels.unavailable), + }); + await supertest(httpSetup.server.listener).get('/api/status?v7format=true').expect(503); + }); + it('respond with a 503 when core.overall.status is critical', async () => { + await setupServer({ + coreOverall: createServiceStatus(ServiceStatusLevels.critical), + }); + await supertest(httpSetup.server.listener).get('/api/status?v7format=true').expect(503); + }); + }); + }); }); diff --git a/src/core/server/status/routes/status.ts b/src/core/server/status/routes/status.ts index df0300c9fa0c2..c9c1c9982af8b 100644 --- a/src/core/server/status/routes/status.ts +++ b/src/core/server/status/routes/status.ts @@ -30,6 +30,7 @@ interface Deps { }; metrics: MetricsServiceSetup; status: { + coreOverall$: Observable; overall$: Observable; core$: Observable; plugins$: Observable>; @@ -51,9 +52,11 @@ export const registerStatusRoute = ({ router, config, metrics, status }: Deps) = // Since the status.plugins$ observable is not subscribed to elsewhere, we need to subscribe it here to eagerly load // the plugins status when Kibana starts up so this endpoint responds quickly on first boot. const combinedStatus$ = new ReplaySubject< - [ServiceStatus, CoreStatus, Record>] + [ServiceStatus, ServiceStatus, CoreStatus, Record>] >(1); - combineLatest([status.overall$, status.core$, status.plugins$]).subscribe(combinedStatus$); + combineLatest([status.overall$, status.coreOverall$, status.core$, status.plugins$]).subscribe( + combinedStatus$ + ); router.get( { @@ -71,7 +74,7 @@ export const registerStatusRoute = ({ router, config, metrics, status }: Deps) = async (context, req, res) => { const { version, buildSha, buildNum } = config.packageInfo; const versionWithoutSnapshot = version.replace(SNAPSHOT_POSTFIX, ''); - const [overall, core, plugins] = await combinedStatus$.pipe(first()).toPromise(); + const [overall, coreOverall, core, plugins] = await combinedStatus$.pipe(first()).toPromise(); let statusInfo: StatusInfo | LegacyStatusInfo; if (req.query?.v8format) { @@ -116,7 +119,7 @@ export const registerStatusRoute = ({ router, config, metrics, status }: Deps) = }, }; - const statusCode = overall.level >= ServiceStatusLevels.unavailable ? 503 : 200; + const statusCode = coreOverall.level >= ServiceStatusLevels.unavailable ? 503 : 200; return res.custom({ body, statusCode, bypassErrorFormat: true }); } ); diff --git a/src/core/server/status/status_service.mock.ts b/src/core/server/status/status_service.mock.ts index 8ef34558ca7b2..7241bb2f0479e 100644 --- a/src/core/server/status/status_service.mock.ts +++ b/src/core/server/status/status_service.mock.ts @@ -42,6 +42,7 @@ const createSetupContractMock = () => { const createInternalSetupContractMock = () => { const setupContract: jest.Mocked = { core$: new BehaviorSubject(availableCoreStatus), + coreOverall$: new BehaviorSubject(available), overall$: new BehaviorSubject(available), isStatusPageAnonymous: jest.fn().mockReturnValue(false), plugins: { diff --git a/src/core/server/status/status_service.test.ts b/src/core/server/status/status_service.test.ts index 4ead81a6638dd..e876068efec34 100644 --- a/src/core/server/status/status_service.test.ts +++ b/src/core/server/status/status_service.test.ts @@ -29,6 +29,7 @@ describe('StatusService', () => { }); const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + const available: ServiceStatus = { level: ServiceStatusLevels.available, summary: 'Available', @@ -37,6 +38,10 @@ describe('StatusService', () => { level: ServiceStatusLevels.degraded, summary: 'This is degraded!', }; + const critical: ServiceStatus = { + level: ServiceStatusLevels.critical, + summary: 'This is critical!', + }; type SetupDeps = Parameters[0]; const setupDeps = (overrides: Partial): SetupDeps => { @@ -319,6 +324,177 @@ describe('StatusService', () => { }); }); + describe('coreOverall$', () => { + it('exposes an overall summary of core services', async () => { + const setup = await service.setup( + setupDeps({ + elasticsearch: { + status$: of(degraded), + }, + savedObjects: { + status$: of(degraded), + }, + }) + ); + expect(await setup.coreOverall$.pipe(first()).toPromise()).toMatchObject({ + level: ServiceStatusLevels.degraded, + summary: '[2] services are degraded', + }); + }); + + it('computes the summary depending on the services status', async () => { + const setup = await service.setup( + setupDeps({ + elasticsearch: { + status$: of(degraded), + }, + savedObjects: { + status$: of(critical), + }, + }) + ); + expect(await setup.coreOverall$.pipe(first()).toPromise()).toMatchObject({ + level: ServiceStatusLevels.critical, + summary: '[savedObjects]: This is critical!', + }); + }); + + it('replays last event', async () => { + const setup = await service.setup( + setupDeps({ + elasticsearch: { + status$: of(degraded), + }, + savedObjects: { + status$: of(degraded), + }, + }) + ); + + const subResult1 = await setup.coreOverall$.pipe(first()).toPromise(); + const subResult2 = await setup.coreOverall$.pipe(first()).toPromise(); + const subResult3 = await setup.coreOverall$.pipe(first()).toPromise(); + + expect(subResult1).toMatchObject({ + level: ServiceStatusLevels.degraded, + summary: '[2] services are degraded', + }); + expect(subResult2).toMatchObject({ + level: ServiceStatusLevels.degraded, + summary: '[2] services are degraded', + }); + expect(subResult3).toMatchObject({ + level: ServiceStatusLevels.degraded, + summary: '[2] services are degraded', + }); + }); + + it('does not emit duplicate events', async () => { + const elasticsearch$ = new BehaviorSubject(available); + const savedObjects$ = new BehaviorSubject(degraded); + const setup = await service.setup( + setupDeps({ + elasticsearch: { + status$: elasticsearch$, + }, + savedObjects: { + status$: savedObjects$, + }, + }) + ); + + const statusUpdates: ServiceStatus[] = []; + const subscription = setup.coreOverall$.subscribe((status) => statusUpdates.push(status)); + + // Wait for timers to ensure that duplicate events are still filtered out regardless of debouncing. + elasticsearch$.next(available); + await delay(500); + elasticsearch$.next(available); + await delay(500); + elasticsearch$.next({ + level: ServiceStatusLevels.available, + summary: `Wow another summary`, + }); + await delay(500); + savedObjects$.next(degraded); + await delay(500); + savedObjects$.next(available); + await delay(500); + savedObjects$.next(available); + await delay(500); + subscription.unsubscribe(); + + expect(statusUpdates).toMatchInlineSnapshot(` + Array [ + Object { + "detail": "See the status page for more information", + "level": degraded, + "meta": Object { + "affectedServices": Array [ + "savedObjects", + ], + }, + "summary": "[savedObjects]: This is degraded!", + }, + Object { + "level": available, + "summary": "All services are available", + }, + ] + `); + }); + + it('debounces events in quick succession', async () => { + const savedObjects$ = new BehaviorSubject(available); + const setup = await service.setup( + setupDeps({ + elasticsearch: { + status$: new BehaviorSubject(available), + }, + savedObjects: { + status$: savedObjects$, + }, + }) + ); + + const statusUpdates: ServiceStatus[] = []; + const subscription = setup.coreOverall$.subscribe((status) => statusUpdates.push(status)); + + // All of these should debounced into a single `available` status + savedObjects$.next(degraded); + savedObjects$.next(available); + savedObjects$.next(degraded); + savedObjects$.next(available); + savedObjects$.next(degraded); + savedObjects$.next(available); + savedObjects$.next(degraded); + // Waiting for the debounce timeout should cut a new update + await delay(500); + savedObjects$.next(available); + await delay(500); + subscription.unsubscribe(); + + expect(statusUpdates).toMatchInlineSnapshot(` + Array [ + Object { + "detail": "See the status page for more information", + "level": degraded, + "meta": Object { + "affectedServices": Array [ + "savedObjects", + ], + }, + "summary": "[savedObjects]: This is degraded!", + }, + Object { + "level": available, + "summary": "All services are available", + }, + ] + `); + }); + }); + describe('preboot status routes', () => { let prebootRouterMock: RouterMock; beforeEach(async () => { diff --git a/src/core/server/status/status_service.ts b/src/core/server/status/status_service.ts index 8e9db30bbebd3..eaeaecd5e1de9 100644 --- a/src/core/server/status/status_service.ts +++ b/src/core/server/status/status_service.ts @@ -47,7 +47,7 @@ export class StatusService implements CoreService { private overall$?: Observable; private pluginsStatus?: PluginsStatusService; - private overallSubscription?: Subscription; + private subscriptions: Subscription[] = []; constructor(private readonly coreContext: CoreContext) { this.logger = coreContext.logger.get('status'); @@ -85,8 +85,24 @@ export class StatusService implements CoreService { shareReplay(1) ); - // Create an unused subscription to ensure all underlying lazy observables are started. - this.overallSubscription = this.overall$.subscribe(); + const coreOverall$ = core$.pipe( + // Prevent many emissions at once from dependency status resolution from making this too noisy + debounceTime(25), + map((coreStatus) => { + const coreOverall = getSummaryStatus([...Object.entries(coreStatus)]); + this.logger.debug(`Recalculated core overall status`, { + kibana: { + status: coreOverall, + }, + }); + return coreOverall; + }), + distinctUntilChanged(isDeepStrictEqual), + shareReplay(1) + ); + + // Create unused subscriptions to ensure all underlying lazy observables are started. + this.subscriptions.push(this.overall$.subscribe(), coreOverall$.subscribe()); const commonRouteDeps = { config: { @@ -100,6 +116,7 @@ export class StatusService implements CoreService { overall$: this.overall$, plugins$: this.pluginsStatus.getAll$(), core$, + coreOverall$, }, }; @@ -124,6 +141,7 @@ export class StatusService implements CoreService { return { core$, + coreOverall$, overall$: this.overall$, plugins: { set: this.pluginsStatus.set.bind(this.pluginsStatus), @@ -149,10 +167,10 @@ export class StatusService implements CoreService { this.stop$.next(); this.stop$.complete(); - if (this.overallSubscription) { - this.overallSubscription.unsubscribe(); - this.overallSubscription = undefined; - } + this.subscriptions.forEach((subscription) => { + subscription.unsubscribe(); + }); + this.subscriptions = []; } private setupCoreStatus({ diff --git a/src/core/server/status/types.ts b/src/core/server/status/types.ts index bfca4c74d9365..aab3bf302dfea 100644 --- a/src/core/server/status/types.ts +++ b/src/core/server/status/types.ts @@ -232,6 +232,11 @@ export interface StatusServiceSetup { /** @internal */ export interface InternalStatusServiceSetup extends Pick { + /** + * Overall status of core's service. + */ + coreOverall$: Observable; + // Namespaced under `plugins` key to improve clarity that these are APIs for plugins specifically. plugins: { set(plugin: PluginName, status$: Observable): void; diff --git a/test/server_integration/http/platform/status.ts b/test/server_integration/http/platform/status.ts index 0dcf82c9bea9e..c46eb5b8c0ebd 100644 --- a/test/server_integration/http/platform/status.ts +++ b/test/server_integration/http/platform/status.ts @@ -23,6 +23,9 @@ export default function ({ getService }: FtrProviderContext) { return resp.body.status.plugins[pluginName]; }; + // max debounce of the status observable + 1 + const statusPropagation = () => new Promise((resolve) => setTimeout(resolve, 501)); + const setStatus = async (level: T) => supertest .post(`/internal/status_plugin_a/status/set?level=${level}`) @@ -53,6 +56,7 @@ export default function ({ getService }: FtrProviderContext) { 5_000, async () => (await getStatus('statusPluginA')).level === 'degraded' ); + await statusPropagation(); expect((await getStatus('statusPluginA')).level).to.eql('degraded'); expect((await getStatus('statusPluginB')).level).to.eql('degraded'); @@ -62,6 +66,7 @@ export default function ({ getService }: FtrProviderContext) { 5_000, async () => (await getStatus('statusPluginA')).level === 'available' ); + await statusPropagation(); expect((await getStatus('statusPluginA')).level).to.eql('available'); expect((await getStatus('statusPluginB')).level).to.eql('available'); }); From 92b6603081c2e9765bd5ea6c20cec5521fcc255e Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 6 Oct 2021 15:20:44 +0200 Subject: [PATCH 2/2] fix tests for 7.x --- .../server/status/routes/integration_tests/status.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/server/status/routes/integration_tests/status.test.ts b/src/core/server/status/routes/integration_tests/status.test.ts index 9c0096764079e..ce5c409376e9b 100644 --- a/src/core/server/status/routes/integration_tests/status.test.ts +++ b/src/core/server/status/routes/integration_tests/status.test.ts @@ -362,25 +362,25 @@ describe('GET /api/status', () => { await setupServer({ coreOverall: createServiceStatus(ServiceStatusLevels.available), }); - await supertest(httpSetup.server.listener).get('/api/status?v7format=true').expect(200); + await supertest(httpSetup.server.listener).get('/api/status').expect(200); }); it('respond with a 200 when core.overall.status is degraded', async () => { await setupServer({ coreOverall: createServiceStatus(ServiceStatusLevels.degraded), }); - await supertest(httpSetup.server.listener).get('/api/status?v7format=true').expect(200); + await supertest(httpSetup.server.listener).get('/api/status').expect(200); }); it('respond with a 503 when core.overall.status is unavailable', async () => { await setupServer({ coreOverall: createServiceStatus(ServiceStatusLevels.unavailable), }); - await supertest(httpSetup.server.listener).get('/api/status?v7format=true').expect(503); + await supertest(httpSetup.server.listener).get('/api/status').expect(503); }); it('respond with a 503 when core.overall.status is critical', async () => { await setupServer({ coreOverall: createServiceStatus(ServiceStatusLevels.critical), }); - await supertest(httpSetup.server.listener).get('/api/status?v7format=true').expect(503); + await supertest(httpSetup.server.listener).get('/api/status').expect(503); }); }); });