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.

Sign up for GitHub

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

no longer pass related event stats to process node component #72435

Merged
merged 3 commits into from
Jul 20, 2020
Merged

no longer pass related event stats to process node component #72435

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45066ae
no longer pass related event stats to process node component
Jul 20, 2020
a27b106
remove incorrect comment. restore old name to relatedeventsstats
Jul 20, 2020
06c81b2
fix error
Jul 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/public/resolver/models/resolver_tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function lifecycleEvents(tree: ResolverTree) {
/**
* This returns a map of entity_ids to stats for the related events and alerts.
*/
export function relatedEventsStats(tree: ResolverTree): Map<string, ResolverNodeStats> {
export function relatedEventStats(tree: ResolverTree): Map<string, ResolverNodeStats> {
Copy link
Contributor

Choose a reason for hiding this comment

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

❔ Isn't more correct to say events? Using the singular implies you are providing stats about one related event, which isn't really the case, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see your point. My motivation for changing this was just because I kept typing it as relatedEventStats. How do you feel about nodeStats? @bkimmel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nvm, putting back the original name

Copy link
Contributor

Choose a reason for hiding this comment

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

To be consistent with our naming in a few other places, should we add a ByEntityId at the end, too? Like nodeStatsByEntityId

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dunno

const nodeStats: Map<string, ResolverNodeStats> = new Map();
for (const node of lifecycleNodes(tree)) {
if (node.stats) {
Expand Down
42 changes: 9 additions & 33 deletions x-pack/plugins/security_solution/public/resolver/store/data/selectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,16 @@ export const tree = createSelector(graphableProcesses, function indexedTree(
/**
* This returns a map of entity_ids to stats about the related events and alerts.
*/
export const relatedEventsStats: (
export const relatedEventStats: (
state: DataState
) => Map<string, ResolverNodeStats> | null = createSelector(
) => (nodeID: string) => ResolverNodeStats | undefined = createSelector(
resolverTreeResponse,
(resolverTree?: ResolverTree) => {
if (resolverTree) {
return resolverTreeModel.relatedEventsStats(resolverTree);
const map = resolverTreeModel.relatedEventStats(resolverTree);
return (nodeID: string) => map.get(nodeID);
} else {
return null;
return () => undefined;
}
}
);
Expand Down Expand Up @@ -206,19 +207,15 @@ export const relatedEventInfoByEntityId: (
state: DataState
) => (entityID: string) => RelatedInfoFunctions | null = createSelector(
relatedEventsByEntityId,
relatedEventsStats,
relatedEventStats,
function selectLineageLimitInfo(
/* eslint-disable no-shadow */
relatedEventsByEntityId,
relatedEventsStats
/* eslint-enable no-shadow */
) {
if (!relatedEventsStats) {
// If there are no related event stats, there are no related event info objects
return () => null;
}
return (entityId) => {
const stats = relatedEventsStats.get(entityId);
const stats = relatedEventsStats(entityId);
if (!stats) {
return null;
}
Expand Down Expand Up @@ -524,37 +521,16 @@ export function databaseDocumentIDToAbort(state: DataState): string | null {
}
}

/**
* `ResolverNodeStats` for a process (`ResolverEvent`)
*/
const relatedEventStatsForProcess: (
state: DataState
) => (event: ResolverEvent) => ResolverNodeStats | null = createSelector(
relatedEventsStats,
(statsMap) => {
if (!statsMap) {
return () => null;
}
return (event: ResolverEvent) => {
const nodeStats = statsMap.get(uniquePidForProcess(event));
if (!nodeStats) {
return null;
}
return nodeStats;
};
}
);

/**
* The sum of all related event categories for a process.
*/
export const relatedEventTotalForProcess: (
state: DataState
) => (event: ResolverEvent) => number | null = createSelector(
relatedEventStatsForProcess,
relatedEventStats,
(statsForProcess) => {
return (event: ResolverEvent) => {
const stats = statsForProcess(event);
const stats = statsForProcess(uniquePidForProcess(event));
if (!stats) {
return null;
}
Expand Down
8 changes: 5 additions & 3 deletions x-pack/plugins/security_solution/public/resolver/store/selectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as dataSelectors from './data/selectors';
import * as uiSelectors from './ui/selectors';
import { ResolverState, IsometricTaxiLayout } from '../types';
import { uniquePidForProcess } from '../models/process_event';
import { ResolverEvent } from '../../../common/endpoint/types';
import { ResolverEvent, ResolverNodeStats } from '../../../common/endpoint/types';

/**
* A matrix that when applied to a Vector2 will convert it from world coordinates to screen coordinates.
Expand Down Expand Up @@ -99,9 +99,11 @@ export const terminatedProcesses = composeSelectors(
/**
* Returns a map of `ResolverEvent` entity_id to their related event and alert statistics
*/
export const relatedEventsStats = composeSelectors(
export const relatedEventsStats: (
state: ResolverState
) => (nodeID: string) => ResolverNodeStats | undefined = composeSelectors(
dataStateSelector,
dataSelectors.relatedEventsStats
dataSelectors.relatedEventStats
);

/**
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/security_solution/public/resolver/view/map.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ export const ResolverMap = React.memo(function ({
const { processNodePositions, connectingEdgeLineSegments } = useSelector(
selectors.visibleNodesAndEdgeLines
)(timeAtRender);
const relatedEventsStats = useSelector(selectors.relatedEventsStats);
const terminatedProcesses = useSelector(selectors.terminatedProcesses);
const { projectionMatrix, ref, onMouseDown } = useCamera();
const isLoading = useSelector(selectors.isLoading);
Expand Down Expand Up @@ -110,9 +109,6 @@ export const ResolverMap = React.memo(function ({
position={position}
projectionMatrix={projectionMatrix}
event={processEvent}
relatedEventsStatsForProcess={
relatedEventsStats ? relatedEventsStats.get(entityId(processEvent)) : undefined
}
isProcessTerminated={terminatedProcesses.has(processEntityId)}
isProcessOrigin={false}
timeAtRender={timeAtRender}
Expand Down
25 changes: 11 additions & 14 deletions x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { NodeSubMenu, subMenuAssets } from './submenu';
import { applyMatrix3 } from '../models/vector2';
import { Vector2, Matrix3 } from '../types';
import { SymbolIds, useResolverTheme, calculateResolverFontSize } from './assets';
import { ResolverEvent, ResolverNodeStats } from '../../../common/endpoint/types';
import { ResolverEvent } from '../../../common/endpoint/types';
import { useResolverDispatch } from './use_resolver_dispatch';
import * as eventModel from '../../../common/endpoint/models/event';
import * as processEventModel from '../models/process_event';
Expand Down Expand Up @@ -73,7 +73,6 @@ const UnstyledProcessEventDot = React.memo(
projectionMatrix,
isProcessTerminated,
isProcessOrigin,
relatedEventsStatsForProcess,
timeAtRender,
}: {
/**
Expand All @@ -100,12 +99,6 @@ const UnstyledProcessEventDot = React.memo(
* Whether or not to show the process as the originating event.
*/
isProcessOrigin: boolean;
/**
* A collection of events related to the current node and statistics (e.g. counts indexed by event type)
* to provide the user some visibility regarding the contents thereof.
* Statistics for the number of related events and alerts for this process node
*/
relatedEventsStatsForProcess?: ResolverNodeStats;

/**
* The time (unix epoch) at render.
Expand All @@ -128,6 +121,12 @@ const UnstyledProcessEventDot = React.memo(
const selectedDescendantId = useSelector(selectors.uiSelectedDescendantId);
const nodeID = processEventModel.uniquePidForProcess(event);

/**
* A collection of events related to the current node and statistics (e.g. counts indexed by event type)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 ResolverNodeStats does not contain a collection of events related to the current node unless I'm missing something. I'd suggest we either: 1) Change the comment here or 2) If it does have a collection of events, the selector name should be something more broad like relatedEventsAndStats

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll remove the comment. fwiw, that was the existing comment for relatedEventStatsForProcess, which contained exactly what relatedEventStats now contains.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe at one point it did have that collection with it in whatever form it was being consumed here.

* to provide the user some visibility regarding the contents thereof.
* Statistics for the number of related events and alerts for this process node
*/
const relatedEventStats = useSelector(selectors.relatedEventsStats)(nodeID);
// define a standard way of giving HTML IDs to nodes based on their entity_id/nodeID.
// this is used to link nodes via aria attributes
const nodeHTMLID = useCallback((id: string) => htmlIdGenerator(htmlIDPrefix)(`${id}:node`), [
Expand Down Expand Up @@ -270,15 +269,13 @@ const UnstyledProcessEventDot = React.memo(
const relatedEventOptions = useMemo(() => {
const relatedStatsList = [];

if (!relatedEventsStatsForProcess) {
if (!relatedEventStats) {
// Return an empty set of options if there are no stats to report
return [];
}
// If we have entries to show, map them into options to display in the selectable list

for (const [category, total] of Object.entries(
relatedEventsStatsForProcess.events.byCategory
)) {
for (const [category, total] of Object.entries(relatedEventStats.events.byCategory)) {
relatedStatsList.push({
prefix: <EuiI18nNumber value={total || 0} />,
optionTitle: category,
Expand All @@ -296,9 +293,9 @@ const UnstyledProcessEventDot = React.memo(
});
}
return relatedStatsList;
}, [relatedEventsStatsForProcess, dispatch, event, pushToQueryParams, nodeID]);
}, [relatedEventStats, dispatch, event, pushToQueryParams, nodeID]);

const relatedEventStatusOrOptions = !relatedEventsStatsForProcess
const relatedEventStatusOrOptions = !relatedEventStats
? subMenuAssets.initialMenuStatus
: relatedEventOptions;

Expand Down