chore(slo): cleanup error log message #205334
Conversation
kdelemme
added
release_note:skip
|
Pinging @elastic/obs-ux-management-team (Team:obs-ux-management) |
There was a problem hiding this comment.
Overall, LGTM! 🧹 Added a clarification question.
Also, I was wondering if during logging, we can add a field, such as type or something like that, to categories errors. For example, for this error:
Cannot reset the SLO [id: ${slo.id}, revision: ${slo.revision}]. Rolling back. ${err}
it would be useful if we can set a type like reset-slo-failed for grouping errors, do you know if there is such an option?
| @@ -100,14 +100,14 @@ export class CreateSLO { | |||
| ]); | |||
| } catch (err) { | |||
| this.logger.error( | |||
| `Cannot install the SLO [id: ${slo.id}, revision: ${slo.revision}]. Rolling back.` | |||
| `Cannot create the SLO [id: ${slo.id}, revision: ${slo.revision}]. Rolling back. ${err}` | |||
There was a problem hiding this comment.
Isn't err an object? How would it be logged in this case?
I vaguely remember there was a consideration while logging errors to ensure we don't log sensitive information, do you remember what the issue was and whether it is applicable in this PR?
There was a problem hiding this comment.
in this case, it would be just formatted as error string.
There was a problem hiding this comment.
Yes it's an object, it will be serialized using its toString() method, which by default includes the name, message and stack trace I believe.
I vaguely remember there was a consideration while logging errors to ensure we don't log sensitive information, do you remember what the issue was and whether it is applicable in this PR?
This was mostly about logging request/response details. In this case, the error message should not include any sensitive information.
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/12586272760 |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]
cc @kdelemme |
(cherry picked from commit a0c94f4)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `8.x`: - [chore(slo): cleanup error log message (#205334)](#205334) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Kevin Delemme","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-02T17:27:26Z","message":"chore(slo): cleanup error log message (#205334)","sha":"a0c94f433ac438787b4564750606efd847acfa2c","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","Team:obs-ux-management","v8.18.0"],"title":"chore(slo): cleanup error log message","number":205334,"url":"https://github.com/elastic/kibana/pull/205334","mergeCommit":{"message":"chore(slo): cleanup error log message (#205334)","sha":"a0c94f433ac438787b4564750606efd847acfa2c"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205334","number":205334,"mergeCommit":{"message":"chore(slo): cleanup error log message (#205334)","sha":"a0c94f433ac438787b4564750606efd847acfa2c"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Kevin Delemme <[email protected]>
Resolves #205355
Summary
This PR audits and rewords the SLO error log message. I've mainly added the original error into the logged message. And changed one log level from error to info