[8.6][ML Inference] Add ML inference failure handler #142488

demjened · 2022-10-03T15:47:12Z

Summary

Parent issue: https://github.com/elastic/enterprise-search-team/issues/2680

This PR adds a default failure handler to Machine Learning Inference pipelines, specifically the inference processor. As a result, any errors occurring during ML inference are caught and appended to the _ingest.inference_errors array as a rich object. This will help surface issues to the UI.

Sample:

"_ingest": {
    "inference_errors": [
        {
            "pipeline": "ml-inference-my-ner-inference-pipeline",
            "message": "Processor 'inference' in pipeline 'ml-inference-my-ner-inference-pipeline' failed with message 'Trained model [philschmid__distilroberta-base-ner-conll2003] is not deployed.'",
            "timestamp": "2022-10-03T14:58:13.757978680Z"
        },
        {
            "pipeline": "ml-inference-my-text-embedding-pipeline",
            "message": "Processor 'inference' in pipeline 'ml-inference-my-text-embedding-pipeline' failed with message 'Input field [text_field] does not exist in the source document'",
            "timestamp": "2022-10-03T14:58:13.757978680Z"
        }
    ],
    (...)
}

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

…-fix'

…jened/kibana into create-ml-inference-pipeline

…-fix'

…-ref HEAD~1..HEAD --fix'

…-fix'

…-ref HEAD~1..HEAD --fix'

…-fix'

…-ref HEAD~1..HEAD --fix'

…-fix'

…-ref HEAD~1..HEAD --fix'

demjened · 2022-10-03T15:58:19Z

x-pack/plugins/enterprise_search/server/lib/pipelines/create_pipeline_definitions.test.ts

+    expect(actualResult).toEqual(
+      expect.objectContaining({
+        processors: expect.arrayContaining([
+          {
+            inference: expect.objectContaining({
+              field_map: {
+                [sourceField]: modelInputField,
+              },
+            }),
+          },
+        ]),
+      })
+    );


I think it's unnecessary code duplication that we check the entire result object here - this is already covered in the "should return the pipeline body" test case. I modified the test to only look for the field that is different under the actual conditions.

Nice. Good to know about these expcet.*Containing functions 👍

While it is quite easy to do unwanted side effects, we usually do a full object checks especially for the Redux/Kea states to make sure we only change the part where we want to change. I am curious if that would be useful here too.

Hm, good point Efe. I'll update the code to work with full objects and assign() the modified part that the test is focusing on.

You can use destructuring as well.

i.e.

{ ...EXPECTED_DEFAULTS, only: { the_changed: "parts" } }

