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

[ML] Anomaly Explorer: Hide top influencers panel for jobs without influencers #192987

Merged
merged 8 commits into from
Sep 20, 2024

Conversation

rbrtj
Copy link
Contributor

@rbrtj rbrtj commented Sep 16, 2024

Summary

Fix for #192679
Hiding the top influencers panel when there are no influencers for the selected job.
Added a functional test to ensure the panel is hidden.
Expanded a few types to improve type safety.

@rbrtj
Copy link
Contributor Author

rbrtj commented Sep 16, 2024

/ci

@rbrtj rbrtj self-assigned this Sep 16, 2024
@rbrtj rbrtj added bug Fixes for quality problems that affect the customer experience release_note:fix :ml Team:ML Team label for ML (also use :ml) v8.16.0 labels Sep 16, 2024
@rbrtj rbrtj marked this pull request as ready for review September 16, 2024 12:53
@rbrtj rbrtj requested a review from a team as a code owner September 16, 2024 12:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@rbrtj rbrtj marked this pull request as draft September 16, 2024 15:20

// Creates index pattern in the format expected by the kuery bar/kuery autocomplete provider
// Field objects required fields: name, type, aggregatable, searchable
export function getIndexPattern(influencers: string[]) {
export function getIndexPattern(influencers: ExplorerJob[]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was typed incorrectly, the arg passed here was always of type 'ExplorerJob[]' instead of just 'string[]'.
The IndexPattern in ExplorerState type expects a name as a string, but after mapping, it was always an object of type ExplorerJob. I didn't notice any regressions after this change, but I would appreciate testing to ensure everything works correctly.

@rbrtj
Copy link
Contributor Author

rbrtj commented Sep 16, 2024

/ci

@peteharverson peteharverson added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:fix labels Sep 16, 2024
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and LGTM. Great to have the new functional test for this use case!

@rbrtj rbrtj marked this pull request as ready for review September 17, 2024 10:55
Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Code LGTM, just added a comment about a logging test in the functional tests.

await ml.anomaliesTable.assertAnomalyActionDiscoverButtonExists(0);
await ml.anomaliesTable.ensureAnomalyActionDiscoverButtonClicked(0);
});
await ml.testExecution.logTestStep('displays the influencers list');
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Should this say does not display the influencers list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!
updated

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #53 / console app console onboarding tour allows re-running the tour

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
ml 4.6MB 4.6MB +719.0B

History

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

cc @rbrtj

@rbrtj rbrtj added the backport:skip This commit does not require backporting label Sep 20, 2024
@rbrtj rbrtj merged commit 23b2595 into elastic:main Sep 20, 2024
21 checks passed
@delanni
Copy link
Contributor

delanni commented Sep 20, 2024

@rbrtj - the PR has the v.8.16 label, yet it wasn't backported to 8.16 (8.x) - if this PR is not meant for 9.0 only, please remove the backport:skip label, and use backport:prev-minor or backport:version

@rbrtj rbrtj added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Sep 20, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 20, 2024
…fluencers (elastic#192987)

# Summary

Fix for [elastic#192679](elastic#192679)
Hiding the top influencers panel when there are no influencers for the
selected job.
Added a functional test to ensure the panel is hidden.
Expanded a few types to improve type safety.

(cherry picked from commit 23b2595)
@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 Sep 23, 2024
…out influencers (#192987) (#193568)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ML] Anomaly Explorer: Hide top influencers panel for jobs without
influencers (#192987)](#192987)

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

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

<!--BACKPORT [{"author":{"name":"Robert
Jaszczurek","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-20T12:06:29Z","message":"[ML]
Anomaly Explorer: Hide top influencers panel for jobs without
influencers (#192987)\n\n# Summary\r\n\r\nFix for
[#192679](https://github.com/elastic/kibana/issues/192679)\r\nHiding the
top influencers panel when there are no influencers for the\r\nselected
job.\r\nAdded a functional test to ensure the panel is
hidden.\r\nExpanded a few types to improve type
safety.","sha":"23b2595be39401214a1ef9e39b684f917020b9ad","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug",":ml","release_note:skip","v9.0.0","Team:ML","backport:prev-minor","v8.16.0"],"title":"[ML]
Anomaly Explorer: Hide top influencers panel for jobs without
influencers","number":192987,"url":"https://github.com/elastic/kibana/pull/192987","mergeCommit":{"message":"[ML]
Anomaly Explorer: Hide top influencers panel for jobs without
influencers (#192987)\n\n# Summary\r\n\r\nFix for
[#192679](https://github.com/elastic/kibana/issues/192679)\r\nHiding the
top influencers panel when there are no influencers for the\r\nselected
job.\r\nAdded a functional test to ensure the panel is
hidden.\r\nExpanded a few types to improve type
safety.","sha":"23b2595be39401214a1ef9e39b684f917020b9ad"}},"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/192987","number":192987,"mergeCommit":{"message":"[ML]
Anomaly Explorer: Hide top influencers panel for jobs without
influencers (#192987)\n\n# Summary\r\n\r\nFix for
[#192679](https://github.com/elastic/kibana/issues/192679)\r\nHiding the
top influencers panel when there are no influencers for the\r\nselected
job.\r\nAdded a functional test to ensure the panel is
hidden.\r\nExpanded a few types to improve type
safety.","sha":"23b2595be39401214a1ef9e39b684f917020b9ad"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Robert Jaszczurek <[email protected]>
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) bug Fixes for quality problems that affect the customer experience :ml release_note:skip Skip the PR/issue when compiling release notes Team:ML Team label for ML (also use :ml) v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants