Skip to content

Commit

Permalink
Handle FOLLOWS_FROM ref type (#118)
Browse files Browse the repository at this point in the history
* 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 <joef@uber.com>
  • Loading branch information
tiffon committed Nov 2, 2017
1 parent f31ba92 commit cec107f
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 12 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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).
Expand Down
2 changes: 1 addition & 1 deletion src/components/TracePage/ScrollManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function isSpanHidden(span: Span, childrenAreHidden: Set<string>, 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);
Expand Down
60 changes: 60 additions & 0 deletions src/selectors/trace.fixture.js
Original file line number Diff line number Diff line change
@@ -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,
};
2 changes: 1 addition & 1 deletion src/selectors/trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
27 changes: 17 additions & 10 deletions src/selectors/trace.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 });
Expand All @@ -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', () => {
Expand Down

0 comments on commit cec107f

Please sign in to comment.