Skip to content

[Logs UI] Generalize ML module management#50662

Merged
weltenwort merged 17 commits intoelastic:masterfrom
weltenwort:logs-ui-categorization-generalize-ml-module-management
Dec 10, 2019
Merged

[Logs UI] Generalize ML module management#50662
weltenwort merged 17 commits intoelastic:masterfrom
weltenwort:logs-ui-categorization-generalize-ml-module-management

Conversation

@weltenwort
Copy link
Copy Markdown
Member

@weltenwort weltenwort commented Nov 14, 2019 •
edited
Loading

Summary

This abstracts the specific job details out of the ML module management hooks to enable re-use with the upcoming categorization module.

closes #50322

Implementation notes

I tried to apply the following methodology:

  • hooks in containers/log_analysis have been made agnostic to the specific ML module
  • the log-entry-rate-specific hooks wrap/utilize the agnostic hooks and have been moved to pages/logs/log_entry_rate
  • the module specifics are defined in a moduleDescriptor data structure (i.e. logEntryRateModuleDescriptor)
  • the current source configuration is encapsulated in a ModuleSourceConfiguration
  • log-entry-rate-specific components have been moved to pages/logs/log_entry_rate (mostly a rename from pages/logs/log_analysis)
  • components useful for different ML modules to come have been moved to components/logging

The resulting relationships between the resulting components and hooks on the coarse scale could be described with the following graph:

 pages/logs/log_entry_rate                                                             containers/logging/log_analysis
+---------------------------------------------------------------------------------+   +------------------------------+
|                                                                                 |   |                              |
|                                                                                 |   |                              |
|  +------------------+          +-----------------------+                        |   |  +----------------------+    |
|  | LogEntryRatePage +----------> useLogEntryRateModule +-------------------------------> useLogAnalysisModule |    |
|  +--------+---------+ provides +---^---^---^---+---+---+ uses                   |   |  +--+-------------------+    |
|           |                        |   |   |   |   |                            |   |     |                        |
|           |                        |   |   |   |   |                            |   |     |   +-----------------+  |
|           |                        |   |   |   |   |                            |   |     +---> useModuleStatus |  |
|           |                        |   |   |   | +-v-------------------------+  |   |         +-----------------+  |
|           |                        |   |   |   | | moduleSourceConfiguration |  |   |                              |
|           |                        |   |   |   | +---------------------------+  |   |  +------------------------+  |
|           |                        |   |   |   |                                |   |  | callGetMlModuleAPI     |  |
|           |                        |   |   | +-v----------------------------+   |   |  | callSetupMlModuleAPI   |  |
|           |                        |   |   | | logEntryRateModuleDescriptor +----------> callJobsSummaryAPI     |  |
|           |                        |   |   | +------------------------------+   |   |  | callDeleteJobs         |  |
|           |      accesses (context)|   |   |                                    |   |  | callValidateIndicesAPI |  |
|           |                        |   |   |                                    |   |  +------------------------+  |
|  +--------v----------------+       |   |   |                                    |   |                              |
|  | LogEntryRatePageContent +-------+   |   |                                    |   |                              |
|  +--------+----------------+           |   |                                    |   |                              |
|           |                            |   |                                    |   |                              |
|           |   +------------------------+-+ |                                    |   |  +-----------------------+   |
|           +---> LogEntryRateSetupContent +---------------------------------------------> useAnalysisSetupState |   |
|           |   +--------------------------+ |                                    |   |  +-----------------------+   |
|           |                                |                                    |   |                              |
|           |   +----------------------------+-+   +------------------------+     |   |                              |
|           +---> LogEntryRateResultsContent   +---> useLogEntryRateResults |     |   +------------------------------+
|           |   +------------------------------+   +------------------------+     |
|           |                                                                     |
|           |   +--------------------------------+                                |
|           +---> LogEntryRateUnavailableContent |                                |
|               +--------------------------------+                                |
|                                                                                 |
|                                                                                 |
+---------------------------------------------------------------------------------+

Checklist

