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

[App Search] API logs: Add log detail flyout #96162

Merged
merged 11 commits into from
Apr 5, 2021
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export const mockApiLog = {
timestamp: '1970-01-01T12:00:00.000Z',
http_method: 'POST',
status: 200,
user_agent: 'Mozilla/5.0',
full_request_path: '/api/as/v1/engines/national-parks-demo/search.json',
request_body: '{"query":"test search"}',
response_body:
'{"meta":{"page":{"current":1,"total_pages":0,"total_results":0,"size":20}},"results":[]}',
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { setMockValues, setMockActions } from '../../../../__mocks__';
import { mockApiLog } from '../__mocks__/api_log.mock';

import React from 'react';

import { shallow } from 'enzyme';

import { EuiFlyout, EuiBadge } from '@elastic/eui';

import { ApiLogFlyout, ApiLogHeading } from './api_log_flyout';

describe('ApiLogFlyout', () => {
const values = {
isFlyoutOpen: true,
apiLog: mockApiLog,
};
const actions = {
closeFlyout: jest.fn(),
};

beforeEach(() => {
jest.clearAllMocks();
setMockValues(values);
setMockActions(actions);
});

it('renders', () => {
const wrapper = shallow(<ApiLogFlyout />);

expect(wrapper.find('h2').text()).toEqual('Request details');
expect(wrapper.find(ApiLogHeading).last().dive().find('h3').text()).toEqual('Response body');
expect(wrapper.find(EuiBadge).prop('children')).toEqual('POST');
});

it('closes the flyout', () => {
const wrapper = shallow(<ApiLogFlyout />);

wrapper.find(EuiFlyout).simulate('close');
expect(actions.closeFlyout).toHaveBeenCalled();
});

it('does not render if the flyout is not open', () => {
setMockValues({ ...values, isFlyoutOpen: false });
const wrapper = shallow(<ApiLogFlyout />);

expect(wrapper.isEmptyRender()).toBe(true);
});

it('does not render if a current apiLog has not been set', () => {
setMockValues({ ...values, apiLog: null });
const wrapper = shallow(<ApiLogFlyout />);

expect(wrapper.isEmptyRender()).toBe(true);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';

import { useActions, useValues } from 'kea';

import {
EuiPortal,
EuiFlyout,
EuiFlyoutHeader,
EuiTitle,
EuiFlyoutBody,
EuiFlexGroup,
EuiFlexItem,
EuiSpacer,
EuiBadge,
EuiHealth,
EuiText,
EuiCode,
EuiCodeBlock,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { getStatusColor, safeJsonParseAndStringify } from '../utils';

import { ApiLogLogic } from './';

export const ApiLogFlyout: React.FC = () => {
const { isFlyoutOpen, apiLog } = useValues(ApiLogLogic);
const { closeFlyout } = useActions(ApiLogLogic);

if (!isFlyoutOpen) return null;
if (!apiLog) return null;

return (
<EuiPortal>
<EuiFlyout ownFocus onClose={closeFlyout} aria-labelledby="apiLogFlyout">
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h2 id="apiLogFlyout">
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.flyout.title', {
defaultMessage: 'Request details',
})}
</h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
<EuiFlexGroup>
<EuiFlexItem>
<ApiLogHeading>
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.methodTitle', {
defaultMessage: 'Method',
})}
</ApiLogHeading>
<div>
<EuiBadge color="primary">{apiLog.http_method}</EuiBadge>
</div>
</EuiFlexItem>
<EuiFlexItem>
<ApiLogHeading>
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.statusTitle', {
defaultMessage: 'Status',
})}
</ApiLogHeading>
<EuiHealth color={getStatusColor(apiLog.status)}>{apiLog.status}</EuiHealth>
</EuiFlexItem>
<EuiFlexItem>
<ApiLogHeading>
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.timestampTitle', {
defaultMessage: 'Timestamp',
})}
</ApiLogHeading>
{apiLog.timestamp}
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer />

<ApiLogHeading>
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.userAgentTitle', {
defaultMessage: 'User agent',
})}
</ApiLogHeading>
<EuiText>
<EuiCode>{apiLog.user_agent}</EuiCode>
</EuiText>
<EuiSpacer />

<ApiLogHeading>
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.requestPathTitle', {
defaultMessage: 'Request path',
})}
</ApiLogHeading>
<EuiText>
<EuiCode>{apiLog.full_request_path}</EuiCode>
</EuiText>
<EuiSpacer />

<ApiLogHeading>
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.requestBodyTitle', {
defaultMessage: 'Request body',
})}
</ApiLogHeading>
<EuiCodeBlock language="json" paddingSize="m">
{safeJsonParseAndStringify(apiLog.request_body)}
</EuiCodeBlock>
<EuiSpacer />

<ApiLogHeading>
{i18n.translate('xpack.enterpriseSearch.appSearch.engine.apiLogs.responseBodyTitle', {
defaultMessage: 'Response body',
})}
</ApiLogHeading>
<EuiCodeBlock language="json" paddingSize="m">
{safeJsonParseAndStringify(apiLog.response_body)}
</EuiCodeBlock>
</EuiFlyoutBody>
</EuiFlyout>
</EuiPortal>
);
};

export const ApiLogHeading: React.FC = ({ children }) => (
<EuiTitle size="xs">
<h3>{children}</h3>
</EuiTitle>
);
Comment on lines +133 to +137
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is also probably a little extra but I figured if Davey ever wants to change the size of all the titles at once this would make it easier? lol.

