[ML] Data Frame Analytics: support custom urls in jobs #154287

alvarezmelissa87 · 2023-04-03T20:17:15Z

Summary

Related meta issue: #150375

This PR:

adds the ability to create custom urls for data frame analytics job - accessible in the 'Edit' flyout in the jobs list
adds access to the created custom urls in the data frame analytics results table (in the 'Actions' column)
moves the custom url component to a shared directory and is used in both anomaly detection and data frame analytics

customUrls.mp4

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

elasticmachine · 2023-04-03T20:17:20Z

Pinging @elastic/ml-ui (:ml)

x-pack/plugins/ml/common/types/data_frame_analytics.ts

x-pack/plugins/ml/public/application/components/custom_urls/custom_url_editor/utils.ts

...ics/pages/analytics_exploration/components/expandable_section/expandable_section_results.tsx

x-pack/plugins/ml/public/application/util/custom_url_utils.ts

darnautov · 2023-04-04T15:35:30Z

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

@@ -10,6 +10,7 @@ import { runtimeMappingsSchema } from './runtime_mappings_schema';

 export const dataAnalyticsJobConfigSchema = schema.object({
  description: schema.maybe(schema.string()),
+  _meta: schema.maybe(schema.any()),


Suggested change

_meta: schema.maybe(schema.any()),

_meta: schema.maybe(schema.object()),

This one I left as any because schema.object requires the properties to be defined and _meta can take whatever properties we want to add. So this could be problematic as it's volatile and for _meta properties that get added automatically on job creation, etc.

Since this is passed on to the corresponding ES API endpoint, does this one have any documentation about the expected structure of the _meta field we could align with?

So in the [PR](https://github.com/elastic/elasticsearch/pull/94529) adding it in it's described as
The _meta field stores an arbitrary key-value map. Keys are strings. Values are arbitrary objects (possibly also maps).

I don't believe schema let's you specify a key type without knowing what the key is. Happy to add it in if I'm wrong about that.

The TS spec now defines it as an optional record like _meta: undefined | Record<string, any>; (in other places _meta: undefined | Record<string, unknown>;) but this one just as an optional any, maybe it can be updated to also be like the Record?

@walterra - yep, did some digging and there is an option with an 'unknown' - added in in 7ee455c

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

alvarezmelissa87 · 2023-04-05T19:25:00Z

This has been updated and is ready for another look when you get a chance 🙏 cc @peteharverson, @darnautov, @walterra

x-pack/plugins/ml/public/application/components/custom_urls/custom_urls.tsx

alvarezmelissa87 · 2023-04-06T16:36:06Z

@elasticmachine merge upstream

x-pack/plugins/ml/public/application/components/custom_urls/custom_urls.tsx

…ckup

walterra · 2023-04-05T12:21:22Z

x-pack/plugins/ml/public/application/components/custom_urls/custom_url_editor/editor.tsx

-                onChange={onQueryEntitiesChange}
-                isClearable={true}
-                data-test-subj="mlJobCustomUrlQueryEntitiesInput"
+        entityOptions.length > 0 ? (


Is there a need to switch to this ternary version? Should be fine to keep the && variant.

Just based on something @darnautov brought up before about unexpected things happening because of the nature of && and I thought we all agreed it was better to explicitly return null. Not married to either - happy to stick with whatever we decide is best practice. Just want to be consistent.

Removed in 3d93dc5

walterra · 2023-04-11T06:27:53Z

x-pack/plugins/ml/common/types/anomaly_detection_jobs/job.ts

+
+export function isAnomalyDetectionJob(arg: unknown): arg is Job {
+  return (
+    isPopulatedObject(arg) && typeof arg.job_id === 'string' && arg.analysis_config !== undefined


Nit: You could move part of the check to isPopulatedObject here like this:

isPopulatedObject(arg, ['job_id', 'analysis_config']) && typeof arg.job_id === 'string'

Updated in 7ee455c

walterra · 2023-04-11T06:28:57Z

x-pack/plugins/ml/common/types/custom_urls.ts

+
+export function isKibanaUrlConfigWithTimeRange(arg: unknown): arg is KibanaUrlConfigWithTimeRange {
+  return (
+    isPopulatedObject(arg) &&


Nit: You could move part of the check to isPopulatedObject here like this:

isPopulatedObject(arg, ['url_name', 'url_value', 'time_range']) && typeof arg.time_range === 'string'

Updated in 7ee455c

walterra · 2023-04-11T06:30:33Z

x-pack/plugins/ml/common/types/data_frame_analytics.ts

+
+export function isDataFrameAnalyticsConfigs(arg: unknown): arg is DataFrameAnalyticsConfig {
+  return (
+    isPopulatedObject(arg) &&


Nit: You could move part of the check to isPopulatedObject here like this:

isPopulatedObject(arg, ['dest', 'analysis', 'id']) && typeof arg.id === 'string'

Updated in 7ee455c

walterra · 2023-04-11T06:33:25Z

...public/application/components/custom_urls/custom_url_editor/__snapshots__/list.test.tsx.snap

@@ -61,33 +61,6 @@ exports[`CustomUrlList renders a list of custom URLs 1`] = `
        />
      </EuiFormRow>
    </EuiFlexItem>
-    <EuiFlexItem


I'd expect these snapshots not to change what do you think? Maybe it's worth updating the configs in the tests so they render the same.

Yep - it wasn't recognizing the job as an anomaly detection job so was removing the time range selector. I just added in the properties so it would be recognized and updated the result 👍 in 7ee455c

walterra · 2023-04-11T06:57:37Z

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

@@ -10,6 +10,7 @@ import { runtimeMappingsSchema } from './runtime_mappings_schema';

 export const dataAnalyticsJobConfigSchema = schema.object({
  description: schema.maybe(schema.string()),
+  _meta: schema.maybe(schema.any()),


The TS spec now defines it as an optional record like _meta: undefined | Record<string, any>; (in other places _meta: undefined | Record<string, unknown>;) but this one just as an optional any, maybe it can be updated to also be like the Record?

@walterra - yep, did some digging and there is an option with an 'unknown' - added in in 7ee455c

alvarezmelissa87 · 2023-04-11T15:50:45Z

@elasticmachine merge upstream

alvarezmelissa87 · 2023-04-11T15:51:03Z

Updated with all suggestions 🙏 cc @walterra, @peteharverson

walterra

Latest changes LGTM

peteharverson

Tested latest changes and looks good.

Would it be possible to add some functional test coverage for this, as we have for anomaly detection jobs?

darnautov · 2023-04-12T13:00:45Z

...ion/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/state.ts

@@ -87,6 +87,7 @@ export interface State {
    maxNumThreads: undefined | number;
    maxOptimizationRoundsPerHyperparameter: undefined | number;
    maxTrees: undefined | number;
+    _meta: undefined | Record<string, any>;


Suggested change

_meta: undefined | Record<string, any>;

_meta: undefined | Record<string, unknown>;

darnautov · 2023-04-12T13:01:27Z

x-pack/plugins/ml/common/types/data_frame_analytics.ts

@@ -56,16 +57,26 @@ export interface ClassificationAnalysis {
 }

 export type AnalysisConfig = estypes.MlDataframeAnalysisContainer;
+interface DataFrameAnalyticsMeta {
+  custom_urls: any[];


we have a type for custom_urls

darnautov · 2023-04-12T13:01:36Z

x-pack/plugins/ml/common/types/data_frame_analytics.ts

@@ -56,16 +57,26 @@ export interface ClassificationAnalysis {
 }

 export type AnalysisConfig = estypes.MlDataframeAnalysisContainer;
+interface DataFrameAnalyticsMeta {
+  custom_urls: any[];
+  [key: string]: any;


Suggested change

[key: string]: any;

[key: string]: unknown;

darnautov · 2023-04-12T13:03:00Z

x-pack/plugins/ml/public/application/components/custom_urls/custom_url_editor/editor.tsx

+                          defaultMessage="Interval"
+                        />
+                      }
+                      className="url-time-range"


this css class doesn't exist anymore

Suggested change

className="url-time-range"

darnautov · 2023-04-12T13:05:01Z

x-pack/plugins/ml/public/application/components/custom_urls/custom_url_editor/utils.ts

@@ -179,22 +255,22 @@ async function buildDashboardUrlFromSettings(settings) {
  });

  // Temp workaround
-  const state = location.state;
+  const state: any = location?.state;


do we have a better type definition for the state?

darnautov · 2023-04-12T13:06:20Z

x-pack/plugins/ml/public/application/components/custom_urls/custom_url_editor/utils.ts

@@ -328,7 +401,7 @@ export async function getTestUrl(job, customUrl) {
      // attempt load the non-combined job and datafeed so they can be used in the datafeed preview
      const [{ jobs }, { datafeeds }] = await Promise.all([
        ml.getJobs({ jobId: job.job_id }),
-        ml.getDatafeeds({ datafeedId: job.datafeed_config.datafeed_id }),
+        ml.getDatafeeds({ datafeedId: job.datafeed_config?.datafeed_id ?? '' }),


ah sorry, ❓ shall we pass undefined instead of empty string here?

darnautov · 2023-04-12T13:10:19Z

...ata_frame_analytics/pages/analytics_management/components/action_edit/edit_action_flyout.tsx

@@ -54,12 +58,20 @@ export const EditActionFlyout: FC<Required<EditAction>> = ({ closeFlyout, item }
  );
  const [mmlValidationError, setMmlValidationError] = useState<string | undefined>();
  const [maxNumThreads, setMaxNumThreads] = useState<number | undefined>(config.max_num_threads);
+  const [activeTabId, setActiveTabId] = useState<string>('job-details');
+  const [customUrls, setCustomUrls] = useState<UrlConfig[]>([]);
+  const [analyticsJob, setAnalyticsJob] = useState<any>();


could you please set an appropriate type for the list of DFA jobs here?

peteharverson

Makes sense to add functional tests once the other items for custom URLs for DFA jobs are in. I added it as an item to #152009.
LGTM.

kibana-ci · 2023-04-12T23:20:24Z

💚 Build Succeeded

Buildkite Build
Commit: 9e9c28e

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1721	1726	+5

Async chunks

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

id	before	after	diff
`ml`	3.4MB	3.4MB	+5.9KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	433	436	+3

References to deprecated APIs

id	before	after	diff
`ml`	160	161	+1

Total ESLint disabled count

id	before	after	diff
`securitySolution`	513	516	+3

History

💚 Build #119099 succeeded 3d93dc5
💚 Build #118453 succeeded 5b70da1
💚 Build #118417 succeeded 26a5075
💚 Build #118391 succeeded 10615c1
💔 Build #118109 failed 642ce6d
💔 Build #117739 failed 9e1bfee

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

cc @alvarezmelissa87

alvarezmelissa87 added 3 commits April 3, 2023 13:24

Move custom urls to shared. Add to DFA job edit

de44829

add open url behavior and ensure urls are cloned

3ba8ef3

fix delete functionality for dfa

b5d645f

alvarezmelissa87 added release_note:enhancement :ml Feature:Data Frame Analytics ML data frame analytics features v8.8.0 labels Apr 3, 2023

alvarezmelissa87 self-assigned this Apr 3, 2023

alvarezmelissa87 requested a review from a team as a code owner April 3, 2023 20:17

alvarezmelissa87 mentioned this pull request Apr 3, 2023

[ML] Data Frame Analytics results view: option to drill down from column to dashboard* #150375

Closed

4 tasks

alvarezmelissa87 added 2 commits April 3, 2023 14:45

update types and remove unused comments

90daf01

update import in functional test

881cbb9

alvarezmelissa87 requested review from darnautov and peteharverson April 4, 2023 14:35

fix import typo in functional tests

9e1bfee

darnautov reviewed Apr 4, 2023

View reviewed changes

Update types and fix AD fallback to datafeed preview for test url

642ce6d

peteharverson reviewed Apr 6, 2023

View reviewed changes

x-pack/plugins/ml/public/application/components/custom_urls/custom_urls.tsx Show resolved Hide resolved

fix anomaly job check

10615c1

Merge branch 'main' into ml-dfa-column-drilldown

26a5075

peteharverson reviewed Apr 6, 2023

View reviewed changes

x-pack/plugins/ml/public/application/components/custom_urls/custom_urls.tsx Show resolved Hide resolved

remove resultsfields from dropdown. ensure source index is used as ba…

5b70da1

…ckup

walterra reviewed Apr 11, 2023

View reviewed changes

update jest and schemas

7ee455c

Merge branch 'main' into ml-dfa-column-drilldown

b91193d

remove unnecessary ternary operator

3d93dc5

walterra approved these changes Apr 11, 2023

View reviewed changes

peteharverson reviewed Apr 12, 2023

View reviewed changes

darnautov reviewed Apr 12, 2023

View reviewed changes

peteharverson approved these changes Apr 12, 2023

View reviewed changes

add DFA job type

9e9c28e

alvarezmelissa87 merged commit c7c5a21 into elastic:main Apr 13, 2023

kibanamachine added the backport:skip This commit does not require backporting label Apr 13, 2023

alvarezmelissa87 deleted the ml-dfa-column-drilldown branch April 13, 2023 14:47

peteharverson added release_note:feature Makes this part of the condensed release notes and removed release_note:enhancement labels Apr 21, 2023

	_meta: schema.maybe(schema.any()),
	_meta: schema.maybe(schema.object()),

	_meta: undefined \| Record<string, any>;
	_meta: undefined \| Record<string, unknown>;

[ML] Data Frame Analytics: support custom urls in jobs #154287

[ML] Data Frame Analytics: support custom urls in jobs #154287

Conversation

alvarezmelissa87 commented Apr 3, 2023 • edited Loading

Summary

Checklist

elasticmachine commented Apr 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra Apr 11, 2023 • edited by alvarezmelissa87 Loading

Choose a reason for hiding this comment

alvarezmelissa87 commented Apr 5, 2023

alvarezmelissa87 commented Apr 6, 2023

Choose a reason for hiding this comment

alvarezmelissa87 Apr 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

walterra Apr 11, 2023 • edited by alvarezmelissa87 Loading

Choose a reason for hiding this comment

alvarezmelissa87 commented Apr 11, 2023

alvarezmelissa87 commented Apr 11, 2023

walterra left a comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 12, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

alvarezmelissa87 commented Apr 3, 2023 •

edited

Loading

walterra Apr 11, 2023 •

edited by alvarezmelissa87

Loading

alvarezmelissa87 Apr 11, 2023 •

edited

Loading

walterra Apr 11, 2023 •

edited by alvarezmelissa87

Loading