Re: [jira] Commented: (CAY-1464) Improve QueryLogger to log more information

[Evgeny Ryabitskiy <ev...@gmail.com>]

Hi!


* Looks like you patch will log all fetched objects? If this is the
* case, it is definitely a bad idea. There can be *lots* of them.

Yes... I would suggest to separate this into several levels INFO and
DEBUG, DEBUG is for logging fetched objects.
If it's not applicable we could move it in another Logger implementation.


* What is the benefit of passing start time instead of time delta in
* the logger? IMO the "old" API was cleaner, separating time measurement
* from logging.

Benefit is that we shouldn't do startTime - endTime if Logging is
disabled. -2 Operations (getCurrTime for endTime and operator "-"), so
low CPU is used.
I know it's minor optimization... but still... :) Unfortunately API
become little bit less clear.


* BTW, my other idea about was that we can define a very simple logger
API, while "advanced" loggers would take extra
* information about the current operation from the injectable
environment objects (or maybe from thread-bound Transaction).

Maybe you are right and I could resolve Connection from thread-bound
Transaction.. but it's to much binded on this Transaction object...


Evgeny.



2010/8/2 Andrus Adamchik <an...@objectstyle.org>:
> BTW, my other idea about was that we can define a very simple logger API,
> while "advanced" loggers would take extra information about the current
> operation from the injectable environment objects (or maybe from
> thread-bound Transaction).
>
> Andrus
>
> On Aug 2, 2010, at 11:50 AM, Andrus Adamchik wrote:
>
>> Hi Evgeny,
>>
>> On Aug 1, 2010, at 6:13 PM, Evgeny Ryabitskiy (JIRA) wrote:
>>>
>>> Evgeny Ryabitskiy commented on CAY-1464:
>>> ----------------------------------------
>>>
>>> + 1 for flexibility at this level.
>>>
>>> Now we should think about what to path to this events, I mean some
>>> interface for it.
>>>
>>> And I am still think that default Cayenne logger can be Improved without
>>> "incur much performance overhead", to provide nice logging "out of box".
>>> That was patch about.
>>
>>
>> Just took a brief look at the patch... I have some minor notes about error
>> messages and such, but here are a few things that I have doubts about:
>>
>> * Looks like you patch will log all fetched objects? If this is the case,
>> it is definitely a bad idea. There can be *lots* of them.
>> * What is the benefit of passing start time instead of time delta in the
>> logger? IMO the "old" API was cleaner, separating time measurement from
>> logging.
>>
>>> + There could be several build-in loggers that could be "turned on" just
>>> by one simple option.
>>>
>>> 1) Default Logger
>>> 2) Formatted logger (move format issues here).
>>> 3) Some supper logger that could provide lot's of output.
>>
>> Yep. DI container is already a reality, so switching to a DI-based logger
>> should be fairly starightforward (we'll need to figure out injection into
>> lower level objects underneath DataDomain). This is on my TODO list. If
>> somebody decides to do it before I get to it, please open a Jira and let me
>> know.
>>
>> Andrus
>>
>>
>
>

Re: [jira] Commented: (CAY-1464) Improve QueryLogger to log more information

[Andrus Adamchik <an...@objectstyle.org>]

On Aug 2, 2010, at 12:30 PM, Evgeny Ryabitskiy wrote:

> * Looks like you patch will log all fetched objects? If this is the
> * case, it is definitely a bad idea. There can be *lots* of them.
>
> Yes... I would suggest to separate this into several levels INFO and
> DEBUG, DEBUG is for logging fetched objects.
> If it's not applicable we could move it in another Logger  
> implementation.

Yes please, let's make it a separate logger.


> * What is the benefit of passing start time instead of time delta in
> * the logger? IMO the "old" API was cleaner, separating time  
> measurement
> * from logging.
>
> Benefit is that we shouldn't do startTime - endTime if Logging is
> disabled. -2 Operations (getCurrTime for endTime and operator "-"), so
> low CPU is used.
> I know it's minor optimization... but still... :) Unfortunately API
> become little bit less clear.

Yep. In a case of negligible performance optimization vs. clean API I  
always vote for the later. That's what ORM is about :-)


> * BTW, my other idea about was that we can define a very simple logger
> API, while "advanced" loggers would take extra
> * information about the current operation from the injectable
> environment objects (or maybe from thread-bound Transaction).
>
> Maybe you are right and I could resolve Connection from thread-bound
> Transaction.. but it's to much binded on this Transaction object...

What do you mean "too much"?

Andrus