[tor-commits] [metrics-lib/release] Simplify logging configuration.

commit d7d5303e76a69f5fd0fe2b3b9f6be9f25f1fd824 Author: Karsten Loesing karsten.loesing@gmx.net Date: Tue Mar 31 09:44:06 2020 +0200

Simplify logging configuration.

Implements #33549. --- CHANGELOG.md | 1 + src/build | 2 +- .../descriptor/DescriptorSourceFactory.java | 4 +-- .../descriptor/impl/DescriptorParserImpl.java | 4 +-- .../descriptor/impl/DescriptorReaderImpl.java | 20 ++++++------- .../descriptor/index/DescriptorIndexCollector.java | 34 ++++++++++++---------- .../org/torproject/descriptor/index/FileNode.java | 8 ++--- .../org/torproject/descriptor/index/IndexNode.java | 5 ---- .../descriptor/log/LogDescriptorImpl.java | 6 ---- .../descriptor/log/WebServerAccessLogImpl.java | 6 ---- .../descriptor/log/WebServerAccessLogLine.java | 4 +-- 11 files changed, 39 insertions(+), 55 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a5a49e..f0c9a63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Minor changes - Avoid invoking overridable methods from constructors. - Make all descriptor instances serializable. + - Simplify logging configuration.

# Changes in version 2.10.0 - 2020-01-15 diff --git a/src/build b/src/build index 264e498..fd85646 160000 --- a/src/build +++ b/src/build @@ -1 +1 @@ -Subproject commit 264e498f54a20f7d299daaf2533d043f880e6a8b +Subproject commit fd856466bcb260f53ef69a24c102d0e49d171cc3 diff --git a/src/main/java/org/torproject/descriptor/DescriptorSourceFactory.java b/src/main/java/org/torproject/descriptor/DescriptorSourceFactory.java index 97f93cc..3dc8439 100644 --- a/src/main/java/org/torproject/descriptor/DescriptorSourceFactory.java +++ b/src/main/java/org/torproject/descriptor/DescriptorSourceFactory.java @@ -40,7 +40,7 @@ import org.slf4j.LoggerFactory; */ public final class DescriptorSourceFactory {

