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

fix: do not log data on JSON deserialization errors (MINOR) #6930

Merged
merged 3 commits into from
Feb 2, 2021

Conversation

vcrfxia
Copy link
Contributor

@vcrfxia vcrfxia commented Feb 2, 2021

Description

JsonParseExceptions are automatically populated with location information to indicate where the parse exception occurred. However, we don't want to log this from the ksqlDB JSON deserializer as it may result in topic data being logged. This PR clears the location data to avoid this.

Here's an example of what a stacktrace may look like prior to this PR:

Exception caught during Deserialization, taskId: 1_0, topic: my_topic, partition: 0, offset: 1901512
org.apache.kafka.common.errors.SerializationException: Failed to deserialize value from topic: my_topic. Illegal character ((CTRL-CHAR, code 0)): only regular white space (\r, \n, \t) is allowed between tokens
at [Source: (byte[])"REDACTED -- we don't want to log this"; line: 1, column: 2]
Caused by: com.fasterxml.jackson.core.JsonParseException: Illegal character ((CTRL-CHAR, code 0)): only regular white space (\r, \n, \t) is allowed between tokens
at [Source: (byte[])"REDACTED -- we don't want to log this"; line: 1, column: 2]

And here's what it looks like after this PR:

Exception caught during Deserialization, taskId: 1_0, topic: my_topic, partition: 0, offset: 1901512
org.apache.kafka.common.errors.SerializationException: Failed to deserialize value from topic: my_topic. Illegal character ((CTRL-CHAR, code 0)): only regular white space (\r, \n, \t) is allowed between tokens
Caused by: com.fasterxml.jackson.core.JsonParseException: Illegal character ((CTRL-CHAR, code 0)): only regular white space (\r, \n, \t) is allowed between tokens

Testing done

Added unit test.

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@vcrfxia vcrfxia requested a review from a team as a code owner February 2, 2021 05:40
Copy link
Contributor

@rodesai rodesai left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@guozhangwang guozhangwang left a comment

Choose a reason for hiding this comment

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

LGTM.

@vcrfxia vcrfxia merged commit 1834da6 into confluentinc:master Feb 2, 2021
@vcrfxia vcrfxia deleted the log-data-on-deser branch February 2, 2021 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants