New logging design (was Re: [Box Backup-dev] COMMIT r285 - box/chris)

[Ben Summers]

On 31 Dec 2005, at 01:16, Chris Wilson wrote:

> Hi Martin,
>
>> I'm not sure why both TRACE and DEBUG are needed, nor why you'd  
>> use one
>> over the other. Have you plans for using these for specific things?
>
> No, I didn't have any specific plans, I'll drop the DEBUG level.

DEBUG might be a better description than TRACE, now that it's in a  
generic logging system.

Will there be a configure time option to compile in all DEBUG/TRACE  
loggings stuff, even in the release build?

Also, on a larger project I worked on, it was handy to have TRACEUSER  
macros which only emitted the line if the user compiling the project  
was as specified in the first argument.

>
>> Ok. I take it Filter() sets the minimum level that will be logged?  
>> Since
>> this is on each Logger class I presume it will be possible to set the
>> level differently for each output method?
>
> Yes, that was my intention. If you run box with "increased verbosity"
> from the command line, you probably want more detail on the  
> console, but
> not necessarily in the Windows event log. Similarly for a command-line
> "quiet" option. On the other hand, a single global level would be  
> easier
> to implement.

You could also implement an extra command in bbackupctl to change the  
logging level in a running bbackupd process, which would be handy  
when it's doing something you want to know more about.

[snip]
>
>>> A hidden static boolean to disable all logging temporarily, and a  
>>> class
>>> which you can create on the stack to disable logging and have it
>>> re-enabled automatically when the class is destroyed.
>>
>> Is this needed for a specific purpose?
>
> I have a general idea in mind, but no specific cases where I'd use it
> (yet), so I'll leave it out. I was thinking that some messages get
> double-logged on Win32 because function A (which knows the filename)
> calls function B (which knows the error code) and you need both pieces
> of information for debugging. Perhaps it would be better to add
> temporarily a special filtering logger to the vector, that captures  
> the
> log messages and prevents them being dispatched to the other loggers?

In my experience, it was handy to have this when writing the tests.  
Otherwise it can get very confusing.


>
>> Another question - I notice you branched for this off trunk again.  
>> But
>> you've also got another branch for the visual c stuff. What state is
>> that in - is it ready to be merged yet? Thinking you're just going to
>> end up creating yourself conflicts given the intrusiveness of the
>> logger change.
>
> I'd like to merge everything except the type changes, which need to be
> reviewed. I'm happy to sort out any conflicts, but I might delete the
> logging branch I took the other day and take a fresh one if trunk ends
> up changing significantly.

Is there a svn diff command that can be run to give a diff of the  
changes?

I suspect we need to do proper test runs on "all" platforms before  
merging it in. That's a lot of work, when I've done releases it's  
taken about a day to test everything properly.

BTW, below is a simple lock class for UNIX, in case you want to write  
something which is portable.

Ben


eg...

ThreadLock lock;

int fn()
{
	ThreadLock::Use use(lock);

	// do something interesting...

	return 0;
}



#ifndef THREADLOCK__H
#define THREADLOCK__H

#include <pthread.h>

class ThreadLock
{
public:
	ThreadLock()
	{
		::pthread_mutex_init(&mMutex, 0);
	}
	~ThreadLock()
	{
		::pthread_mutex_destroy(&mMutex);
	}
	
	class Use
	{
	public:
		Use(ThreadLock &rLock)
			: mrLock(rLock)
		{
			::pthread_mutex_lock(&rLock.mMutex);
		}
		~Use()
		{
			::pthread_mutex_unlock(&mrLock.mMutex);
		}
	
	private:
		ThreadLock &mrLock;
	};
	friend class Use;

private:
	pthread_mutex_t mMutex;
};

#endif // THREADLOCK__H