- private static Logger log = LoggerFactory.getLogger( + private static final Logger logger = LoggerFactory.getLogger( DescriptorSourceFactory.class);

/** @@ -147,7 +147,7 @@ public final class DescriptorSourceFactory { } object = ClassLoader.getSystemClassLoader().loadClass(clazzName) .getDeclaredConstructor().newInstance(); - log.info("Serving implementation {} for {}.", clazzName, type); + logger.debug("Serving implementation {} for {}.", clazzName, type); } catch (ReflectiveOperationException ex) { throw new RuntimeException("Cannot load class " + clazzName + "for type " + type, ex); diff --git a/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java b/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java index 0a2444b..160baac 100644 --- a/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java +++ b/src/main/java/org/torproject/descriptor/impl/DescriptorParserImpl.java @@ -23,7 +23,7 @@ import java.util.List;

public class DescriptorParserImpl implements DescriptorParser {

- private static final Logger log + private static final Logger logger = LoggerFactory.getLogger(DescriptorParserImpl.class);

@Override @@ -33,7 +33,7 @@ public class DescriptorParserImpl implements DescriptorParser { return this.detectTypeAndParseDescriptors(rawDescriptorBytes, sourceFile, fileName); } catch (DescriptorParseException e) { - log.debug("Cannot parse descriptor file '{}'.", sourceFile, e); + logger.debug("Cannot parse descriptor file '{}'.", sourceFile, e); List<Descriptor> parsedDescriptors = new ArrayList<>(); parsedDescriptors.add(new UnparseableDescriptorImpl(rawDescriptorBytes, new int[] { 0, rawDescriptorBytes.length }, sourceFile, e)); diff --git a/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java b/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java index 08c82ec..8ec04a5 100644 --- a/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java +++ b/src/main/java/org/torproject/descriptor/impl/DescriptorReaderImpl.java @@ -36,11 +36,9 @@ import java.util.TreeMap;

public class DescriptorReaderImpl implements DescriptorReader {

- private static Logger log = LoggerFactory.getLogger( + private static final Logger logger = LoggerFactory.getLogger( DescriptorReaderImpl.class);

- private static Logger statisticsLog = LoggerFactory.getLogger("statistics"); - private boolean hasStartedReading = false;

private File manualSaveHistoryFile; @@ -161,8 +159,8 @@ public class DescriptorReaderImpl implements DescriptorReader { this.readTarballs(); this.hasFinishedReading = true; } catch (Throwable t) { - log.error("Bug: uncaught exception or error while reading descriptors.", - t); + logger.error("Bug: uncaught exception or error while reading " + + "descriptors.", t); } finally { if (null != this.descriptorQueue) { this.descriptorQueue.setOutOfDescriptors(); @@ -180,7 +178,7 @@ public class DescriptorReaderImpl implements DescriptorReader { StandardCharsets.UTF_8); for (String line : lines) { if (!line.contains(" ")) { - log.warn("Unexpected line structure in old history: {}", line); + logger.warn("Unexpected line structure in old history: {}", line); continue; } long lastModifiedMillis = Long.parseLong(line.substring(0, @@ -189,7 +187,7 @@ public class DescriptorReaderImpl implements DescriptorReader { this.excludedFilesBefore.put(absolutePath, lastModifiedMillis); } } catch (IOException | NumberFormatException e) { - log.warn("Trouble reading given history file {}.", historyFile, e); + logger.warn("Trouble reading given history file {}.", historyFile, e); } }

@@ -212,7 +210,7 @@ public class DescriptorReaderImpl implements DescriptorReader { bw.newLine(); } } catch (IOException e) { - log.warn("Trouble writing new history file '{}'.", + logger.warn("Trouble writing new history file '{}'.", historyFile, e); } } @@ -250,7 +248,7 @@ public class DescriptorReaderImpl implements DescriptorReader { } this.parsedFilesAfter.put(absolutePath, lastModifiedMillis); } catch (IOException e) { - log.warn("Unable to read descriptor file {}.", file, e); + logger.warn("Unable to read descriptor file {}.", file, e); } } } @@ -271,13 +269,13 @@ public class DescriptorReaderImpl implements DescriptorReader { this.parsedFilesAfter.put(tarball.getAbsolutePath(), tarball.lastModified()); } catch (IOException e) { - log.warn("Unable to read tarball {}.", tarball, e); + logger.warn("Unable to read tarball {}.", tarball, e); } long previousPercentDone = 100L * progress / total; progress += tarball.length(); long percentDone = 100L * progress / total; if (percentDone > previousPercentDone) { - statisticsLog.info("Finished reading {}% of tarball bytes.", + logger.info("Finished reading {}% of tarball bytes.", percentDone); } } diff --git a/src/main/java/org/torproject/descriptor/index/DescriptorIndexCollector.java b/src/main/java/org/torproject/descriptor/index/DescriptorIndexCollector.java index b4aae59..b2f4e97 100644 --- a/src/main/java/org/torproject/descriptor/index/DescriptorIndexCollector.java +++ b/src/main/java/org/torproject/descriptor/index/DescriptorIndexCollector.java @@ -30,7 +30,7 @@ import java.util.TreeMap; */ public class DescriptorIndexCollector implements DescriptorCollector {

- private static Logger log = LoggerFactory + private static final Logger logger = LoggerFactory .getLogger(DescriptorIndexCollector.class);

/** @@ -44,7 +44,7 @@ public class DescriptorIndexCollector implements DescriptorCollector { public void collectDescriptors(String collecTorIndexUrlString, String[] remoteDirectories, long minLastModified, File localDirectory, boolean deleteExtraneousLocalFiles) { - log.info("Starting descriptor collection."); + logger.info("Starting descriptor collection."); if (minLastModified < 0) { throw new IllegalArgumentException("A negative minimum " + "last-modified time is not permitted."); @@ -60,7 +60,8 @@ public class DescriptorIndexCollector implements DescriptorCollector { + "fetched files. Move this file away or delete it. Aborting " + "descriptor collection."); } - log.info("Indexing local directory {}.", localDirectory.getAbsolutePath()); + logger.info("Indexing local directory {}.", + localDirectory.getAbsolutePath()); SortedMap<String, Long> localFiles = statLocalDirectory(localDirectory); SortedMap<String, FileNode> remoteFiles; IndexNode index; @@ -71,27 +72,27 @@ public class DescriptorIndexCollector implements DescriptorCollector { if (indexUrl.getPath().isEmpty()) { indexUrlString += "/index/index.json"; } - log.info("Fetching remote index file {}.", indexUrlString); + logger.info("Fetching remote index file {}.", indexUrlString); index = IndexNode.fetchIndex(indexUrlString); remoteFiles = index.retrieveFilesIn(remoteDirectories); } catch (Exception ex) { - log.warn("Cannot fetch index file {} and hence cannot determine which " + logger.warn("Cannot fetch index file {} and hence cannot determine which " + "remote files to fetch. Aborting descriptor collection.", indexUrlString, ex); return; } - log.info("Fetching remote files from {}.", index.path); + logger.info("Fetching remote files from {}.", index.path); if (!this.fetchRemoteFiles(index.path, remoteFiles, minLastModified, localDirectory, localFiles)) { return; } if (deleteExtraneousLocalFiles) { - log.info("Deleting extraneous files from local directory {}.", + logger.info("Deleting extraneous files from local directory {}.", localDirectory); deleteExtraneousLocalFiles(remoteDirectories, remoteFiles, localDirectory, localFiles); } - log.info("Finished descriptor collection."); + logger.info("Finished descriptor collection."); }

boolean fetchRemoteFiles(String baseUrl, SortedMap<String, FileNode> remotes, @@ -108,14 +109,15 @@ public class DescriptorIndexCollector implements DescriptorCollector { continue; } if (!filepath.exists() && !filepath.mkdirs()) { - log.warn("Cannot create local directory {} to store remote file {}. " + logger.warn("Cannot create local directory {} to store remote file {}. " + "Aborting descriptor collection.", filepath, filename); return false; } File destinationFile = new File(filepath, filename); File tempDestinationFile = new File(filepath, "." + filename); - log.debug("Fetching remote file {} with expected size of {} bytes from " - + "{}, storing locally to temporary file {}, then renaming to {}.", + logger.debug("Fetching remote file {} with expected size of {} bytes " + + "from {}, storing locally to temporary file {}, then renaming to " + + "{}.", filepathname, entry.getValue().size, baseUrl, tempDestinationFile.getAbsolutePath(), destinationFile.getAbsolutePath()); @@ -127,14 +129,14 @@ public class DescriptorIndexCollector implements DescriptorCollector { tempDestinationFile.renameTo(destinationFile); destinationFile.setLastModified(lastModifiedMillis); } else { - log.warn("Fetched remote file {} from {} has a size of {} bytes " + logger.warn("Fetched remote file {} from {} has a size of {} bytes " + "which is different from the expected {} bytes. Not storing " + "this file.", filename, baseUrl, tempDestinationFile.length(), entry.getValue().size); } } catch (IOException e) { - log.warn("Cannot fetch remote file {} from {}. Skipping that file.", + logger.warn("Cannot fetch remote file {} from {}. Skipping that file.", filename, baseUrl, e); } } @@ -151,7 +153,7 @@ public class DescriptorIndexCollector implements DescriptorCollector { if (localPath.startsWith(remDir)) { if (!remoteFiles.containsKey(localPath)) { File extraneousLocalFile = new File(localDir, localPath); - log.debug("Deleting extraneous local file {}.", + logger.debug("Deleting extraneous local file {}.", extraneousLocalFile.getAbsolutePath()); extraneousLocalFile.delete(); } @@ -179,8 +181,8 @@ public class DescriptorIndexCollector implements DescriptorCollector { } }); } catch (IOException ioe) { - log.warn("Cannot index local directory {} to skip any remote files that " - + "already exist locally. Continuing with an either empty or " + logger.warn("Cannot index local directory {} to skip any remote files " + + "that already exist locally. Continuing with an either empty or " + "incomplete index of local files.", localDir, ioe); } return locals; diff --git a/src/main/java/org/torproject/descriptor/index/FileNode.java b/src/main/java/org/torproject/descriptor/index/FileNode.java index 6c35146..a433d59 100644 --- a/src/main/java/org/torproject/descriptor/index/FileNode.java +++ b/src/main/java/org/torproject/descriptor/index/FileNode.java @@ -21,7 +21,7 @@ import java.util.TimeZone; */ public class FileNode implements Comparable<FileNode> {

- private static Logger log = LoggerFactory.getLogger(FileNode.class); + private static final Logger logger = LoggerFactory.getLogger(FileNode.class);

/** Path (i.e. file name) is exposed in JSON. */ public final String path; @@ -70,9 +70,9 @@ public class FileNode implements Comparable<FileNode> { try { lastModifiedMillis = dateTimeFormat.parse(this.lastModified).getTime(); } catch (ParseException ex) { - log.warn("Cannot parse last-modified time {} of remote file entry {}. " - + "Fetching remote file regardless of configured last-modified " - + "time. The following error message provides more details.", + logger.warn("Cannot parse last-modified time {} of remote file entry " + + "{}. Fetching remote file regardless of configured last-modified " + + "time. The following error message provides more details.", this.lastModified, this.path, ex); this.lastModifiedMillis = -1L; } diff --git a/src/main/java/org/torproject/descriptor/index/IndexNode.java b/src/main/java/org/torproject/descriptor/index/IndexNode.java index ce3faa4..d5f62ad 100644 --- a/src/main/java/org/torproject/descriptor/index/IndexNode.java +++ b/src/main/java/org/torproject/descriptor/index/IndexNode.java @@ -14,9 +14,6 @@ import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.PropertyNamingStrategy;

-import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -39,8 +36,6 @@ import java.util.TreeSet; @JsonPropertyOrder({ "created", "revision", "path", "directories", "files" }) public class IndexNode {

- private static Logger log = LoggerFactory.getLogger(IndexNode.class); - private static final int READ_TIMEOUT = Integer.parseInt(System .getProperty("sun.net.client.defaultReadTimeout", "60000"));

diff --git a/src/main/java/org/torproject/descriptor/log/LogDescriptorImpl.java b/src/main/java/org/torproject/descriptor/log/LogDescriptorImpl.java index a253c50..fdebc90 100644 --- a/src/main/java/org/torproject/descriptor/log/LogDescriptorImpl.java +++ b/src/main/java/org/torproject/descriptor/log/LogDescriptorImpl.java @@ -8,9 +8,6 @@ import org.torproject.descriptor.DescriptorParseException; import org.torproject.descriptor.LogDescriptor; import org.torproject.descriptor.internal.FileType;

-import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.File; @@ -37,9 +34,6 @@ public abstract class LogDescriptorImpl

private static final int unrecognizedLinesLimit = 3;

- private static final Logger log - = LoggerFactory.getLogger(LogDescriptorImpl.class); - private static Pattern filenamePattern = Pattern.compile( "(?:\S*)" + MARKER + SEP + "(?:[0-9a-zA-Z]*)(?:\.?)([a-zA-Z2]*)");

diff --git a/src/main/java/org/torproject/descriptor/log/WebServerAccessLogImpl.java b/src/main/java/org/torproject/descriptor/log/WebServerAccessLogImpl.java index eb05413..986eafc 100644 --- a/src/main/java/org/torproject/descriptor/log/WebServerAccessLogImpl.java +++ b/src/main/java/org/torproject/descriptor/log/WebServerAccessLogImpl.java @@ -7,9 +7,6 @@ import org.torproject.descriptor.DescriptorParseException; import org.torproject.descriptor.WebServerAccessLog; import org.torproject.descriptor.internal.FileType;

-import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.io.BufferedReader; import java.io.File; import java.io.InputStreamReader; @@ -35,9 +32,6 @@ public class WebServerAccessLogImpl extends LogDescriptorImpl

private static final long serialVersionUID = 7528914359452568309L;

- private static final Logger log - = LoggerFactory.getLogger(WebServerAccessLogImpl.class); - /** The log's name should include this string. */ public static final String MARKER = InternalWebServerAccessLog.MARKER;

diff --git a/src/main/java/org/torproject/descriptor/log/WebServerAccessLogLine.java b/src/main/java/org/torproject/descriptor/log/WebServerAccessLogLine.java index 445df9d..b39c633 100644 --- a/src/main/java/org/torproject/descriptor/log/WebServerAccessLogLine.java +++ b/src/main/java/org/torproject/descriptor/log/WebServerAccessLogLine.java @@ -25,7 +25,7 @@ public class WebServerAccessLogLine implements WebServerAccessLog.Line {

private static final long serialVersionUID = 6160416810587561460L;

- private static final Logger log = LoggerFactory + private static final Logger logger = LoggerFactory .getLogger(WebServerAccessLogLine.class);

private static final String DATE_PATTERN = "dd/MMM/yyyy"; @@ -153,7 +153,7 @@ public class WebServerAccessLogLine implements WebServerAccessLog.Line { res.valid = true; } } catch (Throwable th) { - log.debug("Unmatchable line: '{}'.", line, th); + logger.debug("Unmatchable line: '{}'.", line, th); return new WebServerAccessLogLine(); } return res;