[ML] Add search time runtime support for index based Data Visualizer by qn895 · Pull Request #95252 · elastic/kibana

qn895 · 2021-03-23T22:47:53Z

Summary

Part of #77462. This PR:

Adds search time runtime fields support index-based Data Visualizer

Handles saved search or search queries with runtime fields
Adds capability for the histogram preview to show runtime fields within Transform/Data Frame Analytics.

Before

After

Adds better error message when a query fails (e.g. due to bad scripts or runtime field definitions)

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

elasticmachine · 2021-03-23T22:47:55Z

Pinging @elastic/ml-ui (:ml)

peteharverson

Tested and LGTM.

The only issue I found was when using a saved search that uses a runtime field defined in the index pattern, where the 'Use full data' button gives an error, but that should be fixed by #94760. Can you please double check that after rebasing.

jgowdyelastic · 2021-03-24T10:39:23Z

x-pack/plugins/ml/server/models/data_visualizer/data_visualizer.ts

          cardinality: { field },
        };
-        runtimeMappings.runtime_mappings = datafeedConfig.runtime_mappings;
+        runtimeMappings.runtime_mappings = {


In my recent PR #94760 i've removed this else if altogether and added the population of the runtime_mappings to the else below.

but looking at this loop again, i think the change you've made on line 629 is correct. We should always add runtime_mappings outside of this loop, if they have been supplied.

In short, this if else isn't needed as we've populated them on line 629.

I moved this to outside of the loop here 1e708e1 (#95252)

jgowdyelastic · 2021-03-24T10:40:03Z

x-pack/plugins/ml/server/models/data_visualizer/data_visualizer.ts

    // filter aggregation with exists query.
    const aggs: Aggs = datafeedAggregations !== undefined ? { ...datafeedAggregations } : {};
-    const runtimeMappings: { runtime_mappings?: RuntimeMappings } = {};
+    const runtimeMappings: { runtime_mappings?: RuntimeMappings } = runtimeFields


it would be good to use a isPopulatedObject check here too.

Updated here 1e708e1 (#95252)

qn895 · 2021-03-24T15:50:21Z

Tested and LGTM.

The only issue I found was when using a saved search that uses a runtime field defined in the index pattern, where the 'Use full data' button gives an error, but that should be fixed by #94760. Can you please double check that after rebasing.

Thanks for testing! Looks like the issue is fixed after #94760:

jgowdyelastic · 2021-03-24T15:51:39Z

x-pack/plugins/ml/server/models/data_visualizer/data_visualizer.ts

    latestMs?: number,
-    datafeedConfig?: Datafeed
+    datafeedConfig?: Datafeed,
+    runtimeFields?: RuntimeMappings


nit, should be called runtimeMappings for consistency

Updated here a6ae3ce (#95252)

x-pack/plugins/ml/server/models/data_visualizer/data_visualizer.ts

x-pack/plugins/ml/server/routes/schemas/data_visualizer_schema.ts

peteharverson · 2021-03-25T17:30:04Z

I am still seeing an error when selecting a saved search which uses a runtime field from the index pattern, and then hitting the 'Use full data' button. I am also seeing it in the job wizards, even after #94760. The time_field_range endpoint is returning

{"success":true,"start":{"epoch":null},"end":{"epoch":null}}

I have added it as an additional item to #77462 - we can address it separately to this PR.

…support

qn895 · 2021-03-25T20:01:19Z

I am still seeing an error when selecting a saved search which uses a runtime field from the index pattern, and then hitting the 'Use full data' button. I am also seeing it in the job wizards, even after #94760.

I think this should be fixed now with the latest changes. Let me know if still have the issue. Since it's also happening with the job wizards, I will add a fix for that in a separate follow up PR.

darnautov · 2021-03-26T09:11:22Z

x-pack/plugins/ml/common/util/runtime_field_utils.ts

+} from '../../../../../src/plugins/data/common/index_patterns';
+import type { RuntimeField, RuntimeMappings } from '../types/fields';
+
+export function isRuntimeField(arg: unknown): arg is RuntimeField {


Looking into this typeguard implementation I think it's worth writing some unit tests

This is actually the same implementation in Transform. Considering we have a few other PRs to switch to estypes, I'll leave this here for now so we can consolidate later.

peteharverson

Tested latest changes and Use Full Data button is working in the Data Visualizer for me when using a saved search with a runtime field.

…d transform" This reverts commit ce813f0

…support

jgowdyelastic

LGTM

kibanamachine · 2021-03-29T16:38:41Z

💚 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
`ml`	6.1MB	6.1MB	+695.0B

History

💚 Build #115603 succeeded ce813f0
💚 Build #115336 succeeded b68e736
💚 Build #114944 succeeded a6ae3ce
💚 Build #114762 succeeded 0f5046f

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

cc @qn895

…lastic#95252) * [ML] Add runtime support from index pattern for data viz * [ML] move runtime mappings outside of aggregatableFields loop * [ML] Change arg name to runtimeMappings * [ML] Fix dv full time range broken * [ML] Fix dv broken with time range * [ML] Add better error handling/transparency * [ML] Update to using estypes.RuntimeField * [ML] Update to use some shared common functions between ml and transform * Revert "[ML] Update to use some shared common functions between ml and transform" This reverts commit ce813f0 * [ML] Disable context menu if no charts

kibanamachine · 2021-03-29T17:13:11Z

💚 Backport successful

✅ 7.x / #95684

This backport PR will be merged automatically after passing CI.

…95252) (#95684) * [ML] Add runtime support from index pattern for data viz * [ML] move runtime mappings outside of aggregatableFields loop * [ML] Change arg name to runtimeMappings * [ML] Fix dv full time range broken * [ML] Fix dv broken with time range * [ML] Add better error handling/transparency * [ML] Update to using estypes.RuntimeField * [ML] Update to use some shared common functions between ml and transform * Revert "[ML] Update to use some shared common functions between ml and transform" This reverts commit ce813f0 * [ML] Disable context menu if no charts Co-authored-by: Quynh Nguyen <43350163+qn895@users.noreply.github.com>

[ML] Add runtime support from index pattern for data viz

0f5046f

qn895 added release_note:enhancement :ml v8.0.0 Feature:File and Index Data Viz ML file and index data visualizer v7.13.0 labels Mar 23, 2021

qn895 requested review from darnautov, jgowdyelastic and peteharverson March 23, 2021 22:47

qn895 self-assigned this Mar 23, 2021

qn895 requested a review from a team as a code owner March 23, 2021 22:47

qn895 mentioned this pull request Mar 23, 2021

[ML] Support runtime fields #77462

Closed

12 tasks

peteharverson approved these changes Mar 24, 2021

View reviewed changes

jgowdyelastic reviewed Mar 24, 2021

View reviewed changes

qn895 added 2 commits March 24, 2021 09:01

Merge upstream/master into branch

c542ca5

[ML] move runtime mappings outside of aggregatableFields loop

1e708e1

jgowdyelastic reviewed Mar 24, 2021

View reviewed changes

[ML] Change arg name to runtimeMappings

a6ae3ce

darnautov reviewed Mar 25, 2021

View reviewed changes

qn895 added 4 commits March 25, 2021 13:23

Merge remote-tracking branch 'upstream/master' into data-viz-runtime-…

705d84c

…support

[ML] Fix dv full time range broken

861d28f

[ML] Fix dv broken with time range

97c5890

[ML] Add better error handling/transparency

b68e736

darnautov reviewed Mar 26, 2021

View reviewed changes

peteharverson approved these changes Mar 26, 2021

View reviewed changes

qn895 added 5 commits March 26, 2021 13:54

[ML] Update to using estypes.RuntimeField

54b62f8

[ML] Update to use some shared common functions between ml and transform

ce813f0

Revert "[ML] Update to use some shared common functions between ml an…

db4123e

…d transform" This reverts commit ce813f0

[ML] Disable context menu if no charts

eaa2445

Merge remote-tracking branch 'upstream/master' into data-viz-runtime-…

3e4a475

…support

qn895 requested review from darnautov and jgowdyelastic March 29, 2021 14:56

jgowdyelastic approved these changes Mar 29, 2021

View reviewed changes

qn895 added the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 29, 2021

qn895 merged commit 587f83a into elastic:master Mar 29, 2021

qn895 deleted the data-viz-runtime-support branch March 29, 2021 17:10

kibanamachine mentioned this pull request Mar 29, 2021

[7.x] [ML] Add search time runtime support for index based Data Visualizer (#95252) #95684

Merged

lcawl added release_note:feature Makes this part of the condensed release notes and removed release_note:enhancement labels May 18, 2021

Conversation

qn895 commented Mar 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

elasticmachine commented Mar 23, 2021

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic Mar 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qn895 Mar 24, 2021

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic Mar 24, 2021

Choose a reason for hiding this comment

Uh oh!

qn895 Mar 24, 2021

Choose a reason for hiding this comment

Uh oh!

qn895 commented Mar 24, 2021

Uh oh!

jgowdyelastic Mar 24, 2021

Choose a reason for hiding this comment

Uh oh!

qn895 Mar 24, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

peteharverson commented Mar 25, 2021

Uh oh!

qn895 commented Mar 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darnautov Mar 26, 2021

Choose a reason for hiding this comment

Uh oh!

qn895 Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 29, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

Uh oh!

kibanamachine commented Mar 29, 2021

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

qn895 commented Mar 23, 2021 •

edited

Loading

jgowdyelastic Mar 24, 2021 •

edited

Loading

qn895 commented Mar 25, 2021 •

edited

Loading