[Tickets #10773] Re: Crippling Memory Usage for certain Log entries

[bugs at horde.org]

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/10773
------------------------------------------------------------------------------
  Ticket             | 10773
  Updated By         | Jan Schneider <jan at horde.org>
  Summary            | Crippling Memory Usage for certain Log entries
  Queue              | Horde Framework Packages
  Version            | Git master
  Type               | Bug
  State              | Feedback
  Priority           | 3. High
  Milestone          |
  Patch              |
  Owners             |
------------------------------------------------------------------------------


Jan Schneider <jan at horde.org> (2011-12-16 11:56) wrote:

>> I think the solution here is to implement Request 10805, and then
>> when logging queries, log Stream resources as <Stream> rather than
>> their contents. Any objections?
>
> Unfortunately, I don't think that solves this issue.  Two major  
> problems jump out to me:
>
> 1. Someone can still pass in a multi-MB string.  It is perfectly  
> valid. and will always remain valid, according to the API.  This  
> solution maintains the status quo on this case.

Agreed.

> 2. Logging as <Stream> now prevents us from EVER debugging the  
> contents of the data values.  Just because it is a stream doesn't  
> mean we shouldn't log it (a stream could contain 1 byte of data).

AFAIU request 10805, streams would only be used for binary data,  
because this is where we expect the largest byte streams. And logging  
those wouldn't make sense.

> I still believe the solution needs to be a way of controlling the  
> log level at the DB package level, as opposed to the Log package  
> level.  E.g. an option that can be passed to the Db library that  
> allows three levels of data logging:
>
> * ALL: Log all entries
> * MAX_SIZE: Log all entries smaller than a certain size (1KB?  
> configurable?; use placeholders (e.g. "<Stream>", "<String>")  
> otherwise
> * NEVER: Never log entries; always use placeholders

This sounds to me like overcomplicating things. I suggest we only:
- add the option to disable query logging in Horde_Db
- enable this option in the factory when using DEBUG logging
- don't log Horde_Db_Value_Binary
- don't log streams when we implement this in the future