Skip to content

Consistently use SLF4J's recommended styles #992

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Avery-Dunn merged 8 commits into from
Sep 19, 2025
Merged

Consistently use SLF4J's recommended styles #992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7ccf720
In-progress logging refactor
Avery-Dunn Sep 12, 2025
81719c5
Adjust SLF4J logging to be consistent throughout the codebase
Avery-Dunn Sep 12, 2025
3f1bc42
Make logging consistent in tests
Avery-Dunn Sep 15, 2025
60f9304
Merge branch 'dev' into avdunn/consistent-logging
Avery-Dunn Sep 16, 2025
ac78cbb
Fix log reference
Avery-Dunn Sep 16, 2025
0e9a8f3
Update msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CloudShellMa…
Avery-Dunn Sep 17, 2025
d67fbcf
Update msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/MexParser.java
Avery-Dunn Sep 17, 2025
82cd008
Update msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/MexParser.java
Avery-Dunn Sep 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/AcquireTokenInteractiveIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ void acquireTokenInteractive_Ciam() {
result = pca.acquireToken(parameters).get();

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

Expand Down Expand Up @@ -236,7 +236,7 @@ private IAuthenticationResult acquireTokenInteractive(
result = pca.acquireToken(parameters).get();

} catch (Exception e) {
LOG.error("Error acquiring token with authCode: " + e.getMessage());
LOG.error("Error acquiring token with authCode: {}", e.getMessage());
throw new RuntimeException("Error acquiring token with authCode: " + e.getMessage());
}
return result;
Expand Down Expand Up @@ -266,7 +266,7 @@ private IAuthenticationResult acquireTokenInteractive_instanceAware(
result = pca.acquireToken(parameters).get();

} catch (Exception e) {
LOG.error("Error acquiring token with authCode: " + e.getMessage());
LOG.error("Error acquiring token with authCode: {}", e.getMessage());
throw new RuntimeException("Error acquiring token with authCode: " + e.getMessage());
}
return result;
Expand Down
4 changes: 2 additions & 2 deletions msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/AuthorizationCodeIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ private IAuthenticationResult acquireTokenAuthorizationCodeFlow(
.get();

} catch (Exception e) {
LOG.error("Error acquiring token with authCode: " + e.getMessage());
LOG.error("Error acquiring token with authCode: {}", e.getMessage());
throw new RuntimeException("Error acquiring token with authCode: " + e.getMessage());
}
return result;
Expand All @@ -219,7 +219,7 @@ private IAuthenticationResult acquireTokenInteractiveB2C(ConfidentialClientAppli
.build())
.get();
} catch (Exception e) {
LOG.error("Error acquiring token with authCode: " + e.getMessage());
LOG.error("Error acquiring token with authCode: {}", e.getMessage());
throw new RuntimeException("Error acquiring token with authCode: " + e.getMessage());
}
return result;
Expand Down
2 changes: 1 addition & 1 deletion msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/DeviceCodeIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ private void runAutomatedDeviceCodeFlow(DeviceCode deviceCode, User user) {
if (!isRunningLocally) {
SeleniumExtensions.takeScreenShot(seleniumDriver);
}
LOG.error("Browser automation failed: " + e.getMessage());
LOG.error("Browser automation failed: {}", e.getMessage());
throw new RuntimeException("Browser automation failed: " + e.getMessage());
}
}
Expand Down
24 changes: 12 additions & 12 deletions msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AadInstanceDiscoveryProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class AadInstanceDiscoveryProvider {
static final TreeSet<String> TRUSTED_HOSTS_SET = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
static final TreeSet<String> TRUSTED_SOVEREIGN_HOSTS_SET = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);

private static final Logger log = LoggerFactory.getLogger(AadInstanceDiscoveryProvider.class);
private static final Logger LOG = LoggerFactory.getLogger(AadInstanceDiscoveryProvider.class);

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

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

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

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

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

Expand Down Expand Up @@ -293,7 +293,7 @@ static String discoverRegion(MsalRequest msalRequest, ServiceBundle serviceBundl

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

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

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

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

Expand Down
5 changes: 2 additions & 3 deletions msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,8 @@ public ManagedIdentityResponse handleResponse(
return getSuccessfulResponse(response);
} else {
message = getMessageFromErrorResponse(response);
LOG.error(
String.format("[Managed Identity] request failed, HttpStatusCode: %s, Error message: %s",
response.statusCode(), message));
LOG.error("[Managed Identity] request failed, HttpStatusCode: {}, Error message: {}",
response.statusCode(), message);
throw new MsalServiceException(message, AuthenticationErrorCode.MANAGED_IDENTITY_REQUEST_FAILED, managedIdentitySourceType);
}
} catch (Exception e) {
Expand Down
2 changes: 1 addition & 1 deletion ...4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByClientCredentialSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ AuthenticationResult execute() throws Exception {

return supplier.execute();
} catch (MsalClientException ex) {
LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
LOG.debug("Cache lookup failed: {}", ex.getMessage());
return acquireTokenByClientCredential();
}
}
Expand Down
6 changes: 3 additions & 3 deletions msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByInteractiveFlowSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ private void updateRedirectUrl() {
try {
URI updatedRedirectUrl = new URI("http://localhost:" + httpListener.port());
interactiveRequest.interactiveRequestParameters().redirectUri(updatedRedirectUrl);
LOG.debug("Redirect URI updated to" + updatedRedirectUrl);
LOG.debug("Redirect URI updated to {}", updatedRedirectUrl);
} catch (URISyntaxException ex) {
throw new MsalClientException("Error updating redirect URI. Not a valid URI format",
AuthenticationErrorCode.INVALID_REDIRECT_URI);
Expand Down Expand Up @@ -204,7 +204,7 @@ private AuthorizationResult getAuthorizationResultFromHttpListener() {
long expirationTime;

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

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

Expand Down
10 changes: 5 additions & 5 deletions msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ AuthenticationResult execute() throws Exception {
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, CacheRefreshReason.CLAIMS);
}

LOG.debug(String.format("Refreshing access token. Cache refresh reason: %s", cacheRefreshReason));
LOG.debug("Refreshing access token. Cache refresh reason: {}", cacheRefreshReason);
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, cacheRefreshReason);
}
} catch (MsalClientException ex) {
if (ex.errorCode().equals(AuthenticationErrorCode.CACHE_MISS)) {
LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
LOG.debug("Cache lookup failed: {}", ex.getMessage());
return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, cacheRefreshReason);
} else {
LOG.error(String.format("Error occurred while cache lookup: %s", ex.getMessage()));
LOG.error("Error occurred while cache lookup: {}", ex.getMessage());
throw ex;
}
}
Expand All @@ -108,8 +108,8 @@ private AuthenticationResult fetchNewAccessTokenAndSaveToCache(TokenRequestExecu

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

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

ManagedIdentityResponse managedIdentityResponse = managedIdentityClient
.getManagedIdentityResponse(managedIdentityParameters);
Expand Down
2 changes: 1 addition & 1 deletion msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByOnBehalfOfSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ AuthenticationResult execute() throws Exception {

return supplier.execute();
} catch (MsalClientException ex) {
LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
LOG.debug("Cache lookup failed: {}", ex.getMessage());
return acquireTokenOnBehalfOf();
}
}
Expand Down
16 changes: 8 additions & 8 deletions msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenSilentSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

class AcquireTokenSilentSupplier extends AuthenticationResultSupplier {

private static final Logger log = LoggerFactory.getLogger(AcquireTokenSilentSupplier.class);
private static final Logger LOG = LoggerFactory.getLogger(AcquireTokenSilentSupplier.class);
private SilentRequest silentRequest;
protected static final int ACCESS_TOKEN_EXPIRE_BUFFER_IN_SEC = 5 * 60;

Expand Down Expand Up @@ -74,7 +74,7 @@ AuthenticationResult execute() throws Exception {
throw new MsalClientException(AuthenticationErrorMessage.NO_TOKEN_IN_CACHE, AuthenticationErrorCode.CACHE_MISS);
}

log.debug("Returning token from cache");
LOG.debug("Returning token from cache");

return res;
}
Expand All @@ -99,7 +99,7 @@ private AuthenticationResult makeRefreshRequest(AuthenticationResult cachedResul
refreshedResult.metadata().tokenSource(TokenSource.IDENTITY_PROVIDER);
refreshedResult.metadata().cacheRefreshReason(refreshReason);

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

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

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

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

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

Expand Down
Loading
Loading