-
Notifications
You must be signed in to change notification settings - Fork 8.5k
add licensed feature usage API #63549
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
Changes from 6 commits
962cd8b
835341f
2db26c8
3bda52e
fd753cc
6a9d940
7cf24e1
250081c
eb7fa18
6349736
3ddc32e
90fd4ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License; | ||
| * you may not use this file except in compliance with the Elastic License. | ||
| */ | ||
| import { IRouter, StartServicesAccessor } from 'src/core/server'; | ||
| import { LicensingPluginStart } from '../types'; | ||
|
|
||
| export function registerFeatureUsageRoute( | ||
| router: IRouter, | ||
| getStartServices: StartServicesAccessor<{}, LicensingPluginStart> | ||
| ) { | ||
| router.get( | ||
| { path: '/api/licensing/feature_usage', validate: false }, | ||
| async (context, request, response) => { | ||
| const [, , { featureUsage }] = await getStartServices(); | ||
| return response.ok({ | ||
| body: [...featureUsage.getLastUsages().entries()].reduce( | ||
| (res, [featureName, lastUsage]) => { | ||
| return { | ||
| ...res, | ||
| [featureName]: new Date(lastUsage).toISOString(), | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may have already been discussed but I wonder if there could be any problems with using absolute dates. If clocks are not configured correctly, it could make making sense of this data harder from the Cloud side. One option could be to return a relative date, ie. how many seconds ago it was used.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as #63549 (comment), not sure if we can change the expected format here.
This has different drawbacks btw, such as being vulnerable to latency, and forcing to be processed on the fly or enhanced to not loose the time reference of the consumer call. Also I would be expecting cloud instances to have properly synchronized clocks between their machines/vms/cris so this is probably a non-issue?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another problem could be that a user couldn't know in what timezone the timestamp was created. Not sure if it's a critical problem.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assumed it was not an issue, as the API consumption is just going to pool calls to the API to count the actual number of usages in a given period of time. TBH I would have returned a unix timestamp instead of an ISO date representation if the elasticsearch API counterpart was not returning this format. If we want to have correct timezones, I can just change the feature usage service to store dates instead of unix timestamps, but once again, this is assuming the actual user/client and the kibana server are on the same timezone. My main difficulty on this feature is that it's very unclear who has the power of decision on that API.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this is a safe enough assumption for its usage. There's always the potential for clock drift, but even if we're off a bit it won't have catastrophic effects. Using an ISO-8601 format for dates is 👍 , they can be easily translated to a unix timestamp. |
||
| }; | ||
| }, | ||
| {} | ||
| ), | ||
| }); | ||
| } | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License; | ||
| * you may not use this file except in compliance with the Elastic License. | ||
| */ | ||
|
|
||
| import { | ||
| FeatureUsageService, | ||
| FeatureUsageServiceSetup, | ||
| FeatureUsageServiceStart, | ||
| } from './feature_usage_service'; | ||
|
|
||
| const createSetupMock = (): jest.Mocked<FeatureUsageServiceSetup> => { | ||
| const mock = { | ||
| register: jest.fn(), | ||
| }; | ||
|
|
||
| return mock; | ||
| }; | ||
|
|
||
| const createStartMock = (): jest.Mocked<FeatureUsageServiceStart> => { | ||
| const mock = { | ||
| notifyUsage: jest.fn(), | ||
| getLastUsages: jest.fn(), | ||
| clear: jest.fn(), | ||
| }; | ||
|
|
||
| return mock; | ||
| }; | ||
|
|
||
| const createServiceMock = (): jest.Mocked<PublicMethodsOf<FeatureUsageService>> => { | ||
| const mock = { | ||
| setup: jest.fn(() => createSetupMock()), | ||
| start: jest.fn(() => createStartMock()), | ||
| }; | ||
|
|
||
| return mock; | ||
| }; | ||
|
|
||
| export const featureUsageMock = { | ||
| create: createServiceMock, | ||
| createSetup: createSetupMock, | ||
| createStart: createStartMock, | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License; | ||
| * you may not use this file except in compliance with the Elastic License. | ||
| */ | ||
|
|
||
| import { FeatureUsageService } from './feature_usage_service'; | ||
|
|
||
| describe('FeatureUsageService', () => { | ||
| let service: FeatureUsageService; | ||
|
|
||
| beforeEach(() => { | ||
| service = new FeatureUsageService(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.restoreAllMocks(); | ||
| }); | ||
|
|
||
| const toObj = (map: ReadonlyMap<string, any>): Record<string, any> => | ||
| Object.fromEntries(map.entries()); | ||
|
|
||
| describe('#setup', () => { | ||
| describe('#register', () => { | ||
| it('throws when registering the same feature twice', () => { | ||
| const setup = service.setup(); | ||
| setup.register('foo'); | ||
| expect(() => { | ||
| setup.register('foo'); | ||
| }).toThrowErrorMatchingInlineSnapshot(`"Feature 'foo' has already been registered."`); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('#start', () => { | ||
| describe('#notifyUsage', () => { | ||
| it('allows to notify a feature usage', () => { | ||
| const setup = service.setup(); | ||
| setup.register('feature'); | ||
| const start = service.start(); | ||
| start.notifyUsage('feature', 127001); | ||
|
|
||
| expect(start.getLastUsages().get('feature')).toBe(127001); | ||
| }); | ||
|
|
||
| it('uses the current time when `usedAt` is unspecified', () => { | ||
| jest.spyOn(Date, 'now').mockReturnValue(42); | ||
|
|
||
| const setup = service.setup(); | ||
| setup.register('feature'); | ||
| const start = service.start(); | ||
| start.notifyUsage('feature'); | ||
|
|
||
| expect(start.getLastUsages().get('feature')).toBe(42); | ||
| }); | ||
|
|
||
| it('throws when notifying for an unregistered feature', () => { | ||
| service.setup(); | ||
| const start = service.start(); | ||
| expect(() => { | ||
| start.notifyUsage('unregistered'); | ||
| }).toThrowErrorMatchingInlineSnapshot(`"Feature 'unregistered' is not registered."`); | ||
| }); | ||
| }); | ||
|
|
||
| describe('#getLastUsages', () => { | ||
| it('returns the last usage for all used features', () => { | ||
| const setup = service.setup(); | ||
| setup.register('featureA'); | ||
| setup.register('featureB'); | ||
| const start = service.start(); | ||
| start.notifyUsage('featureA', 127001); | ||
| start.notifyUsage('featureB', 6666); | ||
|
|
||
| expect(toObj(start.getLastUsages())).toEqual({ | ||
| featureA: 127001, | ||
| featureB: 6666, | ||
| }); | ||
| }); | ||
|
|
||
| it('returns the last usage even after notifying for an older usage', () => { | ||
| const setup = service.setup(); | ||
| setup.register('featureA'); | ||
| const start = service.start(); | ||
| start.notifyUsage('featureA', 1000); | ||
| start.notifyUsage('featureA', 500); | ||
|
|
||
| expect(toObj(start.getLastUsages())).toEqual({ | ||
| featureA: 1000, | ||
| }); | ||
| }); | ||
|
|
||
| it('does not return entries for unused registered features', () => { | ||
| const setup = service.setup(); | ||
| setup.register('featureA'); | ||
| setup.register('featureB'); | ||
| const start = service.start(); | ||
| start.notifyUsage('featureA', 127001); | ||
|
|
||
| expect(toObj(start.getLastUsages())).toEqual({ | ||
| featureA: 127001, | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('#clear', () => { | ||
| it('clears the whole last usages map', () => { | ||
| const setup = service.setup(); | ||
| setup.register('featureA'); | ||
| setup.register('featureB'); | ||
|
|
||
| const start = service.start(); | ||
| start.notifyUsage('featureA'); | ||
| start.notifyUsage('featureB'); | ||
|
|
||
| expect([...start.getLastUsages().keys()]).toEqual(['featureA', 'featureB']); | ||
|
|
||
| start.clear(); | ||
|
|
||
| expect([...start.getLastUsages().keys()]).toEqual([]); | ||
| }); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I encountered (yet another) issue with the
getStartServicesmock type in tests. the mockStartServicesAccessoris now explicitly typed asany, any.