Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NP server setup APIs requiring access to start APIs #54886

Closed
pgayvallet opened this issue Jan 15, 2020 · 2 comments · Fixed by #55156
Closed

NP server setup APIs requiring access to start APIs #54886

pgayvallet opened this issue Jan 15, 2020 · 2 comments · Fixed by #55156
Assignees
Labels
blocker discuss Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@pgayvallet
Copy link
Contributor

(this is probably the server-side counterpart of #49691)

This ticket is to discuss with @elastic/kibana-platform the possible solutions around the issue of setup API's requiring access to start API's to register logic that should be executed after startup.

Issue

In NP, lifecycle is split in two steps: setup and start. We ask plugins to perform all configuration/registration during setup.

However in some cases, the registered objects are meant to be using start API, which are not yet accessible.

First exemple that comes to mind is the telemetry usage collector:

export function createCollectorFetch(server: Server) {
return async function fetchUsageStats(): Promise<UsageStats> {
const internalRepo = server.newPlatform.setup.core.savedObjects.createInternalRepository();
const uiSettingsClient = server.newPlatform.start.core.uiSettings.asScopedToClient(
new SavedObjectsClient(internalRepo)
);
const user = await uiSettingsClient.getUserProvided();
const modifiedEntries = Object.keys(user)
.filter((key: string) => key !== 'buildNum')
.reduce((obj: any, key: string) => {
obj[key] = user[key].userValue;
return obj;
}, {});
return modifiedEntries;
};
}
export function registerManagementUsageCollector(
usageCollection: UsageCollectionSetup,
server: any
) {
const collector = usageCollection.makeUsageCollector({
type: KIBANA_MANAGEMENT_STATS_TYPE,
isReady: () => true,
fetch: createCollectorFetch(server),
});
usageCollection.registerCollector(collector);
}

The collector creation requires to access a uiSettingClient, which is only accessible via a core's start API.

    const internalRepo = server.newPlatform.setup.core.savedObjects.createInternalRepository();
    const uiSettingsClient = server.newPlatform.start.core.uiSettings.asScopedToClient(
      new SavedObjectsClient(internalRepo)
    );

but the registration is then done with a usageCollection'ssetup API:

  usageCollectionSetup.registerCollector(collector);

This code works in legacy, as the plugin got access to both setup and start at the same time, but cannot currently be migrated (therefor the blocker label).

Proposal

#49691 address this issue on the client-side by adding the getStartServices API, and at this time we did not see any need to do the same on server-side, but I think we now have a valid reason to do the same.

By implementing getStartServices to the server-side, the previous code could then be properly migrated to NP and be executed during setup:

export function createCollectorFetch(core: CoreSetup) {
  return async function fetchUsageStats(): Promise<UsageStats> {
    const internalRepo = core.savedObjects.createInternalRepository();
    const uiSettingsClient = (await core.getStartService()).uiSettings.asScopedToClient(
      new SavedObjectsClient(internalRepo)
    );
    const user = await uiSettingsClient.getUserProvided();
    const modifiedEntries = Object.keys(user)
      .filter((key: string) => key !== 'buildNum')
      .reduce((obj: any, key: string) => {
        obj[key] = user[key].userValue;
        return obj;
      }, {});

    return modifiedEntries;
  };
}

export function registerManagementUsageCollector(
  core: CoreSetup,
  usageCollection: UsageCollectionSetup,
) {
  const collector = usageCollection.makeUsageCollector({
    type: KIBANA_MANAGEMENT_STATS_TYPE,
    isReady: () => true,
    fetch: createCollectorFetch(core),
  });

  usageCollection.registerCollector(collector);
}
@pgayvallet pgayvallet added blocker discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Jan 15, 2020
@joshdover
Copy link
Contributor

joshdover commented Jan 15, 2020

@pgayvallet and I talked about this over Slack. Though Context would work for usage collection, we still feel it's complex to use.

My only hesitancy with adding this API on the server is that now there are two ways to get access to similar things. However, I think the convenience of this API (not having to rely on plugins registering with usageCollection's context) outweighs that concern.

I'm a 👍 on this.

@rudolf
Copy link
Contributor

rudolf commented Jan 16, 2020

👍 I cannot think of any better alternatives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker discuss Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants