Skip to content

Commit 81719c5

Browse files
Avery-DunnAvery-Dunn
committed
Adjust SLF4J logging to be consistent throughout the codebase
1 parent 7ccf720 commit 81719c5

16 files changed

+50
-51
lines changed

‎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
//flag to check if instance discovery has failed
4242
private static boolean instanceDiscoveryFailed = false;
@@ -67,7 +67,7 @@ static InstanceDiscoveryMetadataEntry getMetadataEntry(URL authorityUrl,
6767
//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
6868
if (msalRequest.application() instanceof ManagedIdentityApplication || !((AbstractClientApplicationBase) msalRequest.application()).instanceDiscovery()) {
6969
if (cache.get(host) == null) {
70-
log.debug("Instance discovery set to false, caching a default entry.");
70+
LOG.debug("Instance discovery set to false, caching a default entry.");
7171
cacheInstanceDiscoveryMetadata(host);
7272
}
7373
return cache.get(host);
@@ -80,10 +80,10 @@ static InstanceDiscoveryMetadataEntry getMetadataEntry(URL authorityUrl,
8080

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

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

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

9898
((AbstractClientApplicationBase) msalRequest.application()).azureRegion = detectedRegion;
9999
host = getRegionalizedHost(authorityUrl.getHost(), ((AbstractClientApplicationBase) msalRequest.application()).azureRegion());
@@ -160,7 +160,7 @@ private static boolean shouldUseRegionalEndpoint(MsalRequest msalRequest){
160160
} else {
161161
//Avoid unnecessary warnings when looking for cached tokens by checking if request was a silent call
162162
if (msalRequest.getClass() != SilentRequest.class) {
163-
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");
163+
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");
164164
}
165165
return false;
166166
}
@@ -241,7 +241,7 @@ static AadInstanceDiscoveryResponse sendInstanceDiscoveryRequest(URL authorityUr
241241
throw MsalServiceExceptionFactory.fromHttpResponse(httpResponse);
242242
}
243243
// instance discovery failed due to reasons other than an invalid authority, do not perform instance discovery again in this environment.
244-
log.debug("Instance discovery failed due to an unknown error, no more instance discovery attempts will be made.");
244+
LOG.debug("Instance discovery failed due to an unknown error, no more instance discovery attempts will be made.");
245245
cacheInstanceDiscoveryMetadata(authorityUrl.getHost());
246246
}
247247

@@ -293,7 +293,7 @@ static String discoverRegion(MsalRequest msalRequest, ServiceBundle serviceBundl
293293

294294
//Check if the REGION_NAME environment variable has a value for the region
295295
if (System.getenv(REGION_NAME) != null) {
296-
log.info("Region found in environment variable: {}", System.getenv(REGION_NAME));
296+
LOG.info("Region found in environment variable: {}", System.getenv(REGION_NAME));
297297
currentRequest.regionSource(RegionTelemetry.REGION_SOURCE_ENV_VARIABLE.telemetryValue);
298298

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

309309
try {
310-
log.info("Starting call to IMDS endpoint.");
310+
LOG.info("Starting call to IMDS endpoint.");
311311
IHttpResponse httpResponse = future.get(IMDS_TIMEOUT, IMDS_TIMEOUT_UNIT);
312312
//If call to IMDS endpoint was successful, return region from response body
313313
if (httpResponse.statusCode() == HttpStatus.HTTP_OK && !httpResponse.body().isEmpty()) {
314-
log.info("Region retrieved from IMDS endpoint: {}", httpResponse.body());
314+
LOG.info("Region retrieved from IMDS endpoint: {}", httpResponse.body());
315315
currentRequest.regionSource(RegionTelemetry.REGION_SOURCE_IMDS.telemetryValue);
316316

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

‎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

@@ -78,7 +78,7 @@ AuthenticationResult execute() throws Exception {
7878
throw new MsalClientException(AuthenticationErrorMessage.NO_TOKEN_IN_CACHE, AuthenticationErrorCode.CACHE_MISS);
7979
}
8080

81-
log.debug("Returning token from cache");
81+
LOG.debug("Returning token from cache");
8282

8383
return res;
8484
}
@@ -103,7 +103,7 @@ private AuthenticationResult makeRefreshRequest(AuthenticationResult cachedResul
103103
refreshedResult.metadata().tokenSource(TokenSource.IDENTITY_PROVIDER);
104104
refreshedResult.metadata().cacheRefreshReason(refreshReason);
105105

106-
log.info("Access token refreshed successfully.");
106+
LOG.info("Access token refreshed successfully.");
107107
return refreshedResult;
108108
} catch (MsalServiceException ex) {
109109
//If the token refresh attempt threw a MsalServiceException but the refresh attempt was done
@@ -121,15 +121,15 @@ private boolean shouldRefresh(SilentParameters parameters, AuthenticationResult
121121
//If forceRefresh is true, no reason to check any other option
122122
if (parameters.forceRefresh()) {
123123
setCacheTelemetry(CacheRefreshReason.FORCE_REFRESH);
124-
log.debug(String.format("Refreshing access token. Cache refresh reason: %s", CacheRefreshReason.FORCE_REFRESH));
124+
LOG.debug("Refreshing access token. Cache refresh reason: {}", CacheRefreshReason.FORCE_REFRESH);
125125
return true;
126126
}
127127

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

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

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

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ private static URI validateAndGetUri(String msiEndpoint, String secret)
7979
ManagedIdentitySourceType.APP_SERVICE);
8080
}
8181

82-
LOG.info("[Managed Identity] Environment variables validation passed for app service managed identity. Endpoint URI: {endpointUri}. Creating App Service managed identity.");
82+
LOG.info("[Managed Identity] Environment variables validation passed for app service managed identity. Endpoint URI: {}. Creating App Service managed identity.", endpointUri);
8383
return endpointUri;
8484
}
8585

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public class AuthorizationRequestUrlParameters {
3737

3838
Map<String, String> requestParameters = new HashMap<>();
3939

40-
Logger log = LoggerFactory.getLogger(AuthorizationRequestUrlParameters.class);
40+
private static final Logger LOG = LoggerFactory.getLogger(AuthorizationRequestUrlParameters.class);
4141

4242
public static Builder builder(String redirectUri,
4343
Set<String> scopes) {
@@ -157,7 +157,7 @@ private AuthorizationRequestUrlParameters(Builder builder) {
157157
String key = entry.getKey();
158158
String value = entry.getValue();
159159
if(requestParameters.containsKey(key)){
160-
log.warn("A query parameter {} has been provided with values multiple times.", key);
160+
LOG.warn("A query parameter {} has been provided with values multiple times.", key);
161161
}
162162
requestParameters.put(key, value);
163163
}
@@ -243,7 +243,7 @@ public Map<String, List<String>> requestParameters() {
243243
}
244244

245245
public Logger log() {
246-
return this.log;
246+
return LOG;
247247
}
248248

249249
public static class Builder {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public void handle(HttpExchange httpExchange) throws IOException {
5252
authorizationResultQueue.put(result);
5353

5454
} catch (InterruptedException ex) {
55-
LOG.error("Error reading response from socket: " + ex.getMessage());
55+
LOG.error("Error reading response from socket: {}", ex.getMessage());
5656
throw new MsalClientException(ex);
5757
} finally {
5858
httpExchange.close();

0 commit comments

Comments
 (0)