From f5af19fb2b24eba414074bdb51ea2868a1b62ea8 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Thu, 29 Aug 2019 08:56:22 -0500 Subject: [PATCH 1/4] Derive DDG from search results Signed-off-by: Ruben Vargas --- .../DeepDependencies/Graph/DdgNodeContent.tsx | 10 +- .../DeepDependencies/Graph/index.tsx | 13 ++- .../Header/HopsSelector/index.tsx | 1 + .../DeepDependencies/Header/index.tsx | 45 +++++---- .../components/DeepDependencies/index.test.js | 15 +-- .../src/components/DeepDependencies/index.tsx | 37 +++++-- .../components/DeepDependencies/traces.tsx | 98 +++++++++++++++++++ .../src/components/DeepDependencies/url.tsx | 10 +- .../SearchResults/AltViewOptions.tsx | 30 ++++++ .../SearchTracePage/SearchResults/index.css | 4 + .../SearchTracePage/SearchResults/index.js | 82 ++++++++++++---- .../SearchResults/index.test.js | 2 +- .../src/components/SearchTracePage/index.js | 3 + .../components/SearchTracePage/index.test.js | 32 ++++-- .../model/ddg/transformTracesToPaths.test.js | 66 +++++++++++++ .../src/model/ddg/transformTracesToPaths.tsx | 74 ++++++++++++++ packages/jaeger-ui/src/model/ddg/types.tsx | 6 +- 17 files changed, 460 insertions(+), 68 deletions(-) create mode 100644 packages/jaeger-ui/src/components/DeepDependencies/traces.tsx create mode 100644 packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx create mode 100644 packages/jaeger-ui/src/model/ddg/transformTracesToPaths.test.js create mode 100644 packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent.tsx index 1ede00c53c..d1fefd2561 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent.tsx @@ -36,7 +36,7 @@ type TProps = { vertexKey: string; }; -// While browsers suport URLs of unlimited length, many server clients do not handle more than this max +// While browsers support URLs of unlimited length, many server clients do not handle more than this max const MAX_LENGTH = 2083; const MAX_LINKED_TRACES = 35; const MIN_LENGTH = getSearchUrl().length; @@ -55,13 +55,17 @@ export default class DdgNodeContent extends React.PureComponent { getVisiblePathElems: (vertexKey: string) => PathElem[] | undefined, setViewModifier: (vertexKey: string, viewModifier: EViewModifier, enable: boolean) => void, density: EDdgDensity, - showOp: boolean + showOp: boolean, + baseUrl: string, + extraUrlArgs: { [key: string]: unknown } | undefined ) { return function renderNode(vertex: TDdgVertex, utils: TRendererUtils, lv: TLayoutVertex | null) { const { isFocalNode, key, operation, service } = vertex; return ( | undefined; vertices: TDdgVertex[]; verticesViewModifiers: Map; + baseUrl: string; + extraUrlArgs?: { [key: string]: unknown }; }; // The dichotomy between w/ & w/o VMs assumes that any edge VM neccesitates unmodified edges are de-emphasized @@ -82,6 +84,8 @@ export default class Graph extends PureComponent { uiFindMatches, vertices, verticesViewModifiers, + baseUrl, + extraUrlArgs, } = this.props; const nodeRenderers = this.getNodeRenderers(uiFindMatches || this.emptyFindSet, verticesViewModifiers); @@ -134,7 +138,14 @@ export default class Graph extends PureComponent { layerType: 'html', measurable: true, measureNode: DdgNodeContent.measureNode, - renderNode: this.getNodeContentRenderer(getVisiblePathElems, setViewModifier, density, showOp), + renderNode: this.getNodeContentRenderer( + getVisiblePathElems, + setViewModifier, + density, + showOp, + baseUrl, + extraUrlArgs + ), }, ]} /> diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/index.tsx index 5463cc5f31..956095764f 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/index.tsx @@ -22,6 +22,7 @@ type TProps = { distanceToPathElems?: TDdgDistanceToPathElems; handleClick: (distance: number, direction: EDirection) => void; visEncoding?: string; + extraUrlArgs?: { [key: string]: unknown }; }; export default memo(function HopsSelector({ distanceToPathElems, handleClick, visEncoding }: TProps) { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx index cbb1ec0abe..49fe515e36 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx @@ -41,10 +41,16 @@ type TProps = { toggleShowOperations: (enable: boolean) => void; uiFindCount: number | undefined; visEncoding?: string; + showParameters?: boolean; + extraUrlArgs?: { [key: string]: unknown }; }; export default class Header extends React.PureComponent { private _uiFindInput: React.RefObject = React.createRef(); + static defaultProps = { + showParameters: true, + }; + focusUiFindInput = () => { if (this._uiFindInput.current) { this._uiFindInput.current.focus(); @@ -103,30 +109,34 @@ export default class Header extends React.PureComponent { showOperations, toggleShowOperations, visEncoding, + showParameters, + extraUrlArgs, } = this.props; return (
-
- - {service && ( + {showParameters && ( +
- )} -
+ {service && ( + + )} +
+ )}
{ distanceToPathElems={distanceToPathElems} handleClick={setDistance} visEncoding={visEncoding} + extraUrlArgs={extraUrlArgs} />
diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js index 1de8724a93..58c4ee611f 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js @@ -104,7 +104,7 @@ describe('DeepDependencyGraphPage', () => { const value = `new ${propName}`; const kwarg = { [propName]: value }; ddgPageImpl.updateUrlState(kwarg); - expect(getUrlSpy).toHaveBeenLastCalledWith(Object.assign({}, props.urlState, kwarg)); + expect(getUrlSpy).toHaveBeenLastCalledWith(Object.assign({}, props.urlState, kwarg), undefined); expect(props.history.push).toHaveBeenCalledTimes(i + 1); }); }); @@ -115,7 +115,7 @@ describe('DeepDependencyGraphPage', () => { start: 'new start', }; ddgPageImpl.updateUrlState(kwarg); - expect(getUrlSpy).toHaveBeenLastCalledWith(Object.assign({}, props.urlState, kwarg)); + expect(getUrlSpy).toHaveBeenLastCalledWith(Object.assign({}, props.urlState, kwarg), undefined); expect(props.history.push).toHaveBeenCalledTimes(1); }); @@ -130,7 +130,7 @@ describe('DeepDependencyGraphPage', () => { }; const ddgPageWithFewerProps = new DeepDependencyGraphPageImpl(otherProps); ddgPageWithFewerProps.updateUrlState(kwarg); - expect(getUrlSpy).toHaveBeenLastCalledWith(Object.assign({}, otherUrlState, kwarg)); + expect(getUrlSpy).toHaveBeenLastCalledWith(Object.assign({}, otherUrlState, kwarg), undefined); expect(getUrlSpy).not.toHaveBeenLastCalledWith(expect.objectContaining({ start: expect.anything() })); expect(props.history.push).toHaveBeenCalledTimes(1); }); @@ -172,7 +172,8 @@ describe('DeepDependencyGraphPage', () => { prevVisEncoding: visEncoding, }); expect(getUrlSpy).toHaveBeenLastCalledWith( - Object.assign({}, props.urlState, { visEncoding: mockNewEncoding }) + Object.assign({}, props.urlState, { visEncoding: mockNewEncoding }), + undefined ); expect(props.history.push).toHaveBeenCalledTimes(1); }); @@ -183,7 +184,8 @@ describe('DeepDependencyGraphPage', () => { const operation = 'newOperation'; ddgPageImpl.setOperation(operation); expect(getUrlSpy).toHaveBeenLastCalledWith( - Object.assign({}, props.urlState, { operation, visEncoding: undefined }) + Object.assign({}, props.urlState, { operation, visEncoding: undefined }), + undefined ); expect(props.history.push).toHaveBeenCalledTimes(1); }); @@ -199,7 +201,8 @@ describe('DeepDependencyGraphPage', () => { it('updates service and clears operation and visEncoding', () => { ddgPageImpl.setService(service); expect(getUrlSpy).toHaveBeenLastCalledWith( - Object.assign({}, props.urlState, { operation: undefined, service, visEncoding: undefined }) + Object.assign({}, props.urlState, { operation: undefined, service, visEncoding: undefined }), + undefined ); expect(props.history.push).toHaveBeenCalledTimes(1); }); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx index 5d638fdab2..32f061fde1 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx @@ -20,7 +20,7 @@ import { connect } from 'react-redux'; import Header from './Header'; import Graph from './Graph'; -import { getUrl, getUrlState } from './url'; +import { getUrl, getUrlState, ROUTE_PATH } from './url'; import ErrorMessage from '../common/ErrorMessage'; import LoadingIndicator from '../common/LoadingIndicator'; import { extractUiFindFromState, TExtractUiFindFromStateReturn } from '../common/UiFindInput'; @@ -43,7 +43,7 @@ import { TDdgStateEntry } from '../../types/TDdgState'; import './index.css'; -type TDispatchProps = { +export type TDispatchProps = { addViewModifier: (kwarg: TDdgModelParams & { viewModifier: number; visibilityIndices: number[] }) => void; fetchDeepDependencyGraph: (query: TDdgModelParams) => void; fetchServices: () => void; @@ -53,7 +53,7 @@ type TDispatchProps = { ) => void; }; -type TReduxProps = TExtractUiFindFromStateReturn & { +export type TReduxProps = TExtractUiFindFromStateReturn & { graph: GraphModel | undefined; graphState?: TDdgStateEntry; operationsForService: Record; @@ -61,15 +61,23 @@ type TReduxProps = TExtractUiFindFromStateReturn & { urlState: TDdgSparseUrlState; }; -type TOwnProps = { +export type TOwnProps = { history: RouterHistory; location: Location; + showServicesOpsHeader: boolean; + baseUrl: string; + extraUrlArgs?: { [key: string]: unknown }; }; -type TProps = TDispatchProps & TReduxProps & TOwnProps; +export type TProps = TDispatchProps & TReduxProps & TOwnProps; // export for tests export class DeepDependencyGraphPageImpl extends React.PureComponent { + static defaultProps = { + showServicesOpsHeader: true, + baseUrl: ROUTE_PATH, + }; + static fetchModelIfStale(props: TProps) { const { fetchDeepDependencyGraph, graphState = null, urlState } = props; const { service, operation } = urlState; @@ -173,12 +181,22 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { toggleShowOperations = (enable: boolean) => this.updateUrlState({ showOp: enable }); updateUrlState = (newValues: Partial) => { - const { uiFind, urlState, history } = this.props; - history.push(getUrl({ uiFind, ...urlState, ...newValues })); + const { uiFind, urlState, history, baseUrl, extraUrlArgs } = this.props; + history.push(getUrl({ uiFind, ...urlState, ...newValues, ...extraUrlArgs }, baseUrl)); }; render() { - const { graph, graphState, operationsForService, services, uiFind, urlState } = this.props; + const { + graph, + graphState, + operationsForService, + services, + uiFind, + urlState, + showServicesOpsHeader, + baseUrl, + extraUrlArgs, + } = this.props; const { density, operation, service, showOp, visEncoding } = urlState; const distanceToPathElems = graphState && graphState.state === fetchedState.DONE ? graphState.model.distanceToPathElems : undefined; @@ -208,6 +226,8 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { uiFindMatches={uiFindMatches} vertices={vertices} verticesViewModifiers={verticesViewModifiers} + baseUrl={baseUrl} + extraUrlArgs={extraUrlArgs} /> ); } else if (graphState.state === fetchedState.LOADING) { @@ -227,6 +247,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent {
): TDispatchProps { + const { fetchDeepDependencyGraph, fetchServiceOperations, fetchServices } = bindActionCreators( + jaegerApiActions, + dispatch + ); + const { addViewModifier, removeViewModifierFromIndices } = bindActionCreators(ddgActions, dispatch); + + return { + addViewModifier, + fetchDeepDependencyGraph, + fetchServiceOperations, + fetchServices, + removeViewModifierFromIndices, + }; +} + +export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxProps { + const { trace } = state; + const urlState = getUrlState(ownProps.location.search); + const { density, operation, service, showOp } = urlState; + let graphState: TDdgStateEntry | undefined; + let graph: GraphModel | undefined; + let payload: TDdgPayload; + if (service) { + payload = transformTracesToPaths(trace.traces, service, operation); + graphState = { + model: transformDdgData(payload, { service, operation }), + state: fetchedState.DONE, + viewModifiers: new Map(), + }; + graph = makeGraph(graphState.model, showOp, density); + } + + return { + graph, + graphState, + services: undefined, + operationsForService: {}, + urlState, + ...extractUiFindFromState(state), + }; +} + +function tracesDDG() { + return class extends React.PureComponent { + render(): React.ReactNode { + const { location } = this.props; + const urlArgs = queryString.parse(location.search); + const { end, start, limit, lookback, maxDuration, minDuration, view } = urlArgs; + const extraArgs = { end, start, limit, lookback, maxDuration, minDuration, view }; + return ( + + ); + } + }; +} + +export default connect( + mapStateToProps, + mapDispatchToProps +)(tracesDDG()); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/url.tsx b/packages/jaeger-ui/src/components/DeepDependencies/url.tsx index 74e630da31..e824e1678d 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/url.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/url.tsx @@ -27,7 +27,11 @@ export function matches(path: string) { return Boolean(matchPath(path, ROUTE_MATCHER)); } -export function getUrl(args?: { [key: string]: unknown; showOp?: boolean }) { +export function getUrl(args?: { [key: string]: unknown; showOp?: boolean }, baseUrl?: string) { + let basePath: string | undefined = baseUrl; + if (!basePath) { + basePath = ROUTE_PATH; + } if (args && !_isEmpty(args)) { const stringifyArgs = Reflect.has(args, 'showOp') ? { @@ -35,9 +39,9 @@ export function getUrl(args?: { [key: string]: unknown; showOp?: boolean }) { showOp: args.showOp ? 1 : 0, } : args; - return `${ROUTE_PATH}?${queryString.stringify(stringifyArgs)}`; + return `${basePath}?${queryString.stringify(stringifyArgs)}`; } - return ROUTE_PATH; + return basePath; } function firstParam(arg: string | string[]): string { diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx new file mode 100644 index 0000000000..8be5a1374b --- /dev/null +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx @@ -0,0 +1,30 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as React from 'react'; +import { Button } from 'antd'; + +type Props = { + traceResultsView: boolean; + onTraceGraphViewClicked: () => void; +}; + +export default function AltViewOptions(props: Props) { + const { onTraceGraphViewClicked, traceResultsView } = props; + return ( + + ); +} diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.css b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.css index ed61fc1911..e729e9f2f2 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.css +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.css @@ -35,3 +35,7 @@ limitations under the License. .SearchResults--resultLink:hover { color: inherit; } + +.SearchResults--ddg-container { + min-height: 600px; +} diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js index 82c6d343cd..c5d71dcc47 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js @@ -16,14 +16,17 @@ import * as React from 'react'; import { Select } from 'antd'; -import { Link } from 'react-router-dom'; -import { Field, reduxForm, formValueSelector } from 'redux-form'; +import { Link, withRouter } from 'react-router-dom'; +import { Field, formValueSelector, reduxForm } from 'redux-form'; +import { History as RouterHistory, Location } from 'history'; +import queryString from 'query-string'; import DiffSelection from './DiffSelection'; import * as markers from './index.markers'; import ResultItem from './ResultItem'; import ScatterPlot from './ScatterPlot'; import { getUrl } from '../url'; + import LoadingIndicator from '../../common/LoadingIndicator'; import NewWindowIcon from '../../common/NewWindowIcon'; import { getLocation } from '../../TracePage/url'; @@ -36,6 +39,8 @@ import type { FetchedTrace } from '../../../types'; import type { SearchQuery } from '../../../types/search'; import './index.css'; +import AltViewOptions from './AltViewOptions'; +import SearchResultsDGG from '../../DeepDependencies/traces'; type SearchResultsProps = { cohortAddTrace: string => void, @@ -50,6 +55,8 @@ type SearchResultsProps = { showStandaloneLink: boolean, skipMessage?: boolean, traces: TraceSummary[], + history: RouterHistory, + location: Location, }; const Option = Select.Option; @@ -81,7 +88,7 @@ const SelectSort = reduxForm({ export const sortFormSelector = formValueSelector('traceResultsSort'); -export default class SearchResults extends React.PureComponent { +export class UnconnectedSearchResults extends React.PureComponent { props: SearchResultsProps; static defaultProps = { skipMessage: false, queryOfResults: undefined }; @@ -95,6 +102,19 @@ export default class SearchResults extends React.PureComponent { + const { location, history } = this.props; + const urlState = queryString.parse(location.search); + history.push(getUrl({ ...urlState, view: 'ddg' })); + }; + + onTraceGraphViewClicked = () => { + const { location, history } = this.props; + const urlState = queryString.parse(location.search); + const view = urlState.view && urlState.view === 'ddg' ? 'traces' : 'ddg'; + history.push(getUrl({ ...urlState, view })); + }; + render() { const { diffCohort, @@ -107,7 +127,16 @@ export default class SearchResults extends React.PureComponent ); @@ -137,7 +166,7 @@ export default class SearchResults extends React.PureComponent
- {!hideGraph && ( + {!hideGraph && traceResultsView && (
({ @@ -157,7 +186,11 @@ export default class SearchResults extends React.PureComponent {traces.length} Trace{traces.length > 1 && 's'} - + {traceResultsView && } + {showStandaloneLink && (
- {diffSelection} -
    - {traces.map(trace => ( -
  • - -
    • - ))} -
+ {!traceResultsView && ( +
+ +
+ )} + {traceResultsView && diffSelection} + {traceResultsView && ( +
    + {traces.map(trace => ( +
  • + +
    • + ))} +
+ )}
); } } + +export default withRouter(UnconnectedSearchResults); diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js index 0cc7bfa82c..08ebd3d34c 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js @@ -15,7 +15,7 @@ import React from 'react'; import { shallow } from 'enzyme'; -import SearchResults from '.'; +import { UnconnectedSearchResults as SearchResults } from '.'; import * as markers from './index.markers'; import ResultItem from './ResultItem'; import ScatterPlot from './ScatterPlot'; diff --git a/packages/jaeger-ui/src/components/SearchTracePage/index.js b/packages/jaeger-ui/src/components/SearchTracePage/index.js index cd4426736a..ad4e546002 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/index.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/index.js @@ -94,6 +94,7 @@ export class SearchTracePageImpl extends Component { const hasTraceResults = traceResults && traceResults.length > 0; const showErrors = errors && !loadingTraces; const showLogo = isHomepage && !hasTraceResults && !loadingTraces && !errors; + return (
@@ -248,9 +249,11 @@ export function mapStateToProps(state) { if (serviceError) { errors.push(serviceError); } + const location = router.location; const sortBy = sortFormSelector(state, 'sortBy'); const traceResults = sortedTracesXformer(traces, sortBy); return { + location, queryOfResults, diffCohort, embedded, diff --git a/packages/jaeger-ui/src/components/SearchTracePage/index.test.js b/packages/jaeger-ui/src/components/SearchTracePage/index.test.js index 65ca7277bd..95035816fa 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/index.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/index.test.js @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +import { MemoryRouter } from 'react-router-dom'; + jest.mock('redux-form', () => { function reduxForm() { return component => component; @@ -23,7 +25,6 @@ jest.mock('redux-form', () => { return { Field, formValueSelector, reduxForm }; }); -jest.mock('react-router-dom'); jest.mock('store'); /* eslint-disable import/first */ @@ -76,7 +77,11 @@ describe('', () => { props.fetchServiceOperations.mockClear(); const oldFn = store.get; store.get = jest.fn(() => ({ service: 'svc-b' })); - wrapper = mount(); + wrapper = mount( + + + + ); expect(props.fetchServices.mock.calls.length).toBe(1); expect(props.fetchServiceOperations.mock.calls.length).toBe(1); store.get = oldFn; @@ -87,8 +92,16 @@ describe('', () => { const query = 'some-query'; const historyPush = jest.fn(); const historyMock = { push: historyPush }; - wrapper = mount(); - wrapper.instance().goToTrace(traceID); + wrapper = mount( + + + + ); + wrapper + .find(SearchTracePage) + .first() + .instance() + .goToTrace(traceID); expect(historyPush.mock.calls.length).toBe(1); expect(historyPush.mock.calls[0][0]).toEqual({ pathname: `/trace/${traceID}`, @@ -155,9 +168,14 @@ describe('mapStateToProps()', () => { services: stateServices, }; - const { maxTraceDuration, traceResults, diffCohort, numberOfTraceResults, ...rest } = mapStateToProps( - state - ); + const { + maxTraceDuration, + traceResults, + diffCohort, + numberOfTraceResults, + location, + ...rest + } = mapStateToProps(state); expect(traceResults).toHaveLength(stateTrace.search.results.length); expect(traceResults[0].traceID).toBe(trace.traceID); expect(maxTraceDuration).toBe(trace.duration); diff --git a/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.test.js b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.test.js new file mode 100644 index 0000000000..9f17500310 --- /dev/null +++ b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.test.js @@ -0,0 +1,66 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import transformTracesToPaths from './transformTracesToPaths'; +import transformTraceData from '../transform-trace-data'; +import { fetchedState } from '../../constants'; + +describe('transform traces to ddg paths', () => { + const tracesResults = [ + { + traceID: 'Trace-1', + spans: [ + { + traceID: '1', + spanID: '1', + startTime: 1570040938479366, + operationName: 'HTTP GET /customer', + references: [ + { + refType: 'CHILD_OF', + traceID: '1', + spanID: '2', + }, + ], + processID: 'p1', + }, + { + traceID: '1', + spanID: '2', + startTime: 1570040938479369, + operationName: 'HTTP GET /', + processID: 'p1', + }, + ], + processes: { + p1: { + serviceName: 'customer', + }, + }, + }, + ]; + it('transforms trace results payload', () => { + const processed = tracesResults.map(transformTraceData); + const resultTraces = {}; + for (let i = 0; i < processed.length; i++) { + const data = processed[i]; + const id = data.traceID; + resultTraces[id] = { data, id, state: fetchedState.DONE }; + } + + const payload = transformTracesToPaths(resultTraces, 'customer'); + expect(payload.length).toBe(1); + expect(payload[0].path.length).toBe(2); + }); +}); diff --git a/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx new file mode 100644 index 0000000000..3bf106c5fd --- /dev/null +++ b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx @@ -0,0 +1,74 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { TDdgPayloadEntry, TDdgPayload } from './types'; +import { Span, Trace } from '../../types/trace'; +import { FetchedTrace } from '../../types'; +import spanAncestorIds from '../../utils/span-ancestor-ids'; + +type Node = { + value?: Span; + children: Node[]; +}; + +const hasFocal = (path: TDdgPayloadEntry[], focalService: string, focalOperation: string | undefined) => { + for (let i = 0; i < path.length; ++i) { + if ( + focalService === path[i].service && + (focalOperation === undefined || focalOperation === path[i].operation) + ) { + return true; + } + } + return false; +}; + +function convertSpan(span: Span, trace: Trace): TDdgPayloadEntry { + const serviceName = trace.processes[span.processID].serviceName; + const operationName = span.operationName; + return { service: serviceName, operation: operationName }; +} + +export default function( + traces: Record, + focalService: string, + focalOperation: string | undefined +) { + const paths: TDdgPayload = []; + Object.values(traces).forEach(trace => { + const map: Map = new Map(); + if (trace.data) { + const traceId = trace.data.traceID; + trace.data.spans + .filter(span => { + map.set(span.spanID, span); + return !span.hasChildren; + }) + .forEach(leaf => { + const path = spanAncestorIds(leaf) + // @ts-ignore + .map(id => convertSpan(map.get(id), trace.data)); + // @ts-ignore + path.push(convertSpan(map.get(leaf.spanID), trace.data)); + if (hasFocal(path, focalService, focalOperation)) { + paths.push({ + path, + trace_id: traceId, + }); + } + }); + } + }); + return paths; +} diff --git a/packages/jaeger-ui/src/model/ddg/types.tsx b/packages/jaeger-ui/src/model/ddg/types.tsx index 8de824774f..350248ee0e 100644 --- a/packages/jaeger-ui/src/model/ddg/types.tsx +++ b/packages/jaeger-ui/src/model/ddg/types.tsx @@ -50,10 +50,12 @@ export type TDdgPayloadEntry = { service: string; }; -export type TDdgPayload = { +export type TDdgPayloadPath = { path: TDdgPayloadEntry[]; trace_id: string; // eslint-disable-line camelcase -}[]; +}; + +export type TDdgPayload = TDdgPayloadPath[]; export type TDdgService = { name: string; From ac5baa00983390d7c715d27cc284c5bd580762e0 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Tue, 15 Oct 2019 18:14:16 -0400 Subject: [PATCH 2/4] Touch up CSS, test coverage, and DRYness Signed-off-by: Everett Ross --- .../jaeger-ui/src/components/App/Page.css | 2 + .../Header/HopsSelector/index.tsx | 1 - .../DeepDependencies/Header/index.css | 6 +- .../DeepDependencies/Header/index.tsx | 3 - .../src/components/DeepDependencies/index.tsx | 18 +- .../DeepDependencies/traces.test.js | 53 ++++++ .../components/DeepDependencies/traces.tsx | 67 +++---- .../components/DeepDependencies/url.test.js | 5 + .../SearchResults/AltViewOptions.tsx | 2 +- .../SearchTracePage/SearchResults/index.css | 15 +- .../SearchTracePage/SearchResults/index.js | 126 ++++++------ .../src/components/SearchTracePage/index.css | 8 + .../src/components/SearchTracePage/index.js | 103 +++++----- .../model/ddg/transformTracesToPaths.test.js | 179 +++++++++++++----- .../src/model/ddg/transformTracesToPaths.tsx | 29 +-- 15 files changed, 373 insertions(+), 244 deletions(-) create mode 100644 packages/jaeger-ui/src/components/DeepDependencies/traces.test.js diff --git a/packages/jaeger-ui/src/components/App/Page.css b/packages/jaeger-ui/src/components/App/Page.css index d0a0896f19..620189931c 100644 --- a/packages/jaeger-ui/src/components/App/Page.css +++ b/packages/jaeger-ui/src/components/App/Page.css @@ -23,6 +23,8 @@ limitations under the License. } .Page--content { + display: flex; + flex-direction: column; left: 0; min-height: calc(100% - 46px); position: absolute; diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/index.tsx index 956095764f..5463cc5f31 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/HopsSelector/index.tsx @@ -22,7 +22,6 @@ type TProps = { distanceToPathElems?: TDdgDistanceToPathElems; handleClick: (distance: number, direction: EDirection) => void; visEncoding?: string; - extraUrlArgs?: { [key: string]: unknown }; }; export default memo(function HopsSelector({ distanceToPathElems, handleClick, visEncoding }: TProps) { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.css b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.css index 19f50a1d90..a54a39c1c4 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.css +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.css @@ -29,19 +29,19 @@ limitations under the License. .DdgHeader--controlHeader { align-items: center; background-color: #f6f6f6; - border-top: 1px solid #eee; display: flex; justify-content: space-between; padding-left: 1.25rem; } .DdgHeader--paramsHeader { - flex-wrap: wrap; - font-size: unset; align-items: center; background: #fafafa; + border-bottom: 1px solid #eee; display: flex; flex: 1; + flex-wrap: wrap; + font-size: unset; margin: 0; padding: 0.75rem 1.25rem 0.75rem 1.25rem; position: relative; diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx index 49fe515e36..43718b513b 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx @@ -42,7 +42,6 @@ type TProps = { uiFindCount: number | undefined; visEncoding?: string; showParameters?: boolean; - extraUrlArgs?: { [key: string]: unknown }; }; export default class Header extends React.PureComponent { private _uiFindInput: React.RefObject = React.createRef(); @@ -110,7 +109,6 @@ export default class Header extends React.PureComponent { toggleShowOperations, visEncoding, showParameters, - extraUrlArgs, } = this.props; return ( @@ -148,7 +146,6 @@ export default class Header extends React.PureComponent { distanceToPathElems={distanceToPathElems} handleClick={setDistance} visEncoding={visEncoding} - extraUrlArgs={extraUrlArgs} />
diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx index 32f061fde1..ca4cb8a4b6 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx @@ -62,11 +62,11 @@ export type TReduxProps = TExtractUiFindFromStateReturn & { }; export type TOwnProps = { - history: RouterHistory; - location: Location; - showServicesOpsHeader: boolean; baseUrl: string; extraUrlArgs?: { [key: string]: unknown }; + history: RouterHistory; + location: Location; + showSvcOpsHeader: boolean; }; export type TProps = TDispatchProps & TReduxProps & TOwnProps; @@ -74,7 +74,7 @@ export type TProps = TDispatchProps & TReduxProps & TOwnProps; // export for tests export class DeepDependencyGraphPageImpl extends React.PureComponent { static defaultProps = { - showServicesOpsHeader: true, + showSvcOpsHeader: true, baseUrl: ROUTE_PATH, }; @@ -187,15 +187,15 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { render() { const { + baseUrl, + extraUrlArgs, graph, graphState, operationsForService, services, uiFind, urlState, - showServicesOpsHeader, - baseUrl, - extraUrlArgs, + showSvcOpsHeader, } = this.props; const { density, operation, service, showOp, visEncoding } = urlState; const distanceToPathElems = @@ -247,7 +247,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent {
): TDispatchProps { const { fetchDeepDependencyGraph, fetchServiceOperations, fetchServices } = bindActionCreators( jaegerApiActions, diff --git a/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js b/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js new file mode 100644 index 0000000000..093a28d27f --- /dev/null +++ b/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js @@ -0,0 +1,53 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as React from 'react'; +import { shallow } from 'enzyme'; +import queryString from 'query-string'; + +import { DeepDependencyGraphPageImpl } from '.'; +import { TracesDdgImpl /* , mapStateToProps */ } from './traces'; +import { ROUTE_PATH } from '../SearchTracePage/url'; + +describe('TracesDdg', () => { + it('renders DeepDependencyGraphPageImpl with specific props', () => { + const passProps = { + propName0: 'propValue0', + propName1: 'propValue1', + }; + const extraUrlArgs = ['end', 'start', 'limit', 'lookback', 'maxDuration', 'minDuration', 'view'].reduce( + (curr, key) => ({ + ...curr, + [key]: `test ${key}`, + }), + {} + ); + const search = queryString.stringify({ ...extraUrlArgs, extraParam: 'extraParam' }); + + const wrapper = shallow(); + const ddgPage = wrapper.find(DeepDependencyGraphPageImpl); + expect(ddgPage.props()).toEqual( + expect.objectContaining({ + ...passProps, + baseUrl: ROUTE_PATH, + extraUrlArgs, + showSvcOpsHeader: false, + }) + ); + }); + + describe('mapStateToProps()', () => { + /* TODO After merging in master */ + }); +}); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx b/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx index 9b7f68d126..a5a8ba1c7c 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx @@ -12,12 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. import queryString from 'query-string'; -import { bindActionCreators, Dispatch } from 'redux'; import * as React from 'react'; import { connect } from 'react-redux'; import { ReduxState } from '../../types'; -import * as jaegerApiActions from '../../actions/jaeger-api'; -import ddgActions from '../../actions/ddg'; import { getUrlState } from './url'; import { TDdgStateEntry } from '../../types/TDdgState'; import GraphModel, { makeGraph } from '../../model/ddg/GraphModel'; @@ -25,36 +22,19 @@ import { fetchedState } from '../../constants'; import { extractUiFindFromState } from '../common/UiFindInput'; import transformDdgData from '../../model/ddg/transformDdgData'; import transformTracesToPaths from '../../model/ddg/transformTracesToPaths'; -import { TDdgPayload } from '../../model/ddg/types'; import { ROUTE_PATH } from '../SearchTracePage/url'; -import { DeepDependencyGraphPageImpl, TDispatchProps, TOwnProps, TProps, TReduxProps } from '.'; +import { DeepDependencyGraphPageImpl, mapDispatchToProps, TOwnProps, TProps, TReduxProps } from '.'; // export for tests -export function mapDispatchToProps(dispatch: Dispatch): TDispatchProps { - const { fetchDeepDependencyGraph, fetchServiceOperations, fetchServices } = bindActionCreators( - jaegerApiActions, - dispatch - ); - const { addViewModifier, removeViewModifierFromIndices } = bindActionCreators(ddgActions, dispatch); - - return { - addViewModifier, - fetchDeepDependencyGraph, - fetchServiceOperations, - fetchServices, - removeViewModifierFromIndices, - }; -} - export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxProps { - const { trace } = state; + const { services: stServices, trace } = state; + const { services, operationsForService } = stServices; const urlState = getUrlState(ownProps.location.search); const { density, operation, service, showOp } = urlState; let graphState: TDdgStateEntry | undefined; let graph: GraphModel | undefined; - let payload: TDdgPayload; if (service) { - payload = transformTracesToPaths(trace.traces, service, operation); + const payload = transformTracesToPaths(trace.traces, service, operation); graphState = { model: transformDdgData(payload, { service, operation }), state: fetchedState.DONE, @@ -66,33 +46,32 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP return { graph, graphState, - services: undefined, - operationsForService: {}, + operationsForService, + services, urlState, ...extractUiFindFromState(state), }; } -function tracesDDG() { - return class extends React.PureComponent { - render(): React.ReactNode { - const { location } = this.props; - const urlArgs = queryString.parse(location.search); - const { end, start, limit, lookback, maxDuration, minDuration, view } = urlArgs; - const extraArgs = { end, start, limit, lookback, maxDuration, minDuration, view }; - return ( - - ); - } - }; +// export for tests +export class TracesDdgImpl extends React.PureComponent { + render(): React.ReactNode { + const { location } = this.props; + const urlArgs = queryString.parse(location.search); + const { end, start, limit, lookback, maxDuration, minDuration, view } = urlArgs; + const extraArgs = { end, start, limit, lookback, maxDuration, minDuration, view }; + return ( + + ); + } } export default connect( mapStateToProps, mapDispatchToProps -)(tracesDDG()); +)(TracesDdgImpl); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/url.test.js b/packages/jaeger-ui/src/components/DeepDependencies/url.test.js index 688de86a1a..f6004758d2 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/url.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/url.test.js @@ -52,6 +52,11 @@ describe('DeepDependencyGraph/url', () => { expect(getUrl({})).toBe('/deep-dependencies'); }); + it('uses given baseUrl', () => { + const baseUrl = 'test base url'; + expect(getUrl({}, baseUrl)).toBe(baseUrl); + }); + it('includes provided args', () => { const paramA = 'aParam'; const paramB = 'bParam'; diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx index 8be5a1374b..a023a90cd9 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx @@ -24,7 +24,7 @@ export default function AltViewOptions(props: Props) { const { onTraceGraphViewClicked, traceResultsView } = props; return ( ); } diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.css b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.css index e729e9f2f2..3b715672c5 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.css +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.css @@ -14,6 +14,12 @@ See the License for the specific language governing permissions and limitations under the License. */ +.SearchResults { + display: flex; + flex-direction: column; + flex-grow: 1; +} + .SearchResults--header { border: 1px solid #e6e6e6; color: var(--tx-color-title); @@ -23,11 +29,14 @@ limitations under the License. .SearchResults--headerOverview { align-items: center; background-color: #f5f5f5; - border-top: 1px solid #e6e6e6; display: flex; padding: 1rem; } +.SearchResults--headerScatterPlot { + border-bottom: 1px solid #e6e6e6; +} + .SearchResults--resultLink { color: inherit; } @@ -37,5 +46,7 @@ limitations under the License. } .SearchResults--ddg-container { - min-height: 600px; + border: #e6e6e6 1px solid; + flex-grow: 1; + position: relative; } diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js index c5d71dcc47..f63e1f14f9 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js @@ -40,7 +40,7 @@ import type { SearchQuery } from '../../../types/search'; import './index.css'; import AltViewOptions from './AltViewOptions'; -import SearchResultsDGG from '../../DeepDependencies/traces'; +import SearchResultsDDG from '../../DeepDependencies/traces'; type SearchResultsProps = { cohortAddTrace: string => void, @@ -102,12 +102,6 @@ export class UnconnectedSearchResults extends React.PureComponent { - const { location, history } = this.props; - const urlState = queryString.parse(location.search); - history.push(getUrl({ ...urlState, view: 'ddg' })); - }; - onTraceGraphViewClicked = () => { const { location, history } = this.props; const urlState = queryString.parse(location.search); @@ -163,71 +157,67 @@ export class UnconnectedSearchResults extends React.PureComponent datum.id)); const searchUrl = queryOfResults ? getUrl(stripEmbeddedState(queryOfResults)) : getUrl(); return ( -
-
-
- {!hideGraph && traceResultsView && ( -
- ({ - x: t.startTime, - y: t.duration, - traceID: t.traceID, - size: t.spans.length, - name: t.traceName, - }))} - onValueClick={t => { - goToTrace(t.traceID); - }} - /> -
- )} -
-

- {traces.length} Trace{traces.length > 1 && 's'} -

- {traceResultsView && } - +
+ {!hideGraph && traceResultsView && ( +
+ ({ + x: t.startTime, + y: t.duration, + traceID: t.traceID, + size: t.spans.length, + name: t.traceName, + }))} + onValueClick={t => { + goToTrace(t.traceID); + }} /> - {showStandaloneLink && ( - - - - )} -
-
-
-
- {!traceResultsView && ( -
-
)} - {traceResultsView && diffSelection} - {traceResultsView && ( -
    - {traces.map(trace => ( -
  • - -
    • - ))} -
- )} +
+

+ {traces.length} Trace{traces.length > 1 && 's'} +

+ {traceResultsView && } + + {showStandaloneLink && ( + + + + )} +
+ {!traceResultsView && ( +
+ +
+ )} + {traceResultsView && diffSelection} + {traceResultsView && ( +
    + {traces.map(trace => ( +
  • + +
    • + ))} +
+ )}
); } diff --git a/packages/jaeger-ui/src/components/SearchTracePage/index.css b/packages/jaeger-ui/src/components/SearchTracePage/index.css index 313943627e..16a0d2ddfa 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/index.css +++ b/packages/jaeger-ui/src/components/SearchTracePage/index.css @@ -14,7 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ +.SearchTracePage--row { + display: flex; + flex-grow: 1; +} + .SearchTracePage--column { + display: flex; + flex-direction: column; + flex-grow: 1; padding: 1rem 0.5rem; } diff --git a/packages/jaeger-ui/src/components/SearchTracePage/index.js b/packages/jaeger-ui/src/components/SearchTracePage/index.js index ad4e546002..993636f0e4 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/index.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/index.js @@ -94,60 +94,57 @@ export class SearchTracePageImpl extends Component { const hasTraceResults = traceResults && traceResults.length > 0; const showErrors = errors && !loadingTraces; const showLogo = isHomepage && !hasTraceResults && !loadingTraces && !errors; - return ( -
- - {!embedded && ( - -
- - - {!loadingServices && services ? : } - - - - - -
- - )} - - {showErrors && ( -
-

There was an error querying for traces:

- {errors.map(err => ( - - ))} -
- )} - {!showErrors && ( - - )} - {showLogo && ( - presentation - )} + + {!embedded && ( + +
+ + + {!loadingServices && services ? : } + + + + + +
- -
+ )} + + {showErrors && ( +
+

There was an error querying for traces:

+ {errors.map(err => ( + + ))} +
+ )} + {!showErrors && ( + + )} + {showLogo && ( + presentation + )} + + ); } } @@ -249,11 +246,9 @@ export function mapStateToProps(state) { if (serviceError) { errors.push(serviceError); } - const location = router.location; const sortBy = sortFormSelector(state, 'sortBy'); const traceResults = sortedTracesXformer(traces, sortBy); return { - location, queryOfResults, diffCohort, embedded, diff --git a/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.test.js b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.test.js index 9f17500310..5edbf671a8 100644 --- a/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.test.js +++ b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.test.js @@ -13,54 +13,141 @@ // limitations under the License. import transformTracesToPaths from './transformTracesToPaths'; -import transformTraceData from '../transform-trace-data'; -import { fetchedState } from '../../constants'; describe('transform traces to ddg paths', () => { - const tracesResults = [ - { - traceID: 'Trace-1', - spans: [ - { - traceID: '1', - spanID: '1', - startTime: 1570040938479366, - operationName: 'HTTP GET /customer', - references: [ - { - refType: 'CHILD_OF', - traceID: '1', - spanID: '2', - }, - ], - processID: 'p1', - }, - { - traceID: '1', - spanID: '2', - startTime: 1570040938479369, - operationName: 'HTTP GET /', - processID: 'p1', - }, - ], - processes: { - p1: { - serviceName: 'customer', - }, - }, + const makeSpan = (spanName, childOf) => ({ + hasChildren: true, + operationName: `${spanName} operation`, + processID: `${spanName} processID`, + references: childOf + ? [ + { + refType: 'CHILD_OF', + span: childOf, + spanID: childOf.spanID, + }, + ] + : [], + spanID: `${spanName} spanID`, + }); + const makeTrace = (spans, traceID) => ({ + data: { + processes: spans.reduce( + (result, span) => ({ + ...result, + [span.processID]: { + serviceName: `${span.spanID.split(' ')[0]} service`, + }, + }), + {} + ), + spans, + traceID, }, - ]; - it('transforms trace results payload', () => { - const processed = tracesResults.map(transformTraceData); - const resultTraces = {}; - for (let i = 0; i < processed.length; i++) { - const data = processed[i]; - const id = data.traceID; - resultTraces[id] = { data, id, state: fetchedState.DONE }; - } - - const payload = transformTracesToPaths(resultTraces, 'customer'); - expect(payload.length).toBe(1); - expect(payload[0].path.length).toBe(2); + }); + + const linearTraceID = 'linearTraceID'; + const missTraceID = 'missTraceID'; + const shortTraceID = 'shortTraceID'; + const rootSpan = makeSpan('root'); + const childSpan = makeSpan('child', rootSpan); + const grandchildSpan = makeSpan('grandchild', childSpan); + + it('transforms single short trace results payload', () => { + const traces = { + [shortTraceID]: makeTrace([rootSpan, { ...childSpan, hasChildren: false }], shortTraceID), + }; + + const result = transformTracesToPaths(traces, 'child service'); + expect(result.length).toBe(1); + expect(result[0].path.length).toBe(2); + }); + + it('transforms multiple traces results payload', () => { + const traces = { + [shortTraceID]: makeTrace([rootSpan, { ...childSpan, hasChildren: false }], shortTraceID), + [linearTraceID]: makeTrace( + [rootSpan, childSpan, { ...grandchildSpan, hasChildren: false }], + linearTraceID + ), + }; + + const result = transformTracesToPaths(traces, 'child service'); + expect(result.length).toBe(2); + expect(result[0].path.length).toBe(2); + expect(result[1].path.length).toBe(3); + }); + + it('ignores paths without focalService', () => { + const branchingTraceID = 'branchingTraceID'; + const uncleSpan = makeSpan('uncle', rootSpan); + uncleSpan.hasChildren = false; + const traces = { + [missTraceID]: makeTrace([rootSpan, childSpan, uncleSpan], missTraceID), + [branchingTraceID]: makeTrace( + [rootSpan, childSpan, uncleSpan, { ...grandchildSpan, hasChildren: false }], + branchingTraceID + ), + }; + + const result = transformTracesToPaths(traces, 'child service'); + expect(result.length).toBe(1); + expect(result[0].path.length).toBe(3); + }); + + it('matches service and operation names', () => { + const childSpanWithDiffOp = { + ...childSpan, + hasChildren: false, + operationName: 'diff operation', + }; + const traces = { + [missTraceID]: makeTrace([rootSpan, childSpanWithDiffOp], missTraceID), + [linearTraceID]: makeTrace( + [rootSpan, childSpan, { ...grandchildSpan, hasChildren: false }], + linearTraceID + ), + }; + + const result = transformTracesToPaths(traces, 'child service'); + expect(result.length).toBe(2); + expect(result[0].path.length).toBe(2); + expect(result[1].path.length).toBe(3); + + const resultWithOp = transformTracesToPaths(traces, 'child service', 'child operation'); + expect(resultWithOp.length).toBe(1); + expect(resultWithOp[0].path.length).toBe(3); + }); + + it('transforms multiple paths from single trace', () => { + const traces = { + [linearTraceID]: makeTrace( + [rootSpan, { ...childSpan, hasChildren: false }, { ...grandchildSpan, hasChildren: false }], + linearTraceID + ), + }; + + const result = transformTracesToPaths(traces, 'child service'); + expect(result.length).toBe(2); + expect(result[0].path.length).toBe(2); + expect(result[1].path.length).toBe(3); + }); + + it('errors if span has ancestor id not in trace data', () => { + const traces = { + [linearTraceID]: makeTrace([rootSpan, { ...grandchildSpan, hasChildren: false }], linearTraceID), + }; + + expect(() => transformTracesToPaths(traces, 'child service')).toThrowError(/Ancestor spanID.*not found/); + }); + + it('skips trace without data', () => { + const traces = { + [shortTraceID]: makeTrace([rootSpan, { ...childSpan, hasChildren: false }], shortTraceID), + noData: {}, + }; + + const result = transformTracesToPaths(traces, 'child service'); + expect(result.length).toBe(1); }); }); diff --git a/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx index 3bf106c5fd..7c422715f0 100644 --- a/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx +++ b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx @@ -23,7 +23,7 @@ type Node = { }; const hasFocal = (path: TDdgPayloadEntry[], focalService: string, focalOperation: string | undefined) => { - for (let i = 0; i < path.length; ++i) { + for (let i = 0; i < path.length; i++) { if ( focalService === path[i].service && (focalOperation === undefined || focalOperation === path[i].operation) @@ -46,25 +46,28 @@ export default function( focalOperation: string | undefined ) { const paths: TDdgPayload = []; - Object.values(traces).forEach(trace => { - const map: Map = new Map(); - if (trace.data) { - const traceId = trace.data.traceID; - trace.data.spans + Object.values(traces).forEach(({ data }) => { + if (data) { + const spanMap: Map = new Map(); + const { traceID } = data; + data.spans .filter(span => { - map.set(span.spanID, span); + spanMap.set(span.spanID, span); return !span.hasChildren; }) .forEach(leaf => { - const path = spanAncestorIds(leaf) - // @ts-ignore - .map(id => convertSpan(map.get(id), trace.data)); - // @ts-ignore - path.push(convertSpan(map.get(leaf.spanID), trace.data)); + const path = spanAncestorIds(leaf).map(id => { + const span = spanMap.get(id); + if (!span) throw new Error(`Ancestor spanID ${id} not found in trace ${traceID}`); + + return convertSpan(span, data); + }); + path.push(convertSpan(leaf, data)); + if (hasFocal(path, focalService, focalOperation)) { paths.push({ path, - trace_id: traceId, + trace_id: traceID, }); } }); From f1a07cffd3dfc42690fc3c0d7f843f3b03140464 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Fri, 18 Oct 2019 15:51:05 -0400 Subject: [PATCH 3/4] Finish tests, fix search bug, optimize Signed-off-by: Everett Ross --- .../components/DeepDependencies/index.test.js | 4 +- .../src/components/DeepDependencies/index.tsx | 51 +++++----- .../DeepDependencies/traces.test.js | 92 ++++++++++++++++++- .../components/DeepDependencies/traces.tsx | 26 +++--- .../SearchTracePage/SearchResults/index.js | 11 +-- .../SearchResults/index.test.js | 46 +++++++++- .../src/model/ddg/transformDdgData.tsx | 5 +- .../src/model/ddg/transformTracesToPaths.tsx | 6 +- 8 files changed, 191 insertions(+), 50 deletions(-) diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js index c5b093afff..97c4bbaac4 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js @@ -388,8 +388,8 @@ describe('DeepDependencyGraphPage', () => { } ); let getUrlStateSpy; - let sanitizeUrlStateSpy; let makeGraphSpy; + let sanitizeUrlStateSpy; beforeAll(() => { getUrlStateSpy = jest.spyOn(url, 'getUrlState'); @@ -401,7 +401,7 @@ describe('DeepDependencyGraphPage', () => { getUrlStateSpy.mockReset(); getUrlStateSpy.mockReturnValue(expected.urlState); makeGraphSpy.mockReset(); - makeGraphSpy.mockReturnValue(mockGraph); + makeGraphSpy.mockReturnValue(mockGraph); // todo beforeall! }); it('uses gets relevant params from location.search', () => { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx index c23b8e3608..aca0fc3e5a 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx @@ -44,11 +44,11 @@ import { TDdgStateEntry } from '../../types/TDdgState'; import './index.css'; export type TDispatchProps = { - addViewModifier: (kwarg: TDdgModelParams & { viewModifier: number; visibilityIndices: number[] }) => void; - fetchDeepDependencyGraph: (query: TDdgModelParams) => void; - fetchServices: () => void; - fetchServiceOperations: (service: string) => void; - removeViewModifierFromIndices: ( + addViewModifier?: (kwarg: TDdgModelParams & { viewModifier: number; visibilityIndices: number[] }) => void; + fetchDeepDependencyGraph?: (query: TDdgModelParams) => void; + fetchServices?: () => void; + fetchServiceOperations?: (service: string) => void; + removeViewModifierFromIndices?: ( kwarg: TDdgModelParams & { viewModifier: number; visibilityIndices: number[] } ) => void; }; @@ -56,7 +56,7 @@ export type TDispatchProps = { export type TReduxProps = TExtractUiFindFromStateReturn & { graph: GraphModel | undefined; graphState?: TDdgStateEntry; - operationsForService: Record; + operationsForService?: Record; services?: string[] | null; urlState: TDdgSparseUrlState; }; @@ -82,7 +82,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { const { fetchDeepDependencyGraph, graphState = null, urlState } = props; const { service, operation } = urlState; // backend temporarily requires service and operation - if (!graphState && service && operation) { + if (!graphState && service && operation && fetchDeepDependencyGraph) { fetchDeepDependencyGraph({ service, operation, start: 0, end: 0 }); } } @@ -94,10 +94,15 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { const { fetchServices, fetchServiceOperations, operationsForService, services, urlState } = props; const { service } = urlState; - if (!services) { + if (!services && fetchServices) { fetchServices(); } - if (service && !Reflect.has(operationsForService, service)) { + if ( + service && + operationsForService && + !Reflect.has(operationsForService, service) && + fetchServiceOperations + ) { fetchServiceOperations(service); } } @@ -141,7 +146,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { setService = (service: string) => { const { fetchServiceOperations, operationsForService } = this.props; - if (!Reflect.has(operationsForService, service)) { + if (operationsForService && !Reflect.has(operationsForService, service) && fetchServiceOperations) { fetchServiceOperations(service); } this.updateUrlState({ operation: undefined, service, visEncoding: undefined }); @@ -159,14 +164,16 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { } const visibilityIndices = pathElems.map(pe => pe.visibilityIdx); const fn = enable ? addViewModifier : removeViewModifierFromIndices; - fn({ - operation, - service, - viewModifier, - visibilityIndices, - end: 0, - start: 0, - }); + if (fn) { + fn({ + operation, + service, + viewModifier, + visibilityIndices, + end: 0, + start: 0, + }); + } }; showVertices = (vertices: TDdgVertex[]) => { @@ -218,17 +225,17 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { content = ( ); } else if (graphState.state === fetchedState.LOADING) { @@ -253,7 +260,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { distanceToPathElems={distanceToPathElems} hiddenUiFindMatches={hiddenUiFindMatches} operation={operation} - operations={operationsForService[service || '']} + operations={operationsForService && operationsForService[service || '']} service={service} services={services} setDensity={this.setDensity} @@ -299,7 +306,7 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP }; } -// export for traces ddg +// export for tests export function mapDispatchToProps(dispatch: Dispatch): TDispatchProps { const { fetchDeepDependencyGraph, fetchServiceOperations, fetchServices } = bindActionCreators( jaegerApiActions, diff --git a/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js b/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js index 093a28d27f..20333e571c 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/traces.test.js @@ -17,8 +17,12 @@ import { shallow } from 'enzyme'; import queryString from 'query-string'; import { DeepDependencyGraphPageImpl } from '.'; -import { TracesDdgImpl /* , mapStateToProps */ } from './traces'; +import { TracesDdgImpl, mapStateToProps } from './traces'; +import * as url from './url'; import { ROUTE_PATH } from '../SearchTracePage/url'; +import * as GraphModel from '../../model/ddg/GraphModel'; +import * as transformDdgData from '../../model/ddg/transformDdgData'; +import * as transformTracesToPaths from '../../model/ddg/transformTracesToPaths'; describe('TracesDdg', () => { it('renders DeepDependencyGraphPageImpl with specific props', () => { @@ -48,6 +52,90 @@ describe('TracesDdg', () => { }); describe('mapStateToProps()', () => { - /* TODO After merging in master */ + const hash = 'test hash'; + const mockModel = { hash }; + const mockGraph = { model: mockModel }; + const mockPayload = 'test payload'; + const urlState = { + service: 'testService', + operation: 'testOperation', + visEncoding: 'testVisEncoding', + }; + const ownProps = { + location: { + search: queryString.stringify(urlState), + }, + }; + const state = { + router: { location: ownProps.location }, + trace: { + traces: { + testTraceID: 'test trace data', + }, + }, + }; + + let getUrlStateSpy; + let makeGraphSpy; + let sanitizeUrlStateSpy; + let spies; + let transformDdgDataSpy; + let transformTracesToPathsSpy; + + beforeAll(() => { + getUrlStateSpy = jest.spyOn(url, 'getUrlState'); + makeGraphSpy = jest.spyOn(GraphModel, 'makeGraph').mockReturnValue(mockGraph); + sanitizeUrlStateSpy = jest.spyOn(url, 'sanitizeUrlState').mockImplementation(u => u); + transformDdgDataSpy = jest.spyOn(transformDdgData, 'default').mockReturnValue(mockModel); + transformTracesToPathsSpy = jest.spyOn(transformTracesToPaths, 'default').mockReturnValue(mockPayload); + spies = [ + getUrlStateSpy, + makeGraphSpy, + sanitizeUrlStateSpy, + transformDdgDataSpy, + transformTracesToPathsSpy, + ]; + }); + + beforeEach(() => { + spies.forEach(spy => spy.mockClear()); + getUrlStateSpy.mockReturnValue(urlState); + }); + + it('gets props from url', () => { + expect(mapStateToProps(state, ownProps)).toEqual(expect.objectContaining({ urlState })); + }); + + it('calculates graphState and graph iff service is provided', () => { + expect(mapStateToProps(state, ownProps)).toEqual( + expect.objectContaining({ + graph: mockGraph, + graphState: expect.objectContaining({ model: mockModel }), + }) + ); + + const { service: _, ...urlStateWithoutService } = urlState; + getUrlStateSpy.mockReturnValue(urlStateWithoutService); + expect(mapStateToProps(state, ownProps)).toEqual( + expect.objectContaining({ + graph: undefined, + graphState: undefined, + }) + ); + }); + + it('feeds memoized functions same arguments for same url and state data', () => { + mapStateToProps(state, ownProps); + mapStateToProps(state, ownProps); + spies.forEach(spy => { + const [call0, call1] = spy.mock.calls; + call0.forEach((arg, i) => expect(call1[i]).toBe(arg)); + }); + }); + + it('sanitizes url', () => { + mapStateToProps(state, ownProps); + expect(sanitizeUrlStateSpy).toHaveBeenLastCalledWith(urlState, hash); + }); }); }); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx b/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx index a5a8ba1c7c..ac2ab65823 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx @@ -11,11 +11,15 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + import queryString from 'query-string'; +import _get from 'lodash/get'; +import memoizeOne from 'memoize-one'; import * as React from 'react'; import { connect } from 'react-redux'; + import { ReduxState } from '../../types'; -import { getUrlState } from './url'; +import { getUrlState, sanitizeUrlState } from './url'; import { TDdgStateEntry } from '../../types/TDdgState'; import GraphModel, { makeGraph } from '../../model/ddg/GraphModel'; import { fetchedState } from '../../constants'; @@ -23,20 +27,21 @@ import { extractUiFindFromState } from '../common/UiFindInput'; import transformDdgData from '../../model/ddg/transformDdgData'; import transformTracesToPaths from '../../model/ddg/transformTracesToPaths'; import { ROUTE_PATH } from '../SearchTracePage/url'; -import { DeepDependencyGraphPageImpl, mapDispatchToProps, TOwnProps, TProps, TReduxProps } from '.'; +import { DeepDependencyGraphPageImpl, TOwnProps, TProps, TReduxProps } from '.'; + +// Required for proper memoization of subsequent function calls +const svcOp = memoizeOne((service, operation) => ({ service, operation })); // export for tests export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxProps { - const { services: stServices, trace } = state; - const { services, operationsForService } = stServices; const urlState = getUrlState(ownProps.location.search); const { density, operation, service, showOp } = urlState; let graphState: TDdgStateEntry | undefined; let graph: GraphModel | undefined; if (service) { - const payload = transformTracesToPaths(trace.traces, service, operation); + const payload = transformTracesToPaths(state.trace.traces, service, operation); graphState = { - model: transformDdgData(payload, { service, operation }), + model: transformDdgData(payload, svcOp(service, operation)), state: fetchedState.DONE, viewModifiers: new Map(), }; @@ -46,9 +51,7 @@ export function mapStateToProps(state: ReduxState, ownProps: TOwnProps): TReduxP return { graph, graphState, - operationsForService, - services, - urlState, + urlState: sanitizeUrlState(urlState, _get(graphState, 'model.hash')), ...extractUiFindFromState(state), }; } @@ -71,7 +74,4 @@ export class TracesDdgImpl extends React.PureComponent void, @@ -125,11 +124,7 @@ export class UnconnectedSearchResults extends React.PureComponent diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js index 08ebd3d34c..67d6a868d4 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js @@ -17,10 +17,13 @@ import { shallow } from 'enzyme'; import { UnconnectedSearchResults as SearchResults } from '.'; import * as markers from './index.markers'; +import AltViewOptions from './AltViewOptions'; +import DiffSelection from './DiffSelection'; import ResultItem from './ResultItem'; import ScatterPlot from './ScatterPlot'; -import DiffSelection from './DiffSelection'; +import { getUrl } from '../url'; import LoadingIndicator from '../../common/LoadingIndicator'; +import SearchResultsDDG from '../../DeepDependencies/traces'; describe('', () => { let wrapper; @@ -32,6 +35,7 @@ describe('', () => { props = { diffCohort: [], goToTrace: () => {}, + location: {}, loading: false, maxTraceDuration: 1, queryOfResults: {}, @@ -68,5 +72,45 @@ describe('', () => { it('shows a result entry for each trace', () => { expect(wrapper.find(ResultItem).length).toBe(traces.length); }); + + describe('ddg', () => { + const searchParam = 'view'; + const viewDdg = 'ddg'; + const viewTraces = 'traces'; + const search = `${searchParam}=${viewDdg}`; + + it('updates url to view ddg and back and back again', () => { + const otherParam = 'param'; + const otherValue = 'value'; + const otherSearch = `?${otherParam}=${otherValue}`; + const push = jest.fn(); + wrapper.setProps({ history: { push }, location: { search: otherSearch } }); + + const toggle = wrapper.find(AltViewOptions).prop('onTraceGraphViewClicked'); + toggle(); + expect(push).toHaveBeenLastCalledWith(getUrl({ [otherParam]: otherValue, [searchParam]: viewDdg })); + + wrapper.setProps({ location: { search: `${otherSearch}&${search}` } }); + toggle(); + expect(push).toHaveBeenLastCalledWith( + getUrl({ [otherParam]: otherValue, [searchParam]: viewTraces }) + ); + + wrapper.setProps({ location: { search: `${otherSearch}&${searchParam}=${viewTraces}` } }); + toggle(); + expect(push).toHaveBeenLastCalledWith(getUrl({ [otherParam]: otherValue, [searchParam]: viewDdg })); + }); + + it('shows ddg instead of scatterplot and results', () => { + expect(wrapper.find(SearchResultsDDG).length).toBe(0); + expect(wrapper.find(ResultItem).length).not.toBe(0); + expect(wrapper.find(ScatterPlot).length).not.toBe(0); + + wrapper.setProps({ location: { search: `?${search}` } }); + expect(wrapper.find(SearchResultsDDG).length).toBe(1); + expect(wrapper.find(ResultItem).length).toBe(0); + expect(wrapper.find(ScatterPlot).length).toBe(0); + }); + }); }); }); diff --git a/packages/jaeger-ui/src/model/ddg/transformDdgData.tsx b/packages/jaeger-ui/src/model/ddg/transformDdgData.tsx index bd6bff9af3..4680fe36c3 100644 --- a/packages/jaeger-ui/src/model/ddg/transformDdgData.tsx +++ b/packages/jaeger-ui/src/model/ddg/transformDdgData.tsx @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +import memoizeOne from 'memoize-one'; import objectHash from 'object-hash'; import { @@ -36,7 +37,7 @@ function group(arg: { key: string; value: any }[]): Record { return result; } -export default function transformDdgData( +function transformDdgData( { dependencies }: TDdgPayload, { service: focalService, operation: focalOperation }: { service: string; operation?: string } ): TDdgModel { @@ -152,3 +153,5 @@ export default function transformDdgData( visIdxToPathElem, }; } + +export default memoizeOne(transformDdgData); diff --git a/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx index fe3d84eba6..3199e686a8 100644 --- a/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx +++ b/packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +import memoizeOne from 'memoize-one'; + import { TDdgPayloadEntry, TDdgPayloadPath, TDdgPayload } from './types'; import { Span, Trace } from '../../types/trace'; import { FetchedTrace } from '../../types'; @@ -40,7 +42,7 @@ function convertSpan(span: Span, trace: Trace): TDdgPayloadEntry { return { service: serviceName, operation: operationName }; } -export default function( +function transformTracesToPaths( traces: Record, focalService: string, focalOperation: string | undefined @@ -80,3 +82,5 @@ export default function( }); return { dependencies }; } + +export default memoizeOne(transformTracesToPaths); From 12f31c1358d81879cd946dc8a18afbee3edb9567 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Mon, 21 Oct 2019 11:25:40 -0400 Subject: [PATCH 4/4] Add last tests, clean up order/unused Signed-off-by: Everett Ross --- .../DeepDependencies/Graph/index.tsx | 4 +- .../DeepDependencies/Header/index.tsx | 2 +- .../components/DeepDependencies/index.test.js | 7 ++-- .../src/components/DeepDependencies/index.tsx | 2 +- .../components/DeepDependencies/traces.tsx | 13 +++--- .../src/components/DeepDependencies/url.tsx | 10 ++--- .../SearchResults/AltViewOptions.test.js | 40 +++++++++++++++++++ .../SearchResults/AltViewOptions.tsx | 2 +- .../SearchTracePage/SearchResults/index.js | 10 ++--- .../src/model/ddg/transformTracesToPaths.tsx | 29 +++++--------- 10 files changed, 72 insertions(+), 47 deletions(-) create mode 100644 packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Graph/index.tsx index bb1ecff3f8..a5deb6932d 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/index.tsx @@ -27,17 +27,17 @@ import { PathElem, TDdgVertex, EDdgDensity, EViewModifier } from '../../../model import './index.css'; type TProps = { + baseUrl: string; density: EDdgDensity; edges: TEdge[]; edgesViewModifiers: Map; + extraUrlArgs?: { [key: string]: unknown }; getVisiblePathElems: (vertexKey: string) => PathElem[] | undefined; setViewModifier: (vertexKey: string, viewModifier: EViewModifier, enable: boolean) => void; showOp: boolean; uiFindMatches: Set | undefined; vertices: TDdgVertex[]; verticesViewModifiers: Map; - baseUrl: string; - extraUrlArgs?: { [key: string]: unknown }; }; // The dichotomy between w/ & w/o VMs assumes that any edge VM neccesitates unmodified edges are de-emphasized diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx index 43718b513b..b0e2a20980 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx @@ -37,11 +37,11 @@ type TProps = { setOperation: (operation: string) => void; setService: (service: string) => void; showOperations: boolean; + showParameters?: boolean; showVertices: (vertices: TDdgVertex[]) => void; toggleShowOperations: (enable: boolean) => void; uiFindCount: number | undefined; visEncoding?: string; - showParameters?: boolean; }; export default class Header extends React.PureComponent { private _uiFindInput: React.RefObject = React.createRef(); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js index 97c4bbaac4..0de5ca36d1 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js @@ -394,14 +394,13 @@ describe('DeepDependencyGraphPage', () => { beforeAll(() => { getUrlStateSpy = jest.spyOn(url, 'getUrlState'); sanitizeUrlStateSpy = jest.spyOn(url, 'sanitizeUrlState'); - makeGraphSpy = jest.spyOn(GraphModel, 'makeGraph'); + makeGraphSpy = jest.spyOn(GraphModel, 'makeGraph').mockReturnValue(mockGraph); }); beforeEach(() => { - getUrlStateSpy.mockReset(); + getUrlStateSpy.mockClear(); getUrlStateSpy.mockReturnValue(expected.urlState); - makeGraphSpy.mockReset(); - makeGraphSpy.mockReturnValue(mockGraph); // todo beforeall! + makeGraphSpy.mockClear(); }); it('uses gets relevant params from location.search', () => { diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx index aca0fc3e5a..50734cce7d 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx @@ -255,7 +255,6 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent {
{ setOperation={this.setOperation} setService={this.setService} showOperations={showOp} + showParameters={showSvcOpsHeader} showVertices={this.showVertices} toggleShowOperations={this.toggleShowOperations} uiFindCount={uiFind ? uiFindMatches && uiFindMatches.size : undefined} diff --git a/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx b/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx index ac2ab65823..e7cf44b0d0 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/traces.tsx @@ -12,22 +12,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -import queryString from 'query-string'; +import * as React from 'react'; import _get from 'lodash/get'; import memoizeOne from 'memoize-one'; -import * as React from 'react'; +import queryString from 'query-string'; import { connect } from 'react-redux'; -import { ReduxState } from '../../types'; +import { DeepDependencyGraphPageImpl, TOwnProps, TProps, TReduxProps } from '.'; import { getUrlState, sanitizeUrlState } from './url'; -import { TDdgStateEntry } from '../../types/TDdgState'; +import { ROUTE_PATH } from '../SearchTracePage/url'; import GraphModel, { makeGraph } from '../../model/ddg/GraphModel'; import { fetchedState } from '../../constants'; import { extractUiFindFromState } from '../common/UiFindInput'; import transformDdgData from '../../model/ddg/transformDdgData'; import transformTracesToPaths from '../../model/ddg/transformTracesToPaths'; -import { ROUTE_PATH } from '../SearchTracePage/url'; -import { DeepDependencyGraphPageImpl, TOwnProps, TProps, TReduxProps } from '.'; + +import { TDdgStateEntry } from '../../types/TDdgState'; +import { ReduxState } from '../../types'; // Required for proper memoization of subsequent function calls const svcOp = memoizeOne((service, operation) => ({ service, operation })); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/url.tsx b/packages/jaeger-ui/src/components/DeepDependencies/url.tsx index 6228f9ad64..e31564d6c6 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/url.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/url.tsx @@ -28,11 +28,7 @@ export function matches(path: string) { return Boolean(matchPath(path, ROUTE_MATCHER)); } -export function getUrl(args?: { [key: string]: unknown; showOp?: boolean }, baseUrl?: string) { - let basePath: string | undefined = baseUrl; - if (!basePath) { - basePath = ROUTE_PATH; - } +export function getUrl(args?: { [key: string]: unknown; showOp?: boolean }, baseUrl: string = ROUTE_PATH) { if (args && !_isEmpty(args)) { const stringifyArgs = Reflect.has(args, 'showOp') ? { @@ -40,9 +36,9 @@ export function getUrl(args?: { [key: string]: unknown; showOp?: boolean }, base showOp: args.showOp ? 1 : 0, } : args; - return `${basePath}?${queryString.stringify(stringifyArgs)}`; + return `${baseUrl}?${queryString.stringify(stringifyArgs)}`; } - return basePath; + return baseUrl; } function firstParam(arg: string | string[]): string { diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js new file mode 100644 index 0000000000..74da688e73 --- /dev/null +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.test.js @@ -0,0 +1,40 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import * as React from 'react'; +import { shallow } from 'enzyme'; +import { Button } from 'antd'; + +import AltViewOptions from './AltViewOptions'; + +describe('AltViewOptions', () => { + const props = { + traceResultsView: true, + onTraceGraphViewClicked: jest.fn(), + }; + let wrapper; + + beforeEach(() => { + props.onTraceGraphViewClicked.mockClear(); + wrapper = shallow(); + }); + + it('renders correct label', () => { + const getLabel = () => wrapper.find(Button).prop('children'); + expect(getLabel()).toBe('Deep Dependency Graph'); + + wrapper.setProps({ traceResultsView: false }); + expect(getLabel()).toBe('Trace Results'); + }); +}); diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx index a023a90cd9..d99d9b1248 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/AltViewOptions.tsx @@ -16,8 +16,8 @@ import * as React from 'react'; import { Button } from 'antd'; type Props = { - traceResultsView: boolean; onTraceGraphViewClicked: () => void; + traceResultsView: boolean; }; export default function AltViewOptions(props: Props) { diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js index 5ea54aa445..912092a583 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js @@ -16,9 +16,9 @@ import * as React from 'react'; import { Select } from 'antd'; +import { History as RouterHistory, Location } from 'history'; import { Link, withRouter } from 'react-router-dom'; import { Field, formValueSelector, reduxForm } from 'redux-form'; -import { History as RouterHistory, Location } from 'history'; import queryString from 'query-string'; import AltViewOptions from './AltViewOptions'; @@ -48,14 +48,14 @@ type SearchResultsProps = { disableComparisons: boolean, goToTrace: string => void, hideGraph: boolean, + history: RouterHistory, loading: boolean, + location: Location, maxTraceDuration: number, queryOfResults?: SearchQuery, showStandaloneLink: boolean, skipMessage?: boolean, traces: TraceSummary[], - history: RouterHistory, - location: Location, }; const Option = Select.Option; @@ -114,14 +114,14 @@ export class UnconnectedSearchResults extends React.PureComponent { - for (let i = 0; i < path.length; i++) { - if ( - focalService === path[i].service && - (focalOperation === undefined || focalOperation === path[i].operation) - ) { - return true; - } - } - return false; -}; +import { TDdgPayloadEntry, TDdgPayloadPath, TDdgPayload } from './types'; +import { FetchedTrace } from '../../types'; +import { Span, Trace } from '../../types/trace'; function convertSpan(span: Span, trace: Trace): TDdgPayloadEntry { const serviceName = trace.processes[span.processID].serviceName; @@ -66,7 +50,12 @@ function transformTracesToPaths( }); path.push(convertSpan(leaf, data)); - if (hasFocal(path, focalService, focalOperation)) { + if ( + path.some( + ({ service, operation }) => + service === focalService && (!focalOperation || operation === focalOperation) + ) + ) { dependencies.push({ path, attributes: [