Skip to content

Commit b6f54ca

Browse files
Avery-DunnAvery-Dunn
authored
Merge pull request #992 from AzureAD/avdunn/consistent-logging
Consistently use SLF4J's recommended styles
2 parents 4382468 + 82cd008 commit b6f54ca

25 files changed

+78
-84
lines changed

‎msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/AcquireTokenInteractiveIT.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ void acquireTokenInteractive_Ciam() {
145145
result = pca.acquireToken(parameters).get();
146146

147147
} catch (Exception e) {
148-
LOG.error("Error acquiring token with authCode: " + e.getMessage());
148+
LOG.error("Error acquiring token with authCode: {}", e.getMessage());
149149
throw new RuntimeException("Error acquiring token with authCode: " + e.getMessage());
150150
}
151151

@@ -236,7 +236,7 @@ private IAuthenticationResult acquireTokenInteractive(
236236
result = pca.acquireToken(parameters).get();
237237

238238
} catch (Exception e) {
239-
LOG.error("Error acquiring token with authCode: " + e.getMessage());
239+
LOG.error("Error acquiring token with authCode: {}", e.getMessage());
240240
throw new RuntimeException("Error acquiring token with authCode: " + e.getMessage());
241241
}
242242
return result;
@@ -266,7 +266,7 @@ private IAuthenticationResult acquireTokenInteractive_instanceAware(
266266
result = pca.acquireToken(parameters).get();
267267

268268
} catch (Exception e) {
269-
LOG.error("Error acquiring token with authCode: " + e.getMessage());
269+
LOG.error("Error acquiring token with authCode: {}", e.getMessage());
270270
throw new RuntimeException("Error acquiring token with authCode: " + e.getMessage());
271271
}
272272
return result;

‎msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/AuthorizationCodeIT.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ private IAuthenticationResult acquireTokenAuthorizationCodeFlow(
202202
.get();
203203

204204
} catch (Exception e) {
205-
LOG.error("Error acquiring token with authCode: " + e.getMessage());
205+
LOG.error("Error acquiring token with authCode: {}", e.getMessage());
206206
throw new RuntimeException("Error acquiring token with authCode: " + e.getMessage());
207207
}
208208
return result;
@@ -219,7 +219,7 @@ private IAuthenticationResult acquireTokenInteractiveB2C(ConfidentialClientAppli
219219
.build())
220220
.get();
221221
} catch (Exception e) {
222-
LOG.error("Error acquiring token with authCode: " + e.getMessage());
222+
LOG.error("Error acquiring token with authCode: {}", e.getMessage());
223223
throw new RuntimeException("Error acquiring token with authCode: " + e.getMessage());
224224
}
225225
return result;

