-
Notifications
You must be signed in to change notification settings - Fork 129
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
UNOMI-846: improve log for event validation #687
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Despite the fact that I think changing this log levels directy in the code is a bad idea, mostly because of:
- security
- amount of log printed by default
- the fact that normally log level is configurable (should be configurable) outside the code.
There is inconsistent logs, I wonder why sometimes for a ValidationException we now sometimes log it as:
LOGGER.warn("An event was not valid - switch to DEBUG log level for more information.");
LOGGER.debug("{}", e.getMessage(), e);
and sometimes like this:
LOGGER.warn("{}", e.getMessage(), e);
Also I am not really sure about the wording in the message: "An event was not valid" in case of ValidationException, to me it's more a crash/error before/during the validation. Which is different than schema validation results.
I would rename the log to: "An error occurred during event validation", to make that difference, if the goal of thoses logs are to be clear for developers to have useful/understandable informations, they should be clear and consistent.
@@ -226,9 +228,9 @@ private Set<ValidationError> validate(JsonNode jsonNode, JsonSchema jsonSchema) | |||
Set<ValidationMessage> validationMessages = jsonSchema.validate(jsonNode); | |||
|
|||
if (LOGGER.isDebugEnabled() && !validationMessages.isEmpty()) { | |||
LOGGER.debug("Schema validation found {} errors while validating against schema: {}", validationMessages.size(), jsonSchema.getCurrentUri()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOGGER.isDebugEnabled()
should be removed
68e0c02
to
813211a
Compare
813211a
to
13c3aed
Compare
https://issues.apache.org/jira/browse/UNOMI-846