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

[SLOs] Added $state into filters schema !! #202887

Merged
merged 1 commit into from
Dec 5, 2024
Merged

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Dec 4, 2024

Summary

fixes #202999

Added $state into filters schema !!

@shahzad31 shahzad31 marked this pull request as ready for review December 4, 2024 10:26
@shahzad31 shahzad31 requested a review from a team as a code owner December 4, 2024 10:26
@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development labels Dec 4, 2024
@elasticmachine
Copy link
Contributor

💚 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 480.5KB 480.6KB +44.0B
slo 849.0KB 849.1KB +44.0B
synthetics 1.1MB 1.1MB +44.0B
total +132.0B

query: t.record(t.string, t.any),
}),
t.partial({
$state: t.any,
Copy link
Member

Choose a reason for hiding this comment

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

Is it going to be the same data that we store in the SLO's instance? Was there an issue when this type was not present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't store this normall anymore from UI. it's just to be backward compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what is that for? Can you open an issue stating the problem that this PR is actually solving please?

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently we were allowing excess keys for a while, resulting in some SLOs created with some $state: { store: "appState" }overhead. Then excess keys become not allowed somewhere in 8.16, which breaks any API using the filterSchema when provided with such $state key.

This part of the API is really just a dump of the UI settings, nothing better we can do here. I don't expect customer to use that directly

Copy link
Member

@maryam-saeidi maryam-saeidi Dec 5, 2024

Choose a reason for hiding this comment

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

I think it would be better to avoid saving $state in the SLO instance since it is only related to UI. Can we do that by sanitizing the data before passing it to the API? (more info)

Reading your comment again:

we don't store this normall anymore from UI.

Does it mean we have different types for the SLO instance that we save and API calls? I think my main question is how our public API will look like and if this type is used there.

Copy link
Member

Choose a reason for hiding this comment

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

Btw, if this issue was introduced in 8.16, then we don't need to backport it to 8.15:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't do that anymore, it's just for backward compatability.

@shahzad31 shahzad31 requested a review from kdelemme December 4, 2024 18:13
Copy link
Member

@maryam-saeidi maryam-saeidi left a comment

Choose a reason for hiding this comment

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

Can we add a test for this?

@shahzad31 shahzad31 merged commit 51e63ee into elastic:main Dec 5, 2024
13 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.15, 8.16, 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/12176632586

@shahzad31 shahzad31 deleted the add-state branch December 5, 2024 09:15
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 5, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!

(cherry picked from commit 51e63ee)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 5, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!

(cherry picked from commit 51e63ee)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 5, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!

(cherry picked from commit 51e63ee)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 5, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!

(cherry picked from commit 51e63ee)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.15
8.16
8.17
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 5, 2024
# Backport

This will backport the following commits from `main` to `8.15`:
- [[SLOs] Added $state into filters schema !!
(#202887)](#202887)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-05T09:12:48Z","message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema
!!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f","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-major"],"title":"[SLOs]
Added $state into filters schema
!!","number":202887,"url":"https://github.com/elastic/kibana/pull/202887","mergeCommit":{"message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema
!!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202887","number":202887,"mergeCommit":{"message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema !!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f"}}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
kibanamachine added a commit that referenced this pull request Dec 5, 2024
# Backport

This will backport the following commits from `main` to `8.17`:
- [[SLOs] Added $state into filters schema !!
(#202887)](#202887)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-05T09:12:48Z","message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema
!!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f","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-major"],"title":"[SLOs]
Added $state into filters schema
!!","number":202887,"url":"https://github.com/elastic/kibana/pull/202887","mergeCommit":{"message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema
!!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202887","number":202887,"mergeCommit":{"message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema !!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f"}}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
kibanamachine added a commit that referenced this pull request Dec 5, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[SLOs] Added $state into filters schema !!
(#202887)](#202887)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-05T09:12:48Z","message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema
!!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f","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-major"],"title":"[SLOs]
Added $state into filters schema
!!","number":202887,"url":"https://github.com/elastic/kibana/pull/202887","mergeCommit":{"message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema
!!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202887","number":202887,"mergeCommit":{"message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema !!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f"}}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
kibanamachine added a commit that referenced this pull request Dec 5, 2024
# Backport

This will backport the following commits from `main` to `8.16`:
- [[SLOs] Added $state into filters schema !!
(#202887)](#202887)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-05T09:12:48Z","message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema
!!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f","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-major"],"title":"[SLOs]
Added $state into filters schema
!!","number":202887,"url":"https://github.com/elastic/kibana/pull/202887","mergeCommit":{"message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema
!!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202887","number":202887,"mergeCommit":{"message":"[SLOs]
Added $state into filters schema !! (#202887)\n\n## Summary\r\n\r\nfixes
https://github.com/elastic/kibana/issues/202999\r\n\r\nAdded $state into
filters schema !!","sha":"51e63eeaccd9998d473ef62d11d530697a836e7f"}}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Dec 9, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Dec 9, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 9, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!
Samiul-TheSoccerFan pushed a commit to Samiul-TheSoccerFan/kibana that referenced this pull request Dec 10, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!
mykolaharmash pushed a commit to mykolaharmash/kibana that referenced this pull request Dec 11, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
## Summary

fixes elastic#202999

Added $state  into filters schema !!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development release_note:skip Skip the PR/issue when compiling release notes v8.15.6 v8.16.2 v8.17.0 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SLOs] _preview API is broken for UI based filters !!
5 participants