[Logs UI] Access ML via the programmatic plugin API #68905
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
weltenwort
added
v8.0.0
Feature:Logs UI
weltenwort
force-pushed
the
logs-ui-use-ml-search-api
branch
from
eaedd20
to
a2db6fa
Compare
weltenwort
commented
Jun 12, 2020
| @@ -94,25 +94,6 @@ export default ({ getService }: FtrProviderContext) => { | |||
| expect(logEntryRateBuckets.data.bucketDuration).to.be(15 * 60 * 1000); | |||
| expect(logEntryRateBuckets.data.histogramBuckets).to.be.empty(); | |||
| }); | |||
|
|
|||
| it('should return a NotFound error when the results index does not exist', async () => { | |||
There was a problem hiding this comment.
This test case doesn't apply anymore, because we're not accessing the results via the alias name anymore. The access is handled internally by the programmatic ML api so we can't determine existence of the results index.
weltenwort
commented
Jun 12, 2020
| import { CoreSetup } from '../../../../../../src/core/server'; | ||
| import { InfraServerPluginDeps } from '../adapters/framework/adapter_types'; | ||
|
|
||
| export function compose(core: CoreSetup, config: InfraConfig, plugins: InfraServerPluginDeps) { |
There was a problem hiding this comment.
This hasn't been used for a while by now.
|
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
|
@elasticmachine merge upstream |
weltenwort
added
the
release_note:skip
Kerry350
reviewed
Jun 23, 2020
| @@ -151,6 +149,25 @@ export class InfraServerPlugin { | |||
| initInfraServer(this.libs); | |||
| registerAlertTypes(plugins.alerts, this.libs); | |||
|
|
|||
| core.http.registerRouteHandlerContext( | |||
Kerry350
approved these changes
Jun 23, 2020
|
|
||
| export function assertHasInfraPlugins<Context extends { infra?: InfraRequestHandlerContext }>( | ||
| context: Context | ||
| ): asserts context is Context & { infra: Context['infra'] } { |
There was a problem hiding this comment.
Thanks for introducing me to Assertion Signatures 😄
jasonrhodes
reviewed
Jun 23, 2020
| @@ -26,12 +32,10 @@ export const createTimeRangeFilters = (startTime: number, endTime: number) => [ | |||
| }, | |||
| ]; | |||
|
|
|||
| export const createResultTypeFilters = (resultType: 'model_plot' | 'record') => [ | |||
| export const createResultTypeFilters = (resultTypes: Array<'model_plot' | 'record'>) => [ | |||
There was a problem hiding this comment.
Just curious: what's the difference/advantage here?
There was a problem hiding this comment.
I wanted to also use this helper in some places where we're filtering for more than one result type. Therefore the helper had to be extended to allow for that.
jasonrhodes
reviewed
Jun 23, 2020
| framework.registerRoute( | ||
| { | ||
| method: 'post', | ||
| path: LOG_ANALYSIS_GET_LOG_ENTRY_CATEGORIES_PATH, | ||
| validate: { | ||
| // short-circuit forced @kbn/config-schema validation so we can do io-ts validation | ||
| body: anyObject, | ||
| body: createValidationFunction(getLogEntryCategoriesRequestPayloadRT), |
There was a problem hiding this comment.
How do we call this function without having to provide the generic values it specifies (<DecodedValue, EncodedValue, InputValue>)?
There was a problem hiding this comment.
I'm not sure I understand. The generic arguments are inferred from the codec passed as the function argument.
There was a problem hiding this comment.
Ah ok, I'll take this question out of this PR :)
jasonrhodes
reviewed
Jun 23, 2020
| }, | ||
| }, | ||
| async (requestContext, request, response) => { | ||
| framework.router.handleLegacyErrors(async (requestContext, request, response) => { |
There was a problem hiding this comment.
What does this handleLegacyErrors function do and where is it documented? I assume it lets us use the Boom stuff inside our handler?
There was a problem hiding this comment.
I don't know of any documentation - I read the http service source. Inside of our handlers we use some platform services, which throw Boom exceptions. I found this helper improves the representation of these errors in the response. Without it we would have to capture and convert them ourselves to avoid masquerading them as generic code 500 errors.
There was a problem hiding this comment.
|
@elasticmachine merge upstream |
💚 Build SucceededBuild metricspage load asset sizebeta
History
To update your PR or re-run it, just comment with: |
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jun 25, 2020
* master: (45 commits) [QA] Unskip functional tests (elastic#69760) [SIEM][Detection Engine] - Update DE to work with new exceptions schema (elastic#69715) Fixes elastic#69639: Ignore url.url fields above 2048 characters (elastic#69863) PR: Provide limit warnings to user when API limits are reached. (elastic#69590) [Maps] Remove broken button (elastic#69853) Makes usage collection methods available on start (elastic#69836) [SIEM][CASE] Improve Jira's labelling (elastic#69892) [Logs UI] Access ML via the programmatic plugin API (elastic#68905) [ML] DF Analytics: Creation wizard part 3 (elastic#69456) Update Resolver generator script documentation (elastic#69912) [ML] Changes View results button text on new job page (elastic#69809) Add master branch to backport config (elastic#69893) [Ingest Manager] Kibana, not EPR, controls removable packages (elastic#69761) unskips 'Events columns' test (elastic#69684) [ML] Changes the ML overview empty analytics panel text (elastic#69801) [DOCS] Emphasizes where File Data Visualizer is located. (elastic#69812) add the `exactRoute` property to app registration (elastic#69772) Bump backport to 5.4.6 (elastic#69880) [Logs UI] ML log integration splash screen (elastic#69288) Clean up TSVB type client code to conform to the schema (elastic#68519) ...
weltenwort
added a commit
that referenced
this pull request
Jun 25, 2020
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jun 25, 2020
* master: (90 commits) [Encrypted Saved Objects] Adds support for migrations in ESO (elastic#69513) [SIEM] Replace WithSource with useWithSource hook (elastic#68722) [Endpoint]EMT-451: add ability to filter endpoint metadata based on presence of unenrolled events (elastic#69708) rename old siem kibana config to securitySolution (elastic#69874) Remove unused Resolver code (elastic#69914) [Observability] Fixing dynamic return type based on the appName (elastic#69894) [SECURITY SOLUTION][INGEST] Task/endpoint list tests (elastic#69419) Fixes special clicks and 3rd party icon sizes in nav (elastic#69767) [APM] Catch annotations index permission error and log warning (elastic#69881) [Endpoint][Ingest Manager] minor code cleanup (elastic#69844) [Logs UI] Logs ui context menu (elastic#69915) Index pattern serialize and de-serialize (elastic#68844) [QA] Unskip functional tests (elastic#69760) [SIEM][Detection Engine] - Update DE to work with new exceptions schema (elastic#69715) Fixes elastic#69639: Ignore url.url fields above 2048 characters (elastic#69863) PR: Provide limit warnings to user when API limits are reached. (elastic#69590) [Maps] Remove broken button (elastic#69853) Makes usage collection methods available on start (elastic#69836) [SIEM][CASE] Improve Jira's labelling (elastic#69892) [Logs UI] Access ML via the programmatic plugin API (elastic#68905) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This modifies the routes related to log rate and category analysis to use the new programmatic APIs provided by the
mlplugin to access the results index and job info. Because that access is facilitated via the request context, the log analysis lib was converted from classes to plain functions.At the same time the routes have been updated to use the most recent validation and error handling patterns.
closes #67861
Reviewing and testing
💡 The conversion of class methods to functions caused a large amount of indentation changes. Ignore white-space during code review to preserve sanity.
The behavior of everything related to the log rate and categories tabs should be unchanged from the user's perspective, so double-checking their functionality would be prudent.
Task breakdown
spaceIdavailable via the request context