@weltenwort weltenwort added v8.0.0 Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Nov 14, 2019
@weltenwort weltenwort self-assigned this Nov 14, 2019
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 14, 2019

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 14, 2019

💔 Build Failed

@weltenwort weltenwort force-pushed the logs-ui-categorization-generalize-ml-module-management branch from 4d0243d to 3344f32 Compare November 19, 2019 22:08
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 19, 2019

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 20, 2019

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 21, 2019

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 25, 2019

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 25, 2019

💚 Build Succeeded

@weltenwort weltenwort force-pushed the logs-ui-categorization-generalize-ml-module-management branch from df3bc37 to 4b4268f Compare November 25, 2019 23:29
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 26, 2019

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 26, 2019

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 27, 2019

💚 Build Succeeded

@weltenwort weltenwort force-pushed the logs-ui-categorization-generalize-ml-module-management branch from 1b4de9c to adb3ea2 Compare November 28, 2019 16:34
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 28, 2019

💚 Build Succeeded

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 28, 2019

💔 Build Failed

@weltenwort
Copy link
Copy Markdown
Member Author

weltenwort commented Nov 28, 2019

jenkins, test this again

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 28, 2019

💚 Build Succeeded

@weltenwort weltenwort marked this pull request as ready for review November 28, 2019 23:23
@weltenwort weltenwort requested a review from a team as a code owner November 28, 2019 23:23
@weltenwort weltenwort requested a review from Kerry350 November 28, 2019 23:24
@weltenwort weltenwort mentioned this pull request Nov 29, 2019
Merged
4 tasks
@weltenwort
Copy link
Copy Markdown
Member Author

weltenwort commented Dec 3, 2019

@elasticmachine update branch

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 3, 2019

💚 Build Succeeded

@weltenwort
Copy link
Copy Markdown
Member Author

weltenwort commented Dec 9, 2019

Looks unrelated:

[00:31:41]                 └- ✖ fail: "Visualize feature controls security global visualize all privileges allow saving via the saved query management component popover with no saved query loaded"

@weltenwort
Copy link
Copy Markdown
Member Author

weltenwort commented Dec 9, 2019

@elasticmachine update branch

Kerry350
Kerry350 approved these changes Dec 10, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@Kerry350 Kerry350 left a comment

Choose a reason for hiding this comment

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

Nice work 👍 This cleaned things up a lot, even with just the one use of generalised parts currently. The new directory structures look good too. I have left a couple of really minor comments.

x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_module_status.tsx
...accumulatedJobStatus,
[jobType]: 'unknown',
}),
{} as Record<JobType, JobStatus>
Copy link
Copy Markdown
Contributor

@Kerry350 Kerry350 Dec 10, 2019

Choose a reason for hiding this comment

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

Small nitpick: Could we use a generic type argument over the casting? (jobTypes.reduce<Record<JobType, JobStatus>>)?

Copy link
Copy Markdown
Member Author

@weltenwort weltenwort Dec 10, 2019

Choose a reason for hiding this comment

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

That's what I usually prefer too, but in this case it doesn't work because the empty default object is not assignable to that record type:

Argument of type '{}' is not assignable to parameter of type 'Record<JobType, JobStatus>'.

x-pack/legacy/plugins/infra/public/containers/logs/log_analysis/log_analysis_module_status.tsx
...accumulatedJobStatus,
[jobType]: 'initializing',
}),
{} as Record<JobType, JobStatus>
Copy link
Copy Markdown
Contributor

@Kerry350 Kerry350 Dec 10, 2019

Choose a reason for hiding this comment

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

Same here, generic type argument over casting?

Copy link
Copy Markdown
Member Author

@weltenwort weltenwort Dec 10, 2019

Choose a reason for hiding this comment

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

same as above 😉

x-pack/legacy/plugins/infra/public/pages/logs/log_entry_rate/page_providers.tsx Outdated

import { useSourceContext } from '../../../containers/source';
import { useKibanaSpaceId } from '../../../utils/use_kibana_space_id';
// import { LogEntryRateJobsProvider } from './use_log_entry_rate_jobs';
Copy link
Copy Markdown
Contributor

