From 9d8261a33c3adaad9aecd290744b651290c72e24 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Wed, 3 Feb 2016 15:51:51 +0100 Subject: [PATCH] Re-organizes Node so that it can be used on objects besides Span We need to create the trace tree in dependency store code. Particularly, we don't want to retrieve the entire span just to count calls. This refactors the Node object so that it can be used when all span details aren't present. --- .../zipkin/internal/CorrectForClockSkew.java | 20 ++- .../src/main/java/zipkin/internal/Node.java | 152 ++++++++++++++++++ .../main/java/zipkin/internal/SpanNode.java | 80 --------- .../test/java/zipkin/internal/NodeTest.java | 81 ++++++++++ 4 files changed, 246 insertions(+), 87 deletions(-) create mode 100644 zipkin/src/main/java/zipkin/internal/Node.java delete mode 100644 zipkin/src/main/java/zipkin/internal/SpanNode.java create mode 100644 zipkin/src/test/java/zipkin/internal/NodeTest.java diff --git a/zipkin/src/main/java/zipkin/internal/CorrectForClockSkew.java b/zipkin/src/main/java/zipkin/internal/CorrectForClockSkew.java index a5679551f23..ec997572fec 100644 --- a/zipkin/src/main/java/zipkin/internal/CorrectForClockSkew.java +++ b/zipkin/src/main/java/zipkin/internal/CorrectForClockSkew.java @@ -13,6 +13,8 @@ */ package zipkin.internal; +import java.util.ArrayList; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -39,9 +41,13 @@ public ClockSkew(Endpoint endpoint, long skew) { public static List apply(List spans) { for (Span s : spans) { if (s.parentId == null) { - SpanNode tree = SpanNode.create(s, spans); + Node tree = Node.constructTree(spans); adjust(tree, null); - return tree.toSpans(); + List result = new ArrayList<>(spans.size()); + for (Iterator> i = tree.traverse(); i.hasNext();) { + result.add(i.next().value()); + } + return result; } } return spans; @@ -51,20 +57,20 @@ public static List apply(List spans) { * Recursively adjust the timestamps on the span tree. Root span is the reference point, all * children's timestamps gets adjusted based on that span's timestamps. */ - private static void adjust(SpanNode node, @Nullable ClockSkew skewFromParent) { + private static void adjust(Node node, @Nullable ClockSkew skewFromParent) { // adjust skew for the endpoint brought over from the parent span if (skewFromParent != null) { - node.span = adjustTimestamps(node.span, skewFromParent); + node.value(adjustTimestamps(node.value(), skewFromParent)); } // Is there any skew in the current span? - ClockSkew skew = getClockSkew(node.span); + ClockSkew skew = getClockSkew(node.value()); if (skew != null) { // the current span's skew may be a different endpoint than skewFromParent, adjust again. - node.span = adjustTimestamps(node.span, skew); + node.value(adjustTimestamps(node.value(), skew)); // propagate skew to any children - for (SpanNode child : node.children) { + for (Node child : node.children()) { adjust(child, skew); } } diff --git a/zipkin/src/main/java/zipkin/internal/Node.java b/zipkin/src/main/java/zipkin/internal/Node.java new file mode 100644 index 00000000000..4bbfce05f50 --- /dev/null +++ b/zipkin/src/main/java/zipkin/internal/Node.java @@ -0,0 +1,152 @@ +/** + * Copyright 2015-2016 The OpenZipkin Authors + * + * 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. + */ +package zipkin.internal; + +import java.util.ArrayDeque; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Queue; +import zipkin.Span; + +/** + * Convenience type representing a tree. This is here because multiple facets in zipkin require + * traversing the trace tree. For example, looking at network boundaries to correct clock skew, or + * counting requests imply visiting the tree. + * + * @param the node's value. Ex a full span or a tuple like {@code (serviceName, isLocal)} + */ +public final class Node { + + /** Set via {@link #addChild(Node)} */ + private Node parent; + /** mutable as some transformations, such as clock skew, adjust this. */ + private V value; + /** mutable to avoid allocating lists for childless nodes */ + private List> children = Collections.emptyList(); + + /** Returns the parent, or null if root */ + @Nullable + public Node parent() { + return parent; + } + + public V value() { + return value; + } + + public Node value(V newValue) { + this.value = newValue; + return this; + } + + public Node addChild(Node child) { + child.parent = this; + if (children.equals(Collections.emptyList())) children = new LinkedList<>(); + children.add(child); + return this; + } + + /** Returns the children of this node. */ + public Collection> children() { + return children; + } + + /** Traverses the tree, breadth-first. */ + public Iterator> traverse() { + return new BreadthFirstIterator<>(this); + } + + static final class BreadthFirstIterator implements Iterator> { + private final Queue> queue = new ArrayDeque<>(); + + BreadthFirstIterator(Node root) { + queue.add(root); + } + + @Override + public boolean hasNext() { + return !queue.isEmpty(); + } + + @Override + public Node next() { + Node result = queue.remove(); + queue.addAll(result.children); + return result; + } + + @Override + public void remove() { + throw new UnsupportedOperationException("remove"); + } + } + + /** + * @param trace spans that belong to the same {@link Span#traceId trace}, in any order. + */ + static Node constructTree(List trace) { + TreeBuilder treeBuilder = new TreeBuilder<>(); + for (Span s : trace) { + treeBuilder.addNode(s.parentId, s.id, s); + } + return treeBuilder.build(); + } + + /** + * Some operations do not require the entire span object. This creates a tree given (parent id, + * id) pairs. + * + * @param same type as {@link Node#value} + */ + public static final class TreeBuilder { + Node rootNode = null; + + // Nodes representing the trace tree + Map> idToNode = new LinkedHashMap<>(); + // Collect the parent-child relationships between all spans. + Map idToParent = new LinkedHashMap<>(idToNode.size()); + + public void addNode(Long parentId, long id, @Nullable V value) { + Node node = new Node().value(value); + if (parentId == null) { // special-case root + rootNode = node; + } else { + idToNode.put(id, node); + idToParent.put(id, parentId); + } + } + + /** Builds a tree from calls to {@link #addNode}, or returns an empty tree. */ + public Node build() { + // Materialize the tree using parent - child relationships + for (Map.Entry entry : idToParent.entrySet()) { + Node node = idToNode.get(entry.getKey()); + Node parent = idToNode.get(entry.getValue()); + if (parent == null && rootNode == null) { // handle headless trace + rootNode = node; + } else if (parent == null) { // attribute missing parents to root + rootNode.addChild(node); + } else { + parent.addChild(node); + } + } + return rootNode != null ? rootNode : new Node(); + } + } +} diff --git a/zipkin/src/main/java/zipkin/internal/SpanNode.java b/zipkin/src/main/java/zipkin/internal/SpanNode.java deleted file mode 100644 index c077ce43399..00000000000 --- a/zipkin/src/main/java/zipkin/internal/SpanNode.java +++ /dev/null @@ -1,80 +0,0 @@ -/** - * Copyright 2015-2016 The OpenZipkin Authors - * - * 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. - */ -package zipkin.internal; - -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import zipkin.Span; - -import static zipkin.internal.Util.checkNotNull; -import static zipkin.internal.Util.sortedList; - -final class SpanNode { - /** mutable to avoid allocating lists for no reason */ - Span span; - List children = Collections.emptyList(); - - private SpanNode(Span span) { - this.span = checkNotNull(span, "span"); - } - - private void addChild(SpanNode node) { - if (children.equals(Collections.emptyList())) children = new LinkedList<>(); - children.add(node); - } - - static SpanNode create(Span span, List spans) { - SpanNode rootNode = new SpanNode(span); - - // Initialize nodes representing the trace tree - Map idToNode = new LinkedHashMap<>(); - for (Span s : spans) { - if (s.parentId == null) continue; // special-case root - idToNode.put(s.id, new SpanNode(s)); - } - - // Collect the parent-child relationships between all spans. - Map idToParent = new LinkedHashMap<>(); - for (Map.Entry entry : idToNode.entrySet()) { - idToParent.put(entry.getKey(), entry.getValue().span.parentId); - } - - // Materialize the tree using parent - child relationships - for (Map.Entry entry : idToParent.entrySet()) { - SpanNode node = idToNode.get(entry.getKey()); - SpanNode parent = idToNode.get(entry.getValue()); - if (parent == null) { // attribute missing parents to root - rootNode.addChild(node); - } else { - parent.addChild(node); - } - } - return rootNode; - } - - List toSpans() { - if (children.isEmpty()) { - return Collections.singletonList(span); - } - List result = new LinkedList<>(); - result.add(span); - for (SpanNode child : children) { - result.addAll(child.toSpans()); - } - return sortedList(result); - } -} diff --git a/zipkin/src/test/java/zipkin/internal/NodeTest.java b/zipkin/src/test/java/zipkin/internal/NodeTest.java new file mode 100644 index 00000000000..767e976be5d --- /dev/null +++ b/zipkin/src/test/java/zipkin/internal/NodeTest.java @@ -0,0 +1,81 @@ +/** + * Copyright 2015-2016 The OpenZipkin Authors + * + * 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. + */ +package zipkin.internal; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import org.junit.Test; +import zipkin.Span; +import zipkin.TestObjects; + +import static org.assertj.core.api.Assertions.assertThat; + +public class NodeTest { + + /** + *

The following tree should traverse in alphabetical order 

{@code
+   *
+   *          a
+   *        / | \
+   *       b  c  d
+   *      /|\     \
+   *     e f g     h
+   * }
+ */ + @Test + public void traversesBreadthFirst() { + Node a = new Node().value('a'); + Node b = new Node().value('b'); + Node c = new Node().value('c'); + Node d = new Node().value('d'); + // root(a) has children b, c, d + a.addChild(b).addChild(c).addChild(d); + Node e = new Node().value('e'); + Node f = new Node().value('f'); + Node g = new Node().value('g'); + // child(b) has children e, f, g + b.addChild(e).addChild(f).addChild(g); + Node h = new Node().value('h'); + // f has no children + // child(g) has child h + g.addChild(h); + + assertThat(a.traverse()).extracting(Node::value) + .containsExactly('a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'); + } + + /** + * Makes sure that the trace tree is constructed based on parent-child, not by parameter order. + */ + @Test + public void constructsTraceTree() { + // TRACE is sorted with root span first, lets shuffle them to make + // sure the trace is stitched together by id. + List copy = new ArrayList<>(TestObjects.TRACE); + + Collections.shuffle(copy); + + Node root = Node.constructTree(copy); + assertThat(root.value()) + .isEqualTo(TestObjects.TRACE.get(0)); + + assertThat(root.children()).extracting(Node::value) + .containsExactly(TestObjects.TRACE.get(1)); + + Node child = root.children().iterator().next(); + assertThat(child.children()).extracting(Node::value) + .containsExactly(TestObjects.TRACE.get(2)); + } +}