Skip to content
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.

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

Fixed Inappropriate Logging and String.format Usage #22450

Merged
merged 4 commits into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ public void visitToken(DetailAST token) {
// logs error if the @Fluent method has 'throws' at the method declaration.
if (token.findFirstToken(TokenTypes.LITERAL_THROWS) != null) {
log(token, String.format(
"Fluent Method ''%s'' must not be declared to throw any checked exceptions"));
"Fluent Method ''%s'' must not be declared to throw any checked exceptions.",
alzimmermsft marked this conversation as resolved.
Show resolved Hide resolved
token.findFirstToken(TokenTypes.IDENT).getText()));
}
break;
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ private static ConfigurationSetting read(JsonNode node) {
return readSecretReferenceConfigurationSetting(node, baseSetting);
}
} catch (Exception exception) {
LOGGER.info(String.format(
"The setting is neither a 'FeatureFlagConfigurationSetting' nor 'SecretReferenceConfigurationSetting'"
+ ", return the setting as 'ConfigurationSetting', error is ", exception));
LOGGER.info("The setting is neither a 'FeatureFlagConfigurationSetting' nor "
+ "'SecretReferenceConfigurationSetting', return the setting as 'ConfigurationSetting'. "
+ "Error: ", exception);
}
return baseSetting;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public void addEvent(String eventName, Map<String, Object> traceEventAttributes,

Span currentSpan = Span.current();
if (currentSpan == null) {
logger.info("Failed to find a starting span to associate the %s with.", eventName);
logger.info("Failed to find a starting span to associate the {} with.", eventName);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public Mono<HttpResponse> process(HttpPipelineCallContext context, HttpPipelineN
context.getHttpRequest().setUrl(urlBuilder.setScheme(protocol).toUrl());
} catch (MalformedURLException e) {
return Mono.error(new RuntimeException(
String.format("Failed to set the HTTP request protocol to %d.", protocol), e));
String.format("Failed to set the HTTP request protocol to %s.", protocol), e));
}
}
return next.process();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ private boolean usePublicSetter(Object deserializedHeaders, ClientLogger logger)
Method setterMethod = deserializedHeaders.getClass().getDeclaredMethod(potentialSetterName, Map.class);
if (Modifier.isPublic(setterMethod.getModifiers())) {
setterMethod.invoke(deserializedHeaders, values);
logger.verbose("User setter %s on class %s to set header collection.", potentialSetterName,
logger.verbose("User setter {} on class {} to set header collection.", potentialSetterName,
deserializedHeaders.getClass().getSimpleName());
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ public static <T> T parse(String itemResponseBodyAsString, Class<T> itemClassTyp
return getSimpleObjectMapper().readValue(itemResponseBodyAsString, itemClassType);
} catch (IOException e) {
throw new IllegalStateException(
String.format("Failed to parse string [%s] to POJO.", itemResponseBodyAsString, e));
String.format("Failed to parse string [%s] to POJO.", itemResponseBodyAsString), e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,15 @@ private static boolean isSameCollection(PartitionKeyRange initiallyResolved, Par
*
* @param request Request in progress
* @param targetRange Target partition key range determined by address resolver
* @*/
* */
private void throwIfTargetChanged(RxDocumentServiceRequest request, PartitionKeyRange targetRange) {
// If new range is child of previous range, we don't need to throw any exceptions
// as LSNs are continued on child ranges.
if (request.requestContext.resolvedPartitionKeyRange != null &&
!isSameCollection(request.requestContext.resolvedPartitionKeyRange, targetRange)) {
if (!request.getIsNameBased()) {
String message = String.format(
"Target should not change for non name based requests. Previous target {}, Current {}",
"Target should not change for non name based requests. Previous target %s, Current %s",
request.requestContext.resolvedPartitionKeyRange, targetRange);
assert false : message;
logger.warn(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public void queryOrderByMixedTypes(String sortOrder) throws Exception {
// Ensure its a cross partition query
assertThat(partitionKeyRanges.size()).isGreaterThan(1);
// We are inserting documents with int, float, string, array, object and missing propMixed.
String query = String.format("SELECT * FROM r ORDER BY r.propMixed ", sortOrder);
String query = "SELECT * FROM r ORDER BY r.propMixed ";
CosmosQueryRequestOptions options = new CosmosQueryRequestOptions();
List<String> sourceIds = createdDocuments.stream()
.map(Resource::getId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ Mono<Response<Iterable<DigitalTwinsModelData>>> createModelsWithResponse(Iterabl
modelsPayload.add(mapper.readValue(model, Object.class));
}
catch (JsonProcessingException e) {
logger.error("Could not parse the model payload [%s]: %s", model, e);
logger.error("Could not parse the model payload [{}]: {}", model, e);
return Mono.error(e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ public void sendBatchPartitionKeyValidate() throws InterruptedException {
logger.info("Event[{}] matched. Countdown: {}", event.getSequenceNumber(), countDownLatch.getCount());
countDownLatch.countDown();
} else {
logger.warning(String.format("Event[%s] matched partition key, but not GUID. Expected: %s. Actual: %s",
event.getSequenceNumber(), messageValue, event.getProperties().get(MESSAGE_ID)));
logger.warning("Event[{}] matched partition key, but not GUID. Expected: {}. Actual: {}",
event.getSequenceNumber(), messageValue, event.getProperties().get(MESSAGE_ID));
}
}, error -> {
Assertions.fail("An error should not have occurred:" + error.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void testEventProcessorBuilderMissingProperties() {
+ "sequence number of event = " + eventContext.getEventData().getSequenceNumber());
})
.processError(errorContext -> {
System.out.printf("Error occurred in partition processor for partition {}, {}",
System.out.printf("Error occurred in partition processor for partition %s, %s%n",
errorContext.getPartitionContext().getPartitionId(),
errorContext.getThrowable());
})
Expand All @@ -69,7 +69,7 @@ public void testEventProcessorBuilderWithProcessEvent() {
+ "sequence number of event = " + eventContext.getEventData().getSequenceNumber());
})
.processError(errorContext -> {
System.out.printf("Error occurred in partition processor for partition %s, %s",
System.out.printf("Error occurred in partition processor for partition %s, %s%n",
errorContext.getPartitionContext().getPartitionId(),
errorContext.getThrowable());
})
Expand Down Expand Up @@ -98,7 +98,7 @@ public void testEventProcessorBuilderWithBothSingleAndBatchConsumers() {
});
}, 5, Duration.ofSeconds(1))
.processError(errorContext -> {
System.out.printf("Error occurred in partition processor for partition {}, {}",
System.out.printf("Error occurred in partition processor for partition %s, %s%n",
errorContext.getPartitionContext().getPartitionId(),
errorContext.getThrowable());
})
Expand All @@ -113,7 +113,7 @@ public void testEventProcessorBuilderWithNoProcessEventConsumer() {
.checkpointStore(new SampleCheckpointStore())
.consumerGroup("consumer-group")
.processError(errorContext -> {
System.out.printf("Error occurred in partition processor for partition {}, {}",
System.out.printf("Error occurred in partition processor for partition %s, %s%n",
errorContext.getPartitionContext().getPartitionId(),
errorContext.getThrowable());
})
Expand All @@ -134,7 +134,7 @@ public void testEventProcessorBuilderWithProcessEventBatch() {
});
}, 5, Duration.ofSeconds(1))
.processError(errorContext -> {
System.out.printf("Error occurred in partition processor for partition %s, %s",
System.out.printf("Error occurred in partition processor for partition %s, %s%n",
errorContext.getPartitionContext().getPartitionId(),
errorContext.getThrowable());
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,7 @@ protected void dispose(Closeable... closeables) {
try {
closeable.close();
} catch (IOException error) {
logger.error(String.format("[%s]: %s didn't close properly.", testName,
closeable.getClass().getSimpleName()), error);
logger.error("[{}]: {} didn't close properly.", testName, closeable.getClass().getSimpleName(), error);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ public Mono<AccessToken> getToken(TokenRequestContext request) {
+ "The Target Azure platform could not be determined from environment variables.")));
}
return managedIdentityServiceCredential.authenticate(request)
.doOnSuccess((t -> logger.info(String.format("Azure Identity => Managed Identity environment: %s",
managedIdentityServiceCredential.getEnvironment()))))
.doOnSuccess(t -> logger.info("Azure Identity => Managed Identity environment: {}",
managedIdentityServiceCredential.getEnvironment()))
.doOnNext(token -> LoggingUtil.logTokenSuccess(logger, request))
.doOnError(error -> LoggingUtil.logTokenError(logger, request, error));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,8 @@ Mono<PagedResponse<KeyVaultRoleDefinition>> listRoleDefinitionsFirstPage(String
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
.doOnRequest(ignored -> logger.verbose("Listing role definitions for roleScope - {}", roleScope))
.doOnSuccess(response -> logger.verbose("Listed role definitions for roleScope - {}", roleScope))
.doOnError(error ->
logger.warning(String.format("Failed to list role definitions for roleScope - %s", roleScope),
error))
.doOnError(error -> logger.warning("Failed to list role definitions for roleScope - {}", roleScope,
error))
.onErrorMap(KeyVaultAdministrationUtils::mapThrowableToKeyVaultAdministrationException)
.map(KeyVaultAccessControlAsyncClient::transformRoleDefinitionsPagedResponse);
} catch (RuntimeException e) {
Expand Down Expand Up @@ -633,9 +632,8 @@ Mono<PagedResponse<KeyVaultRoleAssignment>> listRoleAssignmentsFirstPage(String
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
.doOnRequest(ignored -> logger.verbose("Listing role assignments for roleScope - {}", roleScope))
.doOnSuccess(response -> logger.verbose("Listed role assignments for roleScope - {}", roleScope))
.doOnError(error ->
logger.warning(String.format("Failed to list role assignments for roleScope - %s", roleScope),
error))
.doOnError(error -> logger.warning("Failed to list role assignments for roleScope - {}", roleScope,
error))
.onErrorMap(KeyVaultAdministrationUtils::mapThrowableToKeyVaultAdministrationException)
.map(KeyVaultAccessControlAsyncClient::transformRoleAssignmentsPagedResponse);
} catch (RuntimeException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ private Mono<PagedResponse<CertificateProperties>> listCertificateVersionsFirstP
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
.doOnRequest(ignored -> logger.verbose("Listing certificate versions - {}", certificateName))
.doOnSuccess(response -> logger.verbose("Listed certificate versions - {}", certificateName))
.doOnError(error -> logger.warning(String.format("Failed to list certificate versions - {}", certificateName), error));
.doOnError(error -> logger.warning("Failed to list certificate versions - {}", certificateName, error));
} catch (RuntimeException ex) {
return monoError(logger, ex);
}
Expand Down Expand Up @@ -1454,7 +1454,7 @@ private Mono<PagedResponse<IssuerProperties>> listPropertiesOfIssuersFirstPage(C
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
.doOnRequest(ignored -> logger.verbose("Listing certificate issuers - {}"))
.doOnSuccess(response -> logger.verbose("Listed certificate issuers - {}"))
.doOnError(error -> logger.warning(String.format("Failed to list certificate issuers - {}"), error));
.doOnError(error -> logger.warning("Failed to list certificate issuers - {}", error));
} catch (RuntimeException ex) {
return monoError(logger, ex);
}
Expand Down Expand Up @@ -1580,7 +1580,7 @@ private Mono<PagedResponse<CertificateContact>> setCertificateContactsWithRespon
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
.doOnRequest(ignored -> logger.verbose("Listing certificate contacts - {}"))
.doOnSuccess(response -> logger.verbose("Listed certificate contacts - {}"))
.doOnError(error -> logger.warning(String.format("Failed to list certificate contacts - {}"), error));
.doOnError(error -> logger.warning("Failed to list certificate contacts - {}", error));
}

/**
Expand Down Expand Up @@ -1615,7 +1615,7 @@ private Mono<PagedResponse<CertificateContact>> listCertificateContactsFirstPage
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
.doOnRequest(ignored -> logger.verbose("Listing certificate contacts - {}"))
.doOnSuccess(response -> logger.verbose("Listed certificate contacts - {}"))
.doOnError(error -> logger.warning(String.format("Failed to list certificate contacts - {}"), error));
.doOnError(error -> logger.warning("Failed to list certificate contacts - {}", error));
} catch (RuntimeException ex) {
return monoError(logger, ex);
}
Expand Down Expand Up @@ -1652,7 +1652,7 @@ private Mono<PagedResponse<CertificateContact>> deleteCertificateContactsWithRes
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
.doOnRequest(ignored -> logger.verbose("Deleting certificate contacts - {}"))
.doOnSuccess(response -> logger.verbose("Deleted certificate contacts - {}"))
.doOnError(error -> logger.warning(String.format("Failed to delete certificate contacts - {}"), error));
.doOnError(error -> logger.warning("Failed to delete certificate contacts - {}", error));
}

/**
Expand Down
Loading