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

feat(slo): SLO grouping values selector #202364

Merged
merged 18 commits into from
Dec 5, 2024

Conversation

kdelemme
Copy link
Contributor

@kdelemme kdelemme commented Nov 29, 2024

Resolves #198009

🏇🏻 Summary

This PR introduces a way to switch from one instance to another for a given SLO. It replaces the current displaying of the grouping key/value in the SLO details page by a searchable combobox of all possible values for each grouping key.
The list is searchable and excludes the stale SLO instances.

Note

I refactored the existing internal API GET /slos/{id}/_instances which was not in use from within Kibana. The API is now accessible under GET /slos/{id}/_groupings

This new API takes two mandatory fields: groupingKey and instanceId. We use the provided instanceId to derive the term filters on the other groupingKeys (if more than one).

🧬 Testing

  1. Run data forge: node x-pack/scripts/data_forge.js --events-per-cycle 30 --lookback now-1d --dataset fake_stack --install-kibana-assets --kibana-url http://localhost:5601/kibana
  2. Create some SLOs without groups, with 1 group and 2+ groups.
  3. Assert the groupings are displayed correctly
  4. Assert groupings selector are searchable
  5. Assert selecting a new group value will change the URL and load the new instanceId data
slo_goruping_value_selection_final.mov

@kdelemme kdelemme added release_note:feature Makes this part of the condensed release notes v8.18.0 release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:obs-ux-management Observability Management User Experience Team and removed release_note:feature Makes this part of the condensed release notes labels Nov 29, 2024
@kdelemme kdelemme self-assigned this Nov 29, 2024
}

export function SLOGroupingValueSelector({ slo, groupingKey, value }: Props) {
const isAvailable = window.location.pathname.includes(SLOS_BASE_PATH);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

💬 This page can be displayed from the dashboard flyout, it's a little bit tricky to handle correctly so I've decided to disable the selection in this case.

sloId: string,
params: GetSLOInstancesParams
): Promise<GetSLOInstancesResponse> {
const { slo } = await this.definitionClient.execute(sloId, this.spaceId, params?.remoteName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fetch from the remote cluster if provided, otherwise it uses the repository

@kdelemme kdelemme marked this pull request as ready for review November 29, 2024 20:12
@kdelemme kdelemme requested a review from a team as a code owner November 29, 2024 20:12
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Nov 29, 2024
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

@kdelemme kdelemme force-pushed the slo/instance-selector branch from e6805a6 to 6d04e55 Compare December 2, 2024 16:00
@kdelemme kdelemme removed the request for review from maciejforcone December 2, 2024 18:21
@kdelemme kdelemme marked this pull request as draft December 3, 2024 13:44
@kdelemme kdelemme marked this pull request as ready for review December 3, 2024 16:49
throw new Error(`Something went wrong. Error: ${error}`);
}
},
enabled: Boolean(!!sloId && !!groupingKey && instanceId !== ALL_VALUE),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

💬 Technically the SLOGroupings component should never call this hook without these values, but in order to prevent other misusage, i'm adding this explicitly

Comment on lines +110 to +116
...groupingKeyValuePairs
.filter(([groupingKey]) => groupingKey !== params.groupingKey)
.map(([groupingKey, groupingValue]) => ({
term: {
[`slo.groupings.${groupingKey}`]: groupingValue,
},
})),
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 filter the summary documents to match the other grouping key/value pairs. So the values returned for the specified groupingKey form a valid instanceId when set together with the other pairs

Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

LGTM !!

@kdelemme kdelemme enabled auto-merge (squash) December 5, 2024 15:48
@kdelemme kdelemme disabled auto-merge December 5, 2024 16:52
@kdelemme kdelemme enabled auto-merge (squash) December 5, 2024 17:07
@elasticmachine
Copy link
Contributor

elasticmachine commented Dec 5, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
slo 856 862 +6

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/slo-schema 182 183 +1

Async chunks

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

id before after diff
observability 472.0KB 472.2KB +219.0B
slo 849.0KB 855.0KB +6.0KB
synthetics 1.1MB 1.1MB +220.0B
total +6.4KB
Unknown metric groups

API count

id before after diff
@kbn/slo-schema 182 183 +1

History

cc @kdelemme

@kdelemme kdelemme merged commit 7806861 into elastic:main Dec 5, 2024
9 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 5, 2024
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
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.x`:
- [feat(slo): SLO grouping values selector
(#202364)](#202364)

<!--- 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":"2024-12-05T18:51:03Z","message":"feat(slo):
SLO grouping values selector
(#202364)","sha":"7806861c5fc742cc09156655413d3d5c2ab87fda","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","ci:project-deploy-observability","Team:obs-ux-management","v8.18.0"],"title":"feat(slo):
SLO grouping values
selector","number":202364,"url":"https://github.com/elastic/kibana/pull/202364","mergeCommit":{"message":"feat(slo):
SLO grouping values selector
(#202364)","sha":"7806861c5fc742cc09156655413d3d5c2ab87fda"}},"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/202364","number":202364,"mergeCommit":{"message":"feat(slo):
SLO grouping values selector
(#202364)","sha":"7806861c5fc742cc09156655413d3d5c2ab87fda"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Kevin Delemme <[email protected]>
markov00 pushed a commit to markov00/kibana that referenced this pull request Dec 7, 2024
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Dec 9, 2024
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Dec 9, 2024
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 9, 2024
Samiul-TheSoccerFan pushed a commit to Samiul-TheSoccerFan/kibana that referenced this pull request Dec 10, 2024
mykolaharmash pushed a commit to mykolaharmash/kibana that referenced this pull request Dec 11, 2024
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SLO] Show instanceId selector in SLO details page
4 participants