Skip to content

Commit

Permalink
Search results link spans (jaegertracing#536)
Browse files Browse the repository at this point in the history
* Add span linking to search results
* Add more url tests
* Change span csv to space seperated value
* Handle span param without spanIDs

Signed-off-by: Everett Ross <reverett@uber.com>
  • Loading branch information
everett980 authored Mar 4, 2020
1 parent 916b2ae commit d7bbc7b
Show file tree
Hide file tree
Showing 9 changed files with 321 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type SearchResultsProps = {
queryOfResults?: SearchQuery,
showStandaloneLink: boolean,
skipMessage?: boolean,
spanLinks?: Record<string, string> | undefined | null,
traces: TraceSummary[],
};

Expand Down Expand Up @@ -91,7 +92,7 @@ export const sortFormSelector = formValueSelector('traceResultsSort');
export class UnconnectedSearchResults extends React.PureComponent<SearchResultsProps> {
props: SearchResultsProps;

static defaultProps = { skipMessage: false, queryOfResults: undefined };
static defaultProps = { skipMessage: false, spanLinks: undefined, queryOfResults: undefined };

toggleComparison = (traceID: string, remove: boolean) => {
const { cohortAddTrace, cohortRemoveTrace } = this.props;
Expand Down Expand Up @@ -123,6 +124,7 @@ export class UnconnectedSearchResults extends React.PureComponent<SearchResultsP
queryOfResults,
showStandaloneLink,
skipMessage,
spanLinks,
traces,
} = this.props;

Expand Down Expand Up @@ -203,7 +205,11 @@ export class UnconnectedSearchResults extends React.PureComponent<SearchResultsP
<ResultItem
durationPercent={getPercentageOfDuration(trace.duration, maxTraceDuration)}
isInDiffCohort={cohortIds.has(trace.traceID)}
linkTo={getLocation(trace.traceID, { fromSearch: searchUrl })}
linkTo={getLocation(
trace.traceID,
{ fromSearch: searchUrl },
spanLinks && spanLinks[trace.traceID]
)}
toggleComparison={this.toggleComparison}
trace={trace}
disableComparision={disableComparisons}
Expand Down
7 changes: 4 additions & 3 deletions packages/jaeger-ui/src/components/SearchTracePage/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@
import React, { Component } from 'react';
import { Col, Row, Tabs } from 'antd';
import PropTypes from 'prop-types';
import queryString from 'query-string';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import store from 'store';
import memoizeOne from 'memoize-one';

import SearchForm from './SearchForm';
import SearchResults, { sortFormSelector } from './SearchResults';
import { isSameQuery, getUrl } from './url';
import { isSameQuery, getUrl, getUrlState } from './url';
import * as jaegerApiActions from '../../actions/jaeger-api';
import * as fileReaderActions from '../../actions/file-reader-api';
import ErrorMessage from '../common/ErrorMessage';
Expand Down Expand Up @@ -90,6 +89,7 @@ export class SearchTracePageImpl extends Component {
traceResults,
queryOfResults,
loadJsonTraces,
urlQueryParams,
} = this.props;
const hasTraceResults = traceResults && traceResults.length > 0;
const showErrors = errors && !loadingTraces;
Expand Down Expand Up @@ -132,6 +132,7 @@ export class SearchTracePageImpl extends Component {
queryOfResults={queryOfResults}
showStandaloneLink={Boolean(embedded)}
skipMessage={isHomepage}
spanLinks={urlQueryParams && urlQueryParams.spanLinks}
traces={traceResults}
/>
)}
Expand Down Expand Up @@ -232,7 +233,7 @@ const stateServicesXformer = memoizeOne(stateServices => {
// export to test
export function mapStateToProps(state) {
const { embedded, router, services: stServices, traceDiff } = state;
const query = queryString.parse(router.location.search);
const query = getUrlState(router.location.search);
const isHomepage = !Object.keys(query).length;
const { query: queryOfResults, traces, maxDuration, traceError, loadingTraces } = stateTraceXformer(
state.trace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ describe('<SearchTracePage>', () => {
expect(historyPush.mock.calls.length).toBe(1);
expect(historyPush.mock.calls[0][0]).toEqual({
pathname: `/trace/${traceID}`,
search: undefined,
state: { fromSearch: '/search?' },
});
});
Expand Down
198 changes: 198 additions & 0 deletions packages/jaeger-ui/src/components/SearchTracePage/url.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
// Copyright (c) 2020 The Jaeger Authors.
//
// 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 { getUrl, getUrlState, isSameQuery } from './url';

describe('SearchTracePage/url', () => {
const span0 = 'span-0';
const span1 = 'span-1';
const span2 = 'span-2';
const trace0 = 'trace-0';
const trace1 = 'trace-1';
const trace2 = 'trace-2';

describe('getUrl', () => {
it('handles no args given', () => {
expect(getUrl()).toBe('/search');
});

it('handles empty args', () => {
expect(getUrl({})).toBe('/search?');
});

it('includes provided args', () => {
const paramA = 'aParam';
const paramB = 'bParam';
expect(getUrl({ paramA, paramB })).toBe(`/search?paramA=${paramA}&paramB=${paramB}`);
});

it('preserves traceID without spanLinks', () => {
expect(
getUrl({
traceID: trace0,
})
).toBe(`/search?traceID=${trace0}`);
});

it('converts spanLink and traceID to traceID and span', () => {
expect(
getUrl({
traceID: trace0,
spanLinks: {
[trace0]: span0,
},
})
).toBe(`/search?span=${span0}%40${trace0}`);
});

it('handles missing traceID for spanLinks', () => {
expect(
getUrl({
spanLinks: {
[trace0]: span0,
},
})
).toBe(`/search?span=${span0}%40${trace0}`);
});

it('handles empty spanLinks', () => {
expect(
getUrl({
spanLinks: {},
})
).toBe(`/search?`);
});

it('converts spanLink and other traceID to traceID and span', () => {
expect(
getUrl({
traceID: [trace0, trace2],
spanLinks: {
[trace0]: span0,
},
})
).toBe(`/search?span=${span0}%40${trace0}&traceID=${trace2}`);
});

it('converts spanLinks to traceID and span', () => {
expect(
getUrl({
traceID: [trace0, trace1, trace2],
spanLinks: {
[trace0]: `${span0} ${span1}`,
[trace1]: span2,
},
})
).toBe(`/search?span=${span0}%20${span1}%40${trace0}&span=${span2}%40${trace1}&traceID=${trace2}`);
});
});

describe('getUrlState', () => {
it('gets search params', () => {
const service = 'svc-0';
const operation = 'op-0';
expect(getUrlState(`service=${service}&operation=${operation}`)).toEqual({ service, operation });
});

it('converts span to traceID and spanLinks', () => {
expect(getUrlState(`span=${span0}%40${trace0}`)).toEqual({
traceID: [trace0],
spanLinks: {
[trace0]: span0,
},
});
});

it('converts multiple spans to traceID and spanLinks', () => {
expect(
getUrlState(`span=${span0}%20${span1}%40${trace0}&span=${span2}%40${trace1}&traceID=${trace2}`)
).toEqual({
traceID: expect.arrayContaining([trace0, trace1, trace2]),
spanLinks: {
[trace0]: `${span0} ${span1}`,
[trace1]: span2,
},
});
});

it('converts span param without spanIDs to just traceID', () => {
expect(getUrlState(`span=${span0}%20${span1}%40${trace0}&span=%40${trace1}&traceID=${trace2}`)).toEqual(
{
traceID: expect.arrayContaining([trace0, trace1, trace2]),
spanLinks: {
[trace0]: `${span0} ${span1}`,
},
}
);
});

it('handles duplicate traceIDs', () => {
expect(
getUrlState(
`span=${span0}%40${trace0}&span=${span1}%40${trace0}&span=${span2}%40${trace1}&traceID=${trace1}&traceID=${trace2}`
)
).toEqual({
traceID: expect.arrayContaining([trace0, trace1, trace2]),
spanLinks: {
[trace0]: `${span0} ${span1}`,
[trace1]: span2,
},
});
});
});

describe('isSameQuery', () => {
const queryKeys = [
'end',
'limit',
'lookback',
'maxDuration',
'minDuration',
'operation',
'service',
'start',
'tags',
];
const otherKey = 'other-key';
const baseQuery = queryKeys.reduce(
(res, curr, i) => ({
...res,
[curr]: i % 2 ? curr : i,
}),
{ [otherKey]: otherKey }
);

it('returns `false` if only one argument is falsy', () => {
expect(isSameQuery(baseQuery)).toBe(false);
});

it('returns `false` if a considered key is changed or omitted', () => {
queryKeys.forEach(key => {
// eslint-disable-next-line camelcase
const { [key]: _omitted, ...rest } = baseQuery;
expect(isSameQuery(baseQuery, rest)).toBe(false);
expect(isSameQuery(baseQuery, { ...rest, [key]: 'changed' })).toBe(false);
});
});

it('returns `true` if no considered keys are changed or omitted', () => {
expect(isSameQuery(baseQuery, { ...baseQuery })).toBe(true);

// eslint-disable-next-line camelcase
const { [otherKey]: _omitted, ...copy } = baseQuery;
expect(isSameQuery(baseQuery, copy)).toBe(true);
expect(isSameQuery(baseQuery, { ...copy, [otherKey]: 'changed' })).toBe(true);
});
});
});
50 changes: 47 additions & 3 deletions packages/jaeger-ui/src/components/SearchTracePage/url.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import memoizeOne from 'memoize-one';
import queryString from 'query-string';
import { matchPath } from 'react-router-dom';

Expand All @@ -31,11 +32,54 @@ export function matches(path: string) {
return Boolean(matchPath(path, ROUTE_MATCHER));
}

export function getUrl(query?: Record<string, unknown> | null | undefined) {
const search = query ? `?${queryString.stringify(query)}` : '';
return prefixUrl(`/search${search}`);
type TUrlState = Record<string, string | string[] | undefined | Record<string, string>> & {
traceID?: string | string[];
spanLinks?: Record<string, string>;
};

export function getUrl(query?: TUrlState) {
const searchUrl = prefixUrl(`/search`);
if (!query) return searchUrl;

const { traceID, spanLinks, ...rest } = query;
let ids = traceID;
if (spanLinks && traceID) {
ids = (Array.isArray(traceID) ? traceID : [traceID]).filter((id: string) => !spanLinks[id]);
}
const stringifyArg = {
...rest,
span:
spanLinks &&
Object.keys(spanLinks).reduce((res: string[], trace: string) => {
return [...res, `${spanLinks[trace]}@${trace}`];
}, []),
traceID: ids && ids.length ? ids : undefined,
};
return `${searchUrl}?${queryString.stringify(stringifyArg)}`;
}

export const getUrlState: (search: string) => TUrlState = memoizeOne(function getUrlState(
search: string
): TUrlState {
const { traceID, span, ...rest } = queryString.parse(search);
const rv: TUrlState = { ...rest };
const traceIDs = new Set(!traceID || Array.isArray(traceID) ? traceID : [traceID]);
const spanLinks: Record<string, string> = {};
if (span && span.length) {
(Array.isArray(span) ? span : [span]).forEach(s => {
const [spansStr, trace] = s.split('@');
traceIDs.add(trace);
if (spansStr) {
if (spanLinks[trace]) spanLinks[trace] = spanLinks[trace].concat(' ', spansStr);
else spanLinks[trace] = spansStr;
}
});
rv.spanLinks = spanLinks;
}
if (traceIDs.size) rv.traceID = [...traceIDs];
return rv;
});

export function isSameQuery(a: SearchQuery, b: SearchQuery) {
if (Boolean(a) !== Boolean(b)) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe(ReferenceLink, () => {

it('render for external trace', () => {
const component = shallow(<ReferenceLink reference={externalRef} focusSpan={focusMock} />);
const link = component.find('a[href="/trace/trace2/uiFind?=span2"]');
const link = component.find('a[href="/trace/trace2?uiFind=span2"]');
expect(link.length).toBe(1);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ type ReferenceLinkProps = {
onClick?: () => void;
};

const linkToExternalSpan = (traceID: string, spanID: string) => `${getUrl(traceID)}/uiFind?=${spanID}`;

export default function ReferenceLink(props: ReferenceLinkProps) {
const { reference, children, className, focusSpan, ...otherProps } = props;
delete otherProps.onClick;
Expand All @@ -38,7 +36,7 @@ export default function ReferenceLink(props: ReferenceLinkProps) {
}
return (
<a
href={linkToExternalSpan(reference.traceID, reference.spanID)}
href={getUrl(reference.traceID, reference.spanID)}
target="_blank"
rel="noopener noreferrer"
className={className}
Expand Down
Loading

0 comments on commit d7bbc7b

Please sign in to comment.