@Kerry350 Kerry350 Dec 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Can this be deleted?

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 10, 2019

💚 Build Succeeded

History

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

@weltenwort weltenwort merged commit cf28280 into elastic:master Dec 10, 2019
@weltenwort weltenwort deleted the logs-ui-categorization-generalize-ml-module-management branch December 10, 2019 18:21
@weltenwort
Copy link
Copy Markdown
Member Author

weltenwort commented Dec 10, 2019

ℹ️ backport to 7.x is waiting for #52613

@weltenwort weltenwort mentioned this pull request Dec 11, 2019
Closed
weltenwort added a commit to weltenwort/kibana that referenced this pull request Dec 11, 2019
@weltenwort
[Logs UI] Generalize ML module management (elastic#50662)
e8da63a 
This abstracts the specific job details out of the ML module management hooks to enable re-use with the upcoming categorization module.

closes elastic#50322
jloleysens added a commit to jloleysens/kibana that referenced this pull request Dec 11, 2019
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into feature/conso…
6f57d30 
…le-sql-highlighting

* 'master' of github.com:elastic/kibana: (56 commits)
  Migrate url shortener service (elastic#50896)
  Re-enable datemath in from/to canvas timelion args (elastic#52159)
  [Logs + Metrics UI] Remove eslint exceptions (elastic#50979)
  [Logs + Metrics UI] Add missing headers in Logs & metrics (elastic#52405)
  [ML] API integration tests - initial tests for bucket span estimator (elastic#52636)
  [Watcher] New Platform (NP) Migration (elastic#50908)
  Decouple Authorization subsystem from Legacy API. (elastic#52638)
  [APM] Fix some warnings logged in APM tests (elastic#52487)
  [ui/public/utils] Delete unused base_object & find_by_param (elastic#52500)
  [ui/public/utils] Move items into ui/vis (elastic#52615)
  fix newlines in kbn-analytics build script
  Add top level examples folder and command to run, `--run-examples`. (elastic#52027)
  feat(NA): add trap for SIGINT in the git precommit hook (elastic#52662)
  [DOCS] Updtes description of elasticsearch.requestHeadersWhitelist (elastic#52675)
  [Telemetry/Pulse] Updates advanced settings text for usage data (elastic#52657)
  [SIEM][Detection Engine] Adds the default name space to the end of the signals index
  [Logs UI] Generalize ML module management (elastic#50662)
  Removing stateful saved object finder (elastic#52166)
  Shim oss telemetry (elastic#51168)
  [Reporting/Screenshots] Do not fail the report if request is aborted (elastic#52344)
  ...

# Conflicts:
#	src/legacy/core_plugins/console/public/legacy.ts
#	src/legacy/core_plugins/console/public/np_ready/application/models/legacy_core_editor/mode/elasticsearch_sql_highlight_rules.ts
#	src/legacy/core_plugins/console/public/np_ready/lib/autocomplete/components/full_request_component.ts
#	src/legacy/core_plugins/console/public/quarantined/src/sense_editor/row_parser.js
@weltenwort weltenwort mentioned this pull request Dec 11, 2019
Merged
weltenwort added a commit to weltenwort/kibana that referenced this pull request Dec 11, 2019
@weltenwort
[Logs UI] Generalize ML module management (elastic#50662)
dbfc8f7 
This abstracts the specific job details out of the ML module management hooks to enable re-use with the upcoming categorization module.

closes elastic#50322
weltenwort added a commit that referenced this pull request Dec 11, 2019
@weltenwort
[7.x] [Logs UI] Generalize ML module management (#50662) (#52777)
737ce28 
Backports the following commits to 7.x:
 - [Logs UI] Generalize ML module management (#50662)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kerry350 Kerry350 Kerry350 approved these changes

Assignees

@weltenwort weltenwort

Labels

Feature:Logs UI Logs UI feature release_note:skip Skip the PR/issue when compiling release notes review Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.6.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Logs UI] Refactor ML job management hooks to be module-agnostic

3 participants

@weltenwort @elasticmachine @Kerry350