-
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 plugin status API #75819
Add plugin status API #75819
Conversation
f2b5f56
to
1440cc5
Compare
Array [ | ||
Object { | ||
"level": available, | ||
"summary": "All services are available", | ||
}, | ||
Object { | ||
"level": degraded, | ||
"summary": "[a]: This is degraded!", | ||
}, | ||
Object { | ||
"level": degraded, | ||
"summary": "[2] services are degraded", | ||
}, |
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.
This highlights a current problem with this Observable-based architecture: a single update to one plugin's status may result in many overall updates as it propagates through the dependency tree.
I plan to experiment more tomorrow with RxJS's various schedulers and debouncing utilities, but I'm not sure yet there is a great solution to this. Even if there is, we still would need to provide guidance or utilities so that plugin authors can also avoid this same issue in their custom status implementations.
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.
https://www.learnrxjs.io/learn-rxjs/operators/filtering/debouncetime after combineLatest
? Can it be backed in the core in order not to enforce plugins to wrap it manually?
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.
+1 for debounce effect of some kind to the overall$
and plugins$
observables. We probably want to keep the derivedStatus$
exposed to plugins 'fresh' to avoid delay in status recomputation though?
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.
My goal was to avoid any time-based approach in order to make the behavior very consistent for synchronous Observable sources. I was able to get this to work using debounce
with a variable number of microtasks based on the number of known observables. Seems to be working correctly! Now the observables should only emit once per macro-task.
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 decided to just go with a time-based apporoach after all. Microtasks seemed like too much magic and very fragile to break. It's also easy to write a test which ends up changing the microtasks queue itself, leading to strange results in tests.
We probably want to keep the derivedStatus$ exposed to plugins 'fresh' to avoid delay in status recomputation though?
+1 to that. I am only going to debounce the overall$ and plugins$ observables
1440cc5
to
a0a091e
Compare
Pinging @elastic/kibana-platform (Team:Platform) |
a0a091e
to
edd46bb
Compare
edd46bb
to
3ea7db2
Compare
// TODO: include URL to status page | ||
detail: status.detail ?? `See the status page for more information`, | ||
meta: { | ||
affectedServices: { [serviceName]: status }, | ||
}, |
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.
Changed the summary output for when only a single service is unavailable to be more consistent with the case where more than one is unavailable.
// We schedule the number of microtasks expected to resolve all of the dependencies of this update. | ||
// Waiting for this ensures that we do not emit 'partial updates' to reduce noise. | ||
debounce(([coreStatus, pluginsStatus]) => | ||
waitForMicrotasks(1 + Object.keys(pluginsStatus).length) |
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.
does it mean that all this.pluginsStatus
API consumers have to implement debouncing
manually?
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.
Now that debouncing is configured on the Observable used to wrap a plugin's dependencies (the core.status.dependencies$
Observable), I don't believe this will be a problem that "fans out" to consumers, so manual debouncing should be unnecessary.
💚 Build SucceededBuild metricsoss distributable file count
distributable file count
History
To update your PR or re-run it, just comment with: |
# Conflicts: # rfcs/text/0010_service_status.md
Co-authored-by: Elastic Machine <[email protected]>
This reverts commit 2006eca.
Co-authored-by: spalger <[email protected]>
Co-authored-by: spalger <[email protected]> # Conflicts: # rfcs/text/0010_service_status.md
…rok/new-patterns-component-use-array * 'master' of github.com:elastic/kibana: (75 commits) Remove legacy ui-apps-mixin (elastic#76604) remove unused test_utils (elastic#76528) [ML] Functional tests - add UI permission tests (elastic#76368) [APM] @ts-error -> @ts-expect-error (elastic#76492) [APM] Avoid negative offset for error marker on timeline (elastic#76638) [Reporting] rename interfaces to align with task manager integration (elastic#76716) Revert back ESO migration for alerting, added try/catch logic to avoid failing Kibana on start (elastic#76220) Test reverting "Add plugin status API (elastic#75819)" (elastic#76707) [Security Solution][Detections] Removes ML Job Settings SIEM copy and fixes link to ML app for creating custom jobs (elastic#76595) [Maps] remove region/coordinate-maps visualizations from sample data (elastic#76399) [DOCS] Dashboard-first docs refresh (elastic#76194) Updated ServiceNow description in docs and actions management UI to contains correct info (elastic#76344) [DOCS] Identifies cloud settings in reporting (elastic#76691) [Security Solution] Refactor timeline details to use search strategy (elastic#75591) es-archiver: Drop invalid index settings, support --query flag (elastic#76522) [DOCS] Identifies graph settings available on cloud (elastic#76661) Add more info about a11y tests (elastic#76045) [data.search.SearchSource] Remove legacy ES client APIs. (elastic#75943) [release notes] automatically retry on Github API 5xx errors (elastic#76447) [es_ui_shared] Fix eslint exhaustive deps rule (elastic#76392) ...
Summary
Related to #41983
Implements the
core.status.set
,core.status.plugins$
, andcore.status.derivedStatus$
APIs as specified in RFC-0010.I did not write any e2e tests in this PR as I thought it would be easier to write them directly against the migrated status HTTP API that I am working on in parallel.
To Do:
StatusServiceSetup
that explains how plugin statuses work, how they are inherited, and how to customize them.Dev Docs
Kibana Platform plugins may now read the status of their dependencies, their plugin's default status, and manually override that status as reported to the end user and on the
/api/status
endpoint.Checklist
Delete any items that are not applicable to this PR.
For maintainers