patch to mod_arm4

[Kai Engels <ka...@gmx.de>]

Hello,

I write to this address since I have made some changes to the mod_arm4 
module which I hope you will find usefull.

Patch it by unzipping the patch, changing into the mod_arm4 directory 
and say

 patch -p1 < mod_arm4_20052009.patch

Then change to the mod_arm4 source directory and say

autoreconf -ivf
 ./configure
 make
 make install

This should do the job. Check if apxs is in your PATH, otherwise use 
"--with-apxs=path/where/apxs/is".

--------------------------------------
My changes are:

1. added a configure structure to the module

2.  added a new metric to each transaction which allows to measure the 
amount of bytes sent during that transaction. I currently use the 
variable "bytes_sent" from the request_rec structure. I also tried 
"clength" but it seemed not to give as good results as "bytes sent". 
Maybe you guys can give me a hint which of them better suites my needs 
since I made my decision on "try and error" but was not sure what 
exactly each variable contains.

3. Added the environment variable "ARM_CORRELATOR" with the current 
stringified correlator to the "subprocess_env" table in order to be able 
to hand over the correlator to other processes (e.g. cgi scripts, 
databases etc.). The apache server transaction can then be identified as 
parent transaction  by these processes (in case they interpret this 
environment variable). I have currently instrumented sqlite and used the 
same mechanism for this purpose.

I did not fix yet all compiler warnings (since I am not sure what 
"warning: dereferencing type-punned pointer will break strict-aliasing 
rules" exactly means). How shall I handle future changes to this module 
(e.g. above mentioned compiler warnings)? Send them to this list again?

BTW: Is the arm4 module not listed on modules.apache.org?  Or did I just 
not see it?
Greetings
Kai

Re: patch to mod_arm4

[Kai Engels <ka...@gmx.de>]

Hi Bill,

>
> Kai,
> mod_arm4 is not on modules.apache.org.  

Ok, I am starting with a patch that adds the configure structure.
The code is almost not touched, I only removed some compiler warnings 
(unused variables etc.)
I also changed the README accordingly and added a file called arm4.conf 
which holds an example configuration.

Hope that is ok, even hope that a configure structure is wanted.
Other patches will follow..

Greetings
Kai

BTW: Why is it not on modules.apache.org?

Re: patch to mod_arm4

[Bill Stoddard <bi...@wstoddard.com>]

William A. Rowe, Jr. wrote:
> Bill Stoddard wrote:
> 
>> Bill Stoddard wrote:
>>
>>> Kai Engels wrote:
>>>
>>>> Hello,
>>>>
>>>> I write to this address since I have made some changes to the 
>>>> mod_arm4 module which I hope you will find usefull.
>>>>
>>
>> Hello Kai,
>> Your patch is difficult to review because there are too many unrelated 
>> changes. Please break the mod_arm4.c patch into smaller, independently 
>> reviewable patches.
> 
> 
> Kai, before you say 'OMG, they are making me jump through hoops just to
> offer back all the things I've fixed!!!'...
> 
> ...just know that Stoddard did the same thing to me 4+yrs ago, and now,
> heh, I'm how deep in Win32/Apache stuff?  Listen to FirstBill LOL.
> 
> Bill

Kai,
Just to drive the point home I'll get on my soap-box for a minute...

<soapbox>
One of the strengths of open source development is peer-review.  A thriving open source project enables many 
eyes to see every line of code going into the project which improves the quality of the project's code.  As 
open source developers (and you are an open source developer because you are submitting patches publically to 
an open source project development mailing list) we have the responsibility to the development community to 
make our patches as easy to review as possible.  If I see a 1000 line patch come across the mailing list that 
adds new functions, does some code reformatting, fixes a few bugs, etc., I and most of the other developers 
here are just going to ignore it. If a patch is more than I can understand in maybe 30 minutes, I'm not 
spending my time on it. That's why I suggested breaking your patch up into multiple self standing patches that 
are self contained and easy to understand/consume. I can't eat the whole thing at once, but send it in bite 
sized pieces and maybe I can help. Make sense?
</soapbox>

Thanks
Bill

Re: patch to mod_arm4

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

Bill Stoddard wrote:
> Bill Stoddard wrote:
> 
>> Kai Engels wrote:
>>
>>> Hello,
>>>
>>> I write to this address since I have made some changes to the 
>>> mod_arm4 module which I hope you will find usefull.
>>>
> 
> Hello Kai,
> Your patch is difficult to review because there are too many unrelated 
> changes. Please break the mod_arm4.c patch into smaller, independently 
> reviewable patches.

Kai, before you say 'OMG, they are making me jump through hoops just to
offer back all the things I've fixed!!!'...

...just know that Stoddard did the same thing to me 4+yrs ago, and now,
heh, I'm how deep in Win32/Apache stuff?  Listen to FirstBill LOL.

Bill

Re: patch to mod_arm4

[Kai Engels <ka...@gmx.de>]

Hi Bill,

>
> Patch 1: Add the environment variable "ARM_CORRELATOR" with the 
> current stringified correlator to the "subprocess_env" table
>
>
And the last one, adding the environemtn variable

Besides the added environment variable: I moved the addition of 
variables to any tables to the place where the stop_transaction is 
registered, i.e. the environment variable and the other variables are 
only "activated" in case the start_transaction was successfull.

The applying of patches shopuld work in any order (as far as I know 
patch can handle this), but the order of the pacthes is

1. configure structure
2. arm_destroy_app
3. added metric
4. ARM_CORRELATOR environment


Uff
Hope it works
Greetings
Kai

Now a beer :-)

Re: patch to mod_arm4

[Kai Engels <ka...@gmx.de>]

Hi Bill,

>
> Patch 3: bytes sent metric
>
Here is it. There is still the open question about the right variable to 
use:

2.  added a new metric to each transaction which allows to measure the 
amount of bytes sent during that transaction. I currently use the 
variable "bytes_sent" from the request_rec structure. I also tried 
"clength" but it seemed not to give as good results as "bytes sent". 
Maybe you guys can give me a hint which of them better suites my needs 
since I made my decision on "try and error" but was not sure what 
exactly each variable contains.

Any hints available? Thanks in advance
Greetings
Kai

Re: patch to mod_arm4

[Bill Stoddard <bi...@wstoddard.com>]

Kai Engels wrote:
> Hi Bill
> 
>>
>> Patch 2: Call to arm_destory_application
>>
>>
>>
> This is it. Just a call to arm destroy application() as the standard 
> awaits it
> Greetings
> Kai
> 
Committed.

Bill

Re: patch to mod_arm4

[Kai Engels <ka...@gmx.de>]

Hi Bill

>
> Patch 2: Call to arm_destory_application
>
>
>
This is it. Just a call to arm destroy application() as the standard 
awaits it
Greetings
Kai

Re: patch to mod_arm4

[Bill Stoddard <bi...@wstoddard.com>]

Bill Stoddard wrote:
> Kai Engels wrote:
> 
>> Hello,
>>
>> I write to this address since I have made some changes to the mod_arm4 
>> module which I hope you will find usefull.
>>

Hello Kai,
Your patch is difficult to review because there are too many unrelated changes. Please break the mod_arm4.c 
patch into smaller, independently reviewable patches.

Patch 1: Add the environment variable "ARM_CORRELATOR" with the current stringified correlator to the 
"subprocess_env" table

Patch 2: Call to arm_destory_application

Patch 3: bytes sent metric

Patch 4: ???

Compile warnings... I have no idea what this means:
"warning: dereferencing type-punned pointer will break strict-aliasing rules"
what compiler are you using?  I get no warning compiling mod_arm4 on RH/Suse Linux or Windows.

Bill

Re: patch to mod_arm4

[Bill Stoddard <bi...@wstoddard.com>]

Kai Engels wrote:
> Hello,
> 
> I write to this address since I have made some changes to the mod_arm4 
> module which I hope you will find usefull.
> 
> Patch it by unzipping the patch, changing into the mod_arm4 directory 
> and say
> 
> patch -p1 < mod_arm4_20052009.patch
> 
> Then change to the mod_arm4 source directory and say
> 
> autoreconf -ivf
> ./configure
> make
> make install
> 
> This should do the job. Check if apxs is in your PATH, otherwise use 
> "--with-apxs=path/where/apxs/is".
> 
> --------------------------------------
> My changes are:
> 
> 1. added a configure structure to the module
> 
> 2.  added a new metric to each transaction which allows to measure the 
> amount of bytes sent during that transaction. I currently use the 
> variable "bytes_sent" from the request_rec structure. I also tried 
> "clength" but it seemed not to give as good results as "bytes sent". 
> Maybe you guys can give me a hint which of them better suites my needs 
> since I made my decision on "try and error" but was not sure what 
> exactly each variable contains.
> 
> 3. Added the environment variable "ARM_CORRELATOR" with the current 
> stringified correlator to the "subprocess_env" table in order to be able 
> to hand over the correlator to other processes (e.g. cgi scripts, 
> databases etc.). The apache server transaction can then be identified as 
> parent transaction  by these processes (in case they interpret this 
> environment variable). I have currently instrumented sqlite and used the 
> same mechanism for this purpose.
> 
> I did not fix yet all compiler warnings (since I am not sure what 
> "warning: dereferencing type-punned pointer will break strict-aliasing 
> rules" exactly means). How shall I handle future changes to this module 
> (e.g. above mentioned compiler warnings)? Send them to this list again?
> 
> BTW: Is the arm4 module not listed on modules.apache.org?  Or did I just 
> not see it?
> Greetings
> Kai
> 

Kai,
mod_arm4 is not on modules.apache.org.  Thanks for the patch, I'll review it 'soon'. Bug me if I don't get to 
it within a week.

Thanks
Bill

Re: patch to mod_arm4

[Kai Engels <ka...@gmx.de>]

Bill Stoddard wrote:

> Kai Engels wrote:
>
>> HI,
>> just wanted to move this thread to the top, since there was no 
>> reaction to my patches
>> (I have splitted the one to 4 different) ?
>> Are they too bad :-) ? Or was it just forgotten ?
>>
>
> I've not forgotten, just really busy.  One of the patches I can commit 
> no problem. Need to look at the other 3.
>
> Bill
>
Hi Bill,
ok, no problem. I can wait.
Was just wondering...
:-)
Greetings
Kai

Re: patch to mod_arm4

[Bill Stoddard <bi...@wstoddard.com>]

Kai Engels wrote:
> HI,
> just wanted to move this thread to the top, since there was no reaction 
> to my patches
> (I have splitted the one to 4 different) ?
> Are they too bad :-) ? Or was it just forgotten ?
> 

I've not forgotten, just really busy.  One of the patches I can commit no problem. Need to look at the other 3.

Bill

Re: patch to mod_arm4

[Kai Engels <ka...@gmx.de>]

HI,
just wanted to move this thread to the top, since there was no reaction 
to my patches
(I have splitted the one to 4 different) ?
Are they too bad :-) ? Or was it just forgotten ?

Thanks in advance
Greetings
Kai



> Hello,
>
> I write to this address since I have made some changes to the mod_arm4 
> module which I hope you will find usefull.
>
> Patch it by unzipping the patch, changing into the mod_arm4 directory 
> and say
>
> patch -p1 < mod_arm4_20052009.patch
>
> Then change to the mod_arm4 source directory and say
>
> autoreconf -ivf
> ./configure
> make
> make install
>
> This should do the job. Check if apxs is in your PATH, otherwise use 
> "--with-apxs=path/where/apxs/is".
>
> --------------------------------------
> My changes are:
>
> 1. added a configure structure to the module
>
> 2.  added a new metric to each transaction which allows to measure the 
> amount of bytes sent during that transaction. I currently use the 
> variable "bytes_sent" from the request_rec structure. I also tried 
> "clength" but it seemed not to give as good results as "bytes sent". 
> Maybe you guys can give me a hint which of them better suites my needs 
> since I made my decision on "try and error" but was not sure what 
> exactly each variable contains.
>
> 3. Added the environment variable "ARM_CORRELATOR" with the current 
> stringified correlator to the "subprocess_env" table in order to be 
> able to hand over the correlator to other processes (e.g. cgi scripts, 
> databases etc.). The apache server transaction can then be identified 
> as parent transaction  by these processes (in case they interpret this 
> environment variable). I have currently instrumented sqlite and used 
> the same mechanism for this purpose.
>
> I did not fix yet all compiler warnings (since I am not sure what 
> "warning: dereferencing type-punned pointer will break strict-aliasing 
> rules" exactly means). How shall I handle future changes to this 
> module (e.g. above mentioned compiler warnings)? Send them to this 
> list again?
>
> BTW: Is the arm4 module not listed on modules.apache.org?  Or did I 
> just not see it?
> Greetings
> Kai
>