You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ondrej Sury <on...@sury.org> on 2005/10/02 20:19:22 UTC

Pluggable mod_log_config

Hi,

I am working on mod_log_spread for apache 2.0.  Old mod_log_spread for
apache 1.3 was based on some older version of mod_log_config and so it
is my new version based on (recent) mod_log_config from apache 2.0.54.

Right now any module which wants to implement custom logging has to
reimplement or reuse some parts of mod_log_config (at least log_*).

Maybe I am beating dead horse, but have you been thinking about
pluggable mod_log_config?  It should not be that hard, moving
ap_log_writer *log_writer and ap_log_writer_init *log_writer_init to
config_log_state structure and some modifications to code should do the
trick.  Then there will be hook for open_config_log, which will either
fill in config_log_state or DECLINE, and config_log_transaction will
call cls->write_log(r, cls->log_writer, ...);

Ondrej.
-- 
Ondrej Sury <on...@sury.org>

Re: Pluggable mod_log_config

Posted by Ondrej Sury <on...@sury.org>.
On Mon, 2005-10-03 at 10:26 -0400, Brian Akins wrote:
> Ondrej Sury wrote:
> 
> > 
> > Nope, (void *)cls->log_writer is pointer to log file descriptor.
> 
> this can be anything. (it's a void).  In some modules I have written, 
> it's a wrapper around a socket. See, open_config_log
> 
> cls->log_writer = log_writer_init(p, s, cls->fname);
> 
> log_writer_init retures whatever you function returns.  In 
> mod_log_config, it's an apr_file_t *.

Sorry for not being accurate.  My point was that cls->log_writer hold
logger *data/configuration* and not logging *function*.

> >>Also, init is called for each CustomLog config entry.
> > 
> > 
> > Yep, but only ap_log_writer_init is called.  You need to do something
> > like that:
> > 
> > foreach provider in ap_log_writer_providers {
> > 	cls->log_writer = provider->init(...., &cls->log_data);
> > 
> > 	if (cls->log_writer) {
> > 		break;
> > 	}
> > }
> > 
> > Then later when writing log you need to call
> > 
> > cls->log_writer(..., cls->log_data, ...)
> > 
> > instead of just
> > 
> > log_writer(...)
> 
> yes. basically.
> 
> I figure you could change cls to:
> 
> typedef struct {
>      const char *fname;
>      const char *format_string;
>      apr_array_header_t *format;
>      void *log_writer;
>      void *log_init; /*added*/
>      char *condition_var;
> } config_log_state;

What about:

typedef struct {
	const char *fname;
	const char *format_string;
	apr_array_header_t *format;
	ap_log_writer *log_writer; /* changed */
	ap_log_writer_init *log_writer_init; /* added */
	void *log_writer_config; /* changed from log_writer */
	char *condition_var;
} config_log_state;

If you set log writing function to log_writer then you are missing
config for this writer (ie. socket or apr_file_t * or mysql connection
or whatever else).

> and log_init and log_writer would be set at config time.  log_init would 
> get ran in open_logs.

Agree.

Ondrej.
-- 
Ondrej Sury <on...@sury.org>

Re: Pluggable mod_log_config

Posted by Brian Akins <br...@turner.com>.
Ondrej Sury wrote:

> 
> Nope, (void *)cls->log_writer is pointer to log file descriptor.

this can be anything. (it's a void).  In some modules I have written, 
it's a wrapper around a socket. See, open_config_log

cls->log_writer = log_writer_init(p, s, cls->fname);

log_writer_init retures whatever you function returns.  In 
mod_log_config, it's an apr_file_t *.

Currently, however, log_writer_init is global.  It should be per cls, or 
per server.


> 
>>Also, init is called for each CustomLog config entry.
> 
> 
> Yep, but only ap_log_writer_init is called.  You need to do something
> like that:
> 
> foreach provider in ap_log_writer_providers {
> 	cls->log_writer = provider->init(...., &cls->log_data);
> 
> 	if (cls->log_writer) {
> 		break;
> 	}
> }
> 
> Then later when writing log you need to call
> 
> cls->log_writer(..., cls->log_data, ...)
> 
> instead of just
> 
> log_writer(...)

yes. basically.

I figure you could change cls to:

typedef struct {
     const char *fname;
     const char *format_string;
     apr_array_header_t *format;
     void *log_writer;
     void *log_init; /*added*/
     char *condition_var;
} config_log_state;


and log_init and log_writer would be set at config time.  log_init would 
get ran in open_logs.


-- 
Brian Akins
Lead Systems Engineer
CNN Internet Technologies

Re: Pluggable mod_log_config

Posted by Ondrej Sury <on...@sury.org>.
On Mon, 2005-10-03 at 09:35 -0400, Brian Akins wrote:
> Ondrej Sury wrote:
> 
> > You also need to store which writer to use in each config_log_state.
> 
> This is already done.

Nope, (void *)cls->log_writer is pointer to log file descriptor.

> Also, init is called for each CustomLog config entry.

Yep, but only ap_log_writer_init is called.  You need to do something
like that:

foreach provider in ap_log_writer_providers {
	cls->log_writer = provider->init(...., &cls->log_data);

	if (cls->log_writer) {
		break;
	}
}

Then later when writing log you need to call

cls->log_writer(..., cls->log_data, ...)

instead of just

log_writer(...)

Ondrej.
-- 
Ondrej Sury <on...@sury.org>

Re: Pluggable mod_log_config

Posted by Brian Akins <br...@turner.com>.
Ondrej Sury wrote:

> You also need to store which writer to use in each config_log_state.

This is already done.

Also, init is called for each CustomLog config entry.

-- 
Brian Akins
Lead Systems Engineer
CNN Internet Technologies

Re: Pluggable mod_log_config

Posted by Ondrej Sury <on...@sury.org>.
On Mon, 2005-10-03 at 09:19 -0400, Brian Akins wrote:
> Ondrej Sury wrote:
> 
> > Even now when you want to make some log files buffered and some not you
> > are out of luck and this could also be solved by making plugging more
> > "general".
> 
> 
> Ok I see what you mean and I agree.  Maybe something stupid like a 
> SetLogHandler similar to SetHandler?  or as an argument to CustomLog?

I think that argument to CustomLog is ok...  you just need to check if
logger was already initialized or not (ie. return NULL or data structure
from ap_*_logger_init).  Each ap_*_logger_init function will be run in
some order (AP_HOOK_FIRST, etc.) and it will stop after logger will get
initialized.  Should not be that hard to implement.

> Hmmm... Maybe custom loggers could be providers that have an init and a 
> writer function.

Yep, something like that.

You also need to store which writer to use in each config_log_state.

I'll think about it more and came with some proposal (or patch :-).

Ondrej.
-- 
Ondrej Sury <on...@sury.org>

Re: Pluggable mod_log_config

Posted by Brian Akins <br...@turner.com>.
Akins, Brian wrote:

> Hmmm... Maybe custom loggers could be providers that have an init and a
> writer function.


Maybe have customLog be something like this:

CustomLog mysql://something common env=images
CustomLog file:///logs/my.log combined
CustomLog spread://somegroup refere
CustomLog buffer:///logs/other.log common

with file:// being the default.

Then to create a new log handler, just do a provider:

/*define init and writer function like normal*/

Then register your provider:

static const log_provider_t my_provider = {
   &my_init,
   &my_writer
};

static void register_hooks(apr_pool_t * p)
{
     ap_register_provider(p, LOG_PROVIDER_GROUP, "mysql", 
LOG_PROVIDER_VERSION,
                          &my_provider);
}


Thoughts?

-- 
Brian Akins
Lead Systems Engineer
CNN Internet Technologies

Re: Pluggable mod_log_config

Posted by Brian Akins <br...@turner.com>.
Ondrej Sury wrote:

> Even now when you want to make some log files buffered and some not you
> are out of luck and this could also be solved by making plugging more
> "general".


Ok I see what you mean and I agree.  Maybe something stupid like a 
SetLogHandler similar to SetHandler?  or as an argument to CustomLog?

Hmmm... Maybe custom loggers could be providers that have an init and a 
writer function.

-- 
Brian Akins
Lead Systems Engineer
CNN Internet Technologies

Re: Pluggable mod_log_config

Posted by Ondrej Sury <on...@sury.org>.
On Mon, 2005-10-03 at 07:58 -0400, Brian Akins wrote:
> 
> mod_log_config is "pluggable."   You can "replace" the normal logging 
> functions quite easily.

The word here is _replace_.  If you "replace" ap_log_writer and
ap_log_writer_init you need to implement all types of logging inside
ap_log_writer.

Imagine this type of config:
--cut here--
<VirtualHost 1.2.3.4>
  [...]
  CustomLog mysql://blah/data common

</VirtualHost>

<VirtualHost 1.2.3.5>
  [...]
  CustomLog $apache#1 combined # mod_log_spread syntax

</VirtualHost>

<VirtualHost 1.2.3.6>
  [...]
  CustomLog /var/log/apache2/access.log

</VirtualHost>
--cut here--

Then your replacement function would have to implement all those types
of log backends (ie. logging to mysql, spread, and files).  So you
cannot just easily add support for just another log backend.

Even now when you want to make some log files buffered and some not you
are out of luck and this could also be solved by making plugging more
"general".

Ondrej.
-- 
Ondrej Sury <on...@sury.org>

Re: Pluggable mod_log_config

Posted by Brian Akins <br...@turner.com>.

mod_log_config is "pluggable."   You can "replace" the normal logging 
functions quite easily.


Just do something like:


static apr_status_t my_logger_writer(request_rec *r,
                                            void *handle,
                                       const char **strs,
                                       int *strl,
                                       int nelts,
                                       apr_size_t len)

{
...

}

static void *my_logger_init(apr_pool_t *p, server_rec *s,
                                         const char* path)
{
/*return a log "handle"*/
}

static void register_hooks(apr_pool_t * p)
{

     set_writer_init = APR_RETRIEVE_OPTIONAL_FN(ap_log_set_writer_init);
     set_writer = APR_RETRIEVE_OPTIONAL_FN(ap_log_set_writer);


     set_writer_init(my_logger_init);
     set_writer(my_logger_writer);

}



-- 
Brian Akins
Lead Systems Engineer
CNN Internet Technologies