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.

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

Jump to bottom

Review logging, some code tidy up #1178

Merged
merged 1 commit into from
Jul 13, 2023
Merged

Review logging, some code tidy up #1178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion kafka-rest/src/main/java/io/confluent/kafkarest/DefaultKafkaRestContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class DefaultKafkaRestContext implements KafkaRestContext {
private SchemaRegistryClient schemaRegistryClient;

public DefaultKafkaRestContext(KafkaRestConfig config) {
log.debug("Creating context with config: {}", config);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't actually log the configuration at the moment due to it use default toString() of Object, but changing this just in case.

log.debug("Creating DefaultKafkaRestContext");
this.config = requireNonNull(config);
}

Expand Down
13 changes: 5 additions & 8 deletions kafka-rest/src/main/java/io/confluent/kafkarest/KafkaRestApplication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public KafkaRestApplication(
config,
path,
listenerName,
createRequestLog(config, requestLogWriter, requestLogFormat, log, listenerName));
createRequestLog(config, requestLogWriter, requestLogFormat, listenerName));

restResourceExtensions =
config.getConfiguredInstances(
Expand All @@ -113,7 +113,6 @@ private static RequestLog createRequestLog(
KafkaRestConfig config,
RequestLog.Writer requestLogWriter,
String requestLogFormat,
Logger log,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log is a static object, so we don't need to pass to this function

String listenerName) {
if (config.getBoolean(KafkaRestConfig.USE_CUSTOM_REQUEST_LOGGING_CONFIG)) {
log.info("For rest-app with listener {}, configuring custom request logging", listenerName);
Expand All @@ -127,12 +126,10 @@ private static RequestLog createRequestLog(
requestLogFormat = CustomRequestLog.EXTENDED_NCSA_FORMAT + " %{ms}T";
}

CustomLog customRequestLog =
new CustomLog(
requestLogWriter,
requestLogFormat,
new String[] {CustomLogRequestAttributes.REST_ERROR_CODE});
return customRequestLog;
return new CustomLog(
requestLogWriter,
requestLogFormat,
new String[] {CustomLogRequestAttributes.REST_ERROR_CODE});
}
// Return null, as Application's ctor would set-up a default request-logger.
return null;
Expand Down
2 changes: 1 addition & 1 deletion kafka-rest/src/main/java/io/confluent/kafkarest/converters/AvroConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public JsonNodeAndSize toJson(Object value) {
throw new ConversionException("Failed to convert Avro to JSON: " + e.getMessage());
} catch (RuntimeException e) {
// Catch-all since it's possible for, e.g., Avro to throw many different RuntimeExceptions
log.error("Unexpected exception convertion Avro to JSON: ", e);
log.error("Unexpected exception converting Avro to JSON: ", e);
throw new ConversionException("Failed to convert Avro to JSON: " + e.getMessage());
}
}
Expand Down
2 changes: 1 addition & 1 deletion kafka-rest/src/main/java/io/confluent/kafkarest/response/StreamingResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public final void resume(AsyncResponse asyncResponse) {
executorService.shutdownNow();
}
} catch (InterruptedException e) {
log.debug("Exception thrown when attempting to shutdown executorService {}", e);
log.debug("Exception thrown when attempting to shutdown executorService", e);
}
}
}
Expand Down