Daan, Correct me if I¹m wrong, but all of the logging added by Ding, fall under "to go with it or to indicate passing a certain code path². I¹ve just noticed that some of them were added with DEBUG, and some with WARN level, and wanted to correct that.
So we should: 1) For sure: never print them out in WARN as there is nothing admin should do in this case, because the code just handles them by ignoring. 2) Figure out what would be the correct level to log them with: INFO or DEBUG >From ³Logging best practices² articles, I can see that people use INFO as a ³storyline² of normal application behavior, and DEBUG for sort of information that helps you to track down the failure cases scenarios. To me, stuff added by Ding, falls under second category. But I might be wrong as I don¹t recall on the spot any discussions happening on the debug topic, from the mailing list. -Alena. On 4/2/14, 12:57 PM, "Daan Hoogland" <daan.hoogl...@gmail.com> wrote: >Alena, > >What I read in your comment is a description of INFO vs WARN. Debug >would be only for outputting stacktraces to go with it or to indicate >passing a certain code path. > >Agree? > >On Wed, Apr 2, 2014 at 8:31 PM, Alena Prokharchyk ><alena.prokharc...@citrix.com> wrote: >> >> ----------------------------------------------------------- >> This is an automatically generated e-mail. To reply, visit: >> https://reviews.apache.org/r/19917/#review39324 >> ----------------------------------------------------------- >> >> >> Is there a reason why logs for some exceptions are being logged in >>DEBUG mode, and some in WARN? From my point of view, if the code only >>catches it and doesn't error out, it should be logged in DEBUG. Lots of >>Admins are seeking for WARN statements in the log, and they might be >>confused seeing WARN w/o further failure or retry. >> >> - Alena Prokharchyk >> >> >> On April 2, 2014, 1:55 p.m., Ding Yuan wrote: >>> >>> ----------------------------------------------------------- >>> This is an automatically generated e-mail. To reply, visit: >>> https://reviews.apache.org/r/19917/ >>> ----------------------------------------------------------- >>> >>> (Updated April 2, 2014, 1:55 p.m.) >>> >>> >>> Review request for cloudstack. >>> >>> >>> Repository: cloudstack-git >>> >>> >>> Description >>> ------- >>> >>> This is the patch for JIRA-6242. See >>>https://issues.apache.org/jira/browse/CLOUDSTACK-6242 for more details. >>>Thanks! >>> >>> >>> Diffs >>> ----- >>> >>> >>>engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java >>>0d41bc1 >>> >>>engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerIm >>>pl.java 01508a4 >>> engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java >>>3e088db >>> >>>engine/orchestration/src/org/apache/cloudstack/engine/datacenter/entity/ >>>api/db/dao/EngineDataCenterDaoImpl.java 4b6818e >>> engine/schema/src/com/cloud/dc/dao/DataCenterDaoImpl.java ea5039f >>> engine/schema/src/com/cloud/host/dao/HostDaoImpl.java 426c90d >>> engine/schema/src/com/cloud/storage/dao/StoragePoolHostDaoImpl.java >>>e42eaf4 >>> engine/schema/src/com/cloud/storage/dao/VMTemplateDaoImpl.java >>>34fdca5 >>> engine/schema/src/com/cloud/upgrade/dao/Upgrade2214to30.java 58dd916 >>> engine/schema/src/com/cloud/vm/dao/ConsoleProxyDaoImpl.java 5e9c2f0 >>> engine/schema/src/com/cloud/vm/dao/SecondaryStorageVmDaoImpl.java >>>1f382d6 >>> >>>engine/storage/src/org/apache/cloudstack/storage/datastore/DataObjectMan >>>agerImpl.java 6ed1274 >>> >>>framework/ipc/src/org/apache/cloudstack/framework/serializer/OnwireClass >>>Registry.java 83c8a42 >>> >>>plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/discoverer/XcpServe >>>rDiscoverer.java 0ad6dc4 >>> >>>plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerC >>>onnectionPool.java b779085 >>> >>>plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerS >>>torageProcessor.java e512046 >>> >>>plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datas >>>tore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java af6a77a >>> server/src/com/cloud/resource/ResourceManagerImpl.java f9a59ba >>> server/src/com/cloud/server/ConfigurationServerImpl.java b8da4c8 >>> >>>services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyThu >>>mbnailHandler.java 06f21d3 >>> utils/src/com/cloud/utils/net/NetUtils.java 6350986 >>> >>> Diff: https://reviews.apache.org/r/19917/diff/ >>> >>> >>> Testing >>> ------- >>> >>> >>> Thanks, >>> >>> Ding Yuan >>> >>> >> > > > >-- >Daan
