-
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
Migrate status & stats APIs to KP + remove legacy status lib #76054
Conversation
93ef427
to
76c334e
Compare
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.
Just passing by because I've been working on elastic/beats#20956 and added some comments related to the /api/status
API 🙂
c4e79ef
to
0713257
Compare
00ffe99
to
47354d0
Compare
47354d0
to
499868c
Compare
type HttpService = ReturnType<typeof createHttpServer>; | ||
type HttpSetup = UnwrapPromise<ReturnType<HttpService['setup']>>; | ||
|
||
describe('/api/stats', () => { |
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.
The test coverage on this endpoint is admittedly low. Since this will be owned by @elastic/telemetry and they have more knowledge of usage collectors (which is the bulk of the logic in this endpoint), I'm leaving this to them to flesh out more. This simply serves as a smoke test to ensure the request will complete successfully. There are also existing integration tests that test more permutations of auth & query parameter combos.
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.
Since this will be owned by @elastic/telemetry
Are you going to update CODEOWNERS in the following PR?
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.
They already own this entire plugin, so I don't believe any changes are needed to CODEOWNERS?
💚 Build SucceededBuild metricsdistributable file count
History
To update your PR or re-run it, just comment with: |
That was it! Thank you for fixing it! |
…#76054) * Complete migration of legacy status API * PR comments * Remove optional auth on stats endpoint * Regen docs * PR comments * PR nits * Set ReplaySubject buffer to 1 * Fix exclude_usage logic Co-authored-by: pgayvallet <pierre.gayvallet@elastic.co>
…#77752) * Complete migration of legacy status API * PR comments * Remove optional auth on stats endpoint * Regen docs * PR comments * PR nits * Set ReplaySubject buffer to 1 * Fix exclude_usage logic Co-authored-by: pgayvallet <pierre.gayvallet@elastic.co> Co-authored-by: Josh Dover <me@joshdover.com>
…rok/new-patterns-component-use-array * 'master' of github.com:elastic/kibana: (140 commits) Add telemetry as an automatic privilege grant (elastic#77390) [Security Solutions][Cases] Cases Redesign (elastic#73247) Use Search API in TSVB (elastic#76274) [Mappings editor] Add support for constant_keyword field type (elastic#76564) [ML] Adds ML modules for Metrics UI Integration (elastic#76460) [Drilldowns] {{event.points}} in URL drilldown for VALUE_CLICK_TRIGGER (elastic#76771) Migrate status & stats APIs to KP + remove legacy status lib (elastic#76054) use App updater API instead of deprecated chrome.navLinks.update (elastic#77708) [CSM Dashboard] Remove points from line chart (elastic#77617) [APM] Trace timeline: Replace multi-fold function icons with new EuiIcon glyphs (elastic#77470) [Observability] Overview: Alerts section style improvements (elastic#77670) Bump the Node.js version used by Docker in CI (elastic#77714) Upgrade all minimist (sub)dependencies to version ^1.2.5 (elastic#60284) Remove unneeded forced package resolutions (elastic#77467) [ML] Add metrics app to check made for internal custom URLs (elastic#77627) Functional tests - add supertest for test_user (elastic#77584) [ML] Adding option to create AD jobs without starting the datafeed (elastic#77484) Bump node-fetch to 2.6.1 (elastic#77445) Bump sharkdown from v0.1.0 to v0.1.1 (elastic#77607) [APM]fixing y axis on transaction error rate to 100% (elastic#77609) ... # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/manage_processor_form.container.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/manage_processor_form.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/field_components/drag_and_drop_text_list.scss # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/field_components/drag_and_drop_text_list.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/field_components/text_editor.scss # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processor_form/processors/grok.test.tsx
Summary
Related #41983
This migrates the
/api/status
and/api/stats
endpoints to the Kibana Platform. It also deletes all of thesrc/legacy/server/status
module and adapts any leftover code that was still using it. The updated status endpoint will now report statuses from all Kibana Platform plugins & core services.Some discussion points:
/api/stats
relies on UsageCollectors which are only available to plugins. Because of this, I opted to put the/api/stats
endpoint in the usage_collection plugin while keeping the/api/status
endpoint inside Core. I had to duplicate a bit of the logic for translating KP status levels to the legacy status colors. LMK if you have a better idea or prefer to put these elsewhere./api/status
which is disabled by default but can be opted-into with thev8format
query parameter. My plan is to add a release note that the old format is deprecated and the default will change in 8.0/master. I will put up a quick follow up PR to change this in master after this merges.Release Note
The
/api/status
endpoint response format is now deprecated and will change in 8.0.This change only affects the
status.*
key of the JSON response, all other keys are unchanged:Before
After
Status codes and request parameters are unchanged.
In all 7.x versions, the old format will continue to be returned. In 8.0, the default format will change to the new format. Users of 7.10+ may opt-in to the new format by specifying the
v8format=true
parameter in the URL. This allows admins to update any external health checks at your convenience and provide a smooth upgrade experience to 8.0.Checklist
Delete any items that are not applicable to this PR.
For maintainers