muthusuba / serf

Automatically exported from code.google.com/p/serf
Apache License 2.0
0 stars 0 forks source link

Allow runtime verbosity configuration #132

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Current implementation requires to switch "*_VERBOSE" flags in "serf_private.h" 
at compile time to be able to diagnose any connectivity or authentication 
troubles.

It will be great to be able to set these flags at runtime.

Here are proposals:
. add an API to do so, like neon has, so that Subversion may have a new 
"serf-debug-mask" http://subversion.tigris.org/issues/show_bug.cgi?id=4407
. read an environment variable providing verbosity flags as a binary mask

Original issue reported on code.google.com by ymartin1040 on 3 Oct 2013 at 2:46

GoogleCodeExporter commented 9 years ago
Discussion of this request has started a few weeks ago on the serf-dev mailing 
list:
https://groups.google.com/d/topic/serf-dev/2TNugwqCrpQ/discussion

How it should work in serf is more or less clear, implementation depends on one 
of us devs having time.

Original comment by lieven.govaerts@gmail.com on 3 Oct 2013 at 5:11

GoogleCodeExporter commented 9 years ago
Do you already have a detailed specification so that I implement it ? Or else 
you allow me to submit a first draft as a patch.

Original comment by ymartin1040 on 3 Oct 2013 at 7:19

GoogleCodeExporter commented 9 years ago
The discussion thread describes what we'd like to see in a complete 
implementation. This should include the requirements for Subversion.
The major missing part and probably the first place to start is the API we want 
serf to provide.

If you're interested to make a proposal in line with our reqs that'd be great!
Lieven

Original comment by lieven.govaerts@gmail.com on 4 Oct 2013 at 3:23

GoogleCodeExporter commented 9 years ago
  Hello Lieven,
I read the discussion but this is far beyond what I have expected to implement.
Here is the patch I have written and tested on current trunk.
To avoid to process all log occurrences, I have done a public/private mapping 
for the verbosity binary masks.
I probably made a mistake invoking "serf_read_env_logging" in 
"serf_create_context_ex" but I found no better place yet.

That patch is no perfect and it does not address many needs you have planned 
for logging. 

Feel free to do anything you think relevant with that code.
Yves

Original comment by ymartin1040 on 5 Oct 2013 at 7:58

Attachments:

GoogleCodeExporter commented 9 years ago
Hi Yves.

Just a quick note to say that I have reviewed your patch. There are some good 
things in it, but it fails to respect feature #9 (configure logging per 
context) in my mail (see above link to the discussion).
Now I don't blame you for that, making any feature in the buckets specific to a 
context is a fundamental change in the design of serf, more specifically the 
split between buckets and context. I'll try to propose a solution to this 
problem in the coming days. It's not the first time we are blocked by this 
issue. We have some ad hoc workarounds in place (see 
serf_request_bucket_request_create, serf_context_bucket_socket_create), but 
this are ugly and have to go.

Can you send this patch (as .txt like you have done now) to the serf-dev list? 
It's easier for me and other interest people to review and comment on specific 
parts of the patch when it's available inline in a mail.

thanks,
Lieven

Original comment by lieven.govaerts@gmail.com on 8 Oct 2013 at 8:50

GoogleCodeExporter commented 9 years ago
An API to enable logging at runtime was added to serf trunk recently.

Subversion trunk has in implementation to use this API, so we'll probably wait 
a bit to get some feedback and how it works, and then release it in serf 1.4.0.

Original comment by lieven.govaerts@gmail.com on 29 Nov 2013 at 9:32

GoogleCodeExporter commented 9 years ago
Looks like this is done, both in serf and in subversion.

Original comment by lieven.govaerts@gmail.com on 4 Feb 2014 at 8:15

GoogleCodeExporter commented 9 years ago
While we'll probably improve this further, the basic functionality+API is 
implemented.

Original comment by lieven.govaerts@gmail.com on 27 Apr 2014 at 9:16