You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by "parakhnr (via GitHub)" <gi...@apache.org> on 2023/04/28 19:16:54 UTC
[GitHub] [helix] parakhnr opened a new pull request, #2462: Enabling INFO level for helix logs for TestClusterStatusMonitorLifecycle integration test.
parakhnr opened a new pull request, #2462:
URL: https://github.com/apache/helix/pull/2462
### Issues
- [X] My PR addresses the following Helix issues and references them in the PR description:
N/A.
**NOTE**: Since we aren't fixing the actual issue I am marking this as N/A
### Description
- [X] Here are some details about my PR, including screenshots of any UI changes:
We need more context to debug the failing `TestClusterStatusMonitorLifecyle` integration test as we aren't able to reproduce the issue locally. So, we enable `INFO` level logging for `org.apache.helix` package.
**NOTE:** This change will increase the log size for the tests from ~15 MB to ~50MB. We will revert the logging level back to `ERROR` once we fix the issue.
### Tests
- [X] The following tests are written for this issue:
N/A
### Changes that Break Backward Compatibility (Optional)
N/A
### Documentation (Optional)
N/A
### Commits
- My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Code Quality
- My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] parakhnr commented on a diff in pull request #2462: Enabling INFO level for helix logs for TestClusterStatusMonitorLifecycle integration test.
Posted by "parakhnr (via GitHub)" <gi...@apache.org>.
parakhnr commented on code in PR #2462:
URL: https://github.com/apache/helix/pull/2462#discussion_r1181903156
##########
helix-core/src/test/java/org/apache/helix/LogHelper.java:
##########
@@ -0,0 +1,26 @@
+package org.apache.helix;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+/**
+ * Helper class for Logging related stuff.
+ */
+public class LogHelper {
Review Comment:
Hmmm I made a new class, just so that in future if anybody needs to enable the logs for the test, they can simply use this class rather than starting from scratch. So, should I make it private or keep the helper class?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] junkaixue merged pull request #2462: Enabling INFO level for helix logs for TestClusterStatusMonitorLifecycle integration test.
Posted by "junkaixue (via GitHub)" <gi...@apache.org>.
junkaixue merged PR #2462:
URL: https://github.com/apache/helix/pull/2462
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] parakhnr commented on pull request #2462: Enabling INFO level for helix logs for TestClusterStatusMonitorLifecycle integration test.
Posted by "parakhnr (via GitHub)" <gi...@apache.org>.
parakhnr commented on PR #2462:
URL: https://github.com/apache/helix/pull/2462#issuecomment-1529925345
> Please excuse my ignorance, but isn't there resource settings for log4j which we can work with.
Yeah there is a way to configure the log level for particular package/class in `log4j2.properties`. However, there is no way to configure the log level for a particular test as the setting is applicable for all the tests within the module and enabling `INFO` level for `helix-core` tests is expensive as the log size grew to ~2.5GB. So, unfortunately programatically changing the log level is the only way out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] parakhnr commented on pull request #2462: Enabling INFO level for helix logs for TestClusterStatusMonitorLifecycle integration test.
Posted by "parakhnr (via GitHub)" <gi...@apache.org>.
parakhnr commented on PR #2462:
URL: https://github.com/apache/helix/pull/2462#issuecomment-1530493998
This PR is ready to merge, approved by @desaikomal.
Commit message:
Enabling INFO level for helix logs for TestClusterStatusMonitorLifecycle integration test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] junkaixue commented on a diff in pull request #2462: Enabling INFO level for helix logs for TestClusterStatusMonitorLifecycle integration test.
Posted by "junkaixue (via GitHub)" <gi...@apache.org>.
junkaixue commented on code in PR #2462:
URL: https://github.com/apache/helix/pull/2462#discussion_r1181900319
##########
helix-core/src/test/java/org/apache/helix/LogHelper.java:
##########
@@ -0,0 +1,26 @@
+package org.apache.helix;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+
+/**
+ * Helper class for Logging related stuff.
+ */
+public class LogHelper {
Review Comment:
This is not needed for new class creation. Just move the function as a private function in that test class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org