-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
add client-side feature usage API #75486
add client-side feature usage API #75486
Conversation
Pinging @elastic/kibana-platform (Team:Platform) |
…side-feature-usage
…side-feature-usage
}, | ||
}, | ||
async (context, request, response) => { | ||
const [, , { featureUsage }] = await getStartServices(); |
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.
optional: why not use context to pass the start contract?
return { | ||
notifyUsage: async (featureName, usedAt = Date.now()) => { | ||
const lastUsed = isDate(usedAt) ? usedAt.getTime() : usedAt; | ||
await http.post('/internal/licensing/feature_usage/notify', { |
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.
in theory, we can overload the server-side if notifyUsage
is called frequently. But it might be premature optimization.
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
page load bundle size
distributable file count
History
To update your PR or re-run it, just comment with: |
* add client-side feature_usage API * use route context for notify feature usage route
…s-for-710 * 'master' of github.com:elastic/kibana: (43 commits) [APM] Chart units don't update when toggling the chart legends (elastic#74931) [ILM] Add support for frozen phase in UI (elastic#75968) Hides advanced json for count metric (elastic#74636) add client-side feature usage API (elastic#75486) [Maps] add drilldown support map embeddable (elastic#75598) [Enterprise Search] Request handler refactors/enhancements + update existing routes to use shared handler (elastic#76106) [Resolver] model `location.search` in redux (elastic#76140) [APM] Prevent imports of public in server code (elastic#75979) fix eslint issue skip flaky suite (elastic#76223) [APM] Transaction duration anomaly alerting integration (elastic#75719) [Transforms] Avoid using "Are you sure" (elastic#75932) [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (elastic#76145) [plugin-helpers] improve 3rd party KP plugin support (elastic#75019) [docs/getting-started] link to yarn v1 specifically (elastic#76169) [Security_Solution][Resolver] Resolver loading and error state (elastic#75600) Fixes App Search documentation links (elastic#76133) Fix alerts unable to create / update when the name has trailing whitepace(s) (elastic#76079) [Resolver] Fix useSelector usage (elastic#76129) [Enterprise Search] Migrate util and components from ent-search (elastic#76051) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/data_tier_allocation/node_allocation.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx # x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts # x-pack/plugins/index_lifecycle_management/public/application/services/policies/warm_phase.ts
return { | ||
notifyUsage: async (featureName, usedAt = Date.now()) => { | ||
const lastUsed = isDate(usedAt) ? usedAt.getTime() : usedAt; | ||
await http.post('/internal/licensing/feature_usage/notify', { |
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.
@pgayvallet, should we catch and swallow errors here? I guess consumer shouldn't care about handling errors from this (nor probably wait for a success) and unhandled promise rejection from this call would be a bummer.
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.
idk. Realistically, the only reason this would fails (appart from network failures) would be if the consumer tries to notify usage of an unregistered feature, which it a consumer cause (and should be detected in development).
Also, unhandled rejections on the client-side don't really do much (even if of course they should be avoided)
But that's not a strong opinion, and if really you think this is preferable, I can change that.
Summary
Fix #72572
Add client-side version of the
featureUsage
API to the licensing pluginChecklist