chore(slo): Migrate to server-route-repository#198726
chore(slo): Migrate to server-route-repository#198726kdelemme merged 56 commits intoelastic:mainfrom
Conversation
9f9aac8 to
ab5510c
Compare
dmlemeshko
left a comment
There was a problem hiding this comment.
Deployment-agnostic tests changes LGTM
kdelemme
left a comment
There was a problem hiding this comment.
Left some comments for reviewers
| /> | ||
| </QueryClientProvider> | ||
| </Router> | ||
| <PluginContext.Provider |
There was a problem hiding this comment.
All the embeddables registration would need to be refactored to use a common context providers instead of creating from scratch for the 5 different embeddable we have. Very error prone.
There was a problem hiding this comment.
Linking with this comment here. There is indeed an SloEmbeddableContext which I introduced as part of refactoring the alerts embeddable. I will create an issue for this
| if (error instanceof ObservabilityError) { | ||
| logger.error(error.message); | ||
| return response.customError({ | ||
| statusCode: getHTTPResponseCode(error), | ||
| body: { | ||
| message: error.message, | ||
| }, | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
This was specific to SLO. Since we moved SLO outside of observability/ this handler is not use anymore
| export async function executeWithErrorHandler(fn: () => Promise<any>): Promise<any> { | ||
| try { | ||
| return await fn(); | ||
| } catch (error) { | ||
| if (error instanceof SLOError) { | ||
| throw handleSLOError(error); | ||
| } | ||
|
|
||
| throw error; | ||
| } |
There was a problem hiding this comment.
This handler is used to wrap the various route's application services and handle any domain errors thrown by them, by mapping them to their Boom equivalent. Boom errors are then handled by the core router.
Ideally, the core http router would offer a way to provide a custom error handler, but changing this there would require more consideration and agreement with the core team. Which can be discussed and done outside of this PR.
shahzad31
left a comment
There was a problem hiding this comment.
Things looks normal !!
Smoke tested following !!
- Adding/editing SLO
- SLO details page
- SLO Overview page
- Adding an embeddable from SLO page
- Adding Overview/alerts/burn rate embeddable
Also verified that version header is being provided now in call from UI !!
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
|
|
Starting backport for target branches: 8.x |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
(cherry picked from commit efcc2ab) # Conflicts: # oas_docs/output/kibana.serverless.yaml # x-pack/plugins/observability_solution/slo/public/plugin.ts # x-pack/plugins/observability_solution/slo/public/types.ts # x-pack/plugins/observability_solution/slo/tsconfig.json
# Backport This will backport the following commits from `main` to `8.x`: - [chore(slo): Migrate to server-route-repository (#198726)](#198726) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kevin Delemme","email":"kevin.delemme@elastic.co"},"sourceCommit":{"committedDate":"2024-11-12T21:24:53Z","message":"chore(slo): Migrate to server-route-repository (#198726)","sha":"efcc2ab004f9888b4307e85cb0a604c79fc39027","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport missing","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management","v8.17.0"],"number":198726,"url":"https://github.com/elastic/kibana/pull/198726","mergeCommit":{"message":"chore(slo): Migrate to server-route-repository (#198726)","sha":"efcc2ab004f9888b4307e85cb0a604c79fc39027"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198726","number":198726,"mergeCommit":{"message":"chore(slo): Migrate to server-route-repository (#198726)","sha":"efcc2ab004f9888b4307e85cb0a604c79fc39027"}},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
Resolves #198683
🌸 Summary
This PR migrates the SLO plugin to the new server route repository, so we can leverage the client.
I've added the SLO repository client into the SLO Plugin Context so we can retrieve it from the different Hooks.
The hooks have been updated to use the new SLO repository client. I had to update the SLO embeddable to use this context as well.
I've taken the opportunity to refactor a bit the various Plugin types, but there is a lot left to do, especially around the Context values we provide a bit everywhere with a little bit of everything 🙅🏻
Testing
Create some SLOs, and play around with dashboard embeddable, SLO flyout.
Found issues
💀 Risks
Check the following clients use the API version header: