From 8d6831f21f4b83b379ecceadaed574292f2ba97c Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Mon, 29 Nov 2021 16:19:56 +0100 Subject: [PATCH] Tracing: Refactor span detail logic (#42152) * Tracing: Update incorrect span fix based on review feedback * Prometheus: Use this.query for health check * Revert "Prometheus: Use this.query for health check" This reverts commit 4c1a6a92a5831bf60dc7110fcef6568accaa0610. --- .../features/explore/TraceView/TraceView.tsx | 15 ++------- .../explore/TraceView/useDetailState.test.ts | 31 ++++++++----------- .../explore/TraceView/useDetailState.ts | 11 ++++--- 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/public/app/features/explore/TraceView/TraceView.tsx b/public/app/features/explore/TraceView/TraceView.tsx index de10dc4a6c2..d4427bfb457 100644 --- a/public/app/features/explore/TraceView/TraceView.tsx +++ b/public/app/features/explore/TraceView/TraceView.tsx @@ -18,9 +18,7 @@ import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; import { getTimeZone } from 'app/features/profile/state/selectors'; import { StoreState } from 'app/types'; import { ExploreId } from 'app/types/explore'; -import { isEqual } from 'lodash'; -import React, { useCallback, useMemo, useState, useEffect } from 'react'; -import { usePrevious } from 'react-use'; +import React, { useCallback, useMemo, useState } from 'react'; import { useSelector } from 'react-redux'; import { createSpanLinkFactory } from './createSpanLink'; import { UIElements } from './uiElements'; @@ -43,7 +41,6 @@ type 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 { @@ -56,15 +53,7 @@ export function TraceView(props: Props) { detailTagsToggle, detailWarningsToggle, detailStackTracesToggle, - clearDetailStates, - } = useDetailState(); - - // Clear detail state when new trace arrives - useEffect(() => { - if (frame && prevFrame && !isEqual(prevFrame, frame)) { - clearDetailStates(); - } - }, [frame, prevFrame, clearDetailStates]); + } = useDetailState(frame); const { removeHoverIndentGuideId, addHoverIndentGuideId, hoverIndentGuideIds } = useHoverIndentGuide(); const { viewRange, updateViewRangeTime, updateNextViewRangeTime } = useViewRange(); diff --git a/public/app/features/explore/TraceView/useDetailState.test.ts b/public/app/features/explore/TraceView/useDetailState.test.ts index caf62e1e047..6433dbd5f0d 100644 --- a/public/app/features/explore/TraceView/useDetailState.test.ts +++ b/public/app/features/explore/TraceView/useDetailState.test.ts @@ -1,22 +1,17 @@ import { act, renderHook } from '@testing-library/react-hooks'; import { useDetailState } from './useDetailState'; import { TraceLog } from '@jaegertracing/jaeger-ui-components/src/types/trace'; +import { DataFrame } from '@grafana/data'; + +const sampleFrame: DataFrame = { + name: 'trace', + fields: [], + length: 0, +}; 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 () => { - const { result } = renderHook(() => useDetailState()); + const { result } = renderHook(() => useDetailState(sampleFrame)); expect(result.current.detailStates.size).toBe(0); act(() => result.current.toggleDetail('span1')); @@ -28,7 +23,7 @@ describe('useDetailState', () => { }); it('toggles logs and logs items', async () => { - const { result } = renderHook(() => useDetailState()); + const { result } = renderHook(() => useDetailState(sampleFrame)); act(() => result.current.toggleDetail('span1')); act(() => result.current.detailLogsToggle('span1')); expect(result.current.detailStates.get('span1')?.logs.isOpen).toBe(true); @@ -39,28 +34,28 @@ describe('useDetailState', () => { }); it('toggles warnings', async () => { - const { result } = renderHook(() => useDetailState()); + const { result } = renderHook(() => useDetailState(sampleFrame)); act(() => result.current.toggleDetail('span1')); act(() => result.current.detailWarningsToggle('span1')); expect(result.current.detailStates.get('span1')?.isWarningsOpen).toBe(true); }); it('toggles references', async () => { - const { result } = renderHook(() => useDetailState()); + const { result } = renderHook(() => useDetailState(sampleFrame)); act(() => result.current.toggleDetail('span1')); act(() => result.current.detailReferencesToggle('span1')); expect(result.current.detailStates.get('span1')?.isReferencesOpen).toBe(true); }); it('toggles processes', async () => { - const { result } = renderHook(() => useDetailState()); + const { result } = renderHook(() => useDetailState(sampleFrame)); act(() => result.current.toggleDetail('span1')); act(() => result.current.detailProcessToggle('span1')); expect(result.current.detailStates.get('span1')?.isProcessOpen).toBe(true); }); it('toggles tags', async () => { - const { result } = renderHook(() => useDetailState()); + const { result } = renderHook(() => useDetailState(sampleFrame)); act(() => result.current.toggleDetail('span1')); act(() => result.current.detailTagsToggle('span1')); expect(result.current.detailStates.get('span1')?.isTagsOpen).toBe(true); diff --git a/public/app/features/explore/TraceView/useDetailState.ts b/public/app/features/explore/TraceView/useDetailState.ts index 157d25c7b91..802ec4a74c3 100644 --- a/public/app/features/explore/TraceView/useDetailState.ts +++ b/public/app/features/explore/TraceView/useDetailState.ts @@ -1,4 +1,5 @@ -import { useCallback, useState } from 'react'; +import { useCallback, useState, useEffect } from 'react'; +import { DataFrame } from '@grafana/data'; import { DetailState } from '@jaegertracing/jaeger-ui-components'; import { TraceLog } from '@jaegertracing/jaeger-ui-components/src/types/trace'; @@ -6,10 +7,13 @@ import { TraceLog } from '@jaegertracing/jaeger-ui-components/src/types/trace'; * Keeps state of the span detail. This means whether span details are open but also state of each detail subitem * like logs or tags. */ -export function useDetailState() { +export function useDetailState(frame: DataFrame) { const [detailStates, setDetailStates] = useState(new Map()); - const clearDetailStates = useCallback(() => setDetailStates(new Map()), [setDetailStates]); + // Clear detail state when new trace arrives + useEffect(() => { + setDetailStates(new Map()); + }, [frame, setDetailStates]); const toggleDetail = useCallback( function toggleDetail(spanID: string) { @@ -40,7 +44,6 @@ export function useDetailState() { return { detailStates, - clearDetailStates, toggleDetail, detailLogItemToggle, detailLogsToggle: useCallback(