From 28383bcfac47a2ab26ee59b6701451b329696873 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 11 Nov 2020 12:10:12 +0100 Subject: [PATCH] TraceViewer: Make sure it does not break when no trace is passed (#28909) * TraceViewer: Make sure it does not break when no trace is passed - exit trace conversion early when there is no trace data - added test to cover missing trace for trace viewer * Review feedback * Use RTL for new test * Exit early if trace or traceID is not there --- packages/grafana-data/src/types/trace.ts | 4 ++++ .../src/model/transform-trace-data.tsx | 9 ++++----- public/app/features/explore/Explore.tsx | 3 ++- public/app/features/explore/TraceView/TraceView.test.tsx | 8 ++++++++ public/app/features/explore/TraceView/TraceView.tsx | 7 +++++-- 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/grafana-data/src/types/trace.ts b/packages/grafana-data/src/types/trace.ts index 63b40d1d706..3af2dbdb4a2 100644 --- a/packages/grafana-data/src/types/trace.ts +++ b/packages/grafana-data/src/types/trace.ts @@ -80,6 +80,10 @@ export type TraceData = { warnings?: string[] | null; }; +export type TraceViewData = TraceData & { + spans: TraceSpanData[]; +}; + export type Trace = TraceData & { duration: number; endTime: number; diff --git a/packages/jaeger-ui-components/src/model/transform-trace-data.tsx b/packages/jaeger-ui-components/src/model/transform-trace-data.tsx index f0071f6d7b8..0b6981e3494 100644 --- a/packages/jaeger-ui-components/src/model/transform-trace-data.tsx +++ b/packages/jaeger-ui-components/src/model/transform-trace-data.tsx @@ -17,7 +17,7 @@ import _isEqual from 'lodash/isEqual'; // @ts-ignore import { getTraceSpanIdsAsTree } from '../selectors/trace'; import { getConfigValue } from '../utils/config/get-config'; -import { TraceKeyValuePair, TraceSpan, TraceSpanData, Trace, TraceData } from '@grafana/data'; +import { TraceKeyValuePair, TraceSpan, Trace, TraceViewData } from '@grafana/data'; // @ts-ignore import TreeNode from '../utils/TreeNode'; @@ -71,12 +71,11 @@ export function orderTags(spanTags: TraceKeyValuePair[], topPrefixes?: string[]) * NOTE: Mutates `data` - Transform the HTTP response data into the form the app * generally requires. */ -export default function transformTraceData(data: TraceData & { spans: TraceSpanData[] }): Trace | null { - let { traceID } = data; - if (!traceID) { +export default function transformTraceData(data: TraceViewData | undefined): Trace | null { + if (!data?.traceID) { return null; } - traceID = traceID.toLowerCase(); + const traceID = data.traceID.toLowerCase(); let traceEndTime = 0; let traceStartTime = Number.MAX_SAFE_INTEGER; diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index eea83ec378c..b562dbe7b49 100644 --- a/public/app/features/explore/Explore.tsx +++ b/public/app/features/explore/Explore.tsx @@ -22,6 +22,7 @@ import { LogsModel, EventBusExtended, EventBusSrv, + TraceViewData, } from '@grafana/data'; import store from 'app/core/store'; @@ -401,7 +402,7 @@ export class Explore extends React.PureComponent { // If there is not data (like 404) we show a separate error so no need to show anything here queryResponse.series[0] && ( )} diff --git a/public/app/features/explore/TraceView/TraceView.test.tsx b/public/app/features/explore/TraceView/TraceView.test.tsx index 4b44e4355bf..45eb36af215 100644 --- a/public/app/features/explore/TraceView/TraceView.test.tsx +++ b/public/app/features/explore/TraceView/TraceView.test.tsx @@ -1,5 +1,6 @@ import React from 'react'; import { shallow } from 'enzyme'; +import { render } from '@testing-library/react'; import { TraceView } from './TraceView'; import { TracePageHeader, TraceTimelineViewer } from '@jaegertracing/jaeger-ui-components'; import { TraceSpanData, TraceData } from '@grafana/data'; @@ -20,6 +21,13 @@ describe('TraceView', () => { expect(header).toHaveLength(1); }); + it('does not render anything on missing trace', () => { + // Simulating Explore's access to empty response data + const trace = [][0]; + const { container } = render( {}} />); + expect(container.hasChildNodes()).toBeFalsy(); + }); + it('toggles detailState', () => { let { timeline, wrapper } = renderTraceView(); expect(timeline.props().traceTimeline.detailStates.size).toBe(0); diff --git a/public/app/features/explore/TraceView/TraceView.tsx b/public/app/features/explore/TraceView/TraceView.tsx index b963e1376d2..23e1db961ac 100644 --- a/public/app/features/explore/TraceView/TraceView.tsx +++ b/public/app/features/explore/TraceView/TraceView.tsx @@ -16,15 +16,18 @@ import { useChildrenState } from './useChildrenState'; import { useDetailState } from './useDetailState'; import { useHoverIndentGuide } from './useHoverIndentGuide'; import { colors, useTheme } from '@grafana/ui'; -import { TraceData, TraceSpanData, Trace, TraceSpan, TraceKeyValuePair, TraceLink } from '@grafana/data'; +import { TraceViewData, Trace, TraceSpan, TraceKeyValuePair, TraceLink } from '@grafana/data'; import { createSpanLinkFactory } from './createSpanLink'; type Props = { - trace: TraceData & { spans: TraceSpanData[] }; + trace?: TraceViewData; splitOpenFn: (options: { datasourceUid: string; query: any }) => void; }; export function TraceView(props: Props) { + if (!props.trace?.traceID) { + return null; + } const { expandOne, collapseOne, childrenToggle, collapseAll, childrenHiddenIDs, expandAll } = useChildrenState(); const { detailStates,