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

Null Throwable are generating repetitive logs in Console. #309

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

ndesai-newrelic
Copy link
Contributor

Copy link
Contributor

@cthomas-newrelic cthomas-newrelic left a comment

Choose a reason for hiding this comment

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

Why not just change the MessageValidator to not use IllegalArgumentException?

What behavior does the customer expect, and what happens when the Android Log.class throwable methods receive a null?

if (null != throwable) {
StringWriter sw = new StringWriter();
throwable.printStackTrace(new PrintWriter(sw));
message = String.format(Locale.getDefault(), "%s: %s", message, sw);
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't replace message with a non-null throwable. Both may be valid. Your condition should only be if throwable is null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changes done.

Copy link
Contributor

@cthomas-newrelic cthomas-newrelic left a comment

Choose a reason for hiding this comment

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

Based on the details, I expected this change to come down to changing the validator and probably emitting something like "(null)"when throwable is null.

} else {
StringWriter sw = new StringWriter();
throwable.printStackTrace(new PrintWriter(sw));
message = String.format(Locale.getDefault(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, message may be valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i am creating the message, we are creating it from attributes.

throwable.printStackTrace(new PrintWriter(sw));
if (null == throwable) {
logToAgent(logLevel, message);
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not abort the entire call is throwable is null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am just passing the message if the throwable is null. We don't need to validate the message because it is already getting validated in the logToAgent method.

Copy link
Contributor

@cthomas-newrelic cthomas-newrelic left a comment

Choose a reason for hiding this comment

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

🚢 it

@ndesai-newrelic
Copy link
Contributor Author

Based on the details, I expected this change to come down to changing the validator and probably emitting something like "(null)"when throwable is null.

android log is also not printing null for null throwable.

@ndesai-newrelic ndesai-newrelic merged commit 4876c52 into develop Nov 12, 2024
11 of 12 checks passed
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.

2 participants