Skip to content

Commit a8c080b

Browse files
Delay client-side feature usage registration until start (#79365)
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
1 parent 56c6122 commit a8c080b

File tree

11 files changed

+102
-76
lines changed

11 files changed

+102
-76
lines changed

test/scripts/jenkins_xpack_build_plugins.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,6 @@ node scripts/build_kibana_platform_plugins \
1010
--scan-dir "$XPACK_DIR/test/alerting_api_integration/plugins" \
1111
--scan-dir "$XPACK_DIR/test/plugin_api_integration/plugins" \
1212
--scan-dir "$XPACK_DIR/test/plugin_api_perf/plugins" \
13+
--scan-dir "$XPACK_DIR/test/licensing_plugin/plugins" \
1314
--workers 12 \
1415
--verbose

x-pack/plugins/licensing/public/plugin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export class LicensingPlugin implements Plugin<LicensingPluginSetup, LicensingPl
117117
return {
118118
refresh: refreshManually,
119119
license$,
120-
featureUsage: this.featureUsage.setup({ http: core.http }),
120+
featureUsage: this.featureUsage.setup(),
121121
};
122122
}
123123

x-pack/plugins/licensing/public/services/feature_usage_service.test.ts

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,33 @@ describe('FeatureUsageService', () => {
1818

1919
describe('#setup', () => {
2020
describe('#register', () => {
21-
it('calls the endpoint with the correct parameters', async () => {
22-
const setup = service.setup({ http });
23-
await setup.register('my-feature', 'platinum');
21+
it('calls the endpoint on start with the correct parameters', async () => {
22+
const setup = service.setup();
23+
setup.register('my-feature', 'platinum');
24+
setup.register('my-other-feature', 'gold');
25+
expect(http.post).not.toHaveBeenCalled();
26+
27+
service.start({ http });
2428
expect(http.post).toHaveBeenCalledTimes(1);
2529
expect(http.post).toHaveBeenCalledWith('/internal/licensing/feature_usage/register', {
26-
body: JSON.stringify({
27-
featureName: 'my-feature',
28-
licenseType: 'platinum',
29-
}),
30+
body: JSON.stringify([
31+
{ featureName: 'my-feature', licenseType: 'platinum' },
32+
{ featureName: 'my-other-feature', licenseType: 'gold' },
33+
]),
3034
});
3135
});
3236

33-
it('does not call endpoint if on anonymous path', async () => {
37+
it('does not call endpoint on start if no registrations', async () => {
38+
service.setup();
39+
service.start({ http });
40+
expect(http.post).not.toHaveBeenCalled();
41+
});
42+
43+
it('does not call endpoint on start if on anonymous path', async () => {
3444
http.anonymousPaths.isAnonymous.mockReturnValue(true);
35-
const setup = service.setup({ http });
36-
await setup.register('my-feature', 'platinum');
45+
const setup = service.setup();
46+
setup.register('my-feature', 'platinum');
47+
service.start({ http });
3748
expect(http.post).not.toHaveBeenCalled();
3849
});
3950
});
@@ -42,7 +53,7 @@ describe('FeatureUsageService', () => {
4253
describe('#start', () => {
4354
describe('#notifyUsage', () => {
4455
it('calls the endpoint with the correct parameters', async () => {
45-
service.setup({ http });
56+
service.setup();
4657
const start = service.start({ http });
4758
await start.notifyUsage('my-feature', 42);
4859

@@ -56,7 +67,7 @@ describe('FeatureUsageService', () => {
5667
});
5768

5869
it('correctly convert dates', async () => {
59-
service.setup({ http });
70+
service.setup();
6071
const start = service.start({ http });
6172

6273
const now = new Date();
@@ -74,7 +85,7 @@ describe('FeatureUsageService', () => {
7485

7586
it('does not call endpoint if on anonymous path', async () => {
7687
http.anonymousPaths.isAnonymous.mockReturnValue(true);
77-
service.setup({ http });
88+
service.setup();
7889
const start = service.start({ http });
7990
await start.notifyUsage('my-feature', 42);
8091
expect(http.post).not.toHaveBeenCalled();

x-pack/plugins/licensing/public/services/feature_usage_service.ts

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,15 @@
55
*/
66

77
import { isDate } from 'lodash';
8-
import type { HttpSetup, HttpStart } from 'src/core/public';
8+
import type { HttpStart } from 'src/core/public';
99
import { LicenseType } from '../../common/types';
1010

1111
/** @public */
1212
export interface FeatureUsageServiceSetup {
1313
/**
1414
* Register a feature to be able to notify of it's usages using the {@link FeatureUsageServiceStart | service start contract}.
1515
*/
16-
register(featureName: string, licenseType: LicenseType): Promise<void>;
16+
register(featureName: string, licenseType: LicenseType): void;
1717
}
1818

1919
/** @public */
@@ -27,10 +27,6 @@ export interface FeatureUsageServiceStart {
2727
notifyUsage(featureName: string, usedAt?: Date | number): Promise<void>;
2828
}
2929

30-
interface SetupDeps {
31-
http: HttpSetup;
32-
}
33-
3430
interface StartDeps {
3531
http: HttpStart;
3632
}
@@ -39,29 +35,33 @@ interface StartDeps {
3935
* @internal
4036
*/
4137
export class FeatureUsageService {
42-
public setup({ http }: SetupDeps): FeatureUsageServiceSetup {
38+
private readonly registrations: Array<{ featureName: string; licenseType: LicenseType }> = [];
39+
40+
public setup(): FeatureUsageServiceSetup {
4341
return {
4442
register: async (featureName, licenseType) => {
45-
// Skip registration if on logged-out page
46-
// NOTE: this only works because the login page does a full-page refresh after logging in
47-
// If this is ever changed, this code will need to buffer registrations and call them after the user logs in.
48-
if (http.anonymousPaths.isAnonymous(window.location.pathname)) return;
49-
50-
await http.post('/internal/licensing/feature_usage/register', {
51-
body: JSON.stringify({
52-
featureName,
53-
licenseType,
54-
}),
55-
});
43+
this.registrations.push({ featureName, licenseType });
5644
},
5745
};
5846
}
5947

6048
public start({ http }: StartDeps): FeatureUsageServiceStart {
49+
// Skip registration if on logged-out page
50+
// NOTE: this only works because the login page does a full-page refresh after logging in
51+
// If this is ever changed, this code will need to buffer registrations and call them after the user logs in.
52+
const registrationPromise =
53+
http.anonymousPaths.isAnonymous(window.location.pathname) || this.registrations.length === 0
54+
? Promise.resolve()
55+
: http.post('/internal/licensing/feature_usage/register', {
56+
body: JSON.stringify(this.registrations),
57+
});
58+
6159
return {
6260
notifyUsage: async (featureName, usedAt = Date.now()) => {
6361
// Skip notification if on logged-out page
6462
if (http.anonymousPaths.isAnonymous(window.location.pathname)) return;
63+
// Wait for registrations to complete
64+
await registrationPromise;
6565

6666
const lastUsed = isDate(usedAt) ? usedAt.getTime() : usedAt;
6767
await http.post('/internal/licensing/feature_usage/notify', {

x-pack/plugins/licensing/server/routes/internal/register_feature.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,26 @@ export function registerRegisterFeatureRoute(
1616
{
1717
path: '/internal/licensing/feature_usage/register',
1818
validate: {
19-
body: schema.object({
20-
featureName: schema.string(),
21-
licenseType: schema.string({
22-
validate: (value) => {
23-
if (!(value in LICENSE_TYPE)) {
24-
return `Invalid license type: ${value}`;
25-
}
26-
},
27-
}),
28-
}),
19+
body: schema.arrayOf(
20+
schema.object({
21+
featureName: schema.string(),
22+
licenseType: schema.string({
23+
validate: (value) => {
24+
if (!(value in LICENSE_TYPE)) {
25+
return `Invalid license type: ${value}`;
26+
}
27+
},
28+
}),
29+
})
30+
),
2931
},
3032
},
3133
async (context, request, response) => {
32-
const { featureName, licenseType } = request.body;
34+
const registrations = request.body;
3335

34-
featureUsageSetup.register(featureName, licenseType as LicenseType);
36+
registrations.forEach(({ featureName, licenseType }) => {
37+
featureUsageSetup.register(featureName, licenseType as LicenseType);
38+
});
3539

3640
return response.ok({
3741
body: {

x-pack/plugins/ui_actions_enhanced/public/services/ui_actions_service_enhancements.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -158,17 +158,7 @@ export class UiActionsServiceEnhancements
158158

159159
private registerFeatureUsage = (definition: ActionFactoryDefinition<any, any, any>): void => {
160160
if (!definition.minimalLicense || !definition.licenseFeatureName) return;
161-
162-
// Intentionally don't wait for response because
163-
// happens in setup phase and has to be sync
164-
this.deps.featureUsageSetup
165-
.register(definition.licenseFeatureName, definition.minimalLicense)
166-
.catch(() => {
167-
// eslint-disable-next-line no-console
168-
console.warn(
169-
`ActionFactory [actionFactory.id = ${definition.id}] fail to register feature for featureUsage.`
170-
);
171-
});
161+
this.deps.featureUsageSetup.register(definition.licenseFeatureName, definition.minimalLicense);
172162
};
173163

174164
public readonly telemetry = (state: DynamicActionsState, telemetry: Record<string, any> = {}) => {

x-pack/test/licensing_plugin/config.public.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
2323
KIBANA_ROOT,
2424
'test/plugin_functional/plugins/core_provider_plugin'
2525
)}`,
26+
`--plugin-path=${path.resolve(__dirname, 'plugins/test_feature_usage')}`,
2627
],
2728
},
2829
};
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"id": "testFeatureUsage",
3+
"version": "kibana",
4+
"server": false,
5+
"ui": true,
6+
"requiredPlugins": ["licensing"]
7+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { TestFeatureUsagePlugin } from './plugin';
8+
9+
export const plugin = () => new TestFeatureUsagePlugin();
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { CoreSetup } from 'src/core/server';
8+
import { LicensingPluginSetup } from '../../../../../plugins/licensing/public';
9+
10+
interface SetupPlugins {
11+
licensing: LicensingPluginSetup;
12+
}
13+
14+
export class TestFeatureUsagePlugin {
15+
public setup(core: CoreSetup, plugins: SetupPlugins) {
16+
plugins.licensing.featureUsage.register('test-client-A', 'gold');
17+
plugins.licensing.featureUsage.register('test-client-B', 'enterprise');
18+
}
19+
20+
public start() {}
21+
public stop() {}
22+
}

0 commit comments

Comments
 (0)