Skip to content

Commit

Permalink
ZEPPELIN-541 - increase logging in application
Browse files Browse the repository at this point in the history
### What is this PR for?
To increase logging in the application, so as it

### What type of PR is it?
Refactoring

### Todos
* [x] - replace all printStackTrace() to LOGGER.error
* [x] - all catch block that doesn't throw exception, should have logging

### Is there a relevant Jira issue?
Yes, ZEPPELIN-541

### Screenshots (if appropriate)
N/A

Author: Prabhjyot Singh <[email protected]>

Closes apache#579 from prabhjyotsingh/increaseLogging and squashes the following commits:

ba053f5 [Prabhjyot Singh] revert note.json
648c65c [Prabhjyot Singh] fix CI
6d927fc [Prabhjyot Singh] Every catch should log error/info message or should throw an exception
7f8b70c [Prabhjyot Singh] replace printStackTrace() to LOGGER.error
  • Loading branch information
prabhjyotsingh authored and jongyoul committed Jan 10, 2016
1 parent 51812cb commit ff99ecb
Show file tree
Hide file tree
Showing 46 changed files with 239 additions and 157 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ private Ignite getIgnite() {

initEx = null;
} catch (Exception e) {
logger.error("Error in IgniteInterpreter while getIgnite: " , e);
initEx = e;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ public InterpreterResult interpret(String st, InterpreterContext context) {
}
}
} catch (Exception e) {
logger.error("Exception in IgniteSqlInterpreter while InterpreterResult interpret: ", e);
return IgniteInterpreterUtils.buildErrorResult(e);
} finally {
curStmt = null;
Expand All @@ -169,6 +170,7 @@ public void cancel(InterpreterContext context) {
curStmt.cancel();
} catch (SQLException e) {
// No-op.
logger.info("No-op while cancel in IgniteSqlInterpreter", e);
} finally {
curStmt = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public LensInterpreter(Properties property) {
s_logger.info("LensInterpreter created");
}
catch (Exception e) {
e.printStackTrace();
s_logger.error(e.toString(), e);
s_logger.error("unable to create lens interpreter", e);
}
}
Expand Down Expand Up @@ -375,6 +375,7 @@ public void cancel(InterpreterContext context) {
closeShell(s_paraToQH.get(context.getParagraphId()).getShell());
} catch (Exception e) {
// ignore
s_logger.info("Exception in LensInterpreter while cancel finally, ignore", e);
}
s_paraToQH.remove(context.getParagraphId());
closeShell(shell);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

import java.util.Map;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanFactoryUtils;
import org.springframework.beans.factory.InitializingBean;
Expand Down Expand Up @@ -56,6 +59,8 @@ public class LensJLineShellComponent extends JLineShell
private ExecutionStrategy executionStrategy = new LensSimpleExecutionStrategy();
private SimpleParser parser = new SimpleParser();

private static final Logger LOGGER = LoggerFactory.getLogger(LensJLineShellComponent.class);

public SimpleParser getSimpleParser() {
return parser;
}
Expand Down Expand Up @@ -123,7 +128,7 @@ public void waitForComplete() {
try {
shellThread.join();
} catch (InterruptedException e) {
e.printStackTrace();
LOGGER.error(e.toString(), e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.scheduler.SchedulerFactory;
import org.markdown4j.Markdown4jProcessor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Markdown interpreter for Zeppelin.
*/
public class Markdown extends Interpreter {
private Markdown4jProcessor md;
static final Logger LOGGER = LoggerFactory.getLogger(Markdown.class);

static {
Interpreter.register("md", Markdown.class.getName());
Expand All @@ -58,6 +61,7 @@ public InterpreterResult interpret(String st, InterpreterContext interpreterCont
try {
html = md.process(st);
} catch (IOException | java.lang.RuntimeException e) {
LOGGER.error("Exception in Markdown while interpret ", e);
return new InterpreterResult(Code.ERROR, InterpreterUtils.getMostRelevantMessage(e));
}
return new InterpreterResult(Code.SUCCESS, "%html " + html);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ public void cancel(InterpreterContext context) {
try {
currentStatement.cancel();
} catch (SQLException ex) {
logger.error("SQLException in PostgreSqlInterpreter while cancel ", ex);
} finally {
currentStatement = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import org.apache.zeppelin.interpreter.WrappedInterpreter;
import org.apache.zeppelin.scheduler.Scheduler;
import org.apache.zeppelin.spark.dep.DependencyContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonatype.aether.resolution.ArtifactResolutionException;
import org.sonatype.aether.resolution.DependencyResolutionException;

Expand Down Expand Up @@ -80,6 +82,7 @@ public class DepInterpreter extends Interpreter {
private DependencyContext depc;
private SparkJLineCompletion completor;
private SparkILoop interpreter;
static final Logger LOGGER = LoggerFactory.getLogger(DepInterpreter.class);

public DepInterpreter(Properties property) {
super(property);
Expand Down Expand Up @@ -195,6 +198,7 @@ public InterpreterResult interpret(String st, InterpreterContext context) {
depc.fetch();
} catch (MalformedURLException | DependencyResolutionException
| ArtifactResolutionException e) {
LOGGER.error("Exception in DepInterpreter while interpret ", e);
return new InterpreterResult(Code.ERROR, e.toString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
*
*/
public class SparkInterpreter extends Interpreter {
Logger logger = LoggerFactory.getLogger(SparkInterpreter.class);
public static Logger logger = LoggerFactory.getLogger(SparkInterpreter.class);

static {
Interpreter.register(
Expand Down Expand Up @@ -186,7 +186,7 @@ static JobProgressListener setupListeners(SparkContext context) {
}
} catch (NoSuchMethodException | SecurityException | IllegalAccessException
| IllegalArgumentException | InvocationTargetException e) {
e.printStackTrace();
logger.error(e.toString(), e);
return null;
}
return pl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@
import org.junit.FixMethodOrder;
import org.junit.Test;
import org.junit.runners.MethodSorters;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public class SparkInterpreterTest {
public static SparkInterpreter repl;
private InterpreterContext context;
private File tmpDir;
public static Logger LOGGER = LoggerFactory.getLogger(SparkInterpreterTest.class);


/**
Expand Down Expand Up @@ -177,7 +180,7 @@ public void emptyConfigurationVariablesOnlyForNonSparkProperties() {
for (Object oKey : intpProperty.keySet()) {
String key = (String) oKey;
String value = (String) intpProperty.get(key);
repl.logger.debug(String.format("[%s]: [%s]", key, value));
LOGGER.debug(String.format("[%s]: [%s]", key, value));
if (key.startsWith("spark.") && value.isEmpty()) {
assertTrue(String.format("configuration starting from 'spark.' should not be empty. [%s]", key), !sparkConf.contains(key) || !sparkConf.get(key).isEmpty());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class SparkSqlInterpreterTest {

Expand All @@ -41,6 +43,8 @@ public class SparkSqlInterpreterTest {
private InterpreterContext context;
private InterpreterGroup intpGroup;

Logger LOGGER = LoggerFactory.getLogger(SparkSqlInterpreterTest.class);

@Before
public void setUp() throws Exception {
Properties p = new Properties();
Expand Down Expand Up @@ -96,6 +100,7 @@ public void test() {
fail("Exception not catched");
} catch (Exception e) {
// okay
LOGGER.info("Exception in SparkSqlInterpreterTest while test ", e);
}
assertEquals(InterpreterResult.Code.SUCCESS, sql.interpret("select case when name==\"aa\" then name else name end from test", context).code());
}
Expand Down
14 changes: 11 additions & 3 deletions tajo/src/test/java/org/apache/zeppelin/tajo/TesterResultSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,26 @@
*/
package org.apache.zeppelin.tajo;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.InputStream;
import java.io.Reader;
import java.io.UnsupportedEncodingException;
import java.math.BigDecimal;
import java.sql.*;
import java.util.Calendar;
import java.util.Map;
import java.io.InputStream;
import java.io.Reader;
import java.io.UnsupportedEncodingException;

/**
* This is borrowed from Apache Commons DBCP2.
*
* A dummy {@link java.sql.ResultSet}, for testing purposes.
*/
public class TesterResultSet implements ResultSet {

Logger LOGGER = LoggerFactory.getLogger(TesterResultSet.class);

public TesterResultSet(Statement stmt) {
_statement = stmt;
}
Expand Down Expand Up @@ -262,6 +268,8 @@ public byte[] getBytes(String columnName) throws SQLException {
return columnName.getBytes("UTF-8");
} catch (UnsupportedEncodingException e) {
// Impossible. JVMs are required to support UTF-8
LOGGER.error("Exception in TesterResultSet while getBytes, Impossible. JVMs are required to" +
" support UTF-8", e);
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public void destroy() {



static Logger logger = LoggerFactory.getLogger(Interpreter.class);
public static Logger logger = LoggerFactory.getLogger(Interpreter.class);
private InterpreterGroup interpreterGroup;
private URL [] classloaderUrls;
protected Properties property;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
public class InterpreterGroup extends LinkedList<Interpreter>{
String id;

Logger LOGGER = Logger.getLogger(InterpreterGroup.class);

AngularObjectRegistry angularObjectRegistry;
RemoteInterpreterProcess remoteInterpreterProcess; // attached remote interpreter process

Expand Down Expand Up @@ -100,8 +102,7 @@ public void run() {
try {
t.join();
} catch (InterruptedException e) {
Logger logger = Logger.getLogger(InterpreterGroup.class);
logger.error("Can't close interpreter", e);
LOGGER.error("Can't close interpreter", e);
}
}
}
Expand All @@ -124,8 +125,7 @@ public void run() {
try {
t.join();
} catch (InterruptedException e) {
Logger logger = Logger.getLogger(InterpreterGroup.class);
logger.error("Can't close interpreter", e);
LOGGER.error("Can't close interpreter", e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ public AngularObject addAndNotifyRemoteProcess(String name, Object o, String not
* this method should be used instead of remove()
* @param name
* @param noteId
* @param emit
* @return
*/
public AngularObject removeAndNotifyRemoteProcess(String name, String noteId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ private void waitQuietly() {
wait(1000);
}
} catch (InterruptedException ignored) {
logger.info("Error in RemoteInterpreterEventPoller while waitQuietly : ", ignored);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ public int reference(InterpreterGroup interpreterGroup) {
try {
Thread.sleep(500);
} catch (InterruptedException e) {
logger.error("Exception in RemoteInterpreterProcess while synchronized reference " +
"Thread.sleep", e);
}
}
}
Expand Down Expand Up @@ -177,6 +179,8 @@ public int dereference() {
client.shutdown();
} catch (Exception e) {
// safely ignore exception while client.shutdown() may terminates remote process
logger.info("Exception in RemoteInterpreterProcess while synchronized dereference, can " +
"safely ignore exception while client.shutdown() may terminates remote process", e);
} finally {
if (client != null) {
// no longer used
Expand All @@ -195,6 +199,8 @@ public int dereference() {
try {
Thread.sleep(500);
} catch (InterruptedException e) {
logger.error("Exception in RemoteInterpreterProcess while synchronized dereference " +
"Thread.sleep", e);
}
} else {
break;
Expand Down Expand Up @@ -266,6 +272,8 @@ public void updateRemoteAngularObject(String name, String noteId, Object o) {
client = getClient();
} catch (NullPointerException e) {
// remote process not started
logger.info("NullPointerException in RemoteInterpreterProcess while " +
"updateRemoteAngularObject getClient, remote process not started", e);
return;
} catch (Exception e) {
logger.error("Can't update angular object", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ public void shutdown() throws TException {
try {
Thread.sleep(300);
} catch (InterruptedException e) {
logger.info("Exception in RemoteInterpreterServer while shutdown, Thread.sleep", e);
}
}

Expand Down Expand Up @@ -170,7 +171,7 @@ public void createInterpreter(String className, Map<String, String> properties)
} catch (ClassNotFoundException | NoSuchMethodException | SecurityException
| InstantiationException | IllegalAccessException
| IllegalArgumentException | InvocationTargetException e) {
e.printStackTrace();
logger.error(e.toString(), e);
throw new TException(e);
}
}
Expand Down Expand Up @@ -225,6 +226,7 @@ public RemoteInterpreterResult interpret(String className, String st,
try {
jobListener.wait(1000);
} catch (InterruptedException e) {
logger.info("Exception in RemoteInterpreterServer while interpret, jobListener.wait", e);
}
}
}
Expand Down Expand Up @@ -455,6 +457,7 @@ public RemoteInterpreterEvent getEvent() throws TException {
try {
eventQueue.wait(1000);
} catch (InterruptedException e) {
logger.info("Exception in RemoteInterpreterServer while getEvent, eventQueue.wait", e);
}
}

Expand All @@ -468,7 +471,6 @@ public RemoteInterpreterEvent getEvent() throws TException {

/**
* called when object is updated in client (web) side.
* @param className
* @param name
* @param noteId noteId where the update issues
* @param object
Expand Down Expand Up @@ -499,6 +501,7 @@ public void angularObjectUpdate(String name, String noteId, String object)
return;
} catch (Exception e) {
// no luck
logger.info("Exception in RemoteInterpreterServer while angularObjectUpdate, no luck", e);
}
}

Expand All @@ -510,6 +513,7 @@ public void angularObjectUpdate(String name, String noteId, String object)
}.getType());
} catch (Exception e) {
// no lock
logger.info("Exception in RemoteInterpreterServer while angularObjectUpdate, no lock", e);
}
}

Expand Down Expand Up @@ -544,6 +548,7 @@ public void angularObjectAdd(String name, String noteId, String object)
}.getType());
} catch (Exception e) {
// nolock
logger.info("Exception in RemoteInterpreterServer while angularObjectAdd, nolock", e);
}

// try string object type at last
Expand Down
Loading

0 comments on commit ff99ecb

Please sign in to comment.