+1 best practices about when, how and what to log and also when how and what to return as event/error messages and also when and how handle exceptions and errors returned by services/delegator
Jacopo On Mar 14, 2012, at 8:01 AM, Adrian Crum wrote: > That sounds fine to me. We should also have a discussion about logging best > practices - like under what circumstances should the various logging levels > be used. > > -Adrian > > On 3/14/2012 6:54 AM, Jacopo Cappellato wrote: >> A couple of additional comments: >> >> * now that the commit has fixed all occurrencies, I would suggest that we do >> not use Debug.log(...) anymore: it internally uses the ALWAYS level and so >> it always prints the log, and this may not be obvious to the developer >> * I agree that this should be backported to all the active branches as well, >> for two reasons (in addition to the performance issue Ashish mentioned): >> ** I have noticed that some of the methods that were using Debug.log(...) >> were logging sensitive information (e.g. credit card processor >> transactions); in a production system there are good reasons for avoiding >> this information to be stored in unencrypted log files in the filesystem; >> the log is also visible in the webtools >> ** the risk of backporting bugs with this patch is relatively low >> >> Jacopo >> >> On Mar 14, 2012, at 7:52 AM, Ashish Vijaywargiya wrote: >> >>> Hello, >>> >>> I am thinking to backport similar changes that I did in this commit to >>> release branch 10.04 and 11.04. Such commit will not harm anyone on >>> production systems but can have better control on Debug.* message. >>> >>> This is kind of bug fix because few Debug.log( statements exists in the >>> code base that prints object of list and map. Generating console output and >>> writing such message on file system creates problem problem on production >>> system and can have system performance degraded for few seconds. >>> >>> Please let me know your thoughts on backporting this changes to release >>> branches. >>> Thanks in advance! >>> >>> -- >>> Ashish >>> >>> ---------- Forwarded message ---------- >>> From:<ash...@apache.org> >>> Date: Wed, Mar 14, 2012 at 11:37 AM >>> Subject: svn commit: r1300463 [1/3] - in /ofbiz/trunk: >>> applications/accounting/src/org/ofbiz/accounting/finaccount/ >>> applications/accounting/src/org/ofbiz/accounting/invoice/ >>> applications/accounting/src/org/ofbiz/accounting/payment/ >>> applications/accounting/src/o... >>> To: comm...@ofbiz.apache.org >>> >>> >>> Author: ashish >>> Date: Wed Mar 14 06:07:52 2012 >>> New Revision: 1300463 >>> >>> URL: http://svn.apache.org/viewvc?rev=1300463&view=rev >>> Log: >>> On production systems you can't suppress Debug.log( message by the use of >>> debug.properties file. It is always good to use Debug.* statements that are >>> having log level setup in debug.properties file. The real problem comes >>> with Debug.log( statement when you are printing any list or map object that >>> contains so many records(or data) in it. Here I am changing all the >>> occurrence of Debug.log( with Debug.logInfo(, Debug.logError( or >>> Debug.logWarning( so that we can have better control of Debug.* statements >>> on production system. :-) >>> >>> Modified: >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/invoice/InvoiceServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/GiftCertificateServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/payment/PaymentGatewayServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/tax/TaxAuthorityServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/clearcommerce/CCPaymentServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/cybersource/IcsPaymentServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/gosoftware/PcChargeServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/gosoftware/RitaApi.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/gosoftware/RitaServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/orbital/OrbitalPaymentServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/paypal/PayPalServices.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/sagepay/SagePayUtil.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/valuelink/ValueLinkApi.java >>> >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/valuelink/ValueLinkServices.java >>> ofbiz/trunk/applications/content/src/org/ofbiz/content/ConvertTree.java >>> ofbiz/trunk/applications/content/src/org/ofbiz/content/cms/CmsEvents.java >>> >>> ofbiz/trunk/applications/content/src/org/ofbiz/content/data/DataEvents.java >>> >>> ofbiz/trunk/applications/content/src/org/ofbiz/content/data/DataResourceWorker.java >>> >>> ofbiz/trunk/applications/content/src/org/ofbiz/content/survey/PdfSurveyServices.java >>> >>> ofbiz/trunk/applications/content/src/org/ofbiz/content/view/SimpleContentViewHandler.java >>> >>> ofbiz/trunk/applications/marketing/src/org/ofbiz/marketing/marketing/MarketingServices.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderLookupServices.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReadHelper.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderReturnServices.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/CheckOutEvents.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartItem.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/shipping/ShippingEvents.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppinglist/ShoppingListServices.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/thirdparty/paypal/ExpressCheckoutEvents.java >>> >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/thirdparty/zipsales/ZipSalesServices.java >>> >>> ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyServices.java >>> ofbiz/trunk/applications/party/src/org/ofbiz/party/party/PartyWorker.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryWorker.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/product/feature/ProductFeatureServices.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/product/inventory/InventoryServices.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/product/product/ProductWorker.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/product/store/ProductStoreWorker.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/packing/PackingServices.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/packing/PackingSession.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/packing/PackingSessionLine.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/picklist/PickListServices.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/shipment/ShipmentServices.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/dhl/DhlServices.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/fedex/FedexServices.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/ups/UpsServices.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsMockApiServlet.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServices.java >>> >>> ofbiz/trunk/applications/product/src/org/ofbiz/shipment/thirdparty/usps/UspsServicesTests.java >>> >>> ofbiz/trunk/applications/securityext/src/org/ofbiz/securityext/thirdparty/truition/TruitionCoReg.java >>> >>> ofbiz/trunk/applications/workeffort/src/org/ofbiz/workeffort/workeffort/WorkEffortServices.java >>> >>> ofbiz/trunk/specialpurpose/ebaystore/src/org/ofbiz/ebaystore/EbayEvents.java >>> >>> ofbiz/trunk/specialpurpose/ebaystore/src/org/ofbiz/ebaystore/EbayStore.java >>> >>> ofbiz/trunk/specialpurpose/ebaystore/src/org/ofbiz/ebaystore/EbayStoreAutoPreferences.java >>> >>> ofbiz/trunk/specialpurpose/ebaystore/src/org/ofbiz/ebaystore/EbayStoreCategoryFacade.java >>> >>> ofbiz/trunk/specialpurpose/ebaystore/src/org/ofbiz/ebaystore/EbayStoreHelper.java >>> >>> ofbiz/trunk/specialpurpose/ebaystore/src/org/ofbiz/ebaystore/EbayStoreOrder.java >>> >>> ofbiz/trunk/specialpurpose/googlebase/src/org/ofbiz/googlebase/ProductsExportToGoogle.java >>> >>> ofbiz/trunk/specialpurpose/googlecheckout/src/org/ofbiz/googlecheckout/GoogleRequestServices.java >>> ofbiz/trunk/specialpurpose/ldap/src/org/ofbiz/ldap/LdapLoginWorker.java >>> >>> ofbiz/trunk/specialpurpose/oagis/src/org/ofbiz/oagis/OagisInventoryServices.java >>> >>> ofbiz/trunk/specialpurpose/oagis/src/org/ofbiz/oagis/OagisShipmentServices.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/PosTransaction.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/component/PosButton.java >>> >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/component/PosButtonWrapper.java >>> >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/config/ButtonEventConfig.java >>> >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/device/impl/CashDrawer.java >>> >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/device/impl/Keyboard.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/device/impl/Msr.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/device/impl/Receipt.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/event/ManagerEvents.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/event/PaymentEvents.java >>> >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/event/SecurityEvents.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/event/TestEvents.java >>> >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/jpos/service/KeyboardService.java >>> >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/jpos/service/NullPosPrinter.java >>> >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/screen/ClientProfile.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/screen/LoadSale.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/screen/PosDialog.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/screen/PosScreen.java >>> ofbiz/trunk/specialpurpose/pos/src/org/ofbiz/pos/screen/SaveSale.java >>> ofbiz/trunk/specialpurpose/scrum/src/org/ofbiz/scrum/ScrumEvents.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/instance/Activity.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/instance/ActivityVariable.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/instance/Assignment.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/instance/EntityPersistentMgr.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/instance/Process.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/instance/ProcessMgr.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/instance/ProcessVariable.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/instance/Resource.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/mapping/EntityParticipantMap.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/repository/EntityRepositoryMgr.java >>> >>> ofbiz/trunk/specialpurpose/shark/src/org/ofbiz/shark/requester/LoggingRequester.java >>> >>> ofbiz/trunk/specialpurpose/webpos/src/org/ofbiz/webpos/transaction/WebPosTransaction.java >>> >>> Modified: >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java >>> URL: >>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java?rev=1300463&r1=1300462&r2=1300463&view=diff >>> ============================================================================== >>> --- >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java >>> (original) >>> +++ >>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/finaccount/FinAccountPaymentServices.java >>> Wed Mar 14 06:07:52 2012 >>> @@ -659,7 +659,7 @@ public class FinAccountPaymentServices { >>> "AccountingFinAccountExpired", >>> UtilMisc.toMap("thruDate", >>> finAccount.getTimestamp("thruDate")), locale)); >>> } >>> - Debug.log("Deposit into financial account #" + finAccountId + " [" >>> + amount + "]", module); >>> + Debug.logInfo("Deposit into financial account #" + finAccountId + >>> " [" + amount + "]", module); >>> >>> // get the previous balance >>> BigDecimal previousBalance = >>> finAccount.getBigDecimal("actualBalance");
