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

[Stack Monitoring] Convert elasticsearch routes to TypeScript #130969

Merged

Conversation

weltenwort
Copy link
Member

@weltenwort weltenwort commented Apr 26, 2022

πŸ“ Summary

This converts the elasticsearch-specific stack monitoring routes to TypeScript.

closes #117760

πŸ•΅οΈβ€β™€οΈ Review notes

  • I added type inference to the legacy request to gain more confidence into the request runtime types. In order to teach the compiler the correct inference flow I separated out validateParams, validateQuery and validateBody functions for each route. That way the correct request type is fixed and made available to the req argument of the route registration function. I also retrofitted that to the already converted APM and beats routes.
  • The timeRange request param type performs the conversion to an epoch directly while decoding so I remove several downstream moment usages.
  • Some of the called functions like getClusterStats and getPaginatedNodes don't actually take a ccs argument but the routes passed it as the last argument. It never took effect so I removed it.

@weltenwort weltenwort added Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes Feature:Stack Monitoring v8.3.0 labels Apr 26, 2022
@weltenwort weltenwort self-assigned this Apr 26, 2022
@weltenwort weltenwort force-pushed the monitoring-ts-convert-routes-elasticsearch branch from 73221aa to 1ebc79f Compare April 26, 2022 14:12
@kibana-ci
Copy link
Collaborator

πŸ’š Build Succeeded

Metrics [docs]

βœ… unchanged

History

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

cc @weltenwort

@weltenwort weltenwort marked this pull request as ready for review April 27, 2022 11:51
@weltenwort weltenwort requested a review from a team as a code owner April 27, 2022 11:51
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@Kerry350 Kerry350 self-requested a review April 27, 2022 11:55
Copy link
Contributor

@Kerry350 Kerry350 left a comment

Choose a reason for hiding this comment

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

LGTM βœ…

Thanks for adding the type inference on LegacyRequest and expanding the shared types πŸ™

@weltenwort weltenwort merged commit 194cb16 into elastic:main Apr 28, 2022
@weltenwort weltenwort deleted the monitoring-ts-convert-routes-elasticsearch branch April 28, 2022 15:26
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Stack Monitoring release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Stack Monitoring] Convert files in server/routes/elasticsearch to typescript
5 participants