commit 7985986113f6422e7154e53ca0b4ebecca13b6c1 Author: iwakeh iwakeh@torproject.org Date: Tue Sep 5 13:21:29 2017 +0000

Streamline logging.

slf4j and logback were already used in ExoneraTor. This commit makes all classes use slf4j and replaced j.u.l and System.{err,out} logging. --- build.xml | 1 + .../exonerator/ExoneraTorDatabaseImporter.java | 48 ++++++++++++---------- .../torproject/exonerator/ExoneraTorServlet.java | 10 +++-- .../org/torproject/exonerator/QueryResponse.java | 27 ++++++------ .../org/torproject/exonerator/QueryServlet.java | 34 ++++++--------- 5 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/build.xml b/build.xml index 2dc70f0..68d5c98 100644 --- a/build.xml +++ b/build.xml @@ -158,6 +158,7 @@ <include name="commons-lang-2.6.jar"/> <include name="gson-2.4.jar" /> <include name="postgresql-9.4.1212.jar"/> + <include name="slf4j-api-1.7.22.jar"/> </lib> <classes dir="${classes}"/> <zipfileset dir="${resources}" diff --git a/src/main/java/org/torproject/exonerator/ExoneraTorDatabaseImporter.java b/src/main/java/org/torproject/exonerator/ExoneraTorDatabaseImporter.java index a36e242..155506c 100644 --- a/src/main/java/org/torproject/exonerator/ExoneraTorDatabaseImporter.java +++ b/src/main/java/org/torproject/exonerator/ExoneraTorDatabaseImporter.java @@ -14,6 +14,9 @@ import org.torproject.descriptor.RelayNetworkStatusConsensus;

import org.apache.commons.codec.binary.Hex;

+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.io.BufferedReader; import java.io.BufferedWriter; import java.io.File; @@ -39,6 +42,9 @@ import java.util.TreeMap; /* Import Tor descriptors into the ExoneraTor database. */ public class ExoneraTorDatabaseImporter {

+ private static Logger logger + = LoggerFactory.getLogger(ExoneraTorDatabaseImporter.class); + /** Main function controlling the parsing process. */ public static void main(String[] args) { readConfiguration(); @@ -63,7 +69,7 @@ public class ExoneraTorDatabaseImporter { private static void readConfiguration() { File configFile = new File("config"); if (!configFile.exists()) { - System.err.println("Could not find config file. Exiting."); + logger.error("Could not find config file. Exiting."); System.exit(1); } String line = null; @@ -82,7 +88,7 @@ public class ExoneraTorDatabaseImporter { } br.close(); } catch (IOException e) { - System.err.println("Could not parse config file. Exiting."); + logger.error("Could not parse config file. Exiting.", e); System.exit(1); } } @@ -95,7 +101,7 @@ public class ExoneraTorDatabaseImporter { try { connection = DriverManager.getConnection(jdbcString); } catch (SQLException e) { - System.out.println("Could not connect to database. Exiting."); + logger.error("Could not connect to database. Exiting.", e); System.exit(1); } } @@ -113,8 +119,8 @@ public class ExoneraTorDatabaseImporter { insertExitlistentryStatement = connection.prepareCall( "{call insert_exitlistentry(?, ?, ?, ?, ?)}"); } catch (SQLException e) { - System.out.println("Could not prepare callable statements to " - + "import data into the database. Exiting."); + logger.warn("Could not prepare callable statements to " + + "import data into the database. Exiting.", e); System.exit(1); } } @@ -130,12 +136,12 @@ public class ExoneraTorDatabaseImporter { br.close(); if (System.currentTimeMillis() - runStarted < 6L * 60L * 60L * 1000L) { - System.out.println("File 'exonerator-lock' is less than 6 " + logger.warn("File 'exonerator-lock' is less than 6 " + "hours old. Exiting."); System.exit(1); } else { - System.out.println("File 'exonerator-lock' is at least 6 hours " - + "old. Overwriting and executing anyway."); + logger.warn("File 'exonerator-lock' is at least 6 hours old." + + " Overwriting and executing anyway."); } } BufferedWriter bw = new BufferedWriter(new FileWriter( @@ -143,8 +149,7 @@ public class ExoneraTorDatabaseImporter { bw.append(String.valueOf(System.currentTimeMillis()) + "\n"); bw.close(); } catch (IOException e) { - System.out.println("Could not create 'exonerator-lock' file. " - + "Exiting."); + logger.warn("Could not create 'exonerator-lock' file. Exiting."); System.exit(1); } } @@ -179,9 +184,9 @@ public class ExoneraTorDatabaseImporter { lineNumber++; String[] parts = line.split(","); if (parts.length != 2) { - System.out.println("File 'stats/exonerator-import-history' " - + "contains a corrupt entry in line " + lineNumber - + ". Ignoring parse history file entirely."); + logger.warn("File 'stats/exonerator-import-history' " + + "contains a corrupt entry in line {}. " + + "Ignoring parse history file entirely.", lineNumber); lastImportHistory.clear(); br.close(); return; @@ -192,7 +197,7 @@ public class ExoneraTorDatabaseImporter { } br.close(); } catch (IOException e) { - System.out.println("Could not read import history. Ignoring."); + logger.warn("Could not read import history. Ignoring.", e); lastImportHistory.clear(); } } @@ -306,8 +311,8 @@ public class ExoneraTorDatabaseImporter { insertStatusentryStatement.setString(6, orAddress.replaceAll("[\[\]]", "")); } else { - System.err.println("Could not import status entry with IPv6 " - + "address '" + orAddress + "'. Exiting."); + logger.error("Could not import status entry with IPv6 " + + "address '{}'. Exiting.", orAddress); System.exit(1); } } @@ -315,7 +320,7 @@ public class ExoneraTorDatabaseImporter { insertStatusentryStatement.execute(); } } catch (SQLException e) { - System.out.println("Could not import status entry. Exiting."); + logger.error("Could not import status entry. Exiting.", e); System.exit(1); } } @@ -360,7 +365,7 @@ public class ExoneraTorDatabaseImporter { insertExitlistentryStatement.setBytes(5, rawExitlistentry); insertExitlistentryStatement.execute(); } catch (SQLException e) { - System.out.println("Could not import exit list entry. Exiting."); + logger.error("Could not import exit list entry. Exiting.", e); System.exit(1); } } @@ -379,8 +384,8 @@ public class ExoneraTorDatabaseImporter { } bw.close(); } catch (IOException e) { - System.out.println("File 'stats/exonerator-import-history' could " - + "not be written. Ignoring."); + logger.warn("File 'stats/exonerator-import-history' could " + + "not be written. Ignoring.", e); } }

@@ -389,8 +394,7 @@ public class ExoneraTorDatabaseImporter { try { connection.close(); } catch (SQLException e) { - System.out.println("Could not close database connection. " - + "Ignoring."); + logger.warn("Could not close database connection. Ignoring.", e); } }

diff --git a/src/main/java/org/torproject/exonerator/ExoneraTorServlet.java b/src/main/java/org/torproject/exonerator/ExoneraTorServlet.java index 8e88882..e5dac24 100644 --- a/src/main/java/org/torproject/exonerator/ExoneraTorServlet.java +++ b/src/main/java/org/torproject/exonerator/ExoneraTorServlet.java @@ -5,6 +5,9 @@ package org.torproject.exonerator;

import org.apache.commons.lang.StringEscapeUtils;

+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -23,7 +26,6 @@ import java.util.ResourceBundle; import java.util.SortedMap; import java.util.TimeZone; import java.util.TreeMap; -import java.util.logging.Logger; import java.util.regex.Pattern;

import javax.servlet.ServletConfig; @@ -48,7 +50,7 @@ public class ExoneraTorServlet extends HttpServlet { @Override public void init(ServletConfig config) throws ServletException { super.init(config); - this.logger = Logger.getLogger(ExoneraTorServlet.class.toString()); + this.logger = LoggerFactory.getLogger(ExoneraTorServlet.class); this.availableLanguageNames = new TreeMap<>(); for (String locale : this.availableLanguages) { ResourceBundle rb = ResourceBundle.getBundle("ExoneraTor", @@ -307,7 +309,7 @@ public class ExoneraTorServlet extends HttpServlet { } catch (IOException e) { /* No result from backend, so that we don't have a query response to * process further. */ - logger.severe("Backend query failed: " + e.getMessage()); + logger.error("Backend query failed.", e); } return null; } @@ -584,7 +586,7 @@ public class ExoneraTorServlet extends HttpServlet { content = rb.getString("technicaldetails.exit.no"); break; default: // should never happen - logger.warning("Unknown content: " + content); + logger.warn("Unknown content: '{}'.", content); } } out.print(" <td" + attributes + ">" + content + "</td>"); diff --git a/src/main/java/org/torproject/exonerator/QueryResponse.java b/src/main/java/org/torproject/exonerator/QueryResponse.java index 8053e94..b172b7d 100644 --- a/src/main/java/org/torproject/exonerator/QueryResponse.java +++ b/src/main/java/org/torproject/exonerator/QueryResponse.java @@ -7,28 +7,30 @@ import com.google.gson.Gson; import com.google.gson.annotations.Expose; import com.google.gson.annotations.SerializedName;

+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.io.IOException; import java.io.Reader; import java.util.Objects; -import java.util.logging.Logger;

/** Query response from the ExoneraTor database. */ public class QueryResponse {

@Expose(serialize = false, deserialize = false) - private static Logger logger = Logger.getLogger(QueryResponse.class.toString()); + private static Logger logger = LoggerFactory.getLogger(QueryResponse.class);

- /* Actual response format version implemented by this class. */ + /* Actual version implemented by this class. */ @Expose(serialize = false, deserialize = false) private static final String VERSION = "1.0";

/* Don't accept query responses with versions lower than this. */ @Expose(serialize = false, deserialize = false) - private static final String FIRST_RECOGNIZED_VERSION = "1.0"; + private static final String FIRSTRECOGNIZEDVERSION = "1.0";

/* Don't accept query responses with this version or higher. */ @Expose(serialize = false, deserialize = false) - private static final String FIRST_UNRECOGNIZED_VERSION = "2.0"; + private static final String FIRSTUNRECOGNIZEDVERSION = "2.0";

/** Version of this response format. */ @Expose @@ -96,20 +98,19 @@ public class QueryResponse { try { QueryResponse response = gson.fromJson(reader, QueryResponse.class); if (null == response || null == response.version) { - logger.warning("Response is either empty or does not contain " + logger.warn("Response is either empty or does not contain " + "version information."); return null; - } else if (response.version.compareTo(FIRST_RECOGNIZED_VERSION) < 0 - || response.version.compareTo(FIRST_UNRECOGNIZED_VERSION) >= 0) { - logger.warning("Response has either an older or a newer version (" - + response.version + ") than we can handle (" - + FIRST_RECOGNIZED_VERSION + " <= x < " + FIRST_UNRECOGNIZED_VERSION - + ")."); + } else if (response.version.compareTo(FIRSTRECOGNIZEDVERSION) < 0 + || response.version.compareTo(FIRSTUNRECOGNIZEDVERSION) >= 0) { + logger.error("Response has version {}, which is not in the range " + + "of versions we can handle: {} <= x < {}).", response.version, + FIRSTRECOGNIZEDVERSION, FIRSTUNRECOGNIZEDVERSION); return null; } return response; } catch (RuntimeException e) { - logger.severe("JSON decoding failed: " + e.getMessage()); + logger.error("JSON decoding failed.", e); } return null; } diff --git a/src/main/java/org/torproject/exonerator/QueryServlet.java b/src/main/java/org/torproject/exonerator/QueryServlet.java index 74be053..893a7b9 100644 --- a/src/main/java/org/torproject/exonerator/QueryServlet.java +++ b/src/main/java/org/torproject/exonerator/QueryServlet.java @@ -3,9 +3,11 @@

package org.torproject.exonerator;

-import com.google.gson.Gson; import org.apache.commons.codec.binary.Hex;

+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.io.IOException; import java.sql.CallableStatement; import java.sql.Connection; @@ -19,8 +21,6 @@ import java.util.List; import java.util.SortedSet; import java.util.TimeZone; import java.util.TreeSet; -import java.util.logging.Level; -import java.util.logging.Logger; import java.util.regex.Pattern;

import javax.naming.Context; @@ -44,9 +44,7 @@ public class QueryServlet extends HttpServlet {

@Override public void init() { - - /* Initialize logger. */ - this.logger = Logger.getLogger(QueryServlet.class.toString()); + this.logger = LoggerFactory.getLogger(QueryServlet.class);

/* Look up data source. */ try { @@ -54,7 +52,7 @@ public class QueryServlet extends HttpServlet { this.ds = (DataSource) cxt.lookup("java:comp/env/jdbc/exonerator"); this.logger.info("Successfully looked up data source."); } catch (NamingException e) { - this.logger.log(Level.WARNING, "Could not look up data source", e); + this.logger.warn("Could not look up data source", e); } }

@@ -96,17 +94,12 @@ public class QueryServlet extends HttpServlet { if (null == queryResponse) { response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Database error."); - return; + } else { + /* Write the response. */ + response.setContentType("application/json"); + response.setCharacterEncoding("utf-8"); + response.getWriter().write(QueryResponse.toJson(queryResponse)); } - - /* Format the query response. */ - Gson gson = new Gson(); - String formattedResponse = gson.toJson(queryResponse); - - /* Write the response. */ - response.setContentType("application/json"); - response.setCharacterEncoding("utf-8"); - response.getWriter().write(formattedResponse); }

/* Helper methods for handling the request. */ @@ -314,12 +307,11 @@ public class QueryServlet extends HttpServlet { rs.close(); cs.close(); conn.close(); - this.logger.info("Returned a database connection to the pool " - + "after " + (System.currentTimeMillis() - - requestedConnection) + " millis."); + this.logger.info("Returned a database connection to the pool after {}" + + " millis.", System.currentTimeMillis() - requestedConnection); } catch (SQLException e) { /* Nothing found. */ - this.logger.log(Level.WARNING, "Database error: " + e.getMessage(), e); + this.logger.warn("Database error. Returning 'null'.", e); return null; }