That being said I can get rid of this if people aren't a fan, just let me know

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't be my style but doesn't offend me

Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { LogicMounter } from '../../../../__mocks__';
import { mockApiLog } from '../__mocks__/api_log.mock';

import { ApiLogLogic } from './';

describe('ApiLogLogic', () => {
const { mount } = new LogicMounter(ApiLogLogic);

const DEFAULT_VALUES = {
isFlyoutOpen: false,
apiLog: null,
};

beforeEach(() => {
jest.clearAllMocks();
});

it('has expected default values', () => {
mount();
expect(ApiLogLogic.values).toEqual(DEFAULT_VALUES);
});

describe('actions', () => {
describe('openFlyout', () => {
it('sets isFlyoutOpen to true & sets the current apiLog', () => {
mount({ isFlyoutOpen: false, apiLog: null });
ApiLogLogic.actions.openFlyout(mockApiLog);

expect(ApiLogLogic.values).toEqual({
...DEFAULT_VALUES,
isFlyoutOpen: true,
apiLog: mockApiLog,
});
});
});

describe('closeFlyout', () => {
it('sets isFlyoutOpen to false & resets the current apiLog', () => {
mount({ isFlyoutOpen: true, apiLog: mockApiLog });
ApiLogLogic.actions.closeFlyout();

expect(ApiLogLogic.values).toEqual({
...DEFAULT_VALUES,
isFlyoutOpen: false,
apiLog: null,
});
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { kea, MakeLogicType } from 'kea';

import { ApiLog } from '../types';

interface ApiLogValues {
isFlyoutOpen: boolean;
apiLog: ApiLog | null;
}

interface ApiLogActions {
openFlyout(apiLog: ApiLog): { apiLog: ApiLog };
closeFlyout(): void;
}

export const ApiLogLogic = kea<MakeLogicType<ApiLogValues, ApiLogActions>>({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe a little extra to split this out from the ApiLogsLogic, but I'm a fan of modularity, and arguably ApiLogsLogic is complicated enough w/ the extra polling business logic to be kept as focused on polling as possible

path: ['enterprise_search', 'app_search', 'api_log_logic'],
actions: () => ({
openFlyout: (apiLog) => ({ apiLog }),
closeFlyout: true,
}),
reducers: () => ({
isFlyoutOpen: [
false,
{
openFlyout: () => true,
closeFlyout: () => false,
},
],
apiLog: [
null,
{
openFlyout: (_, { apiLog }) => apiLog,
closeFlyout: () => null,
},
],
}),
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export { ApiLogFlyout } from './api_log_flyout';
export { ApiLogLogic } from './api_log_logic';
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { Loading } from '../../../shared/loading';

import { LogRetentionCallout, LogRetentionTooltip, LogRetentionOptions } from '../log_retention';

import { ApiLogFlyout } from './api_log';
import { ApiLogsTable, NewApiEventsPrompt } from './components';
import { API_LOGS_TITLE, RECENT_API_EVENTS } from './constants';

Expand Down Expand Up @@ -75,6 +76,7 @@ export const ApiLogs: React.FC<Props> = ({ engineBreadcrumb }) => {
<EuiSpacer size="m" />

<ApiLogsTable hasPagination />
<ApiLogFlyout />
Comment on lines 78 to +79
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 debated putting ApiLogFlyout within ApiLogsTable but opted not to in the end. I don't feel super strongly either way, but I landed on this side of the fence because:

  • I like our <Table /> components just being self contained modular tables/components, and it definitely simplifies tests for the table component
  • It's nice having an 'overview' of what's on the page from the view itself

That being said in the end I don't think it super matters either way so happy to be convinced otherwise

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree it should be separate from ApiLogsTable

</EuiPageContentBody>
</EuiPageContent>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { LogicMounter, mockHttpValues, mockFlashMessageHelpers } from '../../../__mocks__';
import { mockApiLog } from './__mocks__/api_log.mock';
import '../../__mocks__/engine_logic.mock';

import { nextTick } from '@kbn/test/jest';
Expand All @@ -29,17 +30,7 @@ describe('ApiLogsLogic', () => {
};

const MOCK_API_RESPONSE = {
results: [
{
timestamp: '1970-01-01T12:00:00.000Z',
http_method: 'POST',
status: 200,
user_agent: 'some browser agent string',
full_request_path: '/api/as/v1/engines/national-parks-demo/search.json',
request_body: '{"someMockRequest":"hello"}',
response_body: '{"someMockResponse":"world"}',
},
],
results: [mockApiLog, mockApiLog],
meta: {
page: {
current: 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ describe('ApiLogsTable', () => {
};
const actions = {
onPaginate: jest.fn(),
openFlyout: jest.fn(),
};

beforeEach(() => {
Expand Down Expand Up @@ -86,7 +87,7 @@ describe('ApiLogsTable', () => {

expect(wrapper.find(EuiButtonEmpty)).toHaveLength(3);
wrapper.find('[data-test-subj="ApiLogsTableDetailsButton"]').first().simulate('click');
// TODO: API log details flyout
expect(actions.openFlyout).toHaveBeenCalled();
Comment on lines 88 to +90
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow-up to #96008 (comment) - @JasonStoltz feel free to yell if you'd like this to be in its own test block and not in the main renders test 🤔

});

it('renders an empty prompt if no items are passed', () => {
Expand Down
Loading