From cec107f8b9795c5891ca41079f60e24a4008f3a3 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 2 Nov 2017 18:22:41 -0400 Subject: [PATCH] Handle FOLLOWS_FROM ref type (#118) * Handle FOLLOWS_FROM refs when making span tree * Add test-case for issue #115 * Handle FOLLOWS_FROM in scrolling shortcuts Signed-off-by: Joe Farro --- CHANGELOG.md | 5 ++ src/components/TracePage/ScrollManager.js | 2 +- src/selectors/trace.fixture.js | 60 +++++++++++++++++++++++ src/selectors/trace.js | 2 +- src/selectors/trace.test.js | 27 ++++++---- 5 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 src/selectors/trace.fixture.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a3bd55db1..2e623674fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ # Changes merged into master +### [#118](https://github.com/jaegertracing/jaeger-ui/pull/118) Handle `FOLLOWS_FROM` reference type + +Fix issue processing `FOLLOWS_FROM` references to parent spans. Fixes [#115](https://github.com/jaegertracing/jaeger-ui/issues/115). + + ### [#110](https://github.com/jaegertracing/jaeger-ui/pull/110) Fix browser back button not working correctly Fix bug causing browser back button to not work correctly. Fixes [#94](https://github.com/jaegertracing/jaeger-ui/issues/94). diff --git a/src/components/TracePage/ScrollManager.js b/src/components/TracePage/ScrollManager.js index be8f7f20cb..9d47bb618c 100644 --- a/src/components/TracePage/ScrollManager.js +++ b/src/components/TracePage/ScrollManager.js @@ -59,7 +59,7 @@ function isSpanHidden(span: Span, childrenAreHidden: Set, spansMap: Map< let { references } = span; let parentID: ?string; const checkRef = ref => { - if (ref.refType === 'CHILD_OF') { + if (ref.refType === 'CHILD_OF' || ref.refType === 'FOLLOWS_FROM') { parentID = ref.spanID; parentIDs.add(parentID); return childrenAreHidden.has(parentID); diff --git a/src/selectors/trace.fixture.js b/src/selectors/trace.fixture.js new file mode 100644 index 0000000000..8f99f996bf --- /dev/null +++ b/src/selectors/trace.fixture.js @@ -0,0 +1,60 @@ +// Copyright (c) 2017 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. + +// See https://github.com/jaegertracing/jaeger-ui/issues/115 for details. +// eslint-disable-next-line import/prefer-default-export +export const followsFromRef = { + processes: { + p1: { + serviceName: 'issue115', + tags: [], + }, + }, + spans: [ + { + duration: 1173, + flags: 1, + logs: [], + operationName: 'thread', + processID: 'p1', + references: [ + { + refType: 'FOLLOWS_FROM', + spanID: 'ea7cfaca83f0724b', + traceID: '2992f2a5b5d037a8aabffd08ef384237', + }, + ], + spanID: '1bdf4201221bb2ac', + startTime: 1509533706521220, + tags: [], + traceID: '2992f2a5b5d037a8aabffd08ef384237', + warnings: null, + }, + { + duration: 70406, + flags: 1, + logs: [], + operationName: 'demo', + processID: 'p1', + references: [], + spanID: 'ea7cfaca83f0724b', + startTime: 1509533706470949, + tags: [], + traceID: '2992f2a5b5d037a8aabffd08ef384237', + warnings: null, + }, + ], + traceID: '2992f2a5b5d037a8aabffd08ef384237', + warnings: null, +}; diff --git a/src/selectors/trace.js b/src/selectors/trace.js index 75c9644d98..44e5d810df 100644 --- a/src/selectors/trace.js +++ b/src/selectors/trace.js @@ -70,7 +70,7 @@ export function getTraceSpanIdsAsTree(trace) { const node = nodesById.get(span.spanID); if (Array.isArray(span.references) && span.references.length) { const { refType, spanID: parentID } = span.references[0]; - if (refType === 'CHILD_OF') { + if (refType === 'CHILD_OF' || refType === 'FOLLOWS_FROM') { const parent = nodesById.get(parentID) || root; parent.children.push(node); } else { diff --git a/src/selectors/trace.test.js b/src/selectors/trace.test.js index 84485fad30..08ed763c83 100644 --- a/src/selectors/trace.test.js +++ b/src/selectors/trace.test.js @@ -14,8 +14,7 @@ import _values from 'lodash/values'; -import traceGenerator from '../../src/demo/trace-generators'; -import * as traceSelectors from '../../src/selectors/trace'; +import { followsFromRef } from './trace.fixture'; import { getSpanId, getSpanName, @@ -24,7 +23,9 @@ import { getSpanProcessId, getSpanServiceName, getSpanTimestamp, -} from '../../src/selectors/span'; +} from './span'; +import * as traceSelectors from './trace'; +import traceGenerator from '../../src/demo/trace-generators'; import { numberSortComparator } from '../../src/utils/sort'; const generatedTrace = traceGenerator.trace({ numberOfSpans: 45 }); @@ -49,16 +50,22 @@ it('getTraceSpansAsMap() should return a map of all of the spans', () => { } }); -it('getTraceSpanIdsAsTree() should build the tree properly', () => { - const tree = traceSelectors.getTraceSpanIdsAsTree(generatedTrace); - const spanMap = traceSelectors.getTraceSpansAsMap(generatedTrace); +describe('getTraceSpanIdsAsTree()', () => { + it('builds the tree properly', () => { + const tree = traceSelectors.getTraceSpanIdsAsTree(generatedTrace); + const spanMap = traceSelectors.getTraceSpansAsMap(generatedTrace); - tree.walk((value, node) => { - const expectedParentValue = value === traceSelectors.TREE_ROOT_ID ? null : value; - node.children.forEach(childNode => { - expect(getSpanParentId(spanMap.get(childNode.value))).toBe(expectedParentValue); + tree.walk((value, node) => { + const expectedParentValue = value === traceSelectors.TREE_ROOT_ID ? null : value; + node.children.forEach(childNode => { + expect(getSpanParentId(spanMap.get(childNode.value))).toBe(expectedParentValue); + }); }); }); + + it('#115 - handles FOLLOW_FROM refs', () => { + expect(() => traceSelectors.getTraceSpanIdsAsTree(followsFromRef)).not.toThrow(); + }); }); it('getParentSpan() should return the parent span of the tree', () => {