[Watcher] More robust handling of license at setup #55831
Conversation
…e first Enable app to react to license updates from backend
jloleysens
added
Feature:Watcher
v8.0.0
Team:Kibana Management
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
| }); | ||
| }, | ||
| }); | ||
| if (!initialLicenseStatus.valid) { |
There was a problem hiding this comment.
What do you think of adding some comments to describe the intended UX? For example, a comment here along the lines of:
// When we first load Kibana, exclude Watcher from Kibana entirely
// if the license doesn't support it.And then a corresponding comment on https://github.com/elastic/kibana/blob/master/x-pack/plugins/watcher/public/application/app.tsx#L57 would be something like:
// If the license changes while the user is in Watcher, we need to explain
// what's happened.| MANAGEMENT_BREADCRUMB: any; | ||
| } | ||
|
|
||
| export const App = (deps: AppDeps) => { | ||
| const { valid, message } = deps.getLicenseStatus(); | ||
| const [{ valid, message }, setLicenseStatus] = useState(deps.initialLicenseStatus); |
There was a problem hiding this comment.
It took me a minute to figure out what role initialLicenseStatus plays here and it seems like it doesn't have one, since plugin.ts won't render this component if initialLicenseStatus.valid is false. If that's the case, maybe it will be clearer to remove initialLicenseStatus from the props and instead use a hardcoded default:
const [{ valid, message }, setLicenseStatus] = useState({ valid: true });There was a problem hiding this comment.
Great point! Will update!
|
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* Only register watcher app if we have received an indication of license first Enable app to react to license updates from backend * Return setup to a synchronous function. Co-authored-by: Elastic Machine <[email protected]>
* Only register watcher app if we have received an indication of license first Enable app to react to license updates from backend * Return setup to a synchronous function. Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
Summary
Fix after #54752 was merged. Watcher, at setup time, subscribes to a
licensing.license$stream and only registers after it has received the first message from that stream. The problem is the setup function is running synchronously and this was causing some flakiness in ordering. One issue I saw was (when navigating to Watcher):This would redirect to home page as if the app has been disabled (even though it was not).
Additionally there was some jankiness with when the app link would be shown.
Changes
Inplugin.tswe now have anasyncsetup method which will await the first license fromlicensing.license$. Only after it has a valid license will it process app registration.setupshould remain synchronous [RFC][skip-ci] Prevent plugins from blocking Kibana startup #45796 (comment). We now do all registration synchronously and then respond tolicense$updates.app.tsxwe receive an initial license value and an observable so that we can react to license updates and handle license changes in a more graceful way.How to test
curl -XDELETE -v http://elastic:changeme@localhost:9200/_license && curl -XPOST -v http://elastic:changeme@localhost:9200/_license/start_basic\?acknowledge\=trueIf you don't have the required min. license in place for watcher, the link should not be there in the ES management section.
Additional Notes
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsUnit or functional tests were updated or added to match the most common scenariosThis was checked for keyboard-only and screenreader accessibilityFor maintainers
This includes a feature addition or change that requires a release note and was labeled appropriately