Skip to content

Commit

Permalink
Memoize getTraceName to improve render time by 3x (#572)
Browse files Browse the repository at this point in the history
Signed-off-by: Everett Ross <reverett@uber.com>
  • Loading branch information
everett980 authored May 12, 2020
1 parent 7b5aecd commit 4d140e5
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded

const title = (
<h1 className={`TracePageHeader--title ${canCollapse ? 'is-collapsible' : ''}`}>
<TraceName traceName={getTraceName(trace.spans)} />{' '}
<TraceName traceName={getTraceName(trace.spans, trace.traceID)} />{' '}
<small className="u-tx-muted">{trace.traceID.slice(0, 7)}</small>
</h1>
);
Expand Down
13 changes: 8 additions & 5 deletions packages/jaeger-ui/src/model/find-trace-name.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,26 +219,29 @@ describe('getTraceName', () => {
],
},
];
const getTraceNameWrapper = jest.fn(spans =>
getTraceName(spans, `${spans.length}${getTraceNameWrapper.mock.calls.length}`)
);

const fullTraceName = `${serviceName}: ${operationName}`;

it('returns an empty string if given spans with no root among them', () => {
expect(getTraceName(spansWithNoRoots)).toEqual('');
expect(getTraceNameWrapper(spansWithNoRoots)).toEqual('');
});

it('returns an id of root span with the earliest startTime', () => {
expect(getTraceName(spansWithMultipleRootsDifferentByStartTime)).toEqual(fullTraceName);
expect(getTraceNameWrapper(spansWithMultipleRootsDifferentByStartTime)).toEqual(fullTraceName);
});

it('returns an id of root span without any refs', () => {
expect(getTraceName(spansWithMultipleRootsWithOneWithoutRefs)).toEqual(fullTraceName);
expect(getTraceNameWrapper(spansWithMultipleRootsWithOneWithoutRefs)).toEqual(fullTraceName);
});

it('returns an id of root span with remote ref', () => {
expect(getTraceName(spansWithOneRootWithRemoteRef)).toEqual(fullTraceName);
expect(getTraceNameWrapper(spansWithOneRootWithRemoteRef)).toEqual(fullTraceName);
});

it('returns an id of root span with no refs', () => {
expect(getTraceName(spansWithOneRootWithNoRefs)).toEqual(fullTraceName);
expect(getTraceNameWrapper(spansWithOneRootWithNoRefs)).toEqual(fullTraceName);
});
});
8 changes: 6 additions & 2 deletions packages/jaeger-ui/src/model/trace-viewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import _memoize from 'lodash/memoize';

import { Span } from '../types/trace';

type spansDict = { [index: string]: Span };

// eslint-disable-next-line import/prefer-default-export
export function getTraceName(spans: Span[]) {
function getTraceNameImpl(spans: Span[], traceID: string) {
const allTraceSpans: spansDict = spans.reduce((dict, span) => ({ ...dict, [span.spanID]: span }), {});
const rootSpan = spans
.filter(sp => {
Expand All @@ -38,3 +39,6 @@ export function getTraceName(spans: Span[]) {

return rootSpan ? `${rootSpan.process.serviceName}: ${rootSpan.operationName}` : '';
}

// eslint-disable-next-line import/prefer-default-export
export const getTraceName = _memoize(getTraceNameImpl, (_spans, traceID) => traceID);
2 changes: 1 addition & 1 deletion packages/jaeger-ui/src/model/transform-trace-data.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
});
spans.push(span);
});
const traceName = getTraceName(spans);
const traceName = getTraceName(spans, traceID);
const services = Object.keys(svcCounts).map(name => ({ name, numberOfSpans: svcCounts[name] }));
return {
services,
Expand Down

0 comments on commit 4d140e5

Please sign in to comment.