From 586690247556928c147f01a8515e0d272a025bb1 Mon Sep 17 00:00:00 2001 From: Karl Godard Date: Tue, 22 Feb 2022 17:02:56 -0800 Subject: [PATCH] Session view kg addressed comments (#62) * comment added to explain use of large page size * made constants of indexes and other query paths. cleaned up process_events_route logic a bit. fixed the test. * removed unused logger * moved inline function out of button into top level const * useMemo removed in favor of memoizing the getDetails and other functions in the ProcessImpl * index constant added to session leader table Co-authored-by: mitodrummer --- .../plugins/session_view/common/constants.ts | 16 +++++ .../public/components/process_tree/hooks.ts | 69 +++++++++++-------- .../components/process_tree_node/index.tsx | 25 +++---- .../components/session_leader_table/index.tsx | 3 +- x-pack/plugins/session_view/server/plugin.ts | 2 +- .../session_view/server/routes/index.ts | 5 +- .../routes/process_events_route.test.ts | 20 +++--- .../server/routes/process_events_route.ts | 46 ++++--------- .../routes/session_entry_leaders_route.ts | 4 +- 9 files changed, 94 insertions(+), 96 deletions(-) diff --git a/x-pack/plugins/session_view/common/constants.ts b/x-pack/plugins/session_view/common/constants.ts index 14ed373901297..fc2760b3be30a 100644 --- a/x-pack/plugins/session_view/common/constants.ts +++ b/x-pack/plugins/session_view/common/constants.ts @@ -8,5 +8,21 @@ export const PROCESS_EVENTS_ROUTE = '/internal/session_view/process_events_route'; export const RECENT_SESSION_ROUTE = '/internal/session_view/recent_session_route'; export const SESSION_ENTRY_LEADERS_ROUTE = '/internal/session_view/session_entry_leaders_route'; +export const PROCESS_EVENTS_INDEX = 'logs-endpoint.events.process-default'; +export const ALERTS_INDEX = '.siem-signals-default'; +export const ENTRY_SESSION_ENTITY_ID_PROPERTY = 'process.entry_leader.entity_id'; +// We fetch a large number of events per page to mitigate a few design caveats in session viewer +// 1. Due to the hierarchical nature of the data (e.g we are rendering a time ordered pid tree) there are common scenarios where there +// are few top level processes, but many nested children. For example, a build script is run on a remote host via ssh. If for example our page +// size is 10 and the build script has 500 nested children, the user would see a load more button that they could continously click without seeing +// anychange since the next 10 events would be for processes nested under a top level process that might not be expanded. That being said, it's quite +// possible there are build scripts with many thousands of events, in which case this initial large page will have the same issue. A technique used +// in previous incarnations of session view included auto expanding the node which is receiving the new page of events so as to not confuse the user. +// We may need to include this trick as part of this implementation as well. +// 2. The plain text search that comes with Session view is currently limited in that it only searches through data that has been loaded into the browser. +// The large page size allows the user to get a broader set of results per page. That being said, this feature is kind of flawed since sessions could be many thousands +// if not 100s of thousands of events, and to be required to page through these sessions to find more search matches is not a great experience. Future iterations of the +// search functionality will instead use a separate ES backend search to avoid this. +// 3. Fewer round trips to the backend! export const PROCESS_EVENTS_PER_PAGE = 1000; diff --git a/x-pack/plugins/session_view/public/components/process_tree/hooks.ts b/x-pack/plugins/session_view/public/components/process_tree/hooks.ts index 81ae0fd32645b..9fea7c5ac9785 100644 --- a/x-pack/plugins/session_view/public/components/process_tree/hooks.ts +++ b/x-pack/plugins/session_view/public/components/process_tree/hooks.ts @@ -5,6 +5,7 @@ * 2.0. */ import _ from 'lodash'; +import memoizeOne from 'memoize-one'; import { useState, useEffect } from 'react'; import { EventAction, @@ -66,12 +67,11 @@ export class ProcessImpl implements Process { } hasOutput() { - // TODO: schema undecided - return !!this.events.find(({ event }) => event.action === EventAction.output); + return !!this.findEventByAction(this.events, EventAction.output); } hasAlerts() { - return !!this.events.find(({ event }) => event.kind === EventKind.signal); + return !!this.findEventByKind(this.events, EventKind.signal); } getAlerts() { @@ -79,15 +79,48 @@ export class ProcessImpl implements Process { } hasExec() { - return !!this.events.find(({ event }) => event.action === EventAction.exec); + return !!this.findEventByAction(this.events, EventAction.exec); } hasExited() { - return !!this.events.find(({ event }) => event.action === EventAction.end); + return !!this.findEventByAction(this.events, EventAction.end); } getDetails() { - const eventsPartition = this.events.reduce( + return this.getDetailsMemo(this.events); + } + + getOutput() { + // not implemented, output ECS schema not defined (for a future release) + return ''; + } + + isUserEntered() { + const event = this.getDetails(); + const { tty } = event.process; + + return !!tty && process.pid !== event.process.group_leader.pid; + } + + getMaxAlertLevel() { + // TODO: as part of alerts details work + tie in with the new alert flyout + return null; + } + + findEventByAction = memoizeOne((events: ProcessEvent[], action: EventAction) => { + return events.find(({ event }) => event.action === action); + }); + + findEventByKind = memoizeOne((events: ProcessEvent[], kind: EventKind) => { + return events.find(({ event }) => event.kind === kind); + }); + + filterEventsByAction = memoizeOne((events: ProcessEvent[], action: EventAction) => { + return events.filter(({ event }) => event.action === action); + }); + + getDetailsMemo = memoizeOne((events: ProcessEvent[]) => { + const eventsPartition = events.reduce( (currEventsParition, processEvent) => { currEventsParition[processEvent.event.action]?.push(processEvent); return currEventsParition; @@ -107,29 +140,7 @@ export class ProcessImpl implements Process { } return this.events[this.events.length - 1] || ({} as ProcessEvent); - } - - getOutput() { - return this.events.reduce((output, event) => { - if (event.event.action === EventAction.output) { - output += ''; // TODO: schema unknown - } - - return output; - }, ''); - } - - isUserEntered() { - const event = this.getDetails(); - const { tty } = event.process; - - return !!tty && process.pid !== event.process.group_leader.pid; - } - - getMaxAlertLevel() { - // TODO: - return null; - } + }); } export const useProcessTree = ({ sessionEntityId, data, searchQuery }: UseProcessTreeDeps) => { diff --git a/x-pack/plugins/session_view/public/components/process_tree_node/index.tsx b/x-pack/plugins/session_view/public/components/process_tree_node/index.tsx index 9caf0cb8fa5d3..738de02060bad 100644 --- a/x-pack/plugins/session_view/public/components/process_tree_node/index.tsx +++ b/x-pack/plugins/session_view/public/components/process_tree_node/index.tsx @@ -11,7 +11,7 @@ *2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import React, { useMemo, useRef, useLayoutEffect, useState, useEffect, MouseEvent } from 'react'; +import React, { useRef, useLayoutEffect, useState, useEffect, MouseEvent } from 'react'; import { EuiButton, EuiIcon, EuiToolTip } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n-react'; import { Process } from '../../../common/types/process_tree'; @@ -46,20 +46,9 @@ export function ProcessTreeNode({ setChildrenExpanded(isSessionLeader || process.autoExpand); }, [isSessionLeader, process.autoExpand]); - const processDetails = useMemo(() => { - return process.getDetails(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [process.events.length]); - - const hasExec = useMemo(() => { - return process.hasExec(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [process.events.length]); - - const alerts = useMemo(() => { - return process.getAlerts(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [process.events.length]); + const processDetails = process.getDetails(); + const hasExec = process.hasExec(); + const alerts = process.getAlerts(); const styles = useStyles({ depth, hasAlerts: !!alerts.length }); @@ -114,6 +103,8 @@ export function ProcessTreeNode({ return expanded ? 'arrowUp' : 'arrowDown'; }; + const onShowGroupLeaderOnlyClick = () => setShowGroupLeadersOnly(!showGroupLeadersOnly); + const renderButtons = () => { const buttons = []; const childCount = process.getChildren().length; @@ -137,9 +128,9 @@ export function ProcessTreeNode({ } > setShowGroupLeadersOnly(!showGroupLeadersOnly)} + onClick={onShowGroupLeaderOnlyClick} data-test-subj="processTreeNodeChildProcessesButton" > { - registerProcessEventsRoute(router, logger); +export const registerRoutes = (router: IRouter) => { + registerProcessEventsRoute(router); registerRecentSessionRoute(router); sessionEntryLeadersRoute(router); }; diff --git a/x-pack/plugins/session_view/server/routes/process_events_route.test.ts b/x-pack/plugins/session_view/server/routes/process_events_route.test.ts index 8a59ea762eca0..76f54eb4b8ab6 100644 --- a/x-pack/plugins/session_view/server/routes/process_events_route.test.ts +++ b/x-pack/plugins/session_view/server/routes/process_events_route.test.ts @@ -10,24 +10,20 @@ import { mockEvents } from '../../common/mocks/constants/session_view_process.mo const getEmptyResponse = async () => { return { - body: { - hits: { - total: { value: 0, relation: 'eq' }, - hits: [], - }, + hits: { + total: { value: 0, relation: 'eq' }, + hits: [], }, }; }; const getResponse = async () => { return { - body: { - hits: { - total: { value: mockEvents.length, relation: 'eq' }, - hits: mockEvents.map((event) => { - return { _source: event }; - }), - }, + hits: { + total: { value: mockEvents.length, relation: 'eq' }, + hits: mockEvents.map((event) => { + return { _source: event }; + }), }, }; }; diff --git a/x-pack/plugins/session_view/server/routes/process_events_route.ts b/x-pack/plugins/session_view/server/routes/process_events_route.ts index 8af41e78e2a9c..47e2d917733d5 100644 --- a/x-pack/plugins/session_view/server/routes/process_events_route.ts +++ b/x-pack/plugins/session_view/server/routes/process_events_route.ts @@ -5,12 +5,18 @@ * 2.0. */ import { schema } from '@kbn/config-schema'; -import type { ElasticsearchClient, Logger } from 'kibana/server'; +import type { ElasticsearchClient } from 'kibana/server'; import { IRouter } from '../../../../../src/core/server'; -import { PROCESS_EVENTS_ROUTE, PROCESS_EVENTS_PER_PAGE } from '../../common/constants'; +import { + PROCESS_EVENTS_ROUTE, + PROCESS_EVENTS_PER_PAGE, + PROCESS_EVENTS_INDEX, + ALERTS_INDEX, + ENTRY_SESSION_ENTITY_ID_PROPERTY, +} from '../../common/constants'; import { expandDottedObject } from '../../common/utils/expand_dotted_object'; -export const registerProcessEventsRoute = (router: IRouter, logger: Logger) => { +export const registerProcessEventsRoute = (router: IRouter) => { router.get( { path: PROCESS_EVENTS_ROUTE, @@ -38,43 +44,20 @@ export const doSearch = async ( cursor: string | undefined, forward = true ) => { - // Temporary hack. Updates .siem-signals-default index to include a mapping for process.entry_leader.entity_id - // TODO: find out how to do proper index mapping migrations... - let siemSignalsExists = true; - - try { - await client.indices.putMapping({ - index: '.siem-signals-default', - body: { - properties: { - 'process.entry_leader.entity_id': { - type: 'keyword', - }, - }, - }, - }); - } catch (err) { - siemSignalsExists = false; - } - - const indices = ['logs-endpoint.events.process-default']; - - if (siemSignalsExists) { - indices.push('.siem-signals-default'); - } - const search = await client.search({ - index: indices, + // TODO: move alerts into it's own route with it's own pagination. + index: [PROCESS_EVENTS_INDEX, ALERTS_INDEX], + ignore_unavailable: true, body: { query: { match: { - 'process.entry_leader.entity_id': sessionEntityId, + [ENTRY_SESSION_ENTITY_ID_PROPERTY]: sessionEntityId, }, }, // This runtime_mappings is a temporary fix, so we are able to Query these ECS fields while they are not available // TODO: Remove the runtime_mappings once process.entry_leader.entity_id is implemented to ECS runtime_mappings: { - 'process.entry_leader.entity_id': { + [ENTRY_SESSION_ENTITY_ID_PROPERTY]: { type: 'keyword', }, }, @@ -85,6 +68,7 @@ export const doSearch = async ( }); const events = search.hits.hits.map((hit: any) => { + // TODO: re-eval if this is needed after moving alerts to it's own route. // the .siem-signals-default index flattens many properties. this util unflattens them. hit._source = expandDottedObject(hit._source); diff --git a/x-pack/plugins/session_view/server/routes/session_entry_leaders_route.ts b/x-pack/plugins/session_view/server/routes/session_entry_leaders_route.ts index 28fab235cb81b..98aee357fb91e 100644 --- a/x-pack/plugins/session_view/server/routes/session_entry_leaders_route.ts +++ b/x-pack/plugins/session_view/server/routes/session_entry_leaders_route.ts @@ -6,7 +6,7 @@ */ import { schema } from '@kbn/config-schema'; import { IRouter } from '../../../../../src/core/server'; -import { SESSION_ENTRY_LEADERS_ROUTE } from '../../common/constants'; +import { SESSION_ENTRY_LEADERS_ROUTE, PROCESS_EVENTS_INDEX } from '../../common/constants'; export const sessionEntryLeadersRoute = (router: IRouter) => { router.get( @@ -23,7 +23,7 @@ export const sessionEntryLeadersRoute = (router: IRouter) => { const { id } = request.query; const result = await client.get({ - index: 'logs-endpoint.events.process-default', + index: PROCESS_EVENTS_INDEX, id, });