From d61d3afe342ef96a0819c05454cbfd851058cba6 Mon Sep 17 00:00:00 2001
From: Nils Christian Ehmke <nils@rhocas.de>
Date: Sat, 14 Mar 2015 09:45:34 +0100
Subject: [PATCH] Fixed a bug in the LegacyTraceReconstructor where the
 reconstructor ignored multiple (hidden) return jumps. I added also a test
 making sure that this bug will (hopefully) never occur again.

---
 .../stages/LegacyTraceReconstructor.java      |  7 +++--
 .../stages/LegacyTraceReconstructorTest.java  | 27 ++++++++++++++++---
 .../model/importer/stages/StageTester.java    | 16 +++++++++++
 3 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/src/main/java/kieker/diagnosis/model/importer/stages/LegacyTraceReconstructor.java b/src/main/java/kieker/diagnosis/model/importer/stages/LegacyTraceReconstructor.java
index 7fcbca9d..aa8fb250 100644
--- a/src/main/java/kieker/diagnosis/model/importer/stages/LegacyTraceReconstructor.java
+++ b/src/main/java/kieker/diagnosis/model/importer/stages/LegacyTraceReconstructor.java
@@ -30,7 +30,7 @@ import kieker.diagnosis.domain.Trace;
 
 /**
  * Reconstruct traces based on the incoming instances of {@code OperationExecutionRecord}.
- *
+ * 
  * @author Nils Christian Ehmke
  */
 final class LegacyTraceReconstructor extends AbstractStage<OperationExecutionRecord, Trace> {
@@ -85,8 +85,11 @@ final class LegacyTraceReconstructor extends AbstractStage<OperationExecutionRec
 						this.traceID);
 				newCall.setDuration(record.getTout() - record.getTin());
 
-				if ((record.getEss() <= ess) && (ess != 0)) {
+				// There can be "jumps" in the ess, as the operation execution records do not log the return jumps of methods. Therefore multiple of these jumps can be hidden.
+				int currentEss = record.getEss();
+				while ((currentEss <= ess) && (ess != 0)) {
 					header = header.getParent();
+					currentEss++;
 				}
 
 				if (root == null) {
diff --git a/src/test/java/kieker/diagnosis/model/importer/stages/LegacyTraceReconstructorTest.java b/src/test/java/kieker/diagnosis/model/importer/stages/LegacyTraceReconstructorTest.java
index 8897b135..61b926ee 100644
--- a/src/test/java/kieker/diagnosis/model/importer/stages/LegacyTraceReconstructorTest.java
+++ b/src/test/java/kieker/diagnosis/model/importer/stages/LegacyTraceReconstructorTest.java
@@ -27,7 +27,6 @@ import java.util.List;
 
 import kieker.common.record.controlflow.OperationExecutionRecord;
 import kieker.diagnosis.domain.Trace;
-import kieker.diagnosis.model.importer.stages.LegacyTraceReconstructor;
 
 import org.junit.Test;
 
@@ -49,7 +48,7 @@ public class LegacyTraceReconstructorTest {
 		records.add(new OperationExecutionRecord("operation", OperationExecutionRecord.NO_SESSION_ID, 42, 15L, 20L, "localhost", 0, 0));
 
 		final LegacyTraceReconstructor reconstructor = new LegacyTraceReconstructor();
-		List<Trace> result = testStageBySending(records).to(reconstructor.getInputPort()).andReceivingFrom(reconstructor.getOutputPort());
+		final List<Trace> result = testStageBySending(records).to(reconstructor.getInputPort()).andReceivingFrom(reconstructor.getOutputPort());
 
 		assertThat(result, hasSize(1));
 		assertThat(result.get(0).getRootOperationCall().getOperation(), is("operation"));
@@ -63,7 +62,7 @@ public class LegacyTraceReconstructorTest {
 		records.add(new OperationExecutionRecord("bookstoreTracing.Catalog.getBook(boolean)", "1", 42, 15L, 20L, "SRV1", 0, 0));
 
 		final LegacyTraceReconstructor reconstructor = new LegacyTraceReconstructor();
-		List<Trace> result = testStageBySending(records).to(reconstructor.getInputPort()).andReceivingFrom(reconstructor.getOutputPort());
+		final List<Trace> result = testStageBySending(records).to(reconstructor.getInputPort()).andReceivingFrom(reconstructor.getOutputPort());
 
 		assertThat(result, hasSize(1));
 		assertThat(result.get(0).getRootOperationCall().getContainer(), is("SRV1"));
@@ -80,7 +79,7 @@ public class LegacyTraceReconstructorTest {
 		records.add(new OperationExecutionRecord("A", OperationExecutionRecord.NO_SESSION_ID, 42, 10L, 20L, "localhost", 0, 0));
 
 		final LegacyTraceReconstructor reconstructor = new LegacyTraceReconstructor();
-		List<Trace> result = testStageBySending(records).to(reconstructor.getInputPort()).andReceivingFrom(reconstructor.getOutputPort());
+		final List<Trace> result = testStageBySending(records).to(reconstructor.getInputPort()).andReceivingFrom(reconstructor.getOutputPort());
 
 		assertThat(result, hasSize(1));
 		assertThat(result.get(0).getRootOperationCall().getOperation(), is("A"));
@@ -90,6 +89,26 @@ public class LegacyTraceReconstructorTest {
 		assertThat(result.get(0).getRootOperationCall().getChildren().get(1).getChildren().get(0).getOperation(), is("D"));
 	}
 
+	@Test
+	public void nestedCallWithEssJumpsReconstructionShouldWork() throws Exception {
+		final List<OperationExecutionRecord> records = new ArrayList<>();
+		records.add(new OperationExecutionRecord("B", OperationExecutionRecord.NO_SESSION_ID, 42, 10L, 20L, "localhost", 1, 1));
+		records.add(new OperationExecutionRecord("C", OperationExecutionRecord.NO_SESSION_ID, 42, 10L, 20L, "localhost", 2, 2));
+		records.add(new OperationExecutionRecord("D", OperationExecutionRecord.NO_SESSION_ID, 42, 10L, 20L, "localhost", 2, 3));
+		records.add(new OperationExecutionRecord("E", OperationExecutionRecord.NO_SESSION_ID, 42, 10L, 20L, "localhost", 3, 1));
+		records.add(new OperationExecutionRecord("A", OperationExecutionRecord.NO_SESSION_ID, 42, 10L, 20L, "localhost", 0, 0));
+
+		final LegacyTraceReconstructor reconstructor = new LegacyTraceReconstructor();
+		final List<Trace> result = testStageBySending(records).to(reconstructor.getInputPort()).andReceivingFrom(reconstructor.getOutputPort());
+
+		assertThat(result, hasSize(1));
+		assertThat(result.get(0).getRootOperationCall().getOperation(), is("A"));
+		assertThat(result.get(0).getRootOperationCall().getDuration(), is(10L));
+		assertThat(result.get(0).getRootOperationCall().getChildren(), hasSize(2));
+		assertThat(result.get(0).getRootOperationCall().getChildren().get(0).getOperation(), is("B"));
+		assertThat(result.get(0).getRootOperationCall().getChildren().get(1).getOperation(), is("E"));
+	}
+
 	@Test
 	public void exampleLogReconstructionShouldWork() throws Exception {
 		final ExampleLogReconstructionConfiguration configuration = new ExampleLogReconstructionConfiguration();
diff --git a/src/test/java/kieker/diagnosis/model/importer/stages/StageTester.java b/src/test/java/kieker/diagnosis/model/importer/stages/StageTester.java
index 93e67a43..3cb0c0ca 100644
--- a/src/test/java/kieker/diagnosis/model/importer/stages/StageTester.java
+++ b/src/test/java/kieker/diagnosis/model/importer/stages/StageTester.java
@@ -1,3 +1,19 @@
+/***************************************************************************
+ * Copyright 2014 Kieker Project (http://kieker-monitoring.net)
+ *
+ * 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 kieker.diagnosis.model.importer.stages;
 
 import java.util.ArrayList;
-- 
GitLab