Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Logs UI] Generalize ML module management #50662

Conversation

weltenwort
Copy link
Member

@weltenwort weltenwort commented Nov 14, 2019

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
Contributor

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

@elasticmachine
Copy link
Contributor

💔 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
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 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
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 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
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@weltenwort
Copy link
Member Author

jenkins, test this again

@elasticmachine
Copy link
Contributor

💚 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
Copy link
Member Author

@elasticmachine update branch

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@weltenwort
Copy link
Member Author

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
Member Author

@elasticmachine update branch

Copy link
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.

...accumulatedJobStatus,
[jobType]: 'unknown',
}),
{} as Record<JobType, JobStatus>
Copy link
Contributor

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
Member Author

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>'.

...accumulatedJobStatus,
[jobType]: 'initializing',
}),
{} as Record<JobType, JobStatus>
Copy link
Contributor

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
Member Author

Choose a reason for hiding this comment

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

same as above 😉


import { useSourceContext } from '../../../containers/source';
import { useKibanaSpaceId } from '../../../utils/use_kibana_space_id';
// import { LogEntryRateJobsProvider } from './use_log_entry_rate_jobs';
Copy link
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.

Can this be deleted?

@elasticmachine
Copy link
Contributor

💚 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
Member Author

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

weltenwort added a commit to weltenwort/kibana that referenced this pull request Dec 11, 2019
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
…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 added a commit to weltenwort/kibana that referenced this pull request Dec 11, 2019
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
Backports the following commits to 7.x:
 - [Logs UI] Generalize ML module management (#50662)
timductive pushed a commit to timductive/kibana that referenced this pull request Dec 16, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

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