‎msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/DeviceCodeIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ private void runAutomatedDeviceCodeFlow(DeviceCode deviceCode, User user) {
152152
if (!isRunningLocally) {
153153
SeleniumExtensions.takeScreenShot(seleniumDriver);
154154
}
155-
LOG.error("Browser automation failed: " + e.getMessage());
155+
LOG.error("Browser automation failed: {}", e.getMessage());
156156
throw new RuntimeException("Browser automation failed: " + e.getMessage());
157157
}
158158
}

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class AadInstanceDiscoveryProvider {
3636
static final TreeSet<String> TRUSTED_HOSTS_SET = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
3737
static final TreeSet<String> TRUSTED_SOVEREIGN_HOSTS_SET = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
3838

39-
private static final Logger log = LoggerFactory.getLogger(AadInstanceDiscoveryProvider.class);
39+
private static final Logger LOG = LoggerFactory.getLogger(AadInstanceDiscoveryProvider.class);
4040

4141
static ConcurrentHashMap<String, InstanceDiscoveryMetadataEntry> cache = new ConcurrentHashMap<>();
4242

@@ -65,7 +65,7 @@ static InstanceDiscoveryMetadataEntry getMetadataEntry(URL authorityUrl,
6565
//If instanceDiscovery flag set to false OR this is a managed identity scenario, cache a basic instance metadata entry to skip this and future lookups
6666
if (msalRequest.application() instanceof ManagedIdentityApplication || !((AbstractClientApplicationBase) msalRequest.application()).instanceDiscovery()) {
6767
if (cache.get(host) == null) {
68-
log.debug("Instance discovery set to false, caching a default entry.");
68+
LOG.debug("Instance discovery set to false, caching a default entry.");
6969
cacheInstanceDiscoveryMetadata(host);
7070
}
7171
return cache.get(host);
@@ -78,10 +78,10 @@ static InstanceDiscoveryMetadataEntry getMetadataEntry(URL authorityUrl,
7878

7979
//If there is no cached instance metadata, do instance discovery cache the result
8080
if (cache.get(host) == null) {
81-
log.debug("No cached instance metadata, will attempt instance discovery.");
81+
LOG.debug("No cached instance metadata, will attempt instance discovery.");
8282

8383
if (shouldUseRegionalEndpoint(msalRequest)) {
84-
log.debug("Region API used, will attempt to discover Azure region.");
84+
LOG.debug("Region API used, will attempt to discover Azure region.");
8585

8686
//Server side telemetry requires the result from region discovery when any part of the region API is used
8787
String detectedRegion = discoverRegion(msalRequest, serviceBundle);
@@ -91,7 +91,7 @@ static InstanceDiscoveryMetadataEntry getMetadataEntry(URL authorityUrl,
9191
if (((AbstractClientApplicationBase) msalRequest.application()).azureRegion() == null
9292
&& ((AbstractClientApplicationBase) msalRequest.application()).autoDetectRegion()
9393
&& detectedRegion != null) {
94-
log.debug(String.format("Region autodetection found %s, this region will be used for future calls.", detectedRegion));
94+
LOG.debug("Region autodetection found {}, this region will be used for future calls.", detectedRegion);
9595

9696
((AbstractClientApplicationBase) msalRequest.application()).azureRegion = detectedRegion;
9797
host = getRegionalizedHost(authorityUrl.getHost(), ((AbstractClientApplicationBase) msalRequest.application()).azureRegion());
@@ -158,7 +158,7 @@ private static boolean shouldUseRegionalEndpoint(MsalRequest msalRequest){
158158
} else {
159159
//Avoid unnecessary warnings when looking for cached tokens by checking if request was a silent call
160160
if (msalRequest.getClass() != SilentRequest.class) {
161-
log.warn("Regional endpoints are only available for client credential flow, request will fall back to using the global endpoint. See here for more information about supported scenarios: https://aka.ms/msal4j-azure-regions");
161+
LOG.warn("Regional endpoints are only available for client credential flow, request will fall back to using the global endpoint. See here for more information about supported scenarios: https://aka.ms/msal4j-azure-regions");
162162
}
163163
return false;
164164
}
@@ -239,7 +239,7 @@ static AadInstanceDiscoveryResponse sendInstanceDiscoveryRequest(URL authorityUr
239239
throw MsalServiceExceptionFactory.fromHttpResponse(httpResponse);
240240
}
241241
// instance discovery failed due to reasons other than an invalid authority, do not perform instance discovery again in this environment.
242-
log.debug("Instance discovery failed due to an unknown error, no more instance discovery attempts will be made.");
242+
LOG.debug("Instance discovery failed due to an unknown error, no more instance discovery attempts will be made.");
243243
cacheInstanceDiscoveryMetadata(authorityUrl.getHost());
244244
}
245245

@@ -291,7 +291,7 @@ static String discoverRegion(MsalRequest msalRequest, ServiceBundle serviceBundl
291291

292292
//Check if the REGION_NAME environment variable has a value for the region
293293
if (System.getenv(REGION_NAME) != null) {
294-
log.info(String.format("Region found in environment variable: %s",System.getenv(REGION_NAME)));
294+
LOG.info("Region found in environment variable: {}", System.getenv(REGION_NAME));
295295
currentRequest.regionSource(RegionTelemetry.REGION_SOURCE_ENV_VARIABLE.telemetryValue);
296296

297297
return System.getenv(REGION_NAME);
@@ -305,23 +305,23 @@ static String discoverRegion(MsalRequest msalRequest, ServiceBundle serviceBundl
305305
Future<IHttpResponse> future = executor.submit(() -> executeRequest(IMDS_ENDPOINT, headers, msalRequest, serviceBundle));
306306

307307
try {
308-
log.info("Starting call to IMDS endpoint.");
308+
LOG.info("Starting call to IMDS endpoint.");
309309
IHttpResponse httpResponse = future.get(IMDS_TIMEOUT, IMDS_TIMEOUT_UNIT);
310310
//If call to IMDS endpoint was successful, return region from response body
311311
if (httpResponse.statusCode() == HttpStatus.HTTP_OK && !httpResponse.body().isEmpty()) {
312-
log.info(String.format("Region retrieved from IMDS endpoint: %s", httpResponse.body()));
312+
LOG.info("Region retrieved from IMDS endpoint: {}", httpResponse.body());
313313
currentRequest.regionSource(RegionTelemetry.REGION_SOURCE_IMDS.telemetryValue);
314314

315315
return httpResponse.body();
316316
}
317-
log.warn(String.format("Call to local IMDS failed with status code: %s, or response was empty", httpResponse.statusCode()));
317+
LOG.warn("Call to local IMDS failed with status code: {}, or response was empty", httpResponse.statusCode());
318318
currentRequest.regionSource(RegionTelemetry.REGION_SOURCE_FAILED_AUTODETECT.telemetryValue);
319319
} catch (Exception ex) {
320320
// handle other exceptions
321321
//IMDS call failed, cannot find region
322322
//The IMDS endpoint is only available from within an Azure environment, so the most common cause of this
323323
// exception will likely be java.net.SocketException: Network is unreachable: connect
324-
log.warn(String.format("Exception during call to local IMDS endpoint: %s", ex.getMessage()));
324+
LOG.warn("Exception during call to local IMDS endpoint: {}", ex.getMessage());
325325
currentRequest.regionSource(RegionTelemetry.REGION_SOURCE_FAILED_AUTODETECT.telemetryValue);
326326
future.cancel(true);
327327

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,8 @@ public ManagedIdentityResponse handleResponse(
7474
return getSuccessfulResponse(response);
7575
} else {
7676
message = getMessageFromErrorResponse(response);
77-
LOG.error(
78-
String.format("[Managed Identity] request failed, HttpStatusCode: %s, Error message: %s",
79-
response.statusCode(), message));
77+
LOG.error("[Managed Identity] request failed, HttpStatusCode: {}, Error message: {}",
78+
response.statusCode(), message);
8079
throw new MsalServiceException(message, AuthenticationErrorCode.MANAGED_IDENTITY_REQUEST_FAILED, managedIdentitySourceType);
8180
}
8281
} catch (Exception e) {

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByClientCredentialSupplier.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ AuthenticationResult execute() throws Exception {
4646

4747
return supplier.execute();
4848
} catch (MsalClientException ex) {
49-
LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
49+
LOG.debug("Cache lookup failed: {}", ex.getMessage());
5050
return acquireTokenByClientCredential();
5151
}
5252
}

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByInteractiveFlowSupplier.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ private void updateRedirectUrl() {
109109
try {
110110
URI updatedRedirectUrl = new URI("http://localhost:" + httpListener.port());
111111
interactiveRequest.interactiveRequestParameters().redirectUri(updatedRedirectUrl);
112-
LOG.debug("Redirect URI updated to" + updatedRedirectUrl);
112+
LOG.debug("Redirect URI updated to {}", updatedRedirectUrl);
113113
} catch (URISyntaxException ex) {
114114
throw new MsalClientException("Error updating redirect URI. Not a valid URI format",
115115
AuthenticationErrorCode.INVALID_REDIRECT_URI);
@@ -204,7 +204,7 @@ private AuthorizationResult getAuthorizationResultFromHttpListener() {
204204
long expirationTime;
205205

206206
if (timeFromParameters > 0) {
207-
LOG.debug(String.format("Listening for authorization result. Listener will timeout after %S seconds.", timeFromParameters));
207+
LOG.debug("Listening for authorization result. Listener will timeout after {} seconds.", timeFromParameters);
208208
expirationTime = TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis()) + timeFromParameters;
209209
} else {
210210
LOG.warn("Listening for authorization result. Timeout configured to less than 1 second, listener will use a 1 second timeout instead.");
@@ -213,7 +213,7 @@ private AuthorizationResult getAuthorizationResultFromHttpListener() {
213213

214214
while (result == null && !interactiveRequest.futureReference().get().isDone()) {
215215
if (TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis()) > expirationTime) {
216-
LOG.warn(String.format("Listener timed out after %S seconds, no authorization code was returned from the server during that time.", timeFromParameters));
216+
LOG.warn("Listener timed out after {} seconds, no authorization code was returned from the server during that time.", timeFromParameters);
217217
break;
218218
}
219219

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,15 +90,15 @@ AuthenticationResult execute() throws Exception {
9090
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, CacheRefreshReason.CLAIMS);
9191
}
9292

93-
LOG.debug(String.format("Refreshing access token. Cache refresh reason: %s", cacheRefreshReason));
93+
LOG.debug("Refreshing access token. Cache refresh reason: {}", cacheRefreshReason);
9494
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, cacheRefreshReason);
9595
}
9696
} catch (MsalClientException ex) {
9797
if (ex.errorCode().equals(AuthenticationErrorCode.CACHE_MISS)) {
98-
LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
98+
LOG.debug("Cache lookup failed: {}", ex.getMessage());
9999
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, cacheRefreshReason);
100100
} else {
101-
LOG.error(String.format("Error occurred while cache lookup: %s", ex.getMessage()));
101+
LOG.error("Error occurred while cache lookup: {}", ex.getMessage());
102102
throw ex;
103103
}
104104
}
@@ -108,8 +108,8 @@ private AuthenticationResult fetchNewAccessTokenAndSaveToCache(TokenRequestExecu
108108

109109
ManagedIdentityClient managedIdentityClient = new ManagedIdentityClient(msalRequest, tokenRequestExecutor.getServiceBundle());
110110

111-
LOG.debug(String.format("[Managed Identity] Managed Identity source and ID type identified and set successfully, request will use Managed Identity for %s",
112-
managedIdentityClient.managedIdentitySource.managedIdentitySourceType.name()));
111+
LOG.debug("[Managed Identity] Managed Identity source and ID type identified and set successfully, request will use Managed Identity for {}",
112+
managedIdentityClient.managedIdentitySource.managedIdentitySourceType.name());
113113

114114
ManagedIdentityResponse managedIdentityResponse = managedIdentityClient
115115
.getManagedIdentityResponse(managedIdentityParameters);

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByOnBehalfOfSupplier.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ AuthenticationResult execute() throws Exception {
4646

4747
return supplier.execute();
4848
} catch (MsalClientException ex) {
49-
LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
49+
LOG.debug("Cache lookup failed: {}", ex.getMessage());
5050
return acquireTokenOnBehalfOf();
5151
}
5252
}

‎msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
class AcquireTokenSilentSupplier extends AuthenticationResultSupplier {
1313

14-
private static final Logger log = LoggerFactory.getLogger(AcquireTokenSilentSupplier.class);
14+
private static final Logger LOG = LoggerFactory.getLogger(AcquireTokenSilentSupplier.class);
1515
private SilentRequest silentRequest;
1616
protected static final int ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC = 5 * 60;
1717

@@ -74,7 +74,7 @@ AuthenticationResult execute() throws Exception {
7474
throw new MsalClientException(AuthenticationErrorMessage.NO_TOKEN_IN_CACHE, AuthenticationErrorCode.CACHE_MISS);
7575
}
7676

77-
log.debug("Returning token from cache");
77+
LOG.debug("Returning token from cache");
7878

7979
return res;
8080
}
@@ -99,7 +99,7 @@ private AuthenticationResult makeRefreshRequest(AuthenticationResult cachedResul
9999
refreshedResult.metadata().tokenSource(TokenSource.IDENTITY_PROVIDER);
100100
refreshedResult.metadata().cacheRefreshReason(refreshReason);
101101

102-
log.info("Access token refreshed successfully.");
102+
LOG.info("Access token refreshed successfully.");
103103
return refreshedResult;
104104
} catch (MsalServiceException ex) {
105105
//If the token refresh attempt threw a MsalServiceException but the refresh attempt was done
@@ -117,15 +117,15 @@ private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult
117117
//If forceRefresh is true, no reason to check any other option
118118
if (parameters.forceRefresh()) {
119119
setCacheTelemetry(CacheRefreshReason.FORCE_REFRESH);
120-
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.FORCE_REFRESH));
120+
LOG.debug("Refreshing access token. Cache refresh reason: {}", CacheRefreshReason.FORCE_REFRESH);
121121
return true;
122122
}
123123

124124
//If the request contains claims then the token should be refreshed, to ensure that the returned token has the correct claims
125125
// Note: these are the types of claims found in (for example) a claims challenge, and do not include client capabilities
126126
if (parameters.claims() != null) {
127127
setCacheTelemetry(CacheRefreshReason.CLAIMS);
128-
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.CLAIMS));
128+
LOG.debug("Refreshing access token. Cache refresh reason: {}", CacheRefreshReason.CLAIMS);
129129
return true;
130130
}
131131

@@ -134,7 +134,7 @@ private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult
134134
//If the access token is expired or within 5 minutes of becoming expired, refresh it
135135
if (!StringHelper.isBlank(cachedResult.accessToken()) && cachedResult.expiresOn() < (currTimeStampSec + ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC)) {
136136
setCacheTelemetry(CacheRefreshReason.EXPIRED);
137-
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.EXPIRED));
137+
LOG.debug("Refreshing access token. Cache refresh reason: {}", CacheRefreshReason.EXPIRED);
138138
return true;
139139
}
140140

@@ -143,14 +143,14 @@ private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult
143143
cachedResult.refreshOn() != null && cachedResult.refreshOn() > 0 &&
144144
cachedResult.refreshOn() < currTimeStampSec && cachedResult.expiresOn() >= (currTimeStampSec + ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC)){
145145
setCacheTelemetry(CacheRefreshReason.PROACTIVE_REFRESH);
146-
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.PROACTIVE_REFRESH));
146+
LOG.debug("Refreshing access token. Cache refresh reason: {}", CacheRefreshReason.PROACTIVE_REFRESH);
147147
return true;
148148
}
149149

150150
//If there is a refresh token but no access token, we should use the refresh token to get the access token
151151
if (StringHelper.isBlank(cachedResult.accessToken()) && !StringHelper.isBlank(cachedResult.refreshToken())) {
152152
setCacheTelemetry(CacheRefreshReason.NO_CACHED_ACCESS_TOKEN);
153-
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.NO_CACHED_ACCESS_TOKEN));
153+
LOG.debug("Refreshing access token. Cache refresh reason: {}", CacheRefreshReason.NO_CACHED_ACCESS_TOKEN);
154154
return true;
155155
}
156156

0 commit comments

Comments
 (0)