From 03d2d209b181fa02868b445da75d2e6b7c08a7bf Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 30 Apr 2018 16:29:35 +0200 Subject: [PATCH] Migrate httpd classes to Flogger This is the first part of the migration to Flogger. This change migrates all classes of the 'http' module to Flogger. Other modules continue to use slf4j. They should be migrated by follow-up changes. During this migration we try to make the log statements more consistent: - avoid string concatenation - avoid usage of String.format(...) Change-Id: I473c41733b00aa1ceab92fe0dc8cd1c6b347174c Signed-off-by: Edwin Kempin --- java/com/google/gerrit/httpd/BUILD | 2 +- .../gerrit/httpd/DirectChangeByCommit.java | 8 +-- .../httpd/HttpServletResponseRecorder.java | 7 +-- .../gerrit/httpd/ProjectBasicAuthFilter.java | 29 ++++----- .../gerrit/httpd/ProjectOAuthFilter.java | 16 ++--- .../httpd/QueryDocumentationFilter.java | 7 +-- java/com/google/gerrit/httpd/RunAsFilter.java | 9 ++- .../gerrit/httpd/WebSessionManager.java | 12 ++-- .../auth/container/HttpLoginServlet.java | 34 +++++----- .../HttpsClientSslCertAuthFilter.java | 7 +-- .../httpd/auth/ldap/LdapLoginServlet.java | 11 ++-- java/com/google/gerrit/httpd/auth/oauth/BUILD | 2 +- .../gerrit/httpd/auth/oauth/OAuthSession.java | 63 +++++++++---------- .../com/google/gerrit/httpd/auth/openid/BUILD | 2 +- .../gerrit/httpd/auth/openid/LoginForm.java | 14 ++--- .../auth/openid/OAuthSessionOverOpenID.java | 63 +++++++++---------- .../httpd/auth/openid/OpenIdServiceImpl.java | 52 +++++++-------- .../gerrit/httpd/gitweb/GitwebServlet.java | 19 +++--- java/com/google/gerrit/httpd/init/BUILD | 2 +- .../gerrit/httpd/init/SiteInitializer.java | 11 ++-- .../gerrit/httpd/init/WebAppInitializer.java | 7 +-- .../httpd/plugins/HttpPluginServlet.java | 36 ++++++----- .../httpd/plugins/LfsPluginServlet.java | 11 ++-- .../httpd/plugins/PluginServletContext.java | 7 +-- .../google/gerrit/httpd/raw/BazelBuild.java | 11 ++-- .../gerrit/httpd/raw/HostPageServlet.java | 11 ++-- .../gerrit/httpd/raw/ResourceServlet.java | 11 ++-- .../google/gerrit/httpd/raw/StaticModule.java | 7 +-- .../gerrit/httpd/restapi/RestApiServlet.java | 8 +-- .../gerrit/httpd/rpc/GerritJsonServlet.java | 18 +++--- .../httpd/rpc/SystemInfoServiceImpl.java | 7 +-- .../httpd/template/SiteHeaderFooter.java | 9 ++- 32 files changed, 245 insertions(+), 268 deletions(-) diff --git a/java/com/google/gerrit/httpd/BUILD b/java/com/google/gerrit/httpd/BUILD index d4045dd4b9..4bd3e2e2aa 100644 --- a/java/com/google/gerrit/httpd/BUILD +++ b/java/com/google/gerrit/httpd/BUILD @@ -33,11 +33,11 @@ java_library( "//lib/auto:auto-value-annotations", "//lib/commons:codec", "//lib/commons:lang", + "//lib/flogger:api", "//lib/guice", "//lib/guice:guice-assistedinject", "//lib/guice:guice-servlet", "//lib/jgit/org.eclipse.jgit.http.server:jgit-servlet", "//lib/jgit/org.eclipse.jgit:jgit", - "//lib/log:api", ], ) diff --git a/java/com/google/gerrit/httpd/DirectChangeByCommit.java b/java/com/google/gerrit/httpd/DirectChangeByCommit.java index 26e41987ab..152a83d1a2 100644 --- a/java/com/google/gerrit/httpd/DirectChangeByCommit.java +++ b/java/com/google/gerrit/httpd/DirectChangeByCommit.java @@ -4,6 +4,7 @@ package com.google.gerrit.httpd; import com.google.common.base.CharMatcher; import com.google.common.collect.ImmutableList; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.PageLinks; import com.google.gerrit.extensions.api.changes.Changes; import com.google.gerrit.extensions.common.ChangeInfo; @@ -17,13 +18,12 @@ import java.util.List; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton class DirectChangeByCommit extends HttpServlet { private static final long serialVersionUID = 1L; - private static final Logger log = LoggerFactory.getLogger(DirectChangeByCommit.class); + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final Changes changes; @@ -39,7 +39,7 @@ class DirectChangeByCommit extends HttpServlet { try { results = changes.query(query).withLimit(2).get(); } catch (RestApiException e) { - log.warn("Cannot process query by URL: /r/" + query, e); + logger.atWarning().withCause(e).log("Cannot process query by URL: /r/%s", query); results = ImmutableList.of(); } String token; diff --git a/java/com/google/gerrit/httpd/HttpServletResponseRecorder.java b/java/com/google/gerrit/httpd/HttpServletResponseRecorder.java index 6774ec8030..397d093267 100644 --- a/java/com/google/gerrit/httpd/HttpServletResponseRecorder.java +++ b/java/com/google/gerrit/httpd/HttpServletResponseRecorder.java @@ -14,13 +14,12 @@ package com.google.gerrit.httpd; +import com.google.common.flogger.FluentLogger; import java.io.IOException; import java.util.HashMap; import java.util.Map; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * HttpServletResponse wrapper to allow response status code override. @@ -29,7 +28,7 @@ import org.slf4j.LoggerFactory; * override the response http status code. */ public class HttpServletResponseRecorder extends HttpServletResponseWrapper { - private static final Logger log = LoggerFactory.getLogger(HttpServletResponseRecorder.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String LOCATION_HEADER = "Location"; private int status; @@ -78,7 +77,7 @@ public class HttpServletResponseRecorder extends HttpServletResponseWrapper { void play() throws IOException { if (status != 0) { - log.debug("Replaying {} {}", status, statusMsg); + logger.atFine().log("Replaying %s %s", status, statusMsg); if (status == SC_MOVED_TEMPORARILY) { super.sendRedirect(headers.get(LOCATION_HEADER)); diff --git a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java index 6174644013..e7b35ec9f7 100644 --- a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java +++ b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java @@ -19,6 +19,7 @@ import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.client.GitBasicAuthPolicy; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.reviewdb.client.Account; @@ -47,8 +48,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; import org.apache.commons.codec.binary.Base64; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Authenticates the current user by HTTP basic authentication. @@ -62,7 +61,7 @@ import org.slf4j.LoggerFactory; */ @Singleton class ProjectBasicAuthFilter implements Filter { - private static final Logger log = LoggerFactory.getLogger(ProjectBasicAuthFilter.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); public static final String REALM_NAME = "Gerrit Code Review"; private static final String AUTHORIZATION = "Authorization"; @@ -131,10 +130,11 @@ class ProjectBasicAuthFilter implements Filter { Optional accountState = accountCache.getByUsername(username).filter(a -> a.getAccount().isActive()); if (!accountState.isPresent()) { - log.warn( - "Authentication failed for " - + username - + ": account inactive or not provisioned in Gerrit"); + logger + .atWarning() + .log( + "Authentication failed for %s: account inactive or not provisioned in Gerrit", + username); rsp.sendError(SC_UNAUTHORIZED); return false; } @@ -163,17 +163,17 @@ class ProjectBasicAuthFilter implements Filter { if (who.checkPassword(password, username)) { return succeedAuthentication(who); } - log.warn(authenticationFailedMsg(username, req), e); + logger.atWarning().withCause(e).log(authenticationFailedMsg(username, req)); rsp.sendError(SC_UNAUTHORIZED); return false; } catch (AuthenticationFailedException e) { // This exception is thrown if the user provided wrong credentials, we don't need to log a // stacktrace for it. - log.warn(authenticationFailedMsg(username, req) + ": " + e.getMessage()); + logger.atWarning().log(authenticationFailedMsg(username, req) + ": %s", e.getMessage()); rsp.sendError(SC_UNAUTHORIZED); return false; } catch (AccountException e) { - log.warn(authenticationFailedMsg(username, req), e); + logger.atWarning().withCause(e).log(authenticationFailedMsg(username, req)); rsp.sendError(SC_UNAUTHORIZED); return false; } @@ -186,10 +186,11 @@ class ProjectBasicAuthFilter implements Filter { private boolean failAuthentication(Response rsp, String username, HttpServletRequest req) throws IOException { - log.warn( - authenticationFailedMsg(username, req) - + ": password does not match the one stored in Gerrit", - username); + logger + .atWarning() + .log( + authenticationFailedMsg(username, req) + + ": password does not match the one stored in Gerrit"); rsp.sendError(SC_UNAUTHORIZED); return false; } diff --git a/java/com/google/gerrit/httpd/ProjectOAuthFilter.java b/java/com/google/gerrit/httpd/ProjectOAuthFilter.java index 2b37378e08..c97c840835 100644 --- a/java/com/google/gerrit/httpd/ProjectOAuthFilter.java +++ b/java/com/google/gerrit/httpd/ProjectOAuthFilter.java @@ -21,6 +21,7 @@ import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.collect.Iterables; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.auth.oauth.OAuthLoginProvider; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.extensions.registration.DynamicMap; @@ -54,8 +55,6 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; import org.apache.commons.codec.binary.Base64; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Authenticates the current user with an OAuth2 server. @@ -64,8 +63,7 @@ import org.slf4j.LoggerFactory; */ @Singleton class ProjectOAuthFilter implements Filter { - - private static final Logger log = LoggerFactory.getLogger(ProjectOAuthFilter.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String REALM_NAME = "Gerrit Code Review"; private static final String AUTHORIZATION = "Authorization"; @@ -156,9 +154,11 @@ class ProjectOAuthFilter implements Filter { Optional who = accountCache.getByUsername(authInfo.username).filter(a -> a.getAccount().isActive()); if (!who.isPresent()) { - log.warn( - authenticationFailedMsg(authInfo.username, req) - + ": account inactive or not provisioned in Gerrit"); + logger + .atWarning() + .log( + authenticationFailedMsg(authInfo.username, req) + + ": account inactive or not provisioned in Gerrit"); rsp.sendError(SC_UNAUTHORIZED); return false; } @@ -179,7 +179,7 @@ class ProjectOAuthFilter implements Filter { ws.setAccessPathOk(AccessPath.REST_API, true); return true; } catch (AccountException e) { - log.warn(authenticationFailedMsg(authInfo.username, req), e); + logger.atWarning().withCause(e).log(authenticationFailedMsg(authInfo.username, req)); rsp.sendError(SC_UNAUTHORIZED); return false; } diff --git a/java/com/google/gerrit/httpd/QueryDocumentationFilter.java b/java/com/google/gerrit/httpd/QueryDocumentationFilter.java index 7a89b3bba6..8b82c001ef 100644 --- a/java/com/google/gerrit/httpd/QueryDocumentationFilter.java +++ b/java/com/google/gerrit/httpd/QueryDocumentationFilter.java @@ -16,6 +16,7 @@ package com.google.gerrit.httpd; import com.google.common.base.Strings; import com.google.common.collect.ImmutableListMultimap; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.httpd.restapi.RestApiServlet; import com.google.gerrit.server.documentation.QueryDocumentationExecutor; import com.google.gerrit.server.documentation.QueryDocumentationExecutor.DocQueryException; @@ -32,12 +33,10 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton public class QueryDocumentationFilter implements Filter { - private final Logger log = LoggerFactory.getLogger(QueryDocumentationFilter.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final QueryDocumentationExecutor searcher; @@ -62,7 +61,7 @@ public class QueryDocumentationFilter implements Filter { List result = searcher.doQuery(request.getParameter("q")); RestApiServlet.replyJson(req, rsp, ImmutableListMultimap.of(), result); } catch (DocQueryException e) { - log.error("Doc search failed:", e); + logger.atSevere().withCause(e).log("Doc search failed"); rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } } else { diff --git a/java/com/google/gerrit/httpd/RunAsFilter.java b/java/com/google/gerrit/httpd/RunAsFilter.java index 9940cd9cd4..f3bf5af0a2 100644 --- a/java/com/google/gerrit/httpd/RunAsFilter.java +++ b/java/com/google/gerrit/httpd/RunAsFilter.java @@ -18,6 +18,7 @@ import static com.google.gerrit.httpd.restapi.RestApiServlet.replyError; import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.reviewdb.client.Account; @@ -41,13 +42,11 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** Allows running a request as another user account. */ @Singleton class RunAsFilter implements Filter { - private static final Logger log = LoggerFactory.getLogger(RunAsFilter.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String RUN_AS = "X-Gerrit-RunAs"; static class Module extends ServletModule { @@ -99,7 +98,7 @@ class RunAsFilter implements Filter { replyError(req, res, SC_FORBIDDEN, "not permitted to use " + RUN_AS, null); return; } catch (PermissionBackendException e) { - log.warn("cannot check runAs", e); + logger.atWarning().withCause(e).log("cannot check runAs"); replyError(req, res, SC_INTERNAL_SERVER_ERROR, RUN_AS + " unavailable", null); return; } @@ -108,7 +107,7 @@ class RunAsFilter implements Filter { try { target = accountResolver.find(runas); } catch (OrmException | IOException | ConfigInvalidException e) { - log.warn("cannot resolve account for " + RUN_AS, e); + logger.atWarning().withCause(e).log("cannot resolve account for %s", RUN_AS); replyError(req, res, SC_INTERNAL_SERVER_ERROR, "cannot resolve " + RUN_AS, e); return; } diff --git a/java/com/google/gerrit/httpd/WebSessionManager.java b/java/com/google/gerrit/httpd/WebSessionManager.java index 8b6694c105..3d2668b339 100644 --- a/java/com/google/gerrit/httpd/WebSessionManager.java +++ b/java/com/google/gerrit/httpd/WebSessionManager.java @@ -29,6 +29,7 @@ import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; import com.google.common.cache.Cache; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.config.ConfigUtil; @@ -43,11 +44,9 @@ import java.io.Serializable; import java.security.SecureRandom; import java.util.concurrent.TimeUnit; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class WebSessionManager { - private static final Logger log = LoggerFactory.getLogger(WebSessionManager.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); public static final String CACHE_NAME = "web_sessions"; private final long sessionMaxAgeMillis; @@ -69,10 +68,11 @@ public class WebSessionManager { SECONDS.convert(MAX_AGE_MINUTES, MINUTES), SECONDS)); if (sessionMaxAgeMillis < MINUTES.toMillis(5)) { - log.warn( - String.format( + logger + .atWarning() + .log( "cache.%s.maxAge is set to %d milliseconds; it should be at least 5 minutes.", - CACHE_NAME, sessionMaxAgeMillis)); + CACHE_NAME, sessionMaxAgeMillis); } } diff --git a/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java b/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java index d86c85aa17..515d694165 100644 --- a/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java +++ b/java/com/google/gerrit/httpd/auth/container/HttpLoginServlet.java @@ -17,6 +17,7 @@ package com.google.gerrit.httpd.auth.container; import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_EXTERNAL; import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.PageLinks; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.httpd.CanonicalWebUrl; @@ -40,8 +41,6 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -56,7 +55,7 @@ import org.w3c.dom.NodeList; @Singleton class HttpLoginServlet extends HttpServlet { private static final long serialVersionUID = 1L; - private static final Logger log = LoggerFactory.getLogger(HttpLoginServlet.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final DynamicItem webSession; private final CanonicalWebUrl urlProvider; @@ -86,10 +85,12 @@ class HttpLoginServlet extends HttpServlet { CacheHeaders.setNotCacheable(rsp); final String user = authFilter.getRemoteUser(req); if (user == null || "".equals(user)) { - log.error( - "Unable to authenticate user by " - + authFilter.getLoginHeader() - + " request header. Check container or server configuration."); + logger + .atSevere() + .log( + "Unable to authenticate user by %s request header." + + " Check container or server configuration.", + authFilter.getLoginHeader()); final Document doc = HtmlDomUtil.parseFile( // @@ -118,7 +119,7 @@ class HttpLoginServlet extends HttpServlet { try { arsp = accountManager.authenticate(areq); } catch (AccountException e) { - log.error("Unable to authenticate user \"" + user + "\"", e); + logger.atSevere().withCause(e).log("Unable to authenticate user \"%s\"", user); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return; } @@ -126,16 +127,17 @@ class HttpLoginServlet extends HttpServlet { String remoteExternalId = authFilter.getRemoteExternalIdToken(req); if (remoteExternalId != null) { try { - log.debug("Associating external identity \"{}\" to user \"{}\"", remoteExternalId, user); + logger + .atFine() + .log("Associating external identity \"%s\" to user \"%s\"", remoteExternalId, user); updateRemoteExternalId(arsp, remoteExternalId); } catch (AccountException | OrmException | ConfigInvalidException e) { - log.error( - "Unable to associate external identity \"" - + remoteExternalId - + "\" to user \"" - + user - + "\"", - e); + logger + .atSevere() + .withCause(e) + .log( + "Unable to associate external identity \"%s\" to user \"%s\"", + remoteExternalId, user); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return; } diff --git a/java/com/google/gerrit/httpd/auth/container/HttpsClientSslCertAuthFilter.java b/java/com/google/gerrit/httpd/auth/container/HttpsClientSslCertAuthFilter.java index 534e50ec12..40807c05ab 100644 --- a/java/com/google/gerrit/httpd/auth/container/HttpsClientSslCertAuthFilter.java +++ b/java/com/google/gerrit/httpd/auth/container/HttpsClientSslCertAuthFilter.java @@ -14,6 +14,7 @@ package com.google.gerrit.httpd.auth.container; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.httpd.WebSession; import com.google.gerrit.server.account.AccountException; @@ -32,14 +33,12 @@ import javax.servlet.FilterConfig; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton class HttpsClientSslCertAuthFilter implements Filter { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final Pattern REGEX_USERID = Pattern.compile("CN=([^,]*)"); - private static final Logger log = LoggerFactory.getLogger(HttpsClientSslCertAuthFilter.class); private final DynamicItem webSession; private final AccountManager accountManager; @@ -77,7 +76,7 @@ class HttpsClientSslCertAuthFilter implements Filter { arsp = accountManager.authenticate(areq); } catch (AccountException e) { String err = "Unable to authenticate user \"" + userName + "\""; - log.error(err, e); + logger.atSevere().withCause(e).log(err); throw new ServletException(err, e); } webSession.get().login(arsp, true); diff --git a/java/com/google/gerrit/httpd/auth/ldap/LdapLoginServlet.java b/java/com/google/gerrit/httpd/auth/ldap/LdapLoginServlet.java index 46714756fb..63704764c9 100644 --- a/java/com/google/gerrit/httpd/auth/ldap/LdapLoginServlet.java +++ b/java/com/google/gerrit/httpd/auth/ldap/LdapLoginServlet.java @@ -18,6 +18,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.httpd.CanonicalWebUrl; @@ -41,8 +42,6 @@ import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -50,7 +49,7 @@ import org.w3c.dom.Element; @SuppressWarnings("serial") @Singleton class LdapLoginServlet extends HttpServlet { - private static final Logger log = LoggerFactory.getLogger(LdapLoginServlet.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final AccountManager accountManager; private final DynamicItem webSession; @@ -130,15 +129,15 @@ class LdapLoginServlet extends HttpServlet { } catch (AuthenticationFailedException e) { // This exception is thrown if the user provided wrong credentials, we don't need to log a // stacktrace for it. - log.warn("'{}' failed to sign in: {}", username, e.getMessage()); + logger.atWarning().log("'%s' failed to sign in: %s", username, e.getMessage()); sendForm(req, res, "Invalid username or password."); return; } catch (AccountException e) { - log.warn("'{}' failed to sign in", username, e); + logger.atWarning().withCause(e).log("'%s' failed to sign in", username); sendForm(req, res, "Authentication failed."); return; } catch (RuntimeException e) { - log.error("LDAP authentication failed", e); + logger.atSevere().withCause(e).log("LDAP authentication failed"); sendForm(req, res, "Authentication unavailable at this time."); return; } diff --git a/java/com/google/gerrit/httpd/auth/oauth/BUILD b/java/com/google/gerrit/httpd/auth/oauth/BUILD index aa63f0d419..96726ad0db 100644 --- a/java/com/google/gerrit/httpd/auth/oauth/BUILD +++ b/java/com/google/gerrit/httpd/auth/oauth/BUILD @@ -15,9 +15,9 @@ java_library( "//lib:gwtorm", "//lib:servlet-api-3_1", "//lib/commons:codec", + "//lib/flogger:api", "//lib/guice", "//lib/guice:guice-servlet", "//lib/jgit/org.eclipse.jgit:jgit", - "//lib/log:api", ], ) diff --git a/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java b/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java index 68b28a9d9d..866aaa0701 100644 --- a/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java +++ b/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java @@ -18,6 +18,7 @@ import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; import com.google.common.base.CharMatcher; import com.google.common.base.Strings; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.auth.oauth.OAuthServiceProvider; import com.google.gerrit.extensions.auth.oauth.OAuthToken; import com.google.gerrit.extensions.auth.oauth.OAuthUserInfo; @@ -47,13 +48,12 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.commons.codec.binary.Base64; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @SessionScoped /* OAuth protocol implementation */ class OAuthSession { - private static final Logger log = LoggerFactory.getLogger(OAuthSession.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final SecureRandom randomState = newRandomGenerator(); private final String state; private final DynamicItem webSession; @@ -93,7 +93,7 @@ class OAuthSession { boolean login( HttpServletRequest request, HttpServletResponse response, OAuthServiceProvider oauth) throws IOException { - log.debug("Login " + this); + logger.atFine().log("Login %s", this); if (isOAuthFinal(request)) { if (!checkState(request)) { @@ -101,19 +101,19 @@ class OAuthSession { return false; } - log.debug("Login-Retrieve-User " + this); + logger.atFine().log("Login-Retrieve-User %s", this); OAuthToken token = oauth.getAccessToken(new OAuthVerifier(request.getParameter("code"))); user = oauth.getUserInfo(token); if (isLoggedIn()) { - log.debug("Login-SUCCESS " + this); + logger.atFine().log("Login-SUCCESS %s", this); authenticateAndRedirect(request, response, token); return true; } response.sendError(SC_UNAUTHORIZED); return false; } - log.debug("Login-PHASE1 " + this); + logger.atFine().log("Login-PHASE1 %s", this); redirectToken = request.getRequestURI(); // We are here in content of filter. // Due to this Jetty limitation: @@ -148,7 +148,7 @@ class OAuthSession { accountId = arsp.getAccountId(); tokenCache.put(accountId, token); } catch (AccountException e) { - log.error("Unable to authenticate user \"" + user + "\"", e); + logger.atSevere().withCause(e).log("Unable to authenticate user \"%s\"", user); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return; } @@ -169,40 +169,33 @@ class OAuthSession { if (claimedId.isPresent() && actualId.isPresent()) { if (claimedId.get().equals(actualId.get())) { // Both link to the same account, that's what we expected. - log.debug("OAuth2: claimed identity equals current id"); + logger.atFine().log("OAuth2: claimed identity equals current id"); } else { // This is (for now) a fatal error. There are two records // for what might be the same user. // - log.error( - "OAuth accounts disagree over user identity:\n" - + " Claimed ID: " - + claimedId.get() - + " is " - + claimedIdentifier - + "\n" - + " Delgate ID: " - + actualId.get() - + " is " - + user.getExternalId()); + logger + .atSevere() + .log( + "OAuth accounts disagree over user identity:\n" + + " Claimed ID: %s is %s\n" + + " Delgate ID: %s is %s", + claimedId.get(), claimedIdentifier, actualId.get(), user.getExternalId()); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return false; } } else if (claimedId.isPresent() && !actualId.isPresent()) { // Claimed account already exists: link to it. // - log.info("OAuth2: linking claimed identity to {}", claimedId.get().toString()); + logger.atInfo().log("OAuth2: linking claimed identity to %s", claimedId.get().toString()); try { accountManager.link(claimedId.get(), req); } catch (OrmException | ConfigInvalidException e) { - log.error( - "Cannot link: " - + user.getExternalId() - + " to user identity:\n" - + " Claimed ID: " - + claimedId.get() - + " is " - + claimedIdentifier); + logger + .atSevere() + .log( + "Cannot link: %s to user identity:\n Claimed ID: %s is %s", + user.getExternalId(), claimedId.get(), claimedIdentifier); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return false; } @@ -215,11 +208,11 @@ class OAuthSession { try { accountManager.link(identifiedUser.get().getAccountId(), areq); } catch (OrmException | ConfigInvalidException e) { - log.error( - "Cannot link: " - + user.getExternalId() - + " to user identity: " - + identifiedUser.get().getAccountId()); + logger + .atSevere() + .log( + "Cannot link: %s to user identity: %s", + user.getExternalId(), identifiedUser.get().getAccountId()); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return false; } finally { @@ -241,7 +234,7 @@ class OAuthSession { private boolean checkState(ServletRequest request) { String s = Strings.nullToEmpty(request.getParameter("state")); if (!s.equals(state)) { - log.error("Illegal request state '" + s + "' on OAuthProtocol " + this); + logger.atSevere().log("Illegal request state '%s' on OAuthProtocol %s", s, this); return false; } return true; diff --git a/java/com/google/gerrit/httpd/auth/openid/BUILD b/java/com/google/gerrit/httpd/auth/openid/BUILD index 44b7bd1ff5..9c48832fea 100644 --- a/java/com/google/gerrit/httpd/auth/openid/BUILD +++ b/java/com/google/gerrit/httpd/auth/openid/BUILD @@ -17,10 +17,10 @@ java_library( "//lib:gwtorm", "//lib:servlet-api-3_1", "//lib/commons:codec", + "//lib/flogger:api", "//lib/guice", "//lib/guice:guice-servlet", "//lib/jgit/org.eclipse.jgit:jgit", - "//lib/log:api", "//lib/openid:consumer", ], ) diff --git a/java/com/google/gerrit/httpd/auth/openid/LoginForm.java b/java/com/google/gerrit/httpd/auth/openid/LoginForm.java index 6090fed767..adf6458751 100644 --- a/java/com/google/gerrit/httpd/auth/openid/LoginForm.java +++ b/java/com/google/gerrit/httpd/auth/openid/LoginForm.java @@ -20,6 +20,7 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.auth.openid.OpenIdUrls; @@ -47,8 +48,6 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -56,7 +55,8 @@ import org.w3c.dom.Element; @SuppressWarnings("serial") @Singleton class LoginForm extends HttpServlet { - private static final Logger log = LoggerFactory.getLogger(LoginForm.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final ImmutableMap ALL_PROVIDERS = ImmutableMap.of( "launchpad", OpenIdUrls.URL_LAUNCHPAD, @@ -91,7 +91,7 @@ class LoginForm extends HttpServlet { this.oauthServiceProviders = oauthServiceProviders; if (urlProvider == null || Strings.isNullOrEmpty(urlProvider.get())) { - log.error("gerrit.canonicalWebUrl must be set in gerrit.config"); + logger.atSevere().log("gerrit.canonicalWebUrl must be set in gerrit.config"); } if (authConfig.getAuthType() == AuthType.OPENID_SSO) { @@ -160,14 +160,14 @@ class LoginForm extends HttpServlet { mode = SignInMode.SIGN_IN; } - log.debug("mode \"{}\"", mode); + logger.atFine().log("mode \"%s\"", mode); OAuthServiceProvider oauthProvider = lookupOAuthServiceProvider(id); if (oauthProvider == null) { - log.debug("OpenId provider \"{}\"", id); + logger.atFine().log("OpenId provider \"%s\"", id); discover(req, res, link, id, remember, token, mode); } else { - log.debug("OAuth provider \"{}\"", id); + logger.atFine().log("OAuth provider \"%s\"", id); OAuthSessionOverOpenID oauthSession = oauthSessionProvider.get(); if (!currentUserProvider.get().isIdentifiedUser() && oauthSession.isLoggedIn()) { oauthSession.logout(); diff --git a/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java b/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java index 878f9eeef0..1cc43becb8 100644 --- a/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java +++ b/java/com/google/gerrit/httpd/auth/openid/OAuthSessionOverOpenID.java @@ -17,6 +17,7 @@ package com.google.gerrit.httpd.auth.openid; import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; import com.google.common.base.Strings; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.auth.oauth.OAuthServiceProvider; import com.google.gerrit.extensions.auth.oauth.OAuthToken; import com.google.gerrit.extensions.auth.oauth.OAuthUserInfo; @@ -45,14 +46,13 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.commons.codec.binary.Base64; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** OAuth protocol implementation */ @SessionScoped class OAuthSessionOverOpenID { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + static final String GERRIT_LOGIN = "/login"; - private static final Logger log = LoggerFactory.getLogger(OAuthSessionOverOpenID.class); private static final SecureRandom randomState = newRandomGenerator(); private final String state; private final DynamicItem webSession; @@ -89,7 +89,7 @@ class OAuthSessionOverOpenID { boolean login( HttpServletRequest request, HttpServletResponse response, OAuthServiceProvider oauth) throws IOException { - log.debug("Login " + this); + logger.atFine().log("Login %s", this); if (isOAuthFinal(request)) { if (!checkState(request)) { @@ -97,19 +97,19 @@ class OAuthSessionOverOpenID { return false; } - log.debug("Login-Retrieve-User " + this); + logger.atFine().log("Login-Retrieve-User %s", this); token = oauth.getAccessToken(new OAuthVerifier(request.getParameter("code"))); user = oauth.getUserInfo(token); if (isLoggedIn()) { - log.debug("Login-SUCCESS " + this); + logger.atFine().log("Login-SUCCESS %s", this); authenticateAndRedirect(request, response); return true; } response.sendError(SC_UNAUTHORIZED); return false; } - log.debug("Login-PHASE1 " + this); + logger.atFine().log("Login-PHASE1 %s", this); redirectToken = LoginUrlToken.getToken(request); response.sendRedirect(oauth.getAuthorizationUrl() + "&state=" + state); return false; @@ -135,50 +135,43 @@ class OAuthSessionOverOpenID { if (!Strings.isNullOrEmpty(claimedIdentifier)) { claimedId = accountManager.lookup(claimedIdentifier); if (!claimedId.isPresent()) { - log.debug("Claimed identity is unknown"); + logger.atFine().log("Claimed identity is unknown"); } } // Use case 1: claimed identity was provided during handshake phase // and user account exists for this identity if (claimedId.isPresent()) { - log.debug("Claimed identity is set and is known"); + logger.atFine().log("Claimed identity is set and is known"); if (actualId.isPresent()) { if (claimedId.get().equals(actualId.get())) { // Both link to the same account, that's what we expected. - log.debug("Both link to the same account. All is fine."); + logger.atFine().log("Both link to the same account. All is fine."); } else { // This is (for now) a fatal error. There are two records // for what might be the same user. The admin would have to // link the accounts manually. - log.error( - "OAuth accounts disagree over user identity:\n" - + " Claimed ID: " - + claimedId.get() - + " is " - + claimedIdentifier - + "\n" - + " Delgate ID: " - + actualId.get() - + " is " - + user.getExternalId()); + logger + .atFine() + .log( + "OAuth accounts disagree over user identity:\n" + + " Claimed ID: %s is %s\n" + + " Delgate ID: %s is %s", + claimedId.get(), claimedIdentifier, actualId.get(), user.getExternalId()); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return; } } else { // Claimed account already exists: link to it. - log.debug("Claimed account already exists: link to it."); + logger.atFine().log("Claimed account already exists: link to it."); try { accountManager.link(claimedId.get(), areq); } catch (OrmException | ConfigInvalidException e) { - log.error( - "Cannot link: " - + user.getExternalId() - + " to user identity:\n" - + " Claimed ID: " - + claimedId.get() - + " is " - + claimedIdentifier); + logger + .atSevere() + .log( + "Cannot link: %s to user identity:\n Claimed ID: %s is %s", + user.getExternalId(), claimedId.get(), claimedIdentifier); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return; } @@ -187,10 +180,12 @@ class OAuthSessionOverOpenID { // Use case 2: link mode activated from the UI Account.Id accountId = identifiedUser.get().getAccountId(); try { - log.debug("Linking \"{}\" to \"{}\"", user.getExternalId(), accountId); + logger.atFine().log("Linking \"%s\" to \"%s\"", user.getExternalId(), accountId); accountManager.link(accountId, areq); } catch (OrmException | ConfigInvalidException e) { - log.error("Cannot link: " + user.getExternalId() + " to user identity: " + accountId); + logger + .atSevere() + .log("Cannot link: %s to user identity: %s", user.getExternalId(), accountId); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return; } finally { @@ -202,7 +197,7 @@ class OAuthSessionOverOpenID { areq.setDisplayName(user.getDisplayName()); arsp = accountManager.authenticate(areq); } catch (AccountException e) { - log.error("Unable to authenticate user \"" + user + "\"", e); + logger.atSevere().withCause(e).log("Unable to authenticate user \"%s\"", user); rsp.sendError(HttpServletResponse.SC_FORBIDDEN); return; } @@ -223,7 +218,7 @@ class OAuthSessionOverOpenID { private boolean checkState(ServletRequest request) { String s = Strings.nullToEmpty(request.getParameter("state")); if (!s.equals(state)) { - log.error("Illegal request state '" + s + "' on OAuthProtocol " + this); + logger.atSevere().log("Illegal request state '%s' on OAuthProtocol %s", s, this); return false; } return true; diff --git a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java index a971fc30d3..4664e3b37a 100644 --- a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java +++ b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java @@ -14,6 +14,7 @@ package com.google.gerrit.httpd.auth.openid; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.auth.openid.OpenIdUrls; import com.google.gerrit.extensions.registration.DynamicItem; @@ -64,12 +65,10 @@ import org.openid4java.message.sreg.SRegMessage; import org.openid4java.message.sreg.SRegRequest; import org.openid4java.message.sreg.SRegResponse; import org.openid4java.util.HttpClientFactory; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton class OpenIdServiceImpl { - private static final Logger log = LoggerFactory.getLogger(OpenIdServiceImpl.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); static final String RETURN_URL = "OpenID"; @@ -151,7 +150,7 @@ class OpenIdServiceImpl { final AuthRequest aReq; try { aReq = manager.authenticate(state.discovered, state.retTo.toString()); - log.debug("OpenID: openid-realm={}", state.contextUrl); + logger.atFine().log("OpenID: openid-realm=%s", state.contextUrl); aReq.setRealm(state.contextUrl); if (requestRegistration(aReq)) { @@ -173,7 +172,7 @@ class OpenIdServiceImpl { aReq.addExtension(pape); } } catch (MessageException | ConsumerException e) { - log.error("Cannot create OpenID redirect for " + openidIdentifier, e); + logger.atSevere().withCause(e).log("Cannot create OpenID redirect for %s" + openidIdentifier); return new DiscoveryResult(DiscoveryResult.Status.ERROR); } @@ -195,7 +194,7 @@ class OpenIdServiceImpl { try { return accountManager.lookup(aReq.getIdentity()) == null; } catch (AccountException e) { - log.warn("Cannot determine if user account exists", e); + logger.atWarning().withCause(e).log("Cannot determine if user account exists"); return true; } } @@ -250,17 +249,18 @@ class OpenIdServiceImpl { if ("Nonce verification failed.".equals(result.getStatusMsg())) { // We might be suffering from clock skew on this system. // - log.error( - "OpenID failure: " - + result.getStatusMsg() - + " Likely caused by clock skew on this server," - + " install/configure NTP."); + logger + .atSevere() + .log( + "OpenID failure: %s Likely caused by clock skew on this server," + + " install/configure NTP.", + result.getStatusMsg()); cancelWithError(req, rsp, result.getStatusMsg()); } else if (result.getStatusMsg() != null) { // Authentication failed. // - log.error("OpenID failure: " + result.getStatusMsg()); + logger.atSevere().log("OpenID failure: %s", result.getStatusMsg()); cancelWithError(req, rsp, result.getStatusMsg()); } else { @@ -286,12 +286,12 @@ class OpenIdServiceImpl { // right now. Instead of blocking all of them log the error and // let the authentication complete anyway. // - log.error("Invalid PAPE response " + openidIdentifier + ": " + err); + logger.atSevere().log("Invalid PAPE response %s: %s", openidIdentifier, err); unsupported = true; ext = null; } if (!unsupported && ext == null) { - log.error("No PAPE extension response from " + openidIdentifier); + logger.atSevere().log("No PAPE extension response from %s", openidIdentifier); cancelWithError(req, rsp, "OpenID provider does not support PAPE."); return; } @@ -354,7 +354,7 @@ class OpenIdServiceImpl { } if (!match) { - log.error("Domain disallowed: " + emailDomain); + logger.atSevere().log("Domain disallowed: %s", emailDomain); cancelWithError(req, rsp, "Domain disallowed"); return; } @@ -376,17 +376,13 @@ class OpenIdServiceImpl { // This is (for now) a fatal error. There are two records // for what might be the same user. // - log.error( - "OpenID accounts disagree over user identity:\n" - + " Claimed ID: " - + claimedId.get() - + " is " - + claimedIdentifier - + "\n" - + " Delgate ID: " - + actualId.get() - + " is " - + areq.getExternalIdKey()); + logger + .atSevere() + .log( + "OpenID accounts disagree over user identity:\n" + + " Claimed ID: %s is %s\n" + + " Delgate ID: %s is %s", + claimedId.get(), claimedIdentifier, actualId.get(), areq.getExternalIdKey()); cancelWithError(req, rsp, "Contact site administrator"); return; } @@ -451,7 +447,7 @@ class OpenIdServiceImpl { } } } catch (AccountException e) { - log.error("OpenID authentication failure", e); + logger.atSevere().withCause(e).log("OpenID authentication failure"); cancelWithError(req, rsp, "Contact site administrator"); } } @@ -531,7 +527,7 @@ class OpenIdServiceImpl { try { list = manager.discover(openidIdentifier); } catch (DiscoveryException e) { - log.error("Cannot discover OpenID " + openidIdentifier, e); + logger.atSevere().withCause(e).log("Cannot discover OpenID %s", openidIdentifier); return null; } if (list == null || list.isEmpty()) { diff --git a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java index cc22d245f4..383efd34ca 100644 --- a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java +++ b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java @@ -34,6 +34,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.CharMatcher; import com.google.common.base.Splitter; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.PageLinks; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; @@ -85,14 +86,12 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Repository; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** Invokes {@code gitweb.cgi} for the project given in {@code p}. */ @SuppressWarnings("serial") @Singleton class GitwebServlet extends HttpServlet { - private static final Logger log = LoggerFactory.getLogger(GitwebServlet.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String PROJECT_LIST_ACTION = "project_list"; @@ -137,7 +136,7 @@ class GitwebServlet extends HttpServlet { try { uri = new URI(url); } catch (URISyntaxException e) { - log.error("Invalid gitweb.url: " + url); + logger.atSevere().log("Invalid gitweb.url: %s", url); } gitwebUrl = uri; } else { @@ -428,7 +427,7 @@ class GitwebServlet extends HttpServlet { sendErrorOrRedirect(req, rsp, HttpServletResponse.SC_NOT_FOUND); return; } catch (IOException | PermissionBackendException err) { - log.error("cannot load " + name, err); + logger.atSevere().withCause(err).log("cannot load %s", name); rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); return; } catch (ResourceConflictException e) { @@ -528,13 +527,13 @@ class GitwebServlet extends HttpServlet { final int status = proc.exitValue(); if (0 != status) { - log.error("Non-zero exit status (" + status + ") from " + gitwebCgi); + logger.atSevere().log("Non-zero exit status (%d) from %s", status, gitwebCgi); if (!rsp.isCommitted()) { rsp.sendError(500); } } } catch (InterruptedException ie) { - log.debug("CGI: interrupted waiting for CGI to terminate"); + logger.atFine().log("CGI: interrupted waiting for CGI to terminate"); } } @@ -659,7 +658,7 @@ class GitwebServlet extends HttpServlet { dst.close(); } } catch (IOException e) { - log.error("Unexpected error copying input to CGI", e); + logger.atSevere().withCause(e).log("Unexpected error copying input to CGI"); } }, "Gitweb-InputFeeder") @@ -679,9 +678,9 @@ class GitwebServlet extends HttpServlet { } b.append("CGI: ").append(line); } - log.error(b.toString()); + logger.atSevere().log(b.toString()); } catch (IOException e) { - log.error("Unexpected error copying stderr from CGI", e); + logger.atSevere().withCause(e).log("Unexpected error copying stderr from CGI"); } }, "Gitweb-ErrorLogger") diff --git a/java/com/google/gerrit/httpd/init/BUILD b/java/com/google/gerrit/httpd/init/BUILD index f240088bcd..292ceff585 100644 --- a/java/com/google/gerrit/httpd/init/BUILD +++ b/java/com/google/gerrit/httpd/init/BUILD @@ -27,10 +27,10 @@ java_library( "//lib:guava", "//lib:gwtorm", "//lib:servlet-api-3_1", + "//lib/flogger:api", "//lib/guice", "//lib/guice:guice-servlet", "//lib/jgit/org.eclipse.jgit:jgit", - "//lib/log:api", "//prolog:gerrit-prolog-common", ], ) diff --git a/java/com/google/gerrit/httpd/init/SiteInitializer.java b/java/com/google/gerrit/httpd/init/SiteInitializer.java index 17a95b57ec..de4f2848e1 100644 --- a/java/com/google/gerrit/httpd/init/SiteInitializer.java +++ b/java/com/google/gerrit/httpd/init/SiteInitializer.java @@ -14,6 +14,7 @@ package com.google.gerrit.httpd.init; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.pgm.init.BaseInit; import com.google.gerrit.pgm.init.PluginsDistribution; import java.nio.file.Path; @@ -23,11 +24,9 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public final class SiteInitializer { - private static final Logger LOG = LoggerFactory.getLogger(SiteInitializer.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final String sitePath; private final String initPath; @@ -49,7 +48,7 @@ public final class SiteInitializer { try { if (sitePath != null) { Path site = Paths.get(sitePath); - LOG.info("Initializing site at " + site.toRealPath().normalize()); + logger.atInfo().log("Initializing site at %s", site.toRealPath().normalize()); new BaseInit(site, false, true, pluginsDistribution, pluginsToInstall).run(); return; } @@ -60,7 +59,7 @@ public final class SiteInitializer { site = Paths.get(initPath); } if (site != null) { - LOG.info("Initializing site at " + site.toRealPath().normalize()); + logger.atInfo().log("Initializing site at %s", site.toRealPath().normalize()); new BaseInit( site, new ReviewDbDataSourceProvider(), @@ -72,7 +71,7 @@ public final class SiteInitializer { } } } catch (Exception e) { - LOG.error("Site init failed", e); + logger.atSevere().withCause(e).log("Site init failed"); throw new RuntimeException(e); } } diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java index 690d1ac1f8..728fcd9003 100644 --- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java +++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java @@ -18,6 +18,7 @@ import static com.google.inject.Scopes.SINGLETON; import static com.google.inject.Stage.PRODUCTION; import com.google.common.base.Splitter; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.elasticsearch.ElasticIndexModule; import com.google.gerrit.extensions.client.AuthType; import com.google.gerrit.gpg.GpgModule; @@ -127,12 +128,10 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.sql.DataSource; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** Configures the web application environment for Gerrit Code Review. */ public class WebAppInitializer extends GuiceServletContextListener implements Filter { - private static final Logger log = LoggerFactory.getLogger(WebAppInitializer.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private Path sitePath; private Injector dbInjector; @@ -194,7 +193,7 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi buf.append("\nResolve above errors before continuing."); buf.append("\nComplete stack trace follows:"); } - log.error(buf.toString(), first.getCause()); + logger.atSevere().withCause(first.getCause()).log(buf.toString()); throw new CreationException(Collections.singleton(first)); } diff --git a/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java b/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java index eb75a97d5d..adb5516b75 100644 --- a/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java +++ b/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java @@ -32,6 +32,7 @@ import com.google.common.base.Strings; import com.google.common.cache.Cache; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.flogger.FluentLogger; import com.google.common.io.ByteStreams; import com.google.common.net.HttpHeaders; import com.google.gerrit.extensions.registration.RegistrationHandle; @@ -92,14 +93,13 @@ import org.apache.commons.lang.StringUtils; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton class HttpPluginServlet extends HttpServlet implements StartPluginListener, ReloadPluginListener { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final int SMALL_RESOURCE = 128 * 1024; private static final long serialVersionUID = 1L; - private static final Logger log = LoggerFactory.getLogger(HttpPluginServlet.class); private final MimeUtilFileTypeRegistry mimeUtil; private final Provider webUrl; @@ -191,7 +191,7 @@ class HttpPluginServlet extends HttpServlet implements StartPluginListener, Relo try { filter = plugin.getHttpInjector().getInstance(GuiceFilter.class); } catch (RuntimeException e) { - log.warn("Plugin {} cannot load GuiceFilter", name, e); + logger.atWarning().withCause(e).log("Plugin %s cannot load GuiceFilter", name); return null; } @@ -199,7 +199,7 @@ class HttpPluginServlet extends HttpServlet implements StartPluginListener, Relo ServletContext ctx = PluginServletContext.create(plugin, wrapper.getFullPath(name)); filter.init(new WrappedFilterConfig(ctx)); } catch (ServletException e) { - log.warn("Plugin {} failed to initialize HTTP", name, e); + logger.atWarning().withCause(e).log("Plugin %s failed to initialize HTTP", name); return null; } @@ -423,12 +423,12 @@ class HttpPluginServlet extends HttpServlet implements StartPluginListener, Relo && (name.endsWith(".md") || name.endsWith(".html")) && size.isPresent()) { if (size.get() <= 0 || size.get() > SMALL_RESOURCE) { - log.warn( - "Plugin {}: {} omitted from document index. Size {} out of range (0,{}).", - pluginName, - name.substring(prefix.length()), - size.get(), - SMALL_RESOURCE); + logger + .atWarning() + .log( + "Plugin %s: %s omitted from document index. " + + "Size %d out of range (0,%d).", + pluginName, name.substring(prefix.length()), size.get(), SMALL_RESOURCE); return false; } return true; @@ -450,10 +450,11 @@ class HttpPluginServlet extends HttpServlet implements StartPluginListener, Relo if (about == null) { about = entry; } else { - log.warn( - "Plugin {}: Multiple 'about' documents found; using {}", - pluginName, - about.getName().substring(prefix.length())); + logger + .atWarning() + .log( + "Plugin %s: Multiple 'about' documents found; using %s", + pluginName, about.getName().substring(prefix.length())); } } else { docs.add(entry); @@ -731,7 +732,10 @@ class HttpPluginServlet extends HttpServlet implements StartPluginListener, Relo } return def; } catch (IOException e) { - log.warn("Error getting {} for plugin {}, using default", attr, plugin.getName(), e); + logger + .atWarning() + .withCause(e) + .log("Error getting %s for plugin %s, using default", attr, plugin.getName()); return null; } } diff --git a/java/com/google/gerrit/httpd/plugins/LfsPluginServlet.java b/java/com/google/gerrit/httpd/plugins/LfsPluginServlet.java index a8a8502149..0ee22fa7aa 100644 --- a/java/com/google/gerrit/httpd/plugins/LfsPluginServlet.java +++ b/java/com/google/gerrit/httpd/plugins/LfsPluginServlet.java @@ -18,6 +18,7 @@ import static com.google.gerrit.extensions.api.lfs.LfsDefinitions.CONTENTTYPE_VN import static java.nio.charset.StandardCharsets.UTF_8; import static javax.servlet.http.HttpServletResponse.SC_NOT_IMPLEMENTED; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.registration.RegistrationHandle; import com.google.gerrit.httpd.resources.Resource; import com.google.gerrit.server.config.GerritServerConfig; @@ -45,14 +46,14 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton public class LfsPluginServlet extends HttpServlet implements StartPluginListener, ReloadPluginListener { private static final long serialVersionUID = 1L; - private static final Logger log = LoggerFactory.getLogger(LfsPluginServlet.class); + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final String MESSAGE_LFS_NOT_CONFIGURED = "{\"message\":\"No LFS plugin is configured to handle LFS requests.\"}"; @@ -139,7 +140,7 @@ public class LfsPluginServlet extends HttpServlet try { guiceFilter = plugin.getHttpInjector().getInstance(GuiceFilter.class); } catch (RuntimeException e) { - log.warn("Plugin {} cannot load GuiceFilter", name, e); + logger.atWarning().withCause(e).log("Plugin %s cannot load GuiceFilter", name); return null; } @@ -147,7 +148,7 @@ public class LfsPluginServlet extends HttpServlet ServletContext ctx = PluginServletContext.create(plugin, "/"); guiceFilter.init(new WrappedFilterConfig(ctx)); } catch (ServletException e) { - log.warn("Plugin {} failed to initialize HTTP", name, e); + logger.atWarning().withCause(e).log("Plugin %s failed to initialize HTTP", name); return null; } diff --git a/java/com/google/gerrit/httpd/plugins/PluginServletContext.java b/java/com/google/gerrit/httpd/plugins/PluginServletContext.java index 8f64d9ff74..6a8ef328f5 100644 --- a/java/com/google/gerrit/httpd/plugins/PluginServletContext.java +++ b/java/com/google/gerrit/httpd/plugins/PluginServletContext.java @@ -15,6 +15,7 @@ package com.google.gerrit.httpd.plugins; import com.google.common.collect.Maps; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Version; import com.google.gerrit.server.plugins.Plugin; import java.io.InputStream; @@ -29,11 +30,9 @@ import java.util.concurrent.ConcurrentMap; import javax.servlet.RequestDispatcher; import javax.servlet.Servlet; import javax.servlet.ServletContext; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; class PluginServletContext { - private static final Logger log = LoggerFactory.getLogger(PluginServletContext.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); static ServletContext create(Plugin plugin, String contextPath) { return (ServletContext) @@ -155,7 +154,7 @@ class PluginServletContext { @Override public void log(String msg, Throwable reason) { - log.warn("[plugin {}] {}", plugin.getName(), msg, reason); + logger.atWarning().withCause(reason).log("[plugin %s] %s", plugin.getName(), msg); } @Override diff --git a/java/com/google/gerrit/httpd/raw/BazelBuild.java b/java/com/google/gerrit/httpd/raw/BazelBuild.java index f52792caf9..92a5aaac80 100644 --- a/java/com/google/gerrit/httpd/raw/BazelBuild.java +++ b/java/com/google/gerrit/httpd/raw/BazelBuild.java @@ -19,6 +19,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.Joiner; import com.google.common.escape.Escaper; +import com.google.common.flogger.FluentLogger; import com.google.common.html.HtmlEscapers; import com.google.common.io.ByteStreams; import com.google.gerrit.common.TimeUtil; @@ -33,11 +34,9 @@ import java.nio.file.Path; import java.util.Properties; import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.util.RawParseUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class BazelBuild { - private static final Logger log = LoggerFactory.getLogger(BazelBuild.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final Path sourceRoot; @@ -49,7 +48,7 @@ public class BazelBuild { public void build(Label label) throws IOException, BuildFailureException { ProcessBuilder proc = newBuildProcess(label); proc.directory(sourceRoot.toFile()).redirectErrorStream(true); - log.info("building " + label.fullName()); + logger.atInfo().log("building %s", label.fullName()); long start = TimeUtil.nowMs(); Process rebuild = proc.start(); byte[] out; @@ -67,12 +66,12 @@ public class BazelBuild { "interrupted waiting for: " + Joiner.on(' ').join(proc.command())); } if (status != 0) { - log.warn("build failed: " + new String(out, UTF_8)); + logger.atWarning().log("build failed: %s", new String(out, UTF_8)); throw new BuildFailureException(out); } long time = TimeUtil.nowMs() - start; - log.info(String.format("UPDATED %s in %.3fs", label.fullName(), time / 1000.0)); + logger.atInfo().log("UPDATED %s in %.3fs", label.fullName(), time / 1000.0); } // Represents a label in bazel. diff --git a/java/com/google/gerrit/httpd/raw/HostPageServlet.java b/java/com/google/gerrit/httpd/raw/HostPageServlet.java index ffecf1b4f1..74868d7235 100644 --- a/java/com/google/gerrit/httpd/raw/HostPageServlet.java +++ b/java/com/google/gerrit/httpd/raw/HostPageServlet.java @@ -18,6 +18,7 @@ import static com.google.gerrit.common.FileUtil.lastModified; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.Strings; +import com.google.common.flogger.FluentLogger; import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; import com.google.common.primitives.Bytes; @@ -60,8 +61,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -70,7 +69,7 @@ import org.w3c.dom.Node; @SuppressWarnings("serial") @Singleton public class HostPageServlet extends HttpServlet { - private static final Logger log = LoggerFactory.getLogger(HostPageServlet.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String HPD_ID = "gerrit_hostpagedata"; private static final int DEFAULT_JS_LOAD_TIMEOUT = 5000; @@ -141,7 +140,7 @@ public class HostPageServlet extends HttpServlet { } src += "?content=" + md.hash().toString(); } else { - log.debug("No " + src + " in webapp root; keeping noncache.js URL"); + logger.atFine().log("No %s in webapp root; keeping noncache.js URL", src); } } catch (IOException e) { throw new IOException("Failed reading " + src, e); @@ -173,7 +172,7 @@ public class HostPageServlet extends HttpServlet { page = p; } } catch (IOException e) { - log.error("Cannot refresh site header/footer", e); + logger.atSevere().withCause(e).log("Cannot refresh site header/footer"); } return p; } @@ -225,7 +224,7 @@ public class HostPageServlet extends HttpServlet { | ConfigInvalidException | IOException | PermissionBackendException e) { - log.warn("Cannot query account diff preferences", e); + logger.atWarning().withCause(e).log("Cannot query account diff preferences"); } return DiffPreferencesInfo.defaults(); } diff --git a/java/com/google/gerrit/httpd/raw/ResourceServlet.java b/java/com/google/gerrit/httpd/raw/ResourceServlet.java index 3ec6bdbd73..035653da4f 100644 --- a/java/com/google/gerrit/httpd/raw/ResourceServlet.java +++ b/java/com/google/gerrit/httpd/raw/ResourceServlet.java @@ -30,6 +30,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.CharMatcher; import com.google.common.cache.Cache; import com.google.common.collect.ImmutableMap; +import com.google.common.flogger.FluentLogger; import com.google.common.hash.Hashing; import com.google.gerrit.common.Nullable; import com.google.gerrit.httpd.HtmlDomUtil; @@ -47,8 +48,6 @@ import java.util.zip.GZIPOutputStream; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Base class for serving static resources. @@ -58,7 +57,7 @@ import org.slf4j.LoggerFactory; public abstract class ResourceServlet extends HttpServlet { private static final long serialVersionUID = 1L; - private static final Logger log = LoggerFactory.getLogger(ResourceServlet.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final int CACHE_FILE_SIZE_LIMIT_BYTES = 100 << 10; @@ -161,7 +160,7 @@ public abstract class ResourceServlet extends HttpServlet { r = cache.get(p, newLoader(p)); } } catch (ExecutionException e) { - log.warn("Cannot load static resource {}", req.getPathInfo(), e); + logger.atWarning().withCause(e).log("Cannot load static resource %s", req.getPathInfo()); CacheHeaders.setNotCacheable(rsp); rsp.setStatus(SC_INTERNAL_SERVER_ERROR); return; @@ -214,12 +213,12 @@ public abstract class ResourceServlet extends HttpServlet { try { Path p = getResourcePath(name); if (p == null) { - log.warn("Path doesn't exist {}", name); + logger.atWarning().log("Path doesn't exist %s", name); return null; } return cache.get(p, newLoader(p)); } catch (ExecutionException | IOException e) { - log.warn("Cannot load static resource {}", name, e); + logger.atWarning().withCause(e).log("Cannot load static resource %s", name); return null; } } diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java index 915e9ed101..06ec799bac 100644 --- a/java/com/google/gerrit/httpd/raw/StaticModule.java +++ b/java/com/google/gerrit/httpd/raw/StaticModule.java @@ -21,6 +21,7 @@ import static java.nio.file.Files.isReadable; import com.google.common.cache.Cache; import com.google.common.collect.ImmutableList; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.client.UiType; import com.google.gerrit.httpd.XsrfCookieFilter; @@ -57,11 +58,9 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class StaticModule extends ServletModule { - private static final Logger log = LoggerFactory.getLogger(StaticModule.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); public static final String CACHE = "static_content"; public static final String GERRIT_UI_COOKIE = "GERRIT_UI"; @@ -184,7 +183,7 @@ public class StaticModule extends ServletModule { if (exists(configPath) && isReadable(configPath)) { return new SingleFileServlet(cache, configPath, true); } - log.warn("Cannot read httpd.robotsFile, using default"); + logger.atWarning().log("Cannot read httpd.robotsFile, using default"); } Paths p = getPaths(); if (p.warFs != null) { diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 913128e9d4..4c9a03508f 100644 --- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -58,6 +58,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; +import com.google.common.flogger.FluentLogger; import com.google.common.io.BaseEncoding; import com.google.common.io.CountingOutputStream; import com.google.common.math.IntMath; @@ -164,12 +165,11 @@ import org.eclipse.jgit.http.server.ServletUtils; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.util.TemporaryBuffer; import org.eclipse.jgit.util.TemporaryBuffer.Heap; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class RestApiServlet extends HttpServlet { private static final long serialVersionUID = 1L; - private static final Logger log = LoggerFactory.getLogger(RestApiServlet.class); + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); /** MIME type used for a JSON response body. */ private static final String JSON_TYPE = "application/json"; @@ -1192,7 +1192,7 @@ public class RestApiServlet extends HttpServlet { if (!Strings.isNullOrEmpty(req.getQueryString())) { uri += "?" + req.getQueryString(); } - log.error("Error in {} {}", req.getMethod(), uri, err); + logger.atSevere().withCause(err).log("Error in %s %s", req.getMethod(), uri); if (!res.isCommitted()) { res.reset(); diff --git a/java/com/google/gerrit/httpd/rpc/GerritJsonServlet.java b/java/com/google/gerrit/httpd/rpc/GerritJsonServlet.java index e787a48aed..f5d2216f57 100644 --- a/java/com/google/gerrit/httpd/rpc/GerritJsonServlet.java +++ b/java/com/google/gerrit/httpd/rpc/GerritJsonServlet.java @@ -16,6 +16,7 @@ package com.google.gerrit.httpd.rpc; import com.google.common.collect.ListMultimap; import com.google.common.collect.MultimapBuilder; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.audit.Audit; import com.google.gerrit.common.auth.SignInRequired; @@ -38,13 +39,12 @@ import java.lang.reflect.Field; import java.lang.reflect.Method; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** Base JSON servlet to ensure the current user is not forged. */ @SuppressWarnings("serial") final class GerritJsonServlet extends JsonServlet { - private static final Logger log = LoggerFactory.getLogger(GerritJsonServlet.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final ThreadLocal currentCall = new ThreadLocal<>(); private static final ThreadLocal currentMethod = new ThreadLocal<>(); private final DynamicItem session; @@ -141,7 +141,7 @@ final class GerritJsonServlet extends JsonServlet result)); } } catch (Throwable all) { - log.error("Unable to log the call", all); + logger.atSevere().withCause(all).log("Unable to log the call"); } } @@ -190,7 +190,7 @@ final class GerritJsonServlet extends JsonServlet declaredField = clazz.getDeclaredField(fieldName); declaredField.setAccessible(true); } catch (Exception e) { - log.error("Unable to expose RPS/JSON result field"); + logger.atSevere().log("Unable to expose RPS/JSON result field"); } return declaredField; } @@ -205,9 +205,9 @@ final class GerritJsonServlet extends JsonServlet Method method = (Method) methodField.get(this.getMethod()); return method.getDeclaringClass(); } catch (IllegalArgumentException e) { - log.error("Cannot access result field"); + logger.atSevere().log("Cannot access result field"); } catch (IllegalAccessException e) { - log.error("No permissions to access result field"); + logger.atSevere().log("No permissions to access result field"); } return null; @@ -222,9 +222,9 @@ final class GerritJsonServlet extends JsonServlet try { return resultField.get(this); } catch (IllegalArgumentException e) { - log.error("Cannot access result field"); + logger.atSevere().log("Cannot access result field"); } catch (IllegalAccessException e) { - log.error("No permissions to access result field"); + logger.atSevere().log("No permissions to access result field"); } return null; diff --git a/java/com/google/gerrit/httpd/rpc/SystemInfoServiceImpl.java b/java/com/google/gerrit/httpd/rpc/SystemInfoServiceImpl.java index 7a7713dcf0..634e8d8502 100644 --- a/java/com/google/gerrit/httpd/rpc/SystemInfoServiceImpl.java +++ b/java/com/google/gerrit/httpd/rpc/SystemInfoServiceImpl.java @@ -14,6 +14,7 @@ package com.google.gerrit.httpd.rpc; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.data.SshHostKey; import com.google.gerrit.common.data.SystemInfoService; import com.google.gerrit.server.ssh.SshInfo; @@ -26,11 +27,9 @@ import com.jcraft.jsch.JSch; import java.util.ArrayList; import java.util.List; import javax.servlet.http.HttpServletRequest; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; class SystemInfoServiceImpl implements SystemInfoService { - private static final Logger log = LoggerFactory.getLogger(SystemInfoServiceImpl.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final JSch JSCH = new JSch(); @@ -63,7 +62,7 @@ class SystemInfoServiceImpl implements SystemInfoService { HttpServletRequest r = httpRequest.get(); String ua = r.getHeader("User-Agent"); message = message.replaceAll("\n", "\n "); - log.error("Client UI JavaScript error: User-Agent=" + ua + ": " + message); + logger.atSevere().log("Client UI JavaScript error: User-Agent=%s: %s", ua, message); callback.onSuccess(VoidResult.INSTANCE); } } diff --git a/java/com/google/gerrit/httpd/template/SiteHeaderFooter.java b/java/com/google/gerrit/httpd/template/SiteHeaderFooter.java index dca4d0f099..655f4ca0e5 100644 --- a/java/com/google/gerrit/httpd/template/SiteHeaderFooter.java +++ b/java/com/google/gerrit/httpd/template/SiteHeaderFooter.java @@ -17,6 +17,7 @@ package com.google.gerrit.httpd.template; import static com.google.gerrit.common.FileUtil.lastModified; import com.google.common.base.Strings; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.httpd.HtmlDomUtil; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; @@ -25,14 +26,12 @@ import com.google.inject.Singleton; import java.io.IOException; import java.nio.file.Path; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.w3c.dom.Document; import org.w3c.dom.Element; @Singleton public class SiteHeaderFooter { - private static final Logger log = LoggerFactory.getLogger(SiteHeaderFooter.class); + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final boolean refreshHeaderFooter; private final SitePaths sitePaths; @@ -48,7 +47,7 @@ public class SiteHeaderFooter { t.load(); template = t; } catch (IOException e) { - log.warn("Cannot load site header or footer", e); + logger.atWarning().withCause(e).log("Cannot load site header or footer"); } } @@ -60,7 +59,7 @@ public class SiteHeaderFooter { t.load(); template = t; } catch (IOException e) { - log.warn("Cannot refresh site header or footer", e); + logger.atWarning().withCause(e).log("Cannot refresh site header or footer"); t = template; } }