Skip to content

Commit

Permalink
## Fixes an issue where the Signals count spinner can spin forever
Browse files Browse the repository at this point in the history
Per the animated gif below, in `7.6` `BC 4`, the `Signals count` spinner on the Overview page spins forever until the signals index is created (in the current Kibana space):

![signals-count-loading-spinner](https://user-images.githubusercontent.com/4459398/73785251-2ca42000-4754-11ea-8671-daa81f351c9b.gif)

The `Signals count` spinner will spin forever until the user clicks the `Detections` tab, which-in turn creates the signals index (if it doesn't exist), per the animated gif below:

![create-signals-index](https://user-images.githubusercontent.com/4459398/73785319-4ba2b200-4754-11ea-9bb0-a745a8b2be5d.gif)

This behavior is an issue because:

- When a fresh deployment is created on Elastic Cloud, a user won't understand why the `Signals count` widget is always spinning on the `Overview` page. (The user must click the `Detections` page to resolve this.)
- In deployments where authentication is disabled, or, for _reasons_, a Detections index will never be created, the `Signals count` spinner on the Overview page will always spin.

To reproduce:

1. Spin up a new `7.6` `BC 4` deployment on Elastic Cloud
2. Login to Kibana for the first time
3. Navigate to the SIEM app

**Expected result**
- All histograms on the Overview page eventually stop displaying their respective loading spinners

**Actual result**
- The `Signals count` widget spinner spins forever. (The user must click the `Detections` tab to create the signals index.)

## Deleting the signals index

To reproduce the issue above when a signals index has already been created (by clicking on the Detections tab), run the following from the Kibana `Dev Tools` `Console`:

```
DELETE /.siem-signals-default-000001
```

It is also possible to reproduce this issue by creating a new space, because it won't have a signals index.

elastic/siem-team#514
  • Loading branch information
andrew-goldstein committed Feb 4, 2020
1 parent e74ab19 commit 96316eb
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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 { showInitialLoadingSpinner } from './helpers';

describe('helpers', () => {
describe('showInitialLoadingSpinner', () => {
test('it should (only) show the spinner during initial loading, while we are fetching data', () => {
expect(showInitialLoadingSpinner({ isInitialLoading: true, isLoadingSignals: true })).toBe(
true
);
});

test('it should STOP showing the spinner (during initial loading) when the first data fetch completes', () => {
expect(showInitialLoadingSpinner({ isInitialLoading: true, isLoadingSignals: false })).toBe(
false
);
});

test('it should NOT show the spinner after initial loading has completed, even if the user requests more data (e.g. by clicking Refresh)', () => {
expect(showInitialLoadingSpinner({ isInitialLoading: false, isLoadingSignals: true })).toBe(
false
);
});

test('it should NOT show the spinner after initial loading has completed', () => {
expect(showInitialLoadingSpinner({ isInitialLoading: false, isLoadingSignals: false })).toBe(
false
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,17 @@ export const getSignalsHistogramQuery = (
},
},
});

/**
* Returns `true` when the signals histogram initial loading spinner should be shown
*
* @param isInitialLoading The loading spinner will only be displayed if this value is `true`, because after initial load, a different, non-spinner loading indicator is displayed
* @param isLoadingSignals When `true`, IO is being performed to request signals (for rendering in the histogram)
*/
export const showInitialLoadingSpinner = ({
isInitialLoading,
isLoadingSignals,
}: {
isInitialLoading: boolean;
isLoadingSignals: boolean;
}): boolean => isInitialLoading && isLoadingSignals;
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { InspectButtonContainer } from '../../../../components/inspect';
import { useQuerySignals } from '../../../../containers/detection_engine/signals/use_query';
import { MatrixLoader } from '../../../../components/matrix_histogram/matrix_loader';

import { formatSignalsData, getSignalsHistogramQuery } from './helpers';
import { formatSignalsData, getSignalsHistogramQuery, showInitialLoadingSpinner } from './helpers';
import * as i18n from './translations';

const DEFAULT_PANEL_HEIGHT = 300;
Expand Down Expand Up @@ -54,7 +54,6 @@ interface SignalsHistogramPanelProps {
from: number;
query?: Query;
legendPosition?: Position;
loadingInitial?: boolean;
panelHeight?: number;
signalIndexName: string | null;
setQuery: (params: RegisterQuery) => void;
Expand All @@ -75,7 +74,6 @@ export const SignalsHistogramPanel = memo<SignalsHistogramPanelProps>(
query,
from,
legendPosition = 'right',
loadingInitial = false,
panelHeight = DEFAULT_PANEL_HEIGHT,
setQuery,
signalIndexName,
Expand All @@ -86,7 +84,7 @@ export const SignalsHistogramPanel = memo<SignalsHistogramPanelProps>(
title = i18n.HISTOGRAM_HEADER,
updateDateRange,
}) => {
const [isInitialLoading, setIsInitialLoading] = useState(loadingInitial || true);
const [isInitialLoading, setIsInitialLoading] = useState(true);
const [defaultNumberFormat] = useUiSetting$<string>(DEFAULT_NUMBER_FORMAT);
const [totalSignalsObj, setTotalSignalsObj] = useState<SignalsTotal>(defaultTotalSignalsObj);
const [selectedStackByOption, setSelectedStackByOption] = useState<SignalsHistogramOption>(
Expand Down Expand Up @@ -124,10 +122,16 @@ export const SignalsHistogramPanel = memo<SignalsHistogramPanelProps>(
const formattedSignalsData = useMemo(() => formatSignalsData(signalsData), [signalsData]);

useEffect(() => {
if (!loadingInitial && isInitialLoading && !isLoadingSignals && signalsData) {
let canceled = false;

if (!canceled && !showInitialLoadingSpinner({ isInitialLoading, isLoadingSignals })) {
setIsInitialLoading(false);
}
}, [loadingInitial, isLoadingSignals, signalsData]);

return () => {
canceled = true; // prevent long running data fetches from updating state after unmounting
};
}, [isInitialLoading, isLoadingSignals, setIsInitialLoading]);

useEffect(() => {
return () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ const DetectionEnginePageComponent: React.FC<DetectionEnginePageComponentProps>
deleteQuery={deleteQuery}
filters={filters}
from={from}
loadingInitial={loading}
query={query}
setQuery={setQuery}
showTotalSignalsCount={true}
Expand Down

0 comments on commit 96316eb

Please sign in to comment.