Skip to content

[8.10] backport SLO fixes - #168231

Merged
kdelemme merged 6 commits into
elastic:8.10from
kdelemme:backport/slo/8.10.4
Oct 10, 2023
Merged

[8.10] backport SLO fixes#168231
kdelemme merged 6 commits into
elastic:8.10from
kdelemme:backport/slo/8.10.4

Conversation

@kdelemme

@kdelemme kdelemme commented Oct 6, 2023 •
edited by simianhacker
Loading

Copy link
Copy Markdown
Contributor

🍒 Summary

This PR backports the following commits into 8.10 for release scheduled on 8.10.4:

Release Notes

This PR is a mix of fixes and enhancements:

  • Improve the SLO search bar, fixing the issue with focus lost when the list was refreshing
  • Handle gracefully any errors when the preview data API was failing to execute for any reason.
  • Fix Historical SLI and error budget calculation when data was intermittent and had gaps.
  • Optimize the summary transforms by running them unattended (always retry)
  • Improve resilliency of the SLO update use case, handling any errors gracefully with proper rollback.
  • Improve error handling from the react hooks

Testing notes

  • For fix(slo): preview data no response error #167909 and fix(slo): rollback when errors happen during the update use case #168142, set up a user with a role with the following:
    • Cluster Privileges: manage_transform
    • Index Privileges:
      • .slo-*: all
      • high-*: read and read_cross_cluster
    • Kibana: All spaces All privileges
    • Create an SLO with the elastic superuser
    • When you log in as this user, try to create a new SLO and then edit (hit save) the SLO the elastic user created. For the new SLO you should get an error that says you don't have the proper permissions and then end up on the create screen (not saved). When you hit save on the SLO (that the elastic user created) you should end up with the same error without changing the original SLO.

@github-actions

github-actions Bot commented Oct 6, 2023

Copy link
Copy Markdown
Contributor

Documentation preview:

@ghost

ghost commented Oct 6, 2023

Copy link
Copy Markdown

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@kdelemme
kdelemme changed the base branch from main to 8.10 October 6, 2023 14:02
@kdelemme
kdelemme marked this pull request as ready for review October 6, 2023 14:04
@simianhacker
simianhacker self-requested a review October 6, 2023 14:07
@kdelemme kdelemme self-assigned this Oct 6, 2023
@kdelemme kdelemme added Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" Feature:SLO labels Oct 6, 2023
@elasticmachine

elasticmachine commented Oct 6, 2023

Copy link
Copy Markdown
Contributor

Pinging @elastic/actionable-observability (Team: Actionable Observability)

@kibana-ci

kibana-ci commented Oct 6, 2023

Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
observability 1.0MB 1.0MB +416.0B

History

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

cc @kdelemme

@mgiota
mgiota self-requested a review October 6, 2023 18:31
simianhacker
simianhacker approved these changes Oct 6, 2023
View reviewed changes

@simianhacker simianhacker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM... I tested it with an under privileged user, it worked as expected. I started ES without a transform node, the new error message handler caught it perfectly and returned me to the "Create SLO" form. I also created and SLO with a user who had the correct privileges, then edit the privileges to remove read access to high-* and the transform continued to run but stopped indexing data due to not being able to read the data. Once I restored the privilege, it caught back up.

@mgiota

mgiota commented Oct 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

I am currently reviewing this. Code review looks good. I am testing it locally.

I started ES without a transform node

@simianhacker @kdelemme How can I do this?

@mgiota

mgiota commented Oct 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@kdelemme I created a user with read only privileges for SLO, but I don't see the 2 SLOs I have.

Screen.Recording.2023-10-09.at.13.53.08.mov

I might probably be missing something, when creating my custom slo read role. How do you create a read only SLO role? I didn't specify anything to the index privileges. Do I have to?

Screenshot 2023-10-09 at 13 58 21

@kdelemme

kdelemme commented Oct 10, 2023

Copy link
Copy Markdown
Contributor Author

@mgiota Yes you are missing the following index Privileges: .slo-* => read

@mgiota

mgiota commented Oct 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@kdelemme Thanks! I added the Index Privilege and now works as expected. Corresponding actions are deactivated
Screenshot 2023-10-10 at 14 44 03 Screenshot 2023-10-10 at 14 44 17

I played a bit more with custom roles. Here's a role I created:

  • Index privilege .slo-* => all
  • Kibana privilege > Custom > SLO > ALL

When I try to edit the slo, timestamp field is not preselected and the SLI preview is empty. Not sure if a real user would create a role the way I created it, so not sure if this is a bug or not. What do you think?

Screen.Recording.2023-10-10.at.14.59.20.mov

@kdelemme

kdelemme commented Oct 10, 2023

Copy link
Copy Markdown
Contributor Author

When I try to edit the slo, timestamp field is not preselected and the SLI preview is empty. Not sure if a real user would create a role the way I created it, so not sure if this is a bug or not. What do you think?

That's expected, since you did not give any privileges (at least read) on the source index (.e.g the high-cardinality admin console index).

@mgiota

mgiota commented Oct 10, 2023

Copy link
Copy Markdown
Contributor

@kdelemme I tested it and everything works as expected!

@kdelemme
kdelemme merged commit afa98f7 into elastic:8.10 Oct 10, 2023
@kdelemme
kdelemme deleted the backport/slo/8.10.4 branch October 10, 2023 16:01
kdelemme referenced this pull request Oct 10, 2023
@kdelemme
[8.11] fix(slo): handle permission error (#167933) (#168248)
636a833 
# Backport

This will backport the following commits from `main` to `8.11`:
- [fix(slo): handle permission error
(#167933)](#167933)

<!--- 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":"2023-10-05T13:17:32Z","message":"fix(slo):
handle permission error
(#167933)","sha":"335fc9b2409855f4aeebf360c0747141b2fcf03b","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:skip","Team:
Actionable
Observability","Feature:SLO","v8.12.0"],"number":167933,"url":"https://github.com/elastic/kibana/pull/167933","mergeCommit":{"message":"fix(slo):
handle permission error
(#167933)","sha":"335fc9b2409855f4aeebf360c0747141b2fcf03b"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/167933","number":167933,"mergeCommit":{"message":"fix(slo):
handle permission error
(#167933)","sha":"335fc9b2409855f4aeebf360c0747141b2fcf03b"}},{"url":"https://github.com/elastic/kibana/pull/168231","number":168231,"branch":"8.10","state":"OPEN"}]}]
BACKPORT-->
@mistic

mistic commented Oct 10, 2023

Copy link
Copy Markdown
Contributor

Those changes didn't make it on time into the latest BC for 8.10.3. Updating the labels.

@mistic mistic added v8.10.4 and removed v8.10.0 labels Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simianhacker simianhacker simianhacker approved these changes
@mgiota mgiota mgiota approved these changes

Assignees

@kdelemme kdelemme

Labels

Feature:SLO release_note:fix Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.10.4

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@kdelemme @elasticmachine @kibana-ci @mgiota @mistic @simianhacker