opendev/gerrit
Code Issues Proposed changes

Fix various usages of the Flogger API

Browse Source 
- Don't use String.format; instead pass the string directly and let
  Flogger do the formatting. This means the formatting is only done
  when thte log level is enabled.

- Pass exception via withCause rather than as an unreferenced vararg.

- When explicitly passing an exception as vararg with the intention of
  not logging a stack trace, call getMessage() instead of just passing
  the exception object.

Change-Id: Icfe0712b5ebd1d05c630363853a311d734674d14
This commit is contained in:
David Pursehouse
2020-04-27 08:49:26 +09:00
parent c91487abbd
commit ec43a2f070
4 changed files with 8 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
View File

@@ -284,7 +284,7 @@ class OpenIdServiceImpl {
// right now. Instead of blocking all of them log the error and // right now. Instead of blocking all of them log the error and
// let the authentication complete anyway. // let the authentication complete anyway.
// //
logger.atSevere().log("Invalid PAPE response %s: %s", openidIdentifier, err); logger.atSevere().withCause(err).log("Invalid PAPE response from %s", openidIdentifier);
unsupported = true; unsupported = true;
ext = null; ext = null;
} }

3
java/com/google/gerrit/server/config/PluginConfigFactory.java
View File

@@ -230,7 +230,8 @@ public class PluginConfigFactory implements ReloadPluginListener {
cfg.load(); cfg.load();
} catch (ConfigInvalidException e) { } catch (ConfigInvalidException e) {
// This is an error in user input, don't spam logs with a stack trace. // This is an error in user input, don't spam logs with a stack trace.
logger.atWarning().log("Failed to load %s: %s", pluginConfigFile.toAbsolutePath(), e); logger.atWarning().log(
"Failed to load %s: %s", pluginConfigFile.toAbsolutePath(), e.getMessage());
} catch (IOException e) { } catch (IOException e) {
logger.atWarning().withCause(e).log("Failed to load %s", pluginConfigFile.toAbsolutePath()); logger.atWarning().withCause(e).log("Failed to load %s", pluginConfigFile.toAbsolutePath());
} }

8
java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
View File

@@ -158,10 +158,8 @@ public class CommentJsonMigrator {
return true; return true;
} }
} catch (IOException e) { } catch (IOException e) {
logger.atInfo().log( logger.atInfo().withCause(e).log(
String.format( "Error reading change %s in %s; attempting migration anyway", changeId, project);
"Error reading change %s in %s; attempting migration anyway", changeId, project),
e);
} }
try { try {
@@ -201,7 +199,7 @@ public class CommentJsonMigrator {
bru.addCommand(new ReceiveCommand(oldId, newId, ref.getName())); bru.addCommand(new ReceiveCommand(oldId, newId, ref.getName()));
return true; return true;
} catch (ConfigInvalidException | IOException e) { } catch (ConfigInvalidException | IOException e) {
logger.atInfo().log(String.format("Error migrating change %s in %s", changeId, project), e); logger.atInfo().withCause(e).log("Error migrating change %s in %s", changeId, project);
return false; return false;
} }
} }

4
java/com/google/gerrit/server/patch/Text.java
View File

@@ -157,11 +157,11 @@ public class Text extends RawText {
return Charset.forName(encoding); return Charset.forName(encoding);
} catch (IllegalCharsetNameException err) { } catch (IllegalCharsetNameException err) {
logger.atSevere().log("Invalid detected charset name '%s': %s", encoding, err); logger.atSevere().log("Invalid detected charset name '%s': %s", encoding, err.getMessage());
return ISO_8859_1; return ISO_8859_1;
} catch (UnsupportedCharsetException err) { } catch (UnsupportedCharsetException err) {
logger.atSevere().log("Detected charset '%s' not supported: %s", encoding, err); logger.atSevere().log("Detected charset '%s' not supported: %s", encoding, err.getMessage());
return ISO_8859_1; return ISO_8859_1;
} }
} }