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

[Cases] Make metrics routes internal #162506

Merged
merged 21 commits into from
Aug 1, 2023

Conversation

adcoelho
Copy link
Contributor

@adcoelho adcoelho commented Jul 25, 2023

Fixes #162406

Summary

  • Made getCaseMetrics and getCasesMetrics internal APIs.
  • There was no documentation to begin with so there was none to remove.
  • The allowed/available features now come from the CaseMetricsFeature enum instead of being hardcoded everywhere.
  • We now also check for the values passed to the features field in case metrics requests using io-ts.
  • There was already some validation before which I decided to leave. When doing buildHandlers the function checkAndThrowIfInvalidFeatures is always called. Right now, this is only used by getCasesMetrics and getCaseMetrics where we already decode the params so that validation is redundant, but if it starts being used somewhere else it will be nice to have this extra security guarantee.

Release Notes

The get case metrics APIs are now internal.

@adcoelho adcoelho added release_note:breaking Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature v8.10.0 labels Jul 25, 2023
@adcoelho adcoelho self-assigned this Jul 25, 2023
@adcoelho adcoelho marked this pull request as ready for review July 26, 2023 12:37
@adcoelho adcoelho requested review from a team as code owners July 26, 2023 12:37
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops-cases (Feature:Cases)

@adcoelho adcoelho requested review from a team as code owners July 28, 2023 09:00
@adcoelho adcoelho requested a review from a team July 28, 2023 09:00
@adcoelho adcoelho requested review from a team as code owners July 28, 2023 09:00
@adcoelho adcoelho removed request for a team, ashokaditya, banderror and dasansol92 July 28, 2023 09:14
Fixed failing tests that were using the old url.
Corrected types for single case metrics features.
@adcoelho adcoelho force-pushed the make-metrics-internals branch from d0d5180 to 883dfd0 Compare July 28, 2023 09:17
Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

🚀

x-pack/plugins/cases/common/api/metrics/case.test.ts Outdated Show resolved Hide resolved
Copy link
Member

@machadoum machadoum left a comment

Choose a reason for hiding this comment

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

Threat hunting explore team changes LGTM!

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Defend Workflows Cypress Tests #1 / Endpoint Policy Response from Fleet Agent Details page should display policy response with errors should display policy response with errors

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
cases 71 72 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cases 421.1KB 421.2KB +52.0B
securitySolution 15.6MB 15.6MB +141.0B
total +193.0B

Page load bundle

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

id before after diff
cases 150.5KB 151.2KB +679.0B
Unknown metric groups

API count

id before after diff
cases 90 91 +1

History

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

cc @adcoelho

@adcoelho adcoelho merged commit 6085444 into elastic:main Aug 1, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 1, 2023
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
Fixes elastic#162406

## Summary

- Made `getCaseMetrics` and `getCasesMetrics` internal APIs.
- There was no documentation to begin with so there was none to remove.
- The allowed/available `features` now come from the
`CaseMetricsFeature` _enum_ instead of being hardcoded everywhere.
- We now **also** check for the values passed to the `features` field in
case metrics requests using io-ts.
- There was already some validation before which I decided to leave.
When doing `buildHandlers` the function `checkAndThrowIfInvalidFeatures`
is always called. Right now, this is only used by `getCasesMetrics` and
`getCaseMetrics` where we already decode the params so that validation
is redundant, but if it starts being used somewhere else it will be nice
to have this extra security guarantee.

### Release Notes

The get case metrics APIs are now internal.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
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:Cases Cases feature release_note:breaking Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Cases] Make metrics routes internals and limit features
6 participants