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

Expose overall status to plugins #75503

Merged
merged 3 commits into from
Aug 24, 2020

Conversation

joshdover
Copy link
Contributor

Summary

Related to #41983

Exposes the API for reading Kibana's overall status to plugins in order to unblock @elastic/stack-monitoring-ui from deleting their legacy plugin. This API should only be used for data collection & reporting to external systems and should not drive application behavior.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@joshdover joshdover added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 NeededFor:Monitoring labels Aug 19, 2020
@joshdover joshdover requested a review from a team as a code owner August 19, 2020 21:48
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@chrisronline
Copy link
Contributor

Thanks for doing this work!

I'm not sure the newly added response is compatible with the legacy response though.

The legacy response looks like (this is the object under the overall key):

{
  "state": "green",
  "title": "Green",
  "nickname": "Looking good",
  "icon": "success",
  "uiColor": "secondary",
  "since": "2020-08-20T14:49:06.720Z"
}

The new response from this PR looks like:

{
  "level": {},
  "summary": "All services are available"
}

All we technically need to report is the state field (which in the above example is Green).

Am I using the API wrong? Or is this an oversight?

@joshdover
Copy link
Contributor Author

Thanks for doing this work!

I'm not sure the newly added response is compatible with the legacy response though.

The legacy response looks like (this is the object under the overall key):

{
  "state": "green",
  "title": "Green",
  "nickname": "Looking good",
  "icon": "success",
  "uiColor": "secondary",
  "since": "2020-08-20T14:49:06.720Z"
}

The new response from this PR looks like:

{
  "level": {},
  "summary": "All services are available"
}

All we technically need to report is the state field (which in the above example is Green).

Am I using the API wrong? Or is this an oversight?

My intention was to keep this legacy data out of the API exposed from Core. We're no longer going to use many of these fields moving forward but we will need to maintain some compatibility in some parts (the /api/status endpoint for example).

I see two options here:

  1. Core could expose a core.status.legacy.overall$ API that has the same shape as the legacy status API
  2. Monitoring can do the translation from the new format to the old one by copying most of the code in src/legacy/server/status/states.js and doing a simple mapping (unavailable -> red, degraded -> yellow, available -> green, etc.)

(1) may be simpler so that we only have this mapping logic in one place (since I'll need to this in Core already for the HTTP API). One question: how is this data ingested by monitoring and do we need all of the fields?

@joshdover
Copy link
Contributor Author

The new response from this PR looks like:

{
  "level": {},
  "summary": "All services are available"
}

FYI I think this may just be being serialized strangely. The level field is an object that contains toString() and valueOf() functions to describe the level (but can also be directly compared to the ServiceStatusLevel.* objects from src/core/server).

@chrisronline
Copy link
Contributor

how is this data ingested by monitoring and do we need all of the fields?

We just need the state field as a string

Core could expose a core.status.legacy.overall$ API that has the same shape as the legacy status API

This would work for us, but I could also the translation on our side. I'd defer to your judgement here as you are the maintainers of the API.

@joshdover
Copy link
Contributor Author

This would work for us, but I could also the translation on our side. I'd defer to your judgement here as you are the maintainers of the API.

If all you need is the state string, let's just do that translation in Stack Monitoring. This will reduce the amount of breaking changes on the Core API for us to manage and think about. I will merge this PR as-is then.

@joshdover
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@joshdover joshdover merged commit c587058 into elastic:master Aug 24, 2020
@joshdover joshdover deleted the status/expose-overall branch August 24, 2020 17:41
joshdover added a commit to joshdover/kibana that referenced this pull request Aug 24, 2020
joshdover added a commit that referenced this pull request Aug 24, 2020
Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
parkiino pushed a commit to parkiino/kibana that referenced this pull request Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform NeededFor:Monitoring 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.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants