I think we agree indeed. Doesn't mean we should start this discuss thread or write a arch guideline on the wiki somewhere. Maybe Ding Yuan wants to do a preliminary version of it?
In the meantime I don't think that it hurts for the present patch to do everything in debug and decide about higher levels needed later. regards, On Wed, Apr 2, 2014 at 10:11 PM, Alena Prokharchyk <alena.prokharc...@citrix.com> wrote: > 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 > -- Daan