Skip to content

Commit

Permalink
[SecuritySolution] Histogram IP legends error fixed (#99468)
Browse files Browse the repository at this point in the history
* make sure stackByField exists

* fix types

* fix unit test

* skip extra request for non-ip queries

* elasticserach query changes to prevent corrupted data response bug

* client changes to split ip stacked histogram queries in two, inspect modal shows all requests and responses

* lint fixes

* test for useMatrixHistogramCombined added

* comment added on new multiple prop

* changed query to always contain value_type:ip for ip queries

Co-authored-by: Angela Chuang <yi-chun.chuang@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
3 people authored May 10, 2021
1 parent 0ffe4c7 commit 518da5b
Show file tree
Hide file tree
Showing 17 changed files with 510 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export interface MatrixHistogramRequestOptions extends RequestBasicOptions {
| undefined;
inspect?: Maybe<Inspect>;
isPtrIncluded?: boolean;
includeMissingData?: boolean;
}

export interface MatrixHistogramStrategyResponse extends IEsSearchResponse {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export interface HeaderSectionProps extends HeaderProps {
titleSize?: EuiTitleSize;
tooltip?: string;
growLeftSplit?: boolean;
inspectMultiple?: boolean;
}

const HeaderSectionComponent: React.FC<HeaderSectionProps> = ({
Expand All @@ -60,6 +61,7 @@ const HeaderSectionComponent: React.FC<HeaderSectionProps> = ({
titleSize = 'm',
tooltip,
growLeftSplit = true,
inspectMultiple = false,
}) => (
<Header data-test-subj="header-section" border={border} height={height}>
<EuiFlexGroup alignItems="center">
Expand All @@ -83,7 +85,7 @@ const HeaderSectionComponent: React.FC<HeaderSectionProps> = ({

{id && (
<EuiFlexItem grow={false}>
<InspectButton queryId={id} inspectIndex={0} title={title} />
<InspectButton queryId={id} multiple={inspectMultiple} title={title} />
</EuiFlexItem>
)}
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ interface OwnProps {
isDisabled?: boolean;
onCloseInspect?: () => void;
title: string | React.ReactElement | React.ReactNode;
multiple?: boolean;
}

type InspectButtonProps = OwnProps & PropsFromRedux;
Expand All @@ -71,6 +72,7 @@ const InspectButtonComponent: React.FC<InspectButtonProps> = ({
isInspected,
loading,
inspectIndex = 0,
multiple = false, // If multiple = true we ignore the inspectIndex and pass all requests and responses to the inspect modal
onCloseInspect,
queryId = '',
selectedInspectIndex,
Expand Down Expand Up @@ -99,6 +101,26 @@ const InspectButtonComponent: React.FC<InspectButtonProps> = ({
});
}, [onCloseInspect, setIsInspected, queryId, inputId, inspectIndex]);

let request: string | null = null;
let additionalRequests: string[] | null = null;
if (inspect != null && inspect.dsl.length > 0) {
if (multiple) {
[request, ...additionalRequests] = inspect.dsl;
} else {
request = inspect.dsl[inspectIndex];
}
}

let response: string | null = null;
let additionalResponses: string[] | null = null;
if (inspect != null && inspect.response.length > 0) {
if (multiple) {
[response, ...additionalResponses] = inspect.response;
} else {
response = inspect.response[inspectIndex];
}
}

return (
<>
{inputId === 'timeline' && !compact && (
Expand Down Expand Up @@ -131,10 +153,10 @@ const InspectButtonComponent: React.FC<InspectButtonProps> = ({
<ModalInspectQuery
closeModal={handleCloseModal}
isShowing={isShowingModal}
request={inspect != null && inspect.dsl.length > 0 ? inspect.dsl[inspectIndex] : null}
response={
inspect != null && inspect.response.length > 0 ? inspect.response[inspectIndex] : null
}
request={request}
response={response}
additionalRequests={additionalRequests}
additionalResponses={additionalResponses}
title={title}
data-test-subj="inspect-modal"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
EuiTabbedContent,
} from '@elastic/eui';
import numeral from '@elastic/numeral';
import React, { ReactNode } from 'react';
import React, { Fragment, ReactNode } from 'react';
import styled from 'styled-components';

import { NO_ALERT_INDEX } from '../../../../common/constants';
Expand All @@ -44,6 +44,8 @@ interface ModalInspectProps {
isShowing: boolean;
request: string | null;
response: string | null;
additionalRequests?: string[] | null;
additionalResponses?: string[] | null;
title: string | React.ReactElement | React.ReactNode;
}

Expand Down Expand Up @@ -73,11 +75,11 @@ const MyEuiModal = styled(EuiModal)`
`;

MyEuiModal.displayName = 'MyEuiModal';
const parseInspectString = function <T>(objectStringify: string): T | null {
const parseInspectStrings = function <T>(stringsArray: string[]): T[] {
try {
return JSON.parse(objectStringify);
return stringsArray.map((objectStringify) => JSON.parse(objectStringify));
} catch {
return null;
return [];
}
};

Expand All @@ -103,13 +105,23 @@ export const ModalInspectQuery = ({
isShowing = false,
request,
response,
additionalRequests,
additionalResponses,
title,
}: ModalInspectProps) => {
if (!isShowing || request == null || response == null) {
return null;
}
const inspectRequest: Request | null = parseInspectString(request);
const inspectResponse: Response | null = parseInspectString(response);

const requests: string[] = [request, ...(additionalRequests != null ? additionalRequests : [])];
const responses: string[] = [
response,
...(additionalResponses != null ? additionalResponses : []),
];

const inspectRequests: Request[] = parseInspectStrings(requests);
const inspectResponses: Response[] = parseInspectStrings(responses);

const statistics: Array<{
title: NonNullable<ReactNode | string>;
description: NonNullable<ReactNode | string>;
Expand All @@ -123,7 +135,7 @@ export const ModalInspectQuery = ({
),
description: (
<span data-test-subj="index-pattern-description">
{formatIndexPatternRequested(inspectRequest?.index ?? [])}
{formatIndexPatternRequested(inspectRequests[0]?.index ?? [])}
</span>
),
},
Expand All @@ -137,8 +149,8 @@ export const ModalInspectQuery = ({
),
description: (
<span data-test-subj="query-time-description">
{inspectResponse != null
? `${numeral(inspectResponse.took).format('0,0')}ms`
{inspectResponses[0]?.took
? `${numeral(inspectResponses[0].took).format('0,0')}ms`
: i18n.SOMETHING_WENT_WRONG}
</span>
),
Expand Down Expand Up @@ -170,42 +182,50 @@ export const ModalInspectQuery = ({
{
id: 'request',
name: 'Request',
content: (
<>
<EuiSpacer />
<EuiCodeBlock
language="js"
fontSize="m"
paddingSize="m"
color="dark"
overflowHeight={300}
isCopyable
>
{inspectRequest != null
? manageStringify(inspectRequest.body)
: i18n.SOMETHING_WENT_WRONG}
</EuiCodeBlock>
</>
),
content:
inspectRequests.length > 0 ? (
inspectRequests.map((inspectRequest, index) => (
<Fragment key={index}>
<EuiSpacer />
<EuiCodeBlock
language="js"
fontSize="m"
paddingSize="m"
color="dark"
overflowHeight={300}
isCopyable
>
{manageStringify(inspectRequest.body)}
</EuiCodeBlock>
</Fragment>
))
) : (
<EuiCodeBlock>{i18n.SOMETHING_WENT_WRONG}</EuiCodeBlock>
),
},
{
id: 'response',
name: 'Response',
content: (
<>
<EuiSpacer />
<EuiCodeBlock
language="js"
fontSize="m"
paddingSize="m"
color="dark"
overflowHeight={300}
isCopyable
>
{response}
</EuiCodeBlock>
</>
),
content:
inspectResponses.length > 0 ? (
responses.map((responseText, index) => (
<Fragment key={index}>
<EuiSpacer />
<EuiCodeBlock
language="js"
fontSize="m"
paddingSize="m"
color="dark"
overflowHeight={300}
isCopyable
>
{responseText}
</EuiCodeBlock>
</Fragment>
))
) : (
<EuiCodeBlock>{i18n.SOMETHING_WENT_WRONG}</EuiCodeBlock>
),
},
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { mount, ReactWrapper } from 'enzyme';
import React from 'react';

import { MatrixHistogram } from '.';
import { useMatrixHistogram } from '../../containers/matrix_histogram';
import { useMatrixHistogramCombined } from '../../containers/matrix_histogram';
import { MatrixHistogramType } from '../../../../common/search_strategy/security_solution';
import { TestProviders } from '../../mock';

Expand All @@ -30,7 +30,7 @@ jest.mock('../charts/barchart', () => ({
}));

jest.mock('../../containers/matrix_histogram', () => ({
useMatrixHistogram: jest.fn(),
useMatrixHistogramCombined: jest.fn(),
}));

jest.mock('../../components/matrix_histogram/utils', () => ({
Expand Down Expand Up @@ -63,7 +63,7 @@ describe('Matrix Histogram Component', () => {
};

beforeAll(() => {
(useMatrixHistogram as jest.Mock).mockReturnValue([
(useMatrixHistogramCombined as jest.Mock).mockReturnValue([
false,
{
data: null,
Expand All @@ -75,6 +75,7 @@ describe('Matrix Histogram Component', () => {
wrappingComponent: TestProviders,
});
});

describe('on initial load', () => {
test('it renders MatrixLoader', () => {
expect(wrapper.find('MatrixLoader').exists()).toBe(true);
Expand All @@ -99,7 +100,7 @@ describe('Matrix Histogram Component', () => {

describe('not initial load', () => {
beforeAll(() => {
(useMatrixHistogram as jest.Mock).mockReturnValue([
(useMatrixHistogramCombined as jest.Mock).mockReturnValue([
false,
{
data: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { HeaderSection } from '../header_section';
import { MatrixLoader } from './matrix_loader';
import { Panel } from '../panel';
import { getBarchartConfigs, getCustomChartData } from './utils';
import { useMatrixHistogram } from '../../containers/matrix_histogram';
import { useMatrixHistogramCombined } from '../../containers/matrix_histogram';
import { MatrixHistogramProps, MatrixHistogramOption, MatrixHistogramQueryProps } from './types';
import { InspectButtonContainer } from '../inspect';
import { MatrixHistogramType } from '../../../../common/search_strategy/security_solution';
Expand All @@ -40,6 +40,7 @@ export type MatrixHistogramComponentProps = MatrixHistogramProps &
id: string;
legendPosition?: Position;
mapping?: MatrixHistogramMappingTypes;
onError?: () => void;
showSpacer?: boolean;
setQuery: GlobalTimeArgs['setQuery'];
setAbsoluteRangeDatePickerTarget?: InputsModelId;
Expand Down Expand Up @@ -77,6 +78,7 @@ export const MatrixHistogramComponent: React.FC<MatrixHistogramComponentProps> =
isPtrIncluded,
legendPosition,
mapping,
onError,
panelHeight = DEFAULT_PANEL_HEIGHT,
setAbsoluteRangeDatePickerTarget = 'global',
setQuery,
Expand Down Expand Up @@ -133,17 +135,22 @@ export const MatrixHistogramComponent: React.FC<MatrixHistogramComponentProps> =
[defaultStackByOption, stackByOptions]
);

const [loading, { data, inspect, totalCount, refetch }] = useMatrixHistogram({
const matrixHistogramRequest = {
endDate,
errorMessage,
filterQuery,
histogramType,
indexNames,
onError,
startDate,
stackByField: selectedStackByOption.value,
isPtrIncluded,
docValueFields,
});
};

const [loading, { data, inspect, totalCount, refetch }] = useMatrixHistogramCombined(
matrixHistogramRequest
);

const titleWithStackByField = useMemo(
() => (title != null && typeof title === 'function' ? title(selectedStackByOption) : title),
Expand Down Expand Up @@ -208,6 +215,7 @@ export const MatrixHistogramComponent: React.FC<MatrixHistogramComponentProps> =
title={titleWithStackByField}
titleSize={titleSize}
subtitle={subtitleWithCounts}
inspectMultiple
>
<EuiFlexGroup alignItems="center" gutterSize="none">
<EuiFlexItem grow={false}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export interface MatrixHistogramQueryProps {
errorMessage: string;
indexNames: string[];
filterQuery?: ESQuery | string | undefined;
onError?: () => void;
setAbsoluteRangeDatePicker?: ActionCreator<{
id: InputsModelId;
from: string;
Expand All @@ -78,6 +79,7 @@ export interface MatrixHistogramQueryProps {
threshold?: Threshold;
skip?: boolean;
isPtrIncluded?: boolean;
includeMissingData?: boolean;
}

export interface MatrixHistogramProps extends MatrixHistogramBasicProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ describe('utils', () => {
expect(result).toEqual([]);
});

test('shoule format data correctly', () => {
test('should format data correctly', () => {
const data = [
{ x: 1, y: 2, g: 'g1' },
{ x: 2, y: 4, g: 'g1' },
Expand All @@ -120,6 +120,7 @@ describe('utils', () => {
expect(result).toEqual([
{
key: 'g1',
color: '#1EA593',
value: [
{ x: 1, y: 2, g: 'g1' },
{ x: 2, y: 4, g: 'g1' },
Expand All @@ -128,6 +129,7 @@ describe('utils', () => {
},
{
key: 'g2',
color: '#2B70F7',
value: [
{ x: 1, y: 1, g: 'g2' },
{ x: 2, y: 3, g: 'g2' },
Expand Down
Loading

0 comments on commit 518da5b

Please sign in to comment.