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

reduce logging #821

Merged

Conversation

jmecosta
Copy link
Member

. silence parse stacktrace and instead print more meaningful information for the analysis
. clean some duplicated logging

@guwirth the current logging is printing stacktraces that are not meaninfull or not working. ex:

[14:39:57] : [Step 3/3] 14:39:57.857 ERROR - {}
[14:39:57] : [Step 3/3] org.sonar.cxx.preprocessor.CxxPreprocessor$MismatchException: null
[14:39:57] : [Step 3/3] at org.sonar.cxx.preprocessor.CxxPreprocessor.matchArgument(CxxPreprocessor.java:795) [cxx-squid-0.9.6-SNAPSHOT.jar:na]
[14:39:57] : [Step 3/3] at org.sonar.cxx.preprocessor.CxxPreprocessor.matchArguments(CxxPreprocessor.java:739) [cxx-squid-0.9.6-SNAPSHOT.jar:na]

For some reason when doing this:

throw new EvaluationException("Unknown expression type '" + nodeType + "'");

those dont show up in stacktrace. instead a 200 lines stack trace is printed, with this change a debug log of 160 MB went to 26 MB

@@ -575,8 +575,6 @@ PreprocessorAction handleIncludeLine(AstNode ast, Token token, String filename)
} finally {
currentFileState = globalStateStack.pop();
}
} else {
LOG.debug("[{}:{}]: skipping already included file '{}'", new Object[]{filename, token.getLine(), includedFile});
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 is not relevant at all, if already included then good. no need to ack.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok

@jmecosta jmecosta force-pushed the reduce-duplicated-logging branch 2 times, most recently from a0d1f67 to ebe6b6c Compare April 7, 2016 16:47
@jmecosta
Copy link
Member Author

jmecosta commented Apr 7, 2016

@guwirth can you check this please, want to know what is you feeling about this logging issue

@guwirth
Copy link
Collaborator

guwirth commented Apr 8, 2016

Hello @jmecosta,

In general I think it's a good idea to clean-up the logging. All changes which provide more meaningful information or remove redundant items are fine. This should focus mainly on non debug mode. In debug mode I would be more careful. Here I would keep most of the information independent how big the resulting LOG file is. It's the only way to give maintenance to other users.

I will add more detailed comments above.

Regards,
Günter

LOG.warn("Undefined functionlike macro '{}' assuming 0", macroName);
String value = "";

value = preprocessor.expandFunctionLikeMacro(macroName, restTokens);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be in one line?

String value = preprocessor.expandFunctionLikeMacro(macroName, restTokens);

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@guwirth
Copy link
Collaborator

guwirth commented Apr 8, 2016

@jmecosta review done

@jmecosta
Copy link
Member Author

jmecosta commented Apr 8, 2016

@guwirth thanks, i will update those and comment on the others

…more meaningful information for the analysis
@jmecosta
Copy link
Member Author

jmecosta commented Apr 9, 2016

@guwirth updated according with your cr comments. the only missing point is https://github.com/SonarOpenCommunity/sonar-cxx/pull/821/files#diff-1b5817791fecc29b992d6f86144b29d7L147 imo we will have time to detect regressions becuase of this once we start using it.

@guwirth guwirth added this to the 0.9.6 milestone Apr 9, 2016
@guwirth
Copy link
Collaborator

guwirth commented Apr 9, 2016

@jmecosta looks good but build with SQ 5.5-RC1 in Travis is failing?

@guwirth
Copy link
Collaborator

guwirth commented Apr 9, 2016

@jmecosta SQ 5.5-RC1 is another problem.
👍

@guwirth guwirth merged commit 50975c2 into SonarOpenCommunity:master Apr 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants