Skip to content

Commit

Permalink
Start the initial status timeout when the service is "started" (#172268)
Browse files Browse the repository at this point in the history
## Summary

The _plugins status service_ listens to some of the plugins' statuses
through Observables.
If these plugins don't emit an initial status after some time, it
injects a first "timed out" value, with an `unavailable` state.

We're currently starting this 30s timeout at service creation time.
This is not accurate, as other services starting before plugins (e.g.
saved objects + migrations) are time consuming and can leave short to no
time for plugins to `start()` before hitting the 30s timeout.

This PR aims at fixing this, by starting to count when the plugins
`start()` methods are called.
This way, we're actually giving plugins 30s to emit a status.
  • Loading branch information
gsoldevila authored Nov 30, 2023
1 parent f89c6fd commit 2eba909
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('PluginStatusService', () => {
pluginDependencies,
});

service.blockNewRegistrations();
service.start();
expect(() => {
service.set(
'a',
Expand Down Expand Up @@ -365,6 +365,8 @@ describe('PluginStatusService', () => {

const pluginA$ = new ReplaySubject<ServiceStatus>(1);
service.set('a', pluginA$);
service.start(); // the plugin emission timeout starts counting when we call pluginsStatus.start()

// the first emission happens right after core$ services emit
const firstEmission = firstValueFrom(service.getAll$().pipe(skip(1)));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,22 @@
* Side Public License, v 1.
*/

import { BehaviorSubject, type Observable, ReplaySubject, type Subscription } from 'rxjs';
import {
BehaviorSubject,
merge,
Observable,
ReplaySubject,
Subject,
type Subscription,
} from 'rxjs';
import {
map,
distinctUntilChanged,
filter,
timeout,
startWith,
tap,
debounceTime,
takeUntil,
delay,
} from 'rxjs/operators';
import { sortBy } from 'lodash';
import { isDeepStrictEqual } from 'util';
Expand Down Expand Up @@ -62,6 +69,7 @@ export class PluginsStatusService {
private pluginData: PluginData;
private rootPlugins: PluginName[]; // root plugins are those that do not have any dependencies
private orderedPluginNames: PluginName[];
private start$ = new Subject<void>();
private pluginData$ = new ReplaySubject<PluginData>(1);
private pluginStatus: PluginsStatus = {};
private pluginStatus$ = new BehaviorSubject<PluginsStatus>(this.pluginStatus);
Expand Down Expand Up @@ -110,26 +118,21 @@ export class PluginsStatusService {
// delete any derived statuses calculated before the custom status Observable was registered
delete this.pluginStatus[plugin];

const statusChanged$ = status$.pipe(distinctUntilChanged());
const firstEmissionTimeout$ = this.start$.pipe(
delay(this.statusTimeoutMs),
map(() => ({
level: ServiceStatusLevels.unavailable,
summary: `Status check timed out after ${
this.statusTimeoutMs < 1000
? `${this.statusTimeoutMs}ms`
: `${this.statusTimeoutMs / 1000}s`
}`,
})),
takeUntil(status$)
);

this.reportedStatusSubscriptions[plugin] = statusChanged$
.pipe(
// Set a timeout for externally-defined status Observables
timeout({
first: this.statusTimeoutMs,
with: () =>
statusChanged$.pipe(
startWith({
level: ServiceStatusLevels.unavailable,
summary: `Status check timed out after ${
this.statusTimeoutMs < 1000
? `${this.statusTimeoutMs}ms`
: `${this.statusTimeoutMs / 1000}s`
}`,
})
),
})
)
this.reportedStatusSubscriptions[plugin] = merge(firstEmissionTimeout$, status$)
.pipe(distinctUntilChanged())
.subscribe((status) => {
const { levelChanged, summaryChanged } = this.updatePluginReportedStatus(plugin, status);

Expand All @@ -143,11 +146,11 @@ export class PluginsStatusService {
});
}

/**
* Prevent plugins from registering status Observables
*/
public blockNewRegistrations() {
public start() {
// Prevent plugins from registering status Observables
this.newRegistrationsAllowed = false;
this.start$.next();
this.start$.complete();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ export class StatusService implements CoreService<InternalStatusServiceSetup> {
if (!this.pluginsStatus || !this.overall$) {
throw new Error(`StatusService#setup must be called before #start`);
}
this.pluginsStatus.blockNewRegistrations();
this.pluginsStatus.start();
this.logStatusChanges();
}

Expand Down

0 comments on commit 2eba909

Please sign in to comment.