[MGNLDAM-531] DamTemplatingFunctions spams logs with useless error messages Created: 17/Dec/14 Updated: 10/Feb/15 Resolved: 02/Feb/15 |
|
| Status: | Closed |
| Project: | Magnolia DAM Module |
| Component/s: | None |
| Affects Version/s: | 1.2.7 |
| Fix Version/s: | 1.2.8 |
| Type: | Improvement | Priority: | Major |
| Reporter: | Richard Unger | Assignee: | Christoph Meier |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | quickwin, requires_documentation, support | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Template: |
|
||||
| Acceptance criteria: |
Empty
|
||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||
| Date of First Response: | |||||
| Description |
|
Please remove the log statements that produce the following output: WARN info.magnolia.dam.asset.functions.DamTemplatingFunctions 16.12.2014 15:55:37 – AssetIdentifier is null or blank, return Null. This is completely useless in a production environment. 1) Please reduce the log priority to DEBUG, not WARN 2) Please include in the debug message the identifier that can't be found (if not null), or even better, include the current URI from the aggregation state as well. |
| Comments |
| Comment by Christoph Meier [ 08/Jan/15 ] |
|
Hello Richard could you please provide the Magnolia and or DAM version you are using when this log messages appear. |
| Comment by Christoph Meier [ 08/Jan/15 ] |
|
This issue is not part of DAM 2.0.x but of DAM 1.2.x. info.magnolia.dam.asset.functions.DamTemplatingFunctions exists on the DAM 1.2.x branch. So i guess propose the fix-version here must be 1.2.8 |
| Comment by Christoph Meier [ 08/Jan/15 ] |
|
Changes committed onto branch " For me it looks like this must not be ported to 2.0.x and master branch. |
| Comment by Magnolia International [ 23/Jan/15 ] |
|
If you're relying on debug logs to figure out those issues (which the patch indicates), then configure log4j to output the current uri. This patch introduces a dependency onto MgnlContext's static current context, which is not desirable here. Please revert. |
| Comment by Christoph Meier [ 02/Feb/15 ] |
|
@Richard Unger |
| Comment by Christoph Meier [ 03/Feb/15 ] |
|
Log level has been standardized for all methods. WARN for caught exceptions; DEBUG when returning null. |
| Comment by Richard Unger [ 10/Feb/15 ] |
|
Thanks for the fix for 5.2.10. We will install and test it. However, I think the 5.3 series will be affected by the same problems? It seems to me the getItem() functions all log WARNING messages, and don't differentiate by exception type. Please also change this warning to debug... |
| Comment by Richard Unger [ 10/Feb/15 ] |
|
@Gregory: Thanks for the feedback. I don't quite see the problem with using the MgnlContext object - that's what it is for, isn't it. I agree you need to be defensive, as there may not be an active WebContext, but this can be checked quite easily: ( ctx = MgnlContext.getWebContextOrNull(); if (ctx!=null) ... ) I will think about adding the URI to the log4j logs, that is a useful idea! Compared to adding it only to this log statement it seems a bit heavy-weight though... |