Tracing: Fix incorrect indentations due to reoccurring spanIDs (#41919)

* Create unique keys for trace rows

* Add tests

* Fix tests

* Update public/app/features/explore/TraceView/TraceView.test.tsx

* Trigger Build
pull/41416/head
Ivana Huckova 4 years ago committed by GitHub
parent 47a7477cff
commit d4ddd2373b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 26
      packages/jaeger-ui-components/src/TraceTimelineViewer/VirtualizedTraceView.test.js
  2. 9
      packages/jaeger-ui-components/src/TraceTimelineViewer/VirtualizedTraceView.tsx
  3. 20
      public/app/features/explore/TraceView/TraceView.test.tsx
  4. 19
      public/app/features/explore/TraceView/TraceView.tsx
  5. 12
      public/app/features/explore/TraceView/useDetailState.test.ts
  6. 3
      public/app/features/explore/TraceView/useDetailState.ts

@ -65,7 +65,7 @@ describe('<VirtualizedTraceViewImpl>', () => {
const spans = [ const spans = [
trace.spans[0], trace.spans[0],
// this span is condidered to have collapsed children // this span is condidered to have collapsed children
{ spanID: newSpanID, depth: 1 }, { spanID: newSpanID, depth: 1, traceID: trace.traceID },
// these two "spans" are children and should be hidden // these two "spans" are children and should be hidden
{ depth: 2 }, { depth: 2 },
{ depth: 3 }, { depth: 3 },
@ -227,20 +227,20 @@ describe('<VirtualizedTraceViewImpl>', () => {
} }
it('works when nothing is expanded or collapsed', () => { it('works when nothing is expanded or collapsed', () => {
verify(0, `${trace.spans[0].spanID}--bar`); verify(0, `${trace.spans[0].traceID}--${trace.spans[0].spanID}--bar`);
}); });
it('works when rows are expanded', () => { it('works when rows are expanded', () => {
expandRow(1); expandRow(1);
verify(1, `${trace.spans[1].spanID}--bar`); verify(1, `${trace.spans[1].traceID}--${trace.spans[1].spanID}--bar`);
verify(2, `${trace.spans[1].spanID}--detail`); verify(2, `${trace.spans[1].traceID}--${trace.spans[1].spanID}--detail`);
verify(3, `${trace.spans[2].spanID}--bar`); verify(3, `${trace.spans[2].traceID}--${trace.spans[2].spanID}--bar`);
}); });
it('works when a parent span is collapsed', () => { it('works when a parent span is collapsed', () => {
const spans = addSpansAndCollapseTheirParent(); const spans = addSpansAndCollapseTheirParent();
verify(1, `${spans[1].spanID}--bar`); verify(1, `${spans[1].traceID}--${spans[1].spanID}--bar`);
verify(2, `${spans[4].spanID}--bar`); verify(2, `${spans[4].traceID}--${spans[4].spanID}--bar`);
}); });
}); });
@ -250,20 +250,20 @@ describe('<VirtualizedTraceViewImpl>', () => {
} }
it('works when nothing is expanded or collapsed', () => { it('works when nothing is expanded or collapsed', () => {
verify(`${trace.spans[0].spanID}--bar`, 0); verify(`${trace.traceID}--${trace.spans[0].spanID}--bar`, 0);
}); });
it('works when rows are expanded', () => { it('works when rows are expanded', () => {
expandRow(1); expandRow(1);
verify(`${trace.spans[1].spanID}--bar`, 1); verify(`${trace.spans[1].traceID}--${trace.spans[1].spanID}--bar`, 1);
verify(`${trace.spans[1].spanID}--detail`, 2); verify(`${trace.spans[1].traceID}--${trace.spans[1].spanID}--detail`, 2);
verify(`${trace.spans[2].spanID}--bar`, 3); verify(`${trace.spans[2].traceID}--${trace.spans[2].spanID}--bar`, 3);
}); });
it('works when a parent span is collapsed', () => { it('works when a parent span is collapsed', () => {
const spans = addSpansAndCollapseTheirParent(); const spans = addSpansAndCollapseTheirParent();
verify(`${spans[1].spanID}--bar`, 1); verify(`${spans[1].traceID}--${spans[1].spanID}--bar`, 1);
verify(`${spans[4].spanID}--bar`, 2); verify(`${spans[4].traceID}--${spans[4].spanID}--bar`, 2);
}); });
}); });

@ -286,17 +286,18 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
// https://github.com/facebook/flow/issues/3076#issuecomment-290944051 // https://github.com/facebook/flow/issues/3076#issuecomment-290944051
getKeyFromIndex = (index: number) => { getKeyFromIndex = (index: number) => {
const { isDetail, span } = this.getRowStates()[index]; const { isDetail, span } = this.getRowStates()[index];
return `${span.spanID}--${isDetail ? 'detail' : 'bar'}`; return `${span.traceID}--${span.spanID}--${isDetail ? 'detail' : 'bar'}`;
}; };
getIndexFromKey = (key: string) => { getIndexFromKey = (key: string) => {
const parts = key.split('--'); const parts = key.split('--');
const _spanID = parts[0]; const _traceID = parts[0];
const _isDetail = parts[1] === 'detail'; const _spanID = parts[1];
const _isDetail = parts[2] === 'detail';
const max = this.getRowStates().length; const max = this.getRowStates().length;
for (let i = 0; i < max; i++) { for (let i = 0; i < max; i++) {
const { span, isDetail } = this.getRowStates()[i]; const { span, isDetail } = this.getRowStates()[i];
if (span.spanID === _spanID && isDetail === _isDetail) { if (span.spanID === _spanID && span.traceID === _traceID && isDetail === _isDetail) {
return i; return i;
} }
} }

@ -143,6 +143,26 @@ describe('TraceView', () => {
table = screen.getByText('', { selector: 'div[data-test-id="KeyValueTable"]' }); table = screen.getByText('', { selector: 'div[data-test-id="KeyValueTable"]' });
expect(table.innerHTML).toContain('client-uuid-3'); expect(table.innerHTML).toContain('client-uuid-3');
}); });
it('resets detail view for new trace with the identical spanID', () => {
const store = configureStore();
const { rerender } = render(
<Provider store={store}>
<TraceView exploreId={ExploreId.left} dataFrames={[frameOld]} splitOpenFn={() => {}} />
</Provider>
);
const span = screen.getAllByText('', { selector: 'div[data-test-id="span-view"]' })[2];
userEvent.click(span);
//Process is in detail view
expect(screen.getByText(/Process/)).toBeInTheDocument();
rerender(
<Provider store={store}>
<TraceView exploreId={ExploreId.left} dataFrames={[frameNew]} splitOpenFn={() => {}} />
</Provider>
);
expect(screen.queryByText(/Process/)).not.toBeInTheDocument();
});
}); });
const response: TraceData & { spans: TraceSpanData[] } = { const response: TraceData & { spans: TraceSpanData[] } = {

@ -18,7 +18,9 @@ import { getDatasourceSrv } from 'app/features/plugins/datasource_srv';
import { getTimeZone } from 'app/features/profile/state/selectors'; import { getTimeZone } from 'app/features/profile/state/selectors';
import { StoreState } from 'app/types'; import { StoreState } from 'app/types';
import { ExploreId } from 'app/types/explore'; import { ExploreId } from 'app/types/explore';
import React, { useCallback, useMemo, useState } from 'react'; import { isEqual } from 'lodash';
import React, { useCallback, useMemo, useState, useEffect } from 'react';
import { usePrevious } from 'react-use';
import { useSelector } from 'react-redux'; import { useSelector } from 'react-redux';
import { createSpanLinkFactory } from './createSpanLink'; import { createSpanLinkFactory } from './createSpanLink';
import { UIElements } from './uiElements'; import { UIElements } from './uiElements';
@ -39,6 +41,10 @@ type Props = {
}; };
export function TraceView(props: Props) { export function TraceView(props: Props) {
// At this point we only show single trace
const frame = props.dataFrames[0];
const prevFrame = usePrevious(frame);
const { expandOne, collapseOne, childrenToggle, collapseAll, childrenHiddenIDs, expandAll } = useChildrenState(); const { expandOne, collapseOne, childrenToggle, collapseAll, childrenHiddenIDs, expandAll } = useChildrenState();
const { const {
detailStates, detailStates,
@ -50,7 +56,16 @@ export function TraceView(props: Props) {
detailTagsToggle, detailTagsToggle,
detailWarningsToggle, detailWarningsToggle,
detailStackTracesToggle, detailStackTracesToggle,
clearDetailStates,
} = useDetailState(); } = useDetailState();
// Clear detail state when new trace arrives
useEffect(() => {
if (frame && prevFrame && !isEqual(prevFrame, frame)) {
clearDetailStates();
}
}, [frame, prevFrame, clearDetailStates]);
const { removeHoverIndentGuideId, addHoverIndentGuideId, hoverIndentGuideIds } = useHoverIndentGuide(); const { removeHoverIndentGuideId, addHoverIndentGuideId, hoverIndentGuideIds } = useHoverIndentGuide();
const { viewRange, updateViewRangeTime, updateNextViewRangeTime } = useViewRange(); const { viewRange, updateViewRangeTime, updateNextViewRangeTime } = useViewRange();
@ -63,8 +78,6 @@ export function TraceView(props: Props) {
*/ */
const [slim, setSlim] = useState(false); const [slim, setSlim] = useState(false);
// At this point we only show single trace.
const frame = props.dataFrames[0];
const traceProp = useMemo(() => transformDataFrames(frame), [frame]); const traceProp = useMemo(() => transformDataFrames(frame), [frame]);
const { search, setSearch, spanFindMatches } = useSearch(traceProp?.spans); const { search, setSearch, spanFindMatches } = useSearch(traceProp?.spans);
const dataSourceName = useSelector((state: StoreState) => state.explore[props.exploreId]?.datasourceInstance?.name); const dataSourceName = useSelector((state: StoreState) => state.explore[props.exploreId]?.datasourceInstance?.name);

@ -3,6 +3,18 @@ import { useDetailState } from './useDetailState';
import { TraceLog } from '@jaegertracing/jaeger-ui-components/src/types/trace'; import { TraceLog } from '@jaegertracing/jaeger-ui-components/src/types/trace';
describe('useDetailState', () => { describe('useDetailState', () => {
it('clears detail state', async () => {
const { result } = renderHook(() => useDetailState());
expect(result.current.detailStates.size).toBe(0);
act(() => result.current.toggleDetail('span1'));
expect(result.current.detailStates.size).toBe(1);
expect(result.current.detailStates.has('span1')).toBe(true);
act(() => result.current.clearDetailStates());
expect(result.current.detailStates.size).toBe(0);
});
it('toggles detail', async () => { it('toggles detail', async () => {
const { result } = renderHook(() => useDetailState()); const { result } = renderHook(() => useDetailState());
expect(result.current.detailStates.size).toBe(0); expect(result.current.detailStates.size).toBe(0);

@ -9,6 +9,8 @@ import { TraceLog } from '@jaegertracing/jaeger-ui-components/src/types/trace';
export function useDetailState() { export function useDetailState() {
const [detailStates, setDetailStates] = useState(new Map<string, DetailState>()); const [detailStates, setDetailStates] = useState(new Map<string, DetailState>());
const clearDetailStates = useCallback(() => setDetailStates(new Map<string, DetailState>()), [setDetailStates]);
const toggleDetail = useCallback( const toggleDetail = useCallback(
function toggleDetail(spanID: string) { function toggleDetail(spanID: string) {
const newDetailStates = new Map(detailStates); const newDetailStates = new Map(detailStates);
@ -38,6 +40,7 @@ export function useDetailState() {
return { return {
detailStates, detailStates,
clearDetailStates,
toggleDetail, toggleDetail,
detailLogItemToggle, detailLogItemToggle,
detailLogsToggle: useCallback( detailLogsToggle: useCallback(

Loading…
Cancel
Save