chore(slo): Migrate to server-route-repository #198726
Conversation
kdelemme
force-pushed
the
chore/migrate-to-core-http-router
branch
from
9f9aac8 to
ab5510c
Compare
| /> | ||
| </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.
💚 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. |
kibanamachine
added
the
backport missing
💚 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":"[email protected]"},"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-->
kibanamachine
removed
the
backport missing
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: