Skip to content

Conversation

@rainerjung
Copy link

No description provided.

@victorhora victorhora requested a review from zimmerle May 16, 2019 19:01
@victorhora victorhora added the 2.x Related to ModSecurity version 2.x label May 16, 2019
@victorhora victorhora added this to the v2.9.4 milestone May 16, 2019
@zimmerle
Copy link
Contributor

Hi @rainerjung,

Sorry for the delay. Can you give us further details about why do you need such a resolution on the logs?
It will increase the processing time and size of the logs. Is that something that you can compute using the timing variables?

@rainerjung
Copy link
Author

Hi,
concerning processing time change, the patch only adds an integer operation plus integer format string in an existing snprintf(). No system calls, additional clock calls or similar are added. All data needed is already available in the existing code, it just isn't used when the timestamp is formatted.
All of the modern server components I am aware of provide sub second time resolution in their logs. Especially the Apache web server does. Adding the finer time resolution to mod_security logs allows a better understanding of processing sequences of Apache and the module. Yes, it will add 7 bytes per line (the separating dot and 6 digits), but typcally the lines are much longer, so 7 bytes per line is not a big increase.
I wrote the patch to help me doing analysis of some mod_security problems (understanding interaction between Apache und the module). For me it was very helpful. Due to its very little change of resource use and only small increase of log lines, I personally find it a low risk change.
Thanks and Regards,
Rainer

zimmerle pushed a commit that referenced this pull request Jan 15, 2021
@zimmerle
Copy link
Contributor

Sorry for the long delay @rainerjung. This is now merged! Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.x Related to ModSecurity version 2.x enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants