Skip to content
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

[Status UI] Use the new output format of API GET /api/status #107937

Merged
merged 3 commits into from
Aug 11, 2021

Conversation

afharo
Copy link
Member

@afharo afharo commented Aug 9, 2021

Summary

Resolves #77624

This PR uses the request GET /api/status?v8format=true to prepare the Status page for the breaking changes in the output of such API coming up for 8.0.0.

No major UI changes are made. The only visible one is that the version detailed as plugin:[email protected] is removed from each plugin name in the list.

I removed it because, IMO, it doesn't add any value, and it wouldn't be accurate for 3rd-party plugins, since we are using Kibana's global version. The current logic has the same issue because GET /api/status injects the version from Kibana as well.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@afharo afharo added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.15.0 labels Aug 9, 2021
@afharo afharo marked this pull request as ready for review August 10, 2021 07:18
@afharo afharo requested review from a team as code owners August 10, 2021 07:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@afharo
Copy link
Member Author

afharo commented Aug 10, 2021

@elasticmachine run elasticsearch-ci/docs

Comment on lines 36 to 37
core: Record<string, ServerStatus>;
plugins: Record<string, ServerStatus>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This StatusResponse type isn't currently being shared across server and public, though we may not really be able to share it properly until we've removed the legacy type.

core/public is using this type, while core/server is using this unexported type:

interface StatusHttpBody {

If it's not possible to align these two to use the same type, can we leave a TODO to do this as part of #94419 or in code?

serverState: formatStatus(response.status.overall).state,
statuses: [
...Object.entries(response.status.core).map(([serviceName, status]) =>
formatStatus(`core:${serviceName}@${response.version.number}`, status)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we should remove the version number from the core services as well. I think it's confusing and unnecessary.

@@ -7,8 +7,9 @@
*/

import { i18n } from '@kbn/i18n';
import { deepFreeze } from '@kbn/std';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, if this is increasing the core bundle size, it's likely not worth it.

Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meant to approve, no reason to block this merge.

@afharo afharo force-pushed the status-page/use-new-api-format branch from 6c0cc4d to 3bf1714 Compare August 11, 2021 09:25
- Use common types on the server and public
- Remove `deepFreeze`
- Remove version from core services
@afharo afharo force-pushed the status-page/use-new-api-format branch from 3bf1714 to 245943c Compare August 11, 2021 09:29
Comment on lines +47 to +48
build_number: number;
build_snapshot: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The moment we linked the types, this screamed as wrong 👌

* We need this type to convert the object `ServiceStatusLevel` to a union of the possible strings.
* This is because of the "stringification" that occurs when serving HTTP requests.
*/
export type ServiceStatusLevel = ReturnType<ServiceStatusLevelFromServer['toString']>;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the biggest change when linking both types: the HTTP response calls the method toString(). So the types are not exactly the same.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 423.4KB 424.2KB +827.0B

History

  • 💚 Build #143860 succeeded 6c0cc4da4ac1ef59a9d878a416cc4cee9a4c4254
  • 💔 Build #143804 failed d2ab67e4d9f235ed0d8a5da6088adc36c6857c5b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@afharo afharo added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 11, 2021
@afharo afharo merged commit def97bd into elastic:master Aug 11, 2021
@afharo afharo deleted the status-page/use-new-api-format branch August 11, 2021 15:56
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 107937

@afharo
Copy link
Member Author

afharo commented Aug 11, 2021

7.x backport PR: #108227

afharo added a commit that referenced this pull request Aug 11, 2021
…) (#108227)

# Conflicts:
#	packages/kbn-optimizer/limits.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adapt status page UI to 8.0 /api/status format
5 participants