[ML] Anomaly Detection: Use data view `esTypes` instead of custom field caps wrapper endpoint. #182588

walterra · 2024-05-03T15:12:44Z

Summary

The links menu for the anomalies table used a custom field caps wrapper API endpoint to identify the type of the categorization field. This PR changes this to use the data view API instead to use esTypes for the same check. This allows us to remove the custom field caps API endpoint completely.

Removes the route POST /internal/ml/indices/field_caps as it is no longer required by the ml plugin.

Checklist

Unit or functional tests were updated or added to match the most common scenarios
This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2024-05-03T16:44:23Z

Pinging @elastic/ml-ui (:ml)

peteharverson · 2024-05-07T15:40:04Z

x-pack/plugins/ml/public/application/components/anomalies_table/links_menu.tsx

+    // Uses the first matching field found in the list of indices in the datafeed_config.
+    let fieldType;
+
+    for (const index of datafeedIndices) {


This is a regression for the behavior when the datafeed uses a comma-separated style of index pattern. With this change, it no longer finds a mapping for a comma-separated pattern and displays this warning:

For this to work, you'll need to see if there is a data view which maps to a datafeed config such as

"indices": [ "gallery-2023-01", "gallery-2023-02", "gallery-2023-03" ],

We have a function that might be useful here.
getDataViewIdFromDatafeed

James, thanks for the hint about getDataViewIdFromDataFeed. I ended up just copying over the approach instead of reusing the function because that function returns just the id and not the full data view. This allowed me to fix the issue Pete found in 1adabee.

peteharverson · 2024-05-07T15:43:54Z

x-pack/plugins/ml/public/application/components/anomalies_table/links_menu.tsx

+        (dv) => dv.getIndexPattern() === index
+      );
+
+      if (!dataView) {


Testing this, I discovered that we no longer gracefully handle (on main) the case where there is no longer a matching data view for the indices used in the datafeed. We now just silently fail, where the menu item used to be disabled as implemented in #125090.

With ad hoc data views, we could actually enhance the behavior where a data view doesn't exist by enabling the link and let Discover create a temporary data view:

I took an attempt at fixing this. The problem is that the way we link to Discover is quite different for the View in Discover and the View examples link. For the first one the temporary data view works because we don't open the link in a new window. To align this would require quite a refactor of the whole links_menu.tsx file. I tried to keep changes to a minimum and tried to restore the previous behavior until we find the time to align: For now View in Discover will work with temporary data views whereas the View examples link will be disabled with a tooltip explaining that the data view doesn't exist. Updated in 0db6610.

…:walterra/kibana into ml-links-menu-remove-field-caps-endpoint

kibana-ci · 2024-05-08T13:51:06Z

💚 Build Succeeded

Buildkite Build
Commit: 7423c65

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	2015	2014	-1

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.1MB	4.1MB	+163.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`ml`	77.2KB	77.0KB	-198.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`ml`	547	548	+1

Total ESLint disabled count

id	before	after	diff
`ml`	550	551	+1

History

💔 Build #208651 failed 0db6610
💛 Build #208587 was flaky 20e196e
💚 Build #208009 succeeded eae4b71
💚 Build #207858 succeeded 78d407e

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

cc @walterra

peteharverson

Tested latest changes and LGTM

jgowdyelastic

LGTM

…83110) ## Summary Fixes #182837. Fixes #182514. The number of fields returned by the field caps API is different across ES versions in forward compatibility tests. In ES 8.15.0, the `_ignored_source` field was added (elastic/elasticsearch#107567). This fixes the API integration test for field caps to assert the correct number of fields across versions. Note that in Kibana `8.15` we refactored away from using fields caps directly in this way and removed the corresponding API endpoint and tests (#182588), that's why there's this dedicated `7.17` PR to just fix the assertions on the existing test. To test this locally, the following commands for the functional tests server and runner can be used to run the tests in different forward compatibility scenarios: ``` # 7.17 tests server node scripts/functional_tests_server.js --config x-pack/test/api_integration/config.ts # 7.17 tests runner node scripts/functional_test_runner --config x-pack/test/api_integration/config.ts # 8.14 tests server ES_SNAPSHOT_MANIFEST="https://storage.googleapis.com/kibana-ci-es-snapshots-daily/8.14.0/manifest-latest-verified.json" node scripts/functional_tests_server.js # 8.14 tests runner node scripts/functional_test_runner --config x-pack/test/api_integration/config.ts --es-version=8.14.0-SNAPSHOT # 8.15 tests server ES_SNAPSHOT_MANIFEST="https://storage.googleapis.com/kibana-ci-es-snapshots-daily/8.15.0/manifest-latest-verified.json" node scripts/functional_tests_server.js # 8.15 tests runner node scripts/functional_test_runner --config x-pack/test/api_integration/config.ts --es-version=8.15.0-SNAPSHOT ``` Note in `7.17` the API integration tests are not split up yet into several configs so the commands above will run ALL Kibaan API integration tests. The command to run the tests server for a specific ES version is also shared in the buildkite reports, for example: https://buildkite.com/elastic/kibana-7-dot-17-es-8-dot-15-forward-compatibility/builds/20#annotation-es-snapshot-manifest The versions the compatibility tests will currently run against can be found here: https://github.com/elastic/kibana/blob/main/versions.json ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

