You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/03/31 08:35:43 UTC
[GitHub] [cloudstack] harikrishna-patnala opened a new pull request #4003:
Logging framework to use only log4j and wrapper slf4j
harikrishna-patnala opened a new pull request #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003
## Description
Currently CloudStack is using logging frameworks as log4j and Java util logging, logging wrappers as slf4j and Apache common logging.
Here to made it uniform, using only log4j framework and slf4j wrapper.
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [x] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#4003: Logging framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#discussion_r401884908
##########
File path: utils/src/test/java/com/cloud/utils/backoff/impl/ConstantTimeBackoffTest.java
##########
@@ -21,13 +21,12 @@
import java.util.HashMap;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
+import org.apache.log4j.Logger;
import org.junit.Assert;
import org.junit.Test;
public class ConstantTimeBackoffTest {
- final static private Log LOG = LogFactory.getLog(ConstantTimeBackoffTest.class);
+ private static final Logger s_logger = Logger.getLogger(ConstantTimeBackoffTest.class.getName());
Review comment:
LOG
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#4003: Logging framework to use only log4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4003: Logging framework to use only log4j
URL: https://github.com/apache/cloudstack/pull/4003#discussion_r405309615
##########
File path: framework/managed-context/src/main/java/org/apache/cloudstack/managed/context/impl/DefaultManagedContext.java
##########
@@ -33,7 +32,7 @@
public class DefaultManagedContext implements ManagedContext {
- private static final Logger log = LoggerFactory.getLogger(DefaultManagedContext.class);
+ private static final Logger log = Logger.getLogger(DefaultManagedContext.class);
Review comment:
again, next iteration: private static final Logger **_LOG_**
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#4003: Logging framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#discussion_r401884652
##########
File path: utils/src/main/java/org/apache/commons/httpclient/contrib/ssl/EasyX509TrustManager.java
##########
@@ -58,7 +57,7 @@
private X509TrustManager standardTrustManager = null;
/** Log object for this class. */
- private static final Log LOG = LogFactory.getLog(EasyX509TrustManager.class);
+ private static final Logger s_logger = Logger.getLogger(EasyX509TrustManager.class.getName());
Review comment:
please rename
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] harikrishna-patnala commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-609768926
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#4003: Logging framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#discussion_r402939700
##########
File path: utils/src/main/java/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java
##########
@@ -44,7 +41,7 @@
public class ConstantTimeBackoff extends AdapterBase implements BackoffAlgorithm, ConstantTimeBackoffMBean {
long _time;
private final Map<String, Thread> _asleep = new ConcurrentHashMap<String, Thread>();
- private final static Log LOG = LogFactory.getLog(ConstantTimeBackoff.class);
+ private static final Logger s_logger = Logger.getLogger(ConstantTimeBackoff.class.getName());
Review comment:
no, please, the s_ is an old convention. LOGGER is ok, but LOG as well. I prefer the latter, but as long as we standardise on something I'm fine with it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] harikrishna-patnala commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-610163841
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-608382934
> > I think the name of this PR is wrong, is it?
> > "replace slf4j to standardise on log4j"
>
> But at very less places we are using slf4j. In order to replace slf4j to standardise log4j, we need to change almost all the files to use slf4j. I kept this name "Logging framework to use only log4j and wrapper slf4j" because java util logging framework and apache common logging wrapper are used and we removed them now.
I want to not use slf4j anywhere, but standardise on log4j. It is used in the far majority of places and needs to be standardised to, to be able to reduce the upgrade working needed in the future. Both slf4j and commons-logging should be phased out completely.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull
request #4003: Logging framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#discussion_r402044408
##########
File path: utils/src/main/java/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java
##########
@@ -44,7 +41,7 @@
public class ConstantTimeBackoff extends AdapterBase implements BackoffAlgorithm, ConstantTimeBackoffMBean {
long _time;
private final Map<String, Thread> _asleep = new ConcurrentHashMap<String, Thread>();
- private final static Log LOG = LogFactory.getLog(ConstantTimeBackoff.class);
+ private static final Logger s_logger = Logger.getLogger(ConstantTimeBackoff.class.getName());
Review comment:
changed it to capitals but used S_LOGGER since this is the name we are using everywhere.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-610191564
@harikrishna-patnala a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] harikrishna-patnala commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-610191309
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-610170060
Packaging result: ✔centos7 ✔debian. JID-1137
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-607470581
I think the name of this PR is wrong, is it?
"replace slf4j to standardise on log4j"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-609769512
@harikrishna-patnala a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#4003: Logging framework to use only log4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4003: Logging framework to use only log4j
URL: https://github.com/apache/cloudstack/pull/4003#discussion_r405309875
##########
File path: framework/spring/lifecycle/src/main/java/org/apache/cloudstack/spring/lifecycle/CloudStackExtendedLifeCycle.java
##########
@@ -40,7 +39,7 @@
public class CloudStackExtendedLifeCycle extends AbstractBeanCollector {
- private static final Logger log = LoggerFactory.getLogger(CloudStackExtendedLifeCycle.class);
+ private static final Logger log = Logger.getLogger(CloudStackExtendedLifeCycle.class);
Review comment:
LOG
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] harikrishna-patnala commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-607614642
changed all the variable names to capitals but used S_LOGGER since this is the name we are using everywhere.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4003: Logging
framework to use only log4j
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4003: Logging framework to use only log4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-610200476
Packaging result: ✔centos7 ✔debian. JID-1138
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] harikrishna-patnala commented on issue #4003: Logging
framework to use only log4j
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on issue #4003: Logging framework to use only log4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-610786098
@DaanHoogland , I have updated this PR with a commit which removes slf4j completely. Can you please take a relook.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] harikrishna-patnala commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-609651100
> > > I think the name of this PR is wrong, is it?
> > > "replace slf4j to standardise on log4j"
> >
> >
> > But at very less places we are using slf4j. In order to replace slf4j to standardise log4j, we need to change almost all the files to use slf4j. I kept this name "Logging framework to use only log4j and wrapper slf4j" because java util logging framework and apache common logging wrapper are used and we removed them now.
>
> I want to not use slf4j anywhere, but standardise on log4j. It is used in the far majority of places and needs to be standardised to, to be able to reduce the upgrade working needed in the future. Both slf4j and commons-logging should be phased out completely.
Okay. let me try taking out slf4j. It is involved in setting contextid and some other places. I'll make the changes and update the PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] harikrishna-patnala commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-607614489
> I think the name of this PR is wrong, is it?
> "replace slf4j to standardise on log4j"
But at very less places we are using slf4j. In order to replace slf4j to standardise log4j, we need to change almost all the files to use slf4j. I kept this name "Logging framework to use only log4j and wrapper slf4j" because java util logging framework and apache common logging wrapper are used and we removed them now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#4003: Logging framework to use only log4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4003: Logging framework to use only log4j
URL: https://github.com/apache/cloudstack/pull/4003#discussion_r405309227
##########
File path: framework/managed-context/src/main/java/org/apache/cloudstack/managed/context/ManagedContextRunnable.java
##########
@@ -18,16 +18,15 @@
*/
package org.apache.cloudstack.managed.context;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.apache.log4j.Logger;
import org.apache.cloudstack.managed.context.impl.DefaultManagedContext;
public abstract class ManagedContextRunnable implements Runnable {
private static final int SLEEP_COUNT = 120;
- private static final Logger log = LoggerFactory.getLogger(ManagedContextRunnable.class);
+ private static final Logger log = Logger.getLogger(ManagedContextRunnable.class);
Review comment:
maybe not for this iteration, but this var name should be all upper i.e. LOG
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#4003: Logging framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#discussion_r401884532
##########
File path: utils/src/main/java/org/apache/commons/httpclient/contrib/ssl/EasySSLProtocolSocketFactory.java
##########
@@ -88,7 +87,7 @@
public class EasySSLProtocolSocketFactory implements ProtocolSocketFactory {
/** Log object for this class. */
- private static final Log LOG = LogFactory.getLog(EasySSLProtocolSocketFactory.class);
+ private static final Logger s_logger = Logger.getLogger(EasySSLProtocolSocketFactory.class.getName());
Review comment:
please rename to LOG
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#4003: Logging framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#discussion_r401884130
##########
File path: utils/src/main/java/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java
##########
@@ -44,7 +41,7 @@
public class ConstantTimeBackoff extends AdapterBase implements BackoffAlgorithm, ConstantTimeBackoffMBean {
long _time;
private final Map<String, Thread> _asleep = new ConcurrentHashMap<String, Thread>();
- private final static Log LOG = LogFactory.getLog(ConstantTimeBackoff.class);
+ private static final Logger s_logger = Logger.getLogger(ConstantTimeBackoff.class.getName());
Review comment:
name convention for private statics is all capitals so LOG is a better name.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-609781665
Packaging result: ✖centos7 ✔debian. JID-1130
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #4003: Logging
framework to use only log4j and wrapper slf4j
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #4003: Logging framework to use only log4j and wrapper slf4j
URL: https://github.com/apache/cloudstack/pull/4003#issuecomment-610164019
@harikrishna-patnala a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services