[gobblin] branch master updated: [GOBBLIN-1536] Fix issue code generation (#3388)

[aplex]

This is an automated email from the ASF dual-hosted git repository.

aplex pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/gobblin.git


The following commit(s) were added to refs/heads/master by this push:
     new 84fb899  [GOBBLIN-1536] Fix issue code generation (#3388)
84fb899 is described below

commit 84fb899a1d94f24a838d83b85c991800eae434c1
Author: Alex Prokofiev <aprokof...@linkedin.com>
AuthorDate: Tue Sep 7 14:06:45 2021 -0700

    [GOBBLIN-1536] Fix issue code generation (#3388)
    
    Issue code should depend only on exception stack trace and type,
    but previously there was a bug when exception message was included
    in generation of the code. Since the message can have variable
    parts, like paths and dataset names, some issue got multiple
    codes assigned to them.
---
 .../AutoTroubleshooterLogAppender.java             | 17 ++++++++++-
 .../AutoTroubleshooterLogAppenderTest.java         | 33 ++++++++++++++++++++--
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git 
a/gobblin-modules/gobblin-troubleshooter/src/main/java/org/apache/gobblin/troubleshooter/AutoTroubleshooterLogAppender.java
 
b/gobblin-modules/gobblin-troubleshooter/src/main/java/org/apache/gobblin/troubleshooter/AutoTroubleshooterLogAppender.java
index dca54ef..bb562bb 100644
--- 
a/gobblin-modules/gobblin-troubleshooter/src/main/java/org/apache/gobblin/troubleshooter/AutoTroubleshooterLogAppender.java
+++ 
b/gobblin-modules/gobblin-troubleshooter/src/main/java/org/apache/gobblin/troubleshooter/AutoTroubleshooterLogAppender.java
@@ -25,6 +25,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang.text.StrBuilder;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.apache.log4j.AppenderSkeleton;
@@ -142,7 +143,21 @@ public class AutoTroubleshooterLogAppender extends 
AppenderSkeleton {
      * multiple application versions.
      * */
 
-    return getHash(throwable.getClass().toString() + 
ExceptionUtils.getStackTrace(throwable));
+    return getHash(getStackTraceWithoutExceptionMessage(throwable));
+  }
+
+  private String getStackTraceWithoutExceptionMessage(Throwable throwable) {
+    StrBuilder sb = new StrBuilder();
+
+    for (Throwable currentThrowable : 
ExceptionUtils.getThrowableList(throwable)) {
+      sb.appendln(currentThrowable.getClass().getName());
+      for (StackTraceElement stackTraceElement : 
currentThrowable.getStackTrace()) {
+        sb.appendln(stackTraceElement);
+      }
+      sb.appendln("---");
+    }
+
+    return sb.toString();
   }
 
   private IssueSeverity convert(Level level) {
diff --git 
a/gobblin-modules/gobblin-troubleshooter/src/test/java/org/apache/gobblin/troubleshooter/AutoTroubleshooterLogAppenderTest.java
 
b/gobblin-modules/gobblin-troubleshooter/src/test/java/org/apache/gobblin/troubleshooter/AutoTroubleshooterLogAppenderTest.java
index 9b75d6a..899c2b0 100644
--- 
a/gobblin-modules/gobblin-troubleshooter/src/test/java/org/apache/gobblin/troubleshooter/AutoTroubleshooterLogAppenderTest.java
+++ 
b/gobblin-modules/gobblin-troubleshooter/src/test/java/org/apache/gobblin/troubleshooter/AutoTroubleshooterLogAppenderTest.java
@@ -20,7 +20,9 @@ package org.apache.gobblin.troubleshooter;
 import java.io.IOException;
 import java.time.ZonedDateTime;
 import java.time.temporal.ChronoUnit;
+import java.util.List;
 
+import org.apache.hadoop.hive.metastore.api.InvalidOperationException;
 import org.apache.log4j.Level;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
@@ -62,7 +64,7 @@ public class AutoTroubleshooterLogAppenderTest {
     Assert.assertEquals(issue.getSeverity(), IssueSeverity.WARN);
     Assert.assertEquals(issue.getSummary(), "test");
     Assert.assertEquals(issue.getSourceClass(), getClass().getName());
-        Assert.assertTrue(issue.getTime().isAfter(ZonedDateTime.now().minus(1, 
ChronoUnit.MINUTES)));
+    Assert.assertTrue(issue.getTime().isAfter(ZonedDateTime.now().minus(1, 
ChronoUnit.MINUTES)));
     Assert.assertTrue(issue.getTime().isBefore(ZonedDateTime.now().plus(1, 
ChronoUnit.MINUTES)));
     Assert.assertTrue(issue.getCode().length() > 1);
 
@@ -94,6 +96,34 @@ public class AutoTroubleshooterLogAppenderTest {
   }
 
   @Test
+  public void willGetSameErrorCodesForSameStackTraces()
+      throws Exception {
+
+    for (int i = 0; i < 5; i++) {
+      Exception exception;
+      try {
+        // Throwing exception to get a real stack trace in it
+        // Messages are intentionally different in every loop. We are checking 
that as all exceptions with
+        // same stack trace will get the same event code
+
+        try {
+          throw new InvalidOperationException("test inner exception " + i);
+        } catch (Exception inner) {
+          throw new IOException("test outer exception " + i, inner);
+        }
+      } catch (Exception e) {
+        exception = e;
+      }
+
+      appender.append(
+          new LoggingEvent(log.getName(), log, System.currentTimeMillis(), 
Level.ERROR, "test message", exception));
+    }
+
+    List<Issue> issues = issueRepository.getAll();
+    Assert.assertEquals(issues.size(), 1); // all issues should have the same 
error code and get deduplicated
+  }
+
+  @Test
   public void canLogExceptionWithSpecificErrorCode()
       throws Exception {
 
@@ -113,7 +143,6 @@ public class AutoTroubleshooterLogAppenderTest {
     Assert.assertEquals(issue.getCode(), "TestCode");
   }
 
-
   private static class TestException extends Exception implements 
ThrowableWithErrorCode {
     String errorCode;