Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Virtualized traced results list #337

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/jaeger-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"json-markup": "^1.1.0",
"lodash": "^4.17.4",
"logfmt": "^1.2.0",
"memoize-one": "^5.0.0",
"moment": "^2.18.1",
"prop-types": "^15.5.10",
"query-string": "^5.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export default class DiffSelection extends React.PureComponent<Props> {
</Button>
);
return (
<div className={`DiffSelection ${traces.length ? 'is-non-empty' : ''} ub-mb3`}>
<div className={`DiffSelection ${traces.length ? 'is-non-empty' : ''}`}>
{traces.length > 0 && (
<div className="DiffSelection--selectedItems">
{traces.map(fetchedTrace => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ limitations under the License.
}

.ResultItem--serviceTag {
border-left-width: 15px;
margin: 0;
background: #fff;
border-left: 15px solid transparent;
border-radius: 4px;
box-shadow: 0 0 1px rgba(0, 0, 0, 0.5);
color: rgba(0, 0, 0, 0.65);
display: inline-block;
font-size: 0.9em;
line-height: 1;
margin: 0.3em 0.35em;
padding: 0.3em;
}

.ResultItem--serviceCount {
font-size: 0.9em;
padding-left: 0.2em;
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
// limitations under the License.

import * as React from 'react';
import { Col, Divider, Row, Tag } from 'antd';
import { Link } from 'react-router-dom';

import { sortBy } from 'lodash';
Expand Down Expand Up @@ -60,55 +59,56 @@ export default class ResultItem extends React.PureComponent<Props> {
const numSpans = spans.length;
const numErredSpans = spans.filter(sp => sp.tags.some(isErrorTag)).length;
return (
<div className="ResultItem">
<ResultItemTitle
duration={duration}
durationPercent={durationPercent}
isInDiffCohort={isInDiffCohort}
linkTo={linkTo}
toggleComparison={toggleComparison}
traceID={traceID}
traceName={traceName}
disableComparision={disableComparision}
/>
<Link to={linkTo}>
<Row>
<Col span={4} className="ub-p2">
<Tag className="ub-m1" data-test={markers.NUM_SPANS}>
{numSpans} Span{numSpans > 1 && 's'}
</Tag>
{Boolean(numErredSpans) && (
<Tag className="ub-m1" color="red">
{numErredSpans} Error{numErredSpans > 1 && 's'}
</Tag>
)}
</Col>
<Col span={16} className="ub-p2">
<ul className="ub-list-reset" data-test={markers.SERVICE_TAGS}>
{sortBy(services, s => s.name).map(service => {
const { name, numberOfSpans: count } = service;
return (
<li key={name} className="ub-inline-block ub-m1">
<Tag
<div className="ub-pt3">
<div className="ResultItem">
<ResultItemTitle
duration={duration}
durationPercent={durationPercent}
isInDiffCohort={isInDiffCohort}
linkTo={linkTo}
toggleComparison={toggleComparison}
traceID={traceID}
traceName={traceName}
disableComparision={disableComparision}
/>
<Link to={linkTo}>
<div className="ant-row">
<div className="ant-col-4 ub-p2">
<div className="ant-tag ub-m1" data-test={markers.NUM_SPANS}>
{numSpans} Span{numSpans > 1 && 's'}
</div>
{Boolean(numErredSpans) && (
<div className="ant-tag ant-tag-red ub-m1">
{numErredSpans} Error{numErredSpans > 1 && 's'}
</div>
)}
</div>
<div className="ant-col-16 ub-p2">
<ul className="ub-list-reset" data-test={markers.SERVICE_TAGS}>
{sortBy(services, s => s.name).map(service => {
const { name, numberOfSpans: count } = service;
return (
<li
key={name}
className="ResultItem--serviceTag"
style={{ borderLeftColor: colorGenerator.getColorByKey(name) }}
>
{name} ({count})
</Tag>
</li>
);
})}
</ul>
</Col>
<Col span={4} className="ub-p3 ub-tx-right-align">
{formatRelativeDate(startTime / 1000)}
<Divider type="vertical" />
{timeStr.slice(0, -3)}&nbsp;{timeStr.slice(-2)}
<br />
<small>{fromNow}</small>
</Col>
</Row>
</Link>
{name} <span className="ResultItem--serviceCount">({count})</span>
</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this, the <Tag...> seems pretty heavy. What about using something home-grown?

<li
  key={name}
  className="ResultItem--serviceTag"
  style={{ borderLeftColor: colorGenerator.getColorByKey(name) }}
>
  {name} <span className="ResultItem--serviceCount">{count}</span>
</li>
.ResultItem--serviceTag {
  background: #fff;
  border-left: 15px solid transparent;
  border-radius: 4px;
  box-shadow: 0 0 1px rgba(0, 0, 0, 0.5);
  color: rgba(0, 0, 0, 0.65);
  display: inline-block;
  font-size: 0.9em;
  line-height: 1;
  margin: 0.3em 0.35em;
  padding: 0.3em;
}

.ResultItem--serviceCount {
  font-size: 0.9em;
  opacity: 0.5;
  padding-left: 0.2em;
}

);
})}
</ul>
</div>
<div className="ant-col-4 ub-p3 ub-tx-right-align">
{formatRelativeDate(startTime / 1000)}
<div className="ant-divider ant-divider-vertical" />
{timeStr.slice(0, -3)}&nbsp;{timeStr.slice(-2)}
<br />
<small>{fromNow}</small>
</div>
</div>
</Link>
</div>
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

import React from 'react';
import { Tag } from 'antd';
import { shallow } from 'enzyme';

import ResultItem from './ResultItem';
Expand All @@ -30,13 +29,13 @@ it('<ResultItem /> should render base case correctly', () => {
.first()
.render()
.text();
const serviceTags = wrapper.find(`[data-test="${markers.SERVICE_TAGS}"]`).find(Tag);
const serviceTags = wrapper.find(`[data-test="${markers.SERVICE_TAGS}"]`).find('li');
expect(numberOfSpanText).toBe(`${trace.spans.length} Spans`);
expect(serviceTags).toHaveLength(trace.services.length);
});

it('<ResultItem /> should not render any ServiceTags when there are no services', () => {
const wrapper = shallow(<ResultItem trace={{ ...trace, services: [] }} durationPercent={50} />);
const serviceTags = wrapper.find(`[data-test="${markers.SERVICE_TAGS}"]`).find(Tag);
const serviceTags = wrapper.find(`[data-test="${markers.SERVICE_TAGS}"]`).find('li');
expect(serviceTags).toHaveLength(0);
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`DiffSelection renders a trace as expected 1`] = `
<div
className="DiffSelection is-non-empty ub-mb3"
className="DiffSelection is-non-empty"
>
<div
className="DiffSelection--selectedItems"
Expand Down Expand Up @@ -48,7 +48,7 @@ exports[`DiffSelection renders a trace as expected 1`] = `

exports[`DiffSelection renders multiple traces as expected 1`] = `
<div
className="DiffSelection is-non-empty ub-mb3"
className="DiffSelection is-non-empty"
>
<div
className="DiffSelection--selectedItems"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import * as React from 'react';
import { Select } from 'antd';
import { Link } from 'react-router-dom';
import { Field, reduxForm, formValueSelector } from 'redux-form';
import memoizeOne from 'memoize-one';

import DiffSelection from './DiffSelection';
import * as markers from './index.markers';
Expand All @@ -36,6 +37,7 @@ import type { FetchedTrace } from '../../../types';
import type { SearchQuery } from '../../../types/search';

import './index.css';
import ListView from '../../common/ListView';

type SearchResultsProps = {
cohortAddTrace: string => void,
Expand Down Expand Up @@ -83,9 +85,11 @@ export const sortFormSelector = formValueSelector('traceResultsSort');

export default class SearchResults extends React.PureComponent<SearchResultsProps> {
props: SearchResultsProps;

static defaultProps = { skipMessage: false };

searchUrl = memoizeOne(queryOfResults => getUrl(stripEmbeddedState(queryOfResults)));
cohortIds = memoizeOne(diffCohort => new Set(diffCohort.map(datum => datum.id)));

toggleComparison = (traceID: string, remove: boolean) => {
const { cohortAddTrace, cohortRemoveTrace } = this.props;
if (remove) {
Expand All @@ -95,14 +99,43 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
}
};

getKeyFromIndex = (index: number) => this.props.traces[index].traceID;

getIndexFromKey = (key: string) => {
const size = this.props.traces.length;
for (let i = 0; i < size; ++i) {
if (this.props.traces[i].traceID === key) {
return i;
}
}
return -1;
};

renderRow = (key: string, style: Style, index: number, attrs: {}) => {
const trace = this.props.traces[index];
const { disableComparisons, maxTraceDuration, queryOfResults, diffCohort } = this.props;

return (
<div className="u-width-100" key={key} style={style} {...attrs}>
<ResultItem
durationPercent={getPercentageOfDuration(trace.duration, maxTraceDuration)}
isInDiffCohort={this.cohortIds(diffCohort).has(trace.traceID)}
linkTo={getLocation(trace.traceID, { fromSearch: this.searchUrl(queryOfResults) })}
toggleComparison={this.toggleComparison}
trace={trace}
disableComparision={disableComparisons}
/>
</div>
);
};

render() {
const {
diffCohort,
disableComparisons,
goToTrace,
hideGraph,
loading,
maxTraceDuration,
queryOfResults,
showStandaloneLink,
skipMessage,
Expand Down Expand Up @@ -131,7 +164,7 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
</React.Fragment>
);
}
const cohortIds = new Set(diffCohort.map(datum => datum.id));

const searchUrl = getUrl(stripEmbeddedState(queryOfResults));
return (
<div>
Expand Down Expand Up @@ -173,20 +206,18 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
</div>
<div>
{diffSelection}
<ul className="ub-list-reset">
{traces.map(trace => (
<li className="ub-my3" key={trace.traceID}>
<ResultItem
durationPercent={getPercentageOfDuration(trace.duration, maxTraceDuration)}
isInDiffCohort={cohortIds.has(trace.traceID)}
linkTo={getLocation(trace.traceID, { fromSearch: searchUrl })}
toggleComparison={this.toggleComparison}
trace={trace}
disableComparision={disableComparisons}
/>
</li>
))}
</ul>
<ListView
dataLength={this.props.traces.length}
itemHeightGetter={() => 155}
Copy link
Member

@tiffon tiffon Mar 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to have the initialDraw prop set, otherwise it will render 300 items, initially.

itemRenderer={this.renderRow}
viewBuffer={5}
viewBufferMin={5}
initialDraw={15}
itemsWrapperClassName="u-width-100"
getKeyFromIndex={this.getKeyFromIndex}
getIndexFromKey={this.getIndexFromKey}
windowScroller
/>
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import { shallow } from 'enzyme';

import SearchResults from '.';
import * as markers from './index.markers';
import ResultItem from './ResultItem';
import ScatterPlot from './ScatterPlot';
import DiffSelection from './DiffSelection.js';
import LoadingIndicator from '../../common/LoadingIndicator';
import ListView from '../../common/ListView';

describe('<SearchResults>', () => {
let wrapper;
Expand Down Expand Up @@ -65,7 +65,7 @@ describe('<SearchResults>', () => {
});

it('shows a result entry for each trace', () => {
expect(wrapper.find(ResultItem).length).toBe(traces.length);
expect(wrapper.find(ListView)).toBeDefined();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { withRouter } from 'react-router-dom';
import type { Location, RouterHistory } from 'react-router-dom';

import { actions } from './duck';
import ListView from './ListView';
import ListView from '../../common/ListView';
import SpanBarRow from './SpanBarRow';
import DetailState from './SpanDetail/DetailState';
import SpanDetailRow from './SpanDetailRow';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import React from 'react';
import { shallow, mount } from 'enzyme';

import ListView from './ListView';
import ListView from '../../common/ListView';
import SpanBarRow from './SpanBarRow';
import DetailState from './SpanDetail/DetailState';
import SpanDetailRow from './SpanDetailRow';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,15 @@ export default class ListView extends React.Component<ListViewProps> {
this._endIndex = this._yPositions.findFloorIndex(yEnd, this._getHeight);
}

_forceUpdate = () => {
this.forceUpdate();
};

/**
* Checked to see if the currently rendered items are sufficient, if not,
* force an update to trigger more items to be rendered.
*/
_positionList = () => {
this._isScrolledOrResized = false;
if (!this._wrapperElm) {
return;
}
Expand All @@ -296,6 +299,7 @@ export default class ListView extends React.Component<ListViewProps> {
if (maxStart < this._startIndexDrawn || minEnd > this._endIndexDrawn) {
this.forceUpdate();
}
this._isScrolledOrResized = false;
};

_initWrapper = (elm: HTMLElement) => {
Expand Down Expand Up @@ -360,7 +364,7 @@ export default class ListView extends React.Component<ListViewProps> {
const imin = getIndexFromKey(lowDirtyKey);
const imax = highDirtyKey === lowDirtyKey ? imin : getIndexFromKey(highDirtyKey);
this._yPositions.calcHeights(imax, this._getHeight, imin);
this.forceUpdate();
window.requestAnimationFrame(this._forceUpdate);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
import React from 'react';
import { mount, shallow } from 'enzyme';

import ListView from './index';
import { polyfill as polyfillAnimationFrame } from '../../../../utils/test/requestAnimationFrame';
import ListView from '.';
import { polyfill as polyfillAnimationFrame } from '../../../utils/test/requestAnimationFrame';

// Util to get list of all callbacks added to an event emitter by event type.
// jest adds "error" event listeners to window, this util makes it easier to
Expand Down
Loading