…astic#183110) ## Summary Fixes elastic#182837. Fixes elastic#182514. The number of fields returned by the field caps API is different across ES versions in forward compatibility tests. In ES 8.15.0, the `_ignored_source` field was added (elastic/elasticsearch#107567). This fixes the API integration test for field caps to assert the correct number of fields across versions. Note that in Kibana `8.15` we refactored away from using fields caps directly in this way and removed the corresponding API endpoint and tests (elastic#182588), that's why there's this dedicated `7.17` PR to just fix the assertions on the existing test. To test this locally, the following commands for the functional tests server and runner can be used to run the tests in different forward compatibility scenarios: ``` # 7.17 tests server node scripts/functional_tests_server.js --config x-pack/test/api_integration/config.ts # 7.17 tests runner node scripts/functional_test_runner --config x-pack/test/api_integration/config.ts # 8.14 tests server ES_SNAPSHOT_MANIFEST="https://storage.googleapis.com/kibana-ci-es-snapshots-daily/8.14.0/manifest-latest-verified.json" node scripts/functional_tests_server.js # 8.14 tests runner node scripts/functional_test_runner --config x-pack/test/api_integration/config.ts --es-version=8.14.0-SNAPSHOT # 8.15 tests server ES_SNAPSHOT_MANIFEST="https://storage.googleapis.com/kibana-ci-es-snapshots-daily/8.15.0/manifest-latest-verified.json" node scripts/functional_tests_server.js # 8.15 tests runner node scripts/functional_test_runner --config x-pack/test/api_integration/config.ts --es-version=8.15.0-SNAPSHOT ``` Note in `7.17` the API integration tests are not split up yet into several configs so the commands above will run ALL Kibaan API integration tests. The command to run the tests server for a specific ES version is also shared in the buildkite reports, for example: https://buildkite.com/elastic/kibana-7-dot-17-es-8-dot-15-forward-compatibility/builds/20#annotation-es-snapshot-manifest The versions the compatibility tests will currently run against can be found here: https://github.com/elastic/kibana/blob/main/versions.json ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

remove field caps endpoint. use data view estypes instead.

78d407e

walterra self-assigned this May 3, 2024

walterra added :ml Feature:Anomaly Detection ML anomaly detection release_note:skip Skip the PR/issue when compiling release notes v8.15.0 labels May 3, 2024

walterra marked this pull request as ready for review May 3, 2024 16:44

walterra requested a review from a team as a code owner May 3, 2024 16:44

walterra requested review from peteharverson and jgowdyelastic May 3, 2024 16:44

Merge branch 'main' into ml-links-menu-remove-field-caps-endpoint

eae4b71

peteharverson reviewed May 7, 2024

View reviewed changes

walterra added 6 commits May 8, 2024 10:03

Merge branch 'main' into ml-links-menu-remove-field-caps-endpoint

8329140

fix getting data view

1adabee

Merge branch 'ml-links-menu-remove-field-caps-endpoint' of github.com…

20e196e

…:walterra/kibana into ml-links-menu-remove-field-caps-endpoint

check data view without creating a temporary one

d828834

refactor data view handling

0db6610

fix i18n

7423c65

peteharverson approved these changes May 9, 2024

View reviewed changes

jgowdyelastic approved these changes May 9, 2024

View reviewed changes

walterra merged commit 71e7f5a into elastic:main May 10, 2024
17 checks passed

kibanamachine added the backport:skip This commit does not require backporting label May 10, 2024

walterra deleted the ml-links-menu-remove-field-caps-endpoint branch May 10, 2024 05:40

walterra mentioned this pull request May 10, 2024

[ML] Anomaly Detection: Fix API integration tests for field caps. #183110

Merged

2 tasks

walterra mentioned this pull request Jul 15, 2024

[ML] Anomaly Detection: Fix API integration tests for field caps. #188309

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Anomaly Detection: Use data view `esTypes` instead of custom field caps wrapper endpoint. #182588

[ML] Anomaly Detection: Use data view `esTypes` instead of custom field caps wrapper endpoint. #182588

walterra commented May 3, 2024 •

edited by peteharverson

Loading

elasticmachine commented May 3, 2024

peteharverson May 7, 2024

jgowdyelastic May 7, 2024

walterra May 8, 2024 •

edited

Loading

peteharverson May 7, 2024

walterra May 8, 2024

kibana-ci commented May 8, 2024

ESLint disabled line counts

Total ESLint disabled count

peteharverson left a comment

jgowdyelastic left a comment

[ML] Anomaly Detection: Use data view esTypes instead of custom field caps wrapper endpoint. #182588

[ML] Anomaly Detection: Use data view esTypes instead of custom field caps wrapper endpoint. #182588

Conversation

walterra commented May 3, 2024 • edited by peteharverson Loading

Summary

Checklist

elasticmachine commented May 3, 2024

peteharverson May 7, 2024

Choose a reason for hiding this comment

jgowdyelastic May 7, 2024

Choose a reason for hiding this comment

walterra May 8, 2024 • edited Loading

Choose a reason for hiding this comment

peteharverson May 7, 2024

Choose a reason for hiding this comment

walterra May 8, 2024

Choose a reason for hiding this comment

kibana-ci commented May 8, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

peteharverson left a comment

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

[ML] Anomaly Detection: Use data view `esTypes` instead of custom field caps wrapper endpoint. #182588

[ML] Anomaly Detection: Use data view `esTypes` instead of custom field caps wrapper endpoint. #182588

walterra commented May 3, 2024 •

edited by peteharverson

Loading

walterra May 8, 2024 •

edited

Loading