I ended up using lodash merge() since destructuring didn't work for changing a single array element's specific property while leaving everything else untouched. (Or maybe I just don't know how to do it 🙂)

demjened · 2022-10-03T16:28:49Z

x-pack/plugins/enterprise_search/common/ml_inference_pipeline/index.ts

+                value: [
+                  {
+                    pipeline: pipelineName,
+                    message: `Processor 'inference' in pipeline '${pipelineName}' failed with message '{{ _ingest.on_failure_message }}'`,


I named this property message instead of error to make it semantically more extensible (e.g. for warnings).

brianmcgue · 2022-10-03T18:24:47Z

x-pack/plugins/enterprise_search/server/lib/pipelines/create_pipeline_definitions.test.ts

+    expect(actualResult).toEqual(
+      expect.objectContaining({
+        processors: expect.arrayContaining([
+          {
+            inference: expect.objectContaining({
+              field_map: {
+                [sourceField]: modelInputField,
+              },
+            }),
+          },
+        ]),
+      })
+    );


Nice. Good to know about these expcet.*Containing functions 👍

…-ref HEAD~1..HEAD --fix'

kibana-ci · 2022-10-04T15:39:50Z

💚 Build Succeeded

Buildkite Build
Commit: 2eefd89

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
`enterpriseSearch`	1.7MB	1.7MB	+230.0B

History

💚 Build #77602 succeeded 6d5a0d9

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

* Add ML Inference failure handler

demjened and others added 30 commits September 13, 2022 13:47

Add ML inference PL creation flow

c9a0545

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

e293860

…-fix'

Add exists check, clean up code a bit

8fb6d63

Merge branch 'create-ml-inference-pipeline' of https://github.com/dem…

05f5eb2

…jened/kibana into create-ml-inference-pipeline

Fix dest name

d69478b

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

2443d8c

…-fix'

Separate concerns

49aebdb

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

aee672f

…-ref HEAD~1..HEAD --fix'

Remove i18n due to linter error, fix src field ref

d838052

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

f4d53ba

…-ref HEAD~1..HEAD --fix'

Add/update unit tests

ef14bad

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

5d3b37d

…-ref HEAD~1..HEAD --fix'

Refactor error handling

8613f5b

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

b972688

…-fix'

Add sub-pipeline to parent ML PL

92b4ec5

Add unit tests and docs

71bf27b

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

7c74509

…-ref HEAD~1..HEAD --fix'

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

0ecad7e

…-fix'

Merge branch 'main' into create-ml-inference-pipeline

ce8f0a7

Refactor error handling

1c15559

Wrap logic into higher level function

bf29f2a

Add route test

aaae199

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

9d2f232

…-ref HEAD~1..HEAD --fix'

Merge branch 'main' into create-ml-inference-pipeline

99a7027

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

63c52cb

…-fix'

Merge branch 'create-ml-inference-pipeline'

85a5020

Merge branch 'main' of https://github.com/elastic/kibana

1f70e59

Merge branch 'main' of https://github.com/elastic/kibana

58f6524

Merge branch 'main' of https://github.com/elastic/kibana

27028f8

Merge branch 'main' of https://github.com/elastic/kibana

8acd6a3

demjened and others added 4 commits September 26, 2022 13:42

Merge branch 'main' of https://github.com/elastic/kibana

2ab2dd1

Merge branch 'main' of https://github.com/elastic/kibana

397851e

Add ML Inference failure handler

61efa06

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

6d5a0d9

…-ref HEAD~1..HEAD --fix'

demjened commented Oct 3, 2022

View reviewed changes

demjened changed the title ~~Demjened/add ml inference failure handler~~ [8.6][ML Inference] Add ML inference failure handler Oct 3, 2022

demjened added Team:EnterpriseSearch v8.6.0 labels Oct 3, 2022

demjened requested review from brianmcgue and markjhoy October 3, 2022 16:20

demjened commented Oct 3, 2022

View reviewed changes

demjened requested a review from tsclausing October 3, 2022 17:53

demjened marked this pull request as ready for review October 3, 2022 17:53

demjened requested a review from a team October 3, 2022 17:53

brianmcgue added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Oct 3, 2022

brianmcgue approved these changes Oct 3, 2022

View reviewed changes

Restore full object body in unit test

4e7f645

demjened force-pushed the demjened/add-ml-inference-failure-handler branch from 17f3132 to 4e7f645 Compare October 4, 2022 14:29

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

2eefd89

…-ref HEAD~1..HEAD --fix'

efegurkan approved these changes Oct 4, 2022

View reviewed changes

demjened merged commit 524363d into elastic:main Oct 4, 2022

demjened deleted the demjened/add-ml-inference-failure-handler branch October 4, 2022 15:48

WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022

[8.6][ML Inference] Add ML inference failure handler (elastic#142488)

dad660f

* Add ML Inference failure handler

WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022

[8.6][ML Inference] Add ML inference failure handler (elastic#142488)

a4c3115

* Add ML Inference failure handler

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[8.6][ML Inference] Add ML inference failure handler #142488

[8.6][ML Inference] Add ML inference failure handler #142488

demjened commented Oct 3, 2022 •

edited

Loading

demjened Oct 3, 2022 •

edited

Loading

brianmcgue Oct 3, 2022

efegurkan Oct 4, 2022

demjened Oct 4, 2022

efegurkan Oct 4, 2022

demjened Oct 4, 2022

demjened Oct 3, 2022

brianmcgue Oct 3, 2022

kibana-ci commented Oct 4, 2022

[8.6][ML Inference] Add ML inference failure handler #142488

[8.6][ML Inference] Add ML inference failure handler #142488

Conversation

demjened commented Oct 3, 2022 • edited Loading

Summary

Checklist

demjened Oct 3, 2022 • edited Loading

Choose a reason for hiding this comment

brianmcgue Oct 3, 2022

Choose a reason for hiding this comment

efegurkan Oct 4, 2022

Choose a reason for hiding this comment

demjened Oct 4, 2022

Choose a reason for hiding this comment

efegurkan Oct 4, 2022

Choose a reason for hiding this comment

demjened Oct 4, 2022

Choose a reason for hiding this comment

demjened Oct 3, 2022

Choose a reason for hiding this comment

brianmcgue Oct 3, 2022

Choose a reason for hiding this comment

kibana-ci commented Oct 4, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

History

demjened commented Oct 3, 2022 •

edited

Loading

demjened Oct 3, 2022 •

edited

Loading