Clean up getTraceName memoization (jaegertracing#573)

* Clean up memoization approach Signed-off-by: Everett Ross <reverett@uber.com>
smanolloff · May 12, 2020 · 8b8a694 · 8b8a694
1 parent 4d140e5
commit 8b8a694
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 14 deletions.
diff --git a/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx b/packages/jaeger-ui/src/components/TracePage/TracePageHeader/TracePageHeader.tsx
@@ -150,7 +150,7 @@ export function TracePageHeaderFn(props: TracePageHeaderEmbedProps & { forwarded
 
   const title = (
     <h1 className={`TracePageHeader--title ${canCollapse ? 'is-collapsible' : ''}`}>
-      <TraceName traceName={getTraceName(trace.spans, trace.traceID)} />{' '}
+      <TraceName traceName={getTraceName(trace.spans)} />{' '}
       <small className="u-tx-muted">{trace.traceID.slice(0, 7)}</small>
     </h1>
   );

diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import { getTraceName } from './trace-viewer';
+import { _getTraceNameImpl as getTraceName } from './trace-viewer';
 
 describe('getTraceName', () => {
   const firstSpanId = 'firstSpanId';
@@ -219,29 +219,26 @@ 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(getTraceNameWrapper(spansWithNoRoots)).toEqual('');
+    expect(getTraceName(spansWithNoRoots)).toEqual('');
   });
 
   it('returns an id of root span with the earliest startTime', () => {
-    expect(getTraceNameWrapper(spansWithMultipleRootsDifferentByStartTime)).toEqual(fullTraceName);
+    expect(getTraceName(spansWithMultipleRootsDifferentByStartTime)).toEqual(fullTraceName);
   });
 
   it('returns an id of root span without any refs', () => {
-    expect(getTraceNameWrapper(spansWithMultipleRootsWithOneWithoutRefs)).toEqual(fullTraceName);
+    expect(getTraceName(spansWithMultipleRootsWithOneWithoutRefs)).toEqual(fullTraceName);
   });
 
   it('returns an id of root span with remote ref', () => {
-    expect(getTraceNameWrapper(spansWithOneRootWithRemoteRef)).toEqual(fullTraceName);
+    expect(getTraceName(spansWithOneRootWithRemoteRef)).toEqual(fullTraceName);
   });
 
   it('returns an id of root span with no refs', () => {
-    expect(getTraceNameWrapper(spansWithOneRootWithNoRefs)).toEqual(fullTraceName);
+    expect(getTraceName(spansWithOneRootWithNoRefs)).toEqual(fullTraceName);
   });
 });
diff --git a/packages/jaeger-ui/src/model/trace-viewer.tsx b/packages/jaeger-ui/src/model/trace-viewer.tsx
@@ -18,7 +18,7 @@ import { Span } from '../types/trace';
 
 type spansDict = { [index: string]: Span };
 
-function getTraceNameImpl(spans: Span[], traceID: string) {
+export function _getTraceNameImpl(spans: Span[]) {
   const allTraceSpans: spansDict = spans.reduce((dict, span) => ({ ...dict, [span.spanID]: span }), {});
   const rootSpan = spans
     .filter(sp => {
@@ -40,5 +40,7 @@ function getTraceNameImpl(spans: Span[], traceID: string) {
   return rootSpan ? `${rootSpan.process.serviceName}: ${rootSpan.operationName}` : '';
 }
 
-// eslint-disable-next-line import/prefer-default-export
-export const getTraceName = _memoize(getTraceNameImpl, (_spans, traceID) => traceID);
+export const getTraceName = _memoize(_getTraceNameImpl, (spans: Span[]) => {
+  if (!spans.length) return 0;
+  return spans[0].traceID;
+});
diff --git a/packages/jaeger-ui/src/model/transform-trace-data.tsx b/packages/jaeger-ui/src/model/transform-trace-data.tsx
@@ -160,7 +160,7 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
     });
     spans.push(span);
   });
-  const traceName = getTraceName(spans, traceID);
+  const traceName = getTraceName(spans);
   const services = Object.keys(svcCounts).map(name => ({ name, numberOfSpans: svcCounts[name] }));
   return {
     services,