Skip to content

Commit

Permalink
[SearchProfiler] Fix handling of bad profile data and update tab beha…
Browse files Browse the repository at this point in the history
…viour (#55806)

* Fix searchprofiler's ability to handle badly formed profile data
Also fix tab changing upon subsequent requests

* Fix comment typo
  • Loading branch information
jloleysens authored Jan 24, 2020
1 parent 2eb6a2f commit b8f7748
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,16 @@ describe('ProfileTree', () => {
const init = registerTestBed(ProfileTree);
await init(props);
});

it('does not throw despite bad profile data', async () => {
// For now, ignore the console.error that gets logged.
const props: Props = {
onHighlight: () => {},
target: 'searches',
data: [{}] as any,
};

const init = registerTestBed(ProfileTree);
await init(props);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';
import { IndexDetails } from './index_details';
import { ShardDetails } from './shard_details';
import { initDataFor } from './init_data';
import { Targets, ShardSerialized } from '../../types';
import { Targets, ShardSerialized, Index } from '../../types';
import { HighlightContextProvider, OnHighlightChangeArgs } from './highlight_context';

export interface Props {
Expand All @@ -24,7 +24,14 @@ export const ProfileTree = memo(({ data, target, onHighlight }: Props) => {
return null;
}

const sortedIndices = initDataFor(target)(data);
let sortedIndices: Index[];
try {
sortedIndices = initDataFor(target)(data);
} catch (e) {
// eslint-disable-next-line no-console
console.error(e);
return null;
}

return (
<HighlightContextProvider onHighlight={onHighlight}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,10 @@ import {
import { useAppContext } from '../../contexts/app_context';

import { EmptyTreePlaceHolder, ProfileLoadingPlaceholder } from './components';
import { Targets, ShardSerialized } from '../../types';
import { Targets } from '../../types';
import { useProfilerActionContext, useProfilerReadContext } from '../../contexts/profiler_context';

function hasSearch(profileResponse: ShardSerialized[]) {
const aggs = _.get(profileResponse, '[0].searches', []);
return aggs.length > 0;
}

function hasAggregations(profileResponse: ShardSerialized[]) {
const aggs = _.get(profileResponse, '[0].aggregations', []);
return aggs.length > 0;
}
import { hasAggregations, hasSearch } from '../../utils';

export const Main = () => {
const { getLicenseStatus } = useAppContext();
Expand Down
23 changes: 21 additions & 2 deletions x-pack/plugins/searchprofiler/public/application/store/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { State } from './store';

import { OnHighlightChangeArgs } from '../components/profile_tree';
import { ShardSerialized, Targets } from '../types';
import { hasSearch, hasAggregations } from '../utils';

export type Action =
| { type: 'setProfiling'; value: boolean }
Expand Down Expand Up @@ -54,8 +55,26 @@ export const reducer: Reducer<State, Action> = (state, action) => {
if (action.type === 'setCurrentResponse') {
nextState.currentResponse = action.value;
if (nextState.currentResponse) {
// Default to the searches tab
nextState.activeTab = 'searches';
const currentResponseHasAggregations = hasAggregations(nextState.currentResponse);
const currentResponseHasSearch = hasSearch(nextState.currentResponse);
if (
nextState.activeTab === 'searches' &&
!currentResponseHasSearch &&
currentResponseHasAggregations
) {
nextState.activeTab = 'aggregations';
} else if (
nextState.activeTab === 'aggregations' &&
!currentResponseHasAggregations &&
currentResponseHasSearch
) {
nextState.activeTab = 'searches';
} else if (!nextState.activeTab) {
// Default to searches tab
nextState.activeTab = 'searches';
}
} else {
nextState.activeTab = null;
}
return nextState;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { get } from 'lodash';
import { ShardSerialized } from '../types';

export function hasAggregations(profileResponse: ShardSerialized[]) {
const aggs = get(profileResponse, '[0].aggregations', []);
return aggs.length > 0;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { get } from 'lodash';
import { ShardSerialized } from '../types';

export function hasSearch(profileResponse: ShardSerialized[]) {
const aggs = get(profileResponse, '[0].searches', []);
return aggs.length > 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export { hasAggregations } from './has_aggregations';
export { hasSearch } from './has_searches';
export { checkForParseErrors } from './check_for_json_errors';
export { msToPretty } from './ms_to_pretty';
export { nsToPretty } from './ns_to_pretty';

0 comments on commit b8f7748

Please sign in to comment.