chore(logs): be more consistent with exception logging (#1237)

spinnaker · Jun 4, 2020 · 043e31b · 043e31b
1 parent 0895823
commit 043e31b
Show file tree

Hide file tree

Showing 9 changed files with 24 additions and 29 deletions.
diff --git a/...core/src/main/groovy/com/netflix/spinnaker/gate/security/anonymous/AnonymousConfig.groovy b/...core/src/main/groovy/com/netflix/spinnaker/gate/security/anonymous/AnonymousConfig.groovy
@@ -85,7 +85,7 @@ class AnonymousConfig extends WebSecurityConfigurerAdapter {
       anonymousAllowedAccounts.removeAll(toRemove)
       anonymousAllowedAccounts.addAll(toAdd)
     } catch (Exception e) {
-      log.warn(ExceptionUtils.getStackTrace(e))
+      log.warn("Error while updating anonymous accounts", e)
     }
   }
 }
diff --git a/...e/src/main/groovy/com/netflix/spinnaker/gate/services/DefaultProviderLookupService.groovy b/...e/src/main/groovy/com/netflix/spinnaker/gate/services/DefaultProviderLookupService.groovy
@@ -82,8 +82,8 @@ class DefaultProviderLookupService implements ProviderLookupService, AccountLook
         }
       }
       accountsCache.set(accounts)
-    } catch (Exception e) {
-      log.error("Unable to refresh account details cache, reason: ${e.message}")
+z    } catch (Exception e) {
+      log.error("Unable to refresh account details cache", e)
     }
   }
 

diff --git a/gate-iap/src/main/java/com/netflix/spinnaker/gate/security/iap/IapAuthenticationFilter.java b/gate-iap/src/main/java/com/netflix/spinnaker/gate/security/iap/IapAuthenticationFilter.java
@@ -122,9 +122,7 @@ protected void doFilterInternal(
       session.setAttribute(SIGNATURE_ATTRIBUTE, jwt.getSignature());
 
     } catch (Exception e) {
-      if (log.isDebugEnabled()) {
-        log.info("Could not verify JWT Token for request: " + request.getPathInfo(), e);
-      }
+      log.error("Could not verify JWT Token for request {}", request.getPathInfo(), e);
     }
     chain.doFilter(request, response);
   }

diff --git a/...n/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy b/...n/groovy/com/netflix/spinnaker/gate/security/oauth2/SpinnakerUserInfoTokenServices.groovy
@@ -132,7 +132,8 @@ class SpinnakerUserInfoTokenServices implements ResourceServerTokenServices {
           username,
           roles.size(),
           roles,
-          fiatClientConfigurationProperties.legacyFallback
+          fiatClientConfigurationProperties.legacyFallback,
+          e
         )
         id = id.withTag("success", false).withTag("fallback", fiatClientConfigurationProperties.legacyFallback)
 

diff --git a/...vy/com/netflix/spinnaker/gate/security/oauth2/provider/GithubProviderTokenServices.groovy b/...vy/com/netflix/spinnaker/gate/security/oauth2/provider/GithubProviderTokenServices.groovy
@@ -58,40 +58,34 @@ class GithubProviderTokenServices implements SpinnakerProviderTokenServices {
 
   boolean checkOrganization(String accessToken, String organizationsUrl, String organization) {
     try {
-      log.debug("Getting user organizations from: " + organizationsUrl)
-      OAuth2RestOperations restTemplate = this.restTemplate;
+      log.debug("Getting user organizations from URL {}", organizationsUrl)
+      OAuth2RestOperations restTemplate = this.restTemplate
       if (restTemplate == null) {
-        BaseOAuth2ProtectedResourceDetails resource = new BaseOAuth2ProtectedResourceDetails();
+        BaseOAuth2ProtectedResourceDetails resource = new BaseOAuth2ProtectedResourceDetails()
         resource.setClientId(sso.clientId)
         restTemplate = new OAuth2RestTemplate(resource)
       }
-      OAuth2AccessToken existingToken = restTemplate.getOAuth2ClientContext()
-        .getAccessToken()
+      OAuth2AccessToken existingToken = restTemplate.getOAuth2ClientContext().getAccessToken()
       if (existingToken == null || !accessToken.equals(existingToken.getValue())) {
-        DefaultOAuth2AccessToken token = new DefaultOAuth2AccessToken(
-          accessToken)
+        DefaultOAuth2AccessToken token = new DefaultOAuth2AccessToken(accessToken)
         token.setTokenType(this.tokenType)
         restTemplate.getOAuth2ClientContext().setAccessToken(token)
       }
-      List<Map<String, String>> organizations = restTemplate
-        .getForEntity(organizationsUrl, List.class).getBody()
+      List<Map<String, String>> organizations = restTemplate.getForEntity(organizationsUrl, List.class).getBody()
       return githubOrganizationMember(organization, organizations)
     }
-    catch (Exception ex) {
-      log.warn("Could not fetch user organizations: " + ex.getClass() + ", "
-        + ex.getMessage())
-      return Collections.<String, Object>singletonMap("error",
-        "Could not fetch user organizations")
+    catch (Exception e) {
+      log.warn("Could not fetch user organizations", e)
+      return Collections.<String, Object>singletonMap("error", "Could not fetch user organizations")
     }
   }
 
   boolean hasAllProviderRequirements(String token, Map details) {
     boolean hasRequirements = true
     if (requirements.organization != null && details.containsKey("organizations_url")) {
-      boolean orgMatch = checkOrganization(token, details['organizations_url'],
-        requirements.organization)
+      boolean orgMatch = checkOrganization(token, details['organizations_url'], requirements.organization)
       if (!orgMatch) {
-        log.debug("User does not include required organization: " + requirements.organization)
+        log.debug("User does not include required organization {}", requirements.organization)
         hasRequirements = false
       }
     }

diff --git a/gate-saml/src/main/groovy/com/netflix/spinnaker/gate/security/saml/SamlSsoConfig.groovy b/gate-saml/src/main/groovy/com/netflix/spinnaker/gate/security/saml/SamlSsoConfig.groovy
@@ -241,7 +241,8 @@ class SamlSsoConfig extends WebSecurityConfigurerAdapter {
               username,
               roles.size(),
               roles,
-              fiatClientConfigurationProperties.legacyFallback
+              fiatClientConfigurationProperties.legacyFallback,
+              e
           )
           id = id.withTag("success", false).withTag("fallback", fiatClientConfigurationProperties.legacyFallback)
 

diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/AuthController.groovy b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/AuthController.groovy
@@ -129,8 +129,8 @@ class AuthController {
     URL toURL
     try {
       toURL = new URL(to)
-    } catch (MalformedURLException malEx) {
-      log.warn "Malformed redirect URL: $to\n${ExceptionUtils.getStackTrace(malEx)}"
+    } catch (MalformedURLException e) {
+      log.warn("Malformed redirect URL {}", to, e)
       return false
     }
 

diff --git a/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ManagedController.java b/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/ManagedController.java
@@ -163,7 +163,7 @@ ResponseEntity<Map> validateManifest(@RequestBody DeliveryConfig manifest) {
           return ResponseEntity.badRequest()
               .body(objectMapper.readValue(e.getResponse().getBody().in(), Map.class));
         } catch (Exception ex) {
-          log.error("Error parsing error response from keel: {}", ex.getMessage(), ex);
+          log.error("Error parsing error response from keel", ex);
           return ResponseEntity.badRequest().body(Collections.emptyMap());
         }
       } else {

diff --git a/...oovy/com/netflix/spinnaker/gate/security/x509/X509AuthenticationUserDetailsService.groovy b/...oovy/com/netflix/spinnaker/gate/security/x509/X509AuthenticationUserDetailsService.groovy
@@ -191,7 +191,8 @@ class X509AuthenticationUserDetailsService implements AuthenticationUserDetailsS
             email,
             roles.size(),
             roles,
-            fiatClientConfigurationProperties.legacyFallback
+            fiatClientConfigurationProperties.legacyFallback,
+            e
           )
           id = id.withTag("success", false).withTag("fallback", fiatClientConfigurationProperties.legacyFallback)