You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Thomas Meyer <th...@m3y3r.de> on 2024/09/21 10:44:51 UTC

Friendly nudge: PR #429 "mod_log_config: Add log_formatter support and a JSON log_formatter" 📝

Hi all,

I hope you're all doing well and keeping your servers running smoothly! 🚀

A little while ago, I submitted PR #429 to add support for log_formatter and a shiny new JSON log formatter to mod_log_config. It's been sitting quietly, waiting for a bit of love (or at least a review), but it seems everyone has been busy keeping the web safe and sound, so it hasn't gotten much attention.

I know you're all swamped, but if anyone has a moment to take a quick look, I'd really appreciate it! The logs are patiently waiting to be formatted in style (and JSON!). If there’s anything I can do to help move it along or make it easier to review, feel free to let me know.

Thanks a lot for your time! I'll buy the first round of virtual coffee ☕ for anyone who can help push this one through. 😉

Best regards,
Thomas

Am 9. September 2024 08:41:44 MESZ schrieb Thomas Meyer <th...@m3y3r.de>:
>
>
>Am 30. Juli 2024 23:28:20 MESZ schrieb Thomas Meyer <th...@m3y3r.de>:
>>Am Sun, Jun 02, 2024 at 04:08:06PM +0200 schrieb Thomas Meyer:
>
>Good day!
>
>Summer seems to come to an end this week in Germany, a lot of rain and colder weather is forecasted this week, so a good chance to greet the beginning autumn and to make yourself a cup of hot chocolate and review some pull requests, like this one:
>
>>> https://github.com/apache/httpd/pull/429
>
>
>>> Mfg
>>> Thomas

-- 
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

Re: Friendly nudge: PR #429 "mod_log_config: Add log_formatter support and a JSON log_formatter" 📝

Posted by Thomas Meyer <th...@m3y3r.de>.
Hi,

Quick question how to raise a pull request or how to get code into the Apache httpd test framework repo? I have some ideas and fixes for this repo.

Mfg
Thomas

Am 21. September 2024 15:49:49 MESZ schrieb Eric Covener <co...@gmail.com>:
>On Sat, Sep 21, 2024 at 6:46 AM Thomas Meyer <th...@m3y3r.de> wrote:
>>
>> Hi all,
>>
>> I hope you're all doing well and keeping your servers running smoothly! 🚀
>>
>> A little while ago, I submitted PR #429 to add support for log_formatter and a shiny new JSON log formatter to mod_log_config. It's been sitting quietly, waiting for a bit of love (or at least a review), but it seems everyone has been busy keeping the web safe and sound, so it hasn't gotten much attention.
>>
>> I know you're all swamped, but if anyone has a moment to take a quick look, I'd really appreciate it! The logs are patiently waiting to be formatted in style (and JSON!). If there’s anything I can do to help move it along or make it easier to review, feel free to let me know.
>>
>> Thanks a lot for your time! I'll buy the first round of virtual coffee ☕ for anyone who can help push this one through. 😉
>
>
>a few minor requests after paging through again:
>
>1. doc (mod_log_config.xml, logs.xml -- not just for the obvious
>reasons, but it helps reviewers too)
>2. changelog entry (is the only expected change that you can pick JSON
>or are there other subtle things to declare?)
>3. Some of the TODOS might be a little alarming to reviewers, it is
>unclear which ones are status quo for example
>4. it's a massive pain to get started,  but test coverage in
>https://svn.apache.org/repos/asf/httpd/test/framework/trunk .  I will
>try to find some time to create a skeletal test for json and non-json
>just so the code is actually running.
>5. replace "gruppenwechsel" in comments

-- 
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.

Re: Friendly nudge: PR #429 "mod_log_config: Add log_formatter support and a JSON log_formatter"

Posted by Thomas Meyer <th...@m3y3r.de>.
On Sat, Sep 21, 2024 at 09:49:49AM -0400, Eric Covener wrote:
> On Sat, Sep 21, 2024 at 6:46 AM Thomas Meyer <th...@m3y3r.de> wrote:
> 
> a few minor requests after paging through again:
> 
> 4. it's a massive pain to get started,  but test coverage in
> https://svn.apache.org/repos/asf/httpd/test/framework/trunk .  I will
> try to find some time to create a skeletal test for json and non-json
> just so the code is actually running.

Hi,

to get some early feedback here some ideas regarding test coverage for
mod_log_config, it's still unfinished but I hope you get the idea what I
am aming for:

https://github.com/apache/httpd-tests/compare/trunk...thomasmey:httpd-tests:tests/mod-log-config?expand=1

With this new test I get around 74% code coverage on mod_log_config.

Two questions from my side:
1.) Not sure how to test the buffered log code, as this probably needs a
separate httpd run as this is a global config
2.) Do anybody has an idea how to test the "request line" contains a
password functionality in log_request_line? How would an HTTP request
look like that triggers this code path? Is that even reachable by normal
http requests or only relevant for mod_ftp or mod_proxy or something
like this?

Please also see this PR https://github.com/apache/httpd/pull/494
which runs httpd in the perl tests under code coverage and uploads the
results as Github Action to the workflow.

Mfg
thomas

Re: Friendly nudge: PR #429 "mod_log_config: Add log_formatter support and a JSON log_formatter"

Posted by Ruediger Pluem <rp...@apache.org>.

On 7/29/25 6:00 PM, Thomas Meyer wrote:
> Hi,
> 
> although I was told this project is not dead, it doesn't feel that way...
> 
> how to get my PRs merged? I did receive no feedback after my last adjustments :-(

Sorry for the long silence and thanks for your reminder. I made some comments on the PR
regarding the actual code.
But if I think a bit further I think the current abstraction via the ap_log_formatted_data
callback is not flexible enough. Instead the provider API (see include/ap_provider.h) should
be used to enable an easy implementation of further providers in separate modules.
The standard rendering (rendering the format string as a string) and your json rendering could
then be implemented as such providers with the json provider moved into a separate module.
The provider implementing the standard rendering would be taken by default if no provider is specified.
This could add further value given the AP_ERRORLOG_PROVIDER_GROUP we have in trunk and which
allows more sophisticated log writer providers to be implemented that might like to get the data they
log in a more binary form.

Regards

Rüdiger


Re: Friendly nudge: PR #429 "mod_log_config: Add log_formatter support and a JSON log_formatter"

Posted by Thomas Meyer <th...@m3y3r.de>.
Hi,

although I was told this project is not dead, it doesn't feel that way...

how to get my PRs merged? I did receive no feedback after my last  
adjustments :-(

mfg
thomas


> On Sat, Sep 21, 2024 at 09:49:49AM -0400, Eric Covener wrote:
>> On Sat, Sep 21, 2024 at 6:46 AM Thomas Meyer <th...@m3y3r.de> wrote:
>> >
>> > Hi all,
>
> Hi,
>
>> >
>> > I hope you're all doing well and keeping your servers running smoothly! 🚀
>> >
>> > A little while ago, I submitted PR #429 to add support for  
>> log_formatter and a shiny new JSON log formatter to mod_log_config.  
>> It's been sitting quietly, waiting for a bit of love (or at least a  
>> review), but it seems everyone has been busy keeping the web safe  
>> and sound, so it hasn't gotten much attention.
>> >
>> > I know you're all swamped, but if anyone has a moment to take a  
>> quick look, I'd really appreciate it! The logs are patiently  
>> waiting to be formatted in style (and JSON!). If there’s anything I  
>> can do to help move it along or make it easier to review, feel free  
>> to let me know.
>> >
>> > Thanks a lot for your time! I'll buy the first round of virtual  
>> coffee ☕ for anyone who can help push this one through. 😉
>>
>>
>> a few minor requests after paging through again:
>>
>> 1. doc (mod_log_config.xml, logs.xml -- not just for the obvious
>> reasons, but it helps reviewers too)
>> 2. changelog entry (is the only expected change that you can pick JSON
>> or are there other subtle things to declare?)
>> 3. Some of the TODOS might be a little alarming to reviewers, it is
>> unclear which ones are status quo for example
>> 4. it's a massive pain to get started,  but test coverage in
>> https://svn.apache.org/repos/asf/httpd/test/framework/trunk .  I will
>> try to find some time to create a skeletal test for json and non-json
>> just so the code is actually running.
>> 5. replace "gruppenwechsel" in comments
>
> I added some documentation in the manual page of mod_log_config, and  
> removed the word gruppenwechsel.
> Please see updated PR:
>
> https://github.com/apache/httpd/pull/429/files
>
> For testing I already did bid for some feedback regarding some  
> changes I made, which did add
> code coverage to the existing tests and add basic coverage for  
> existing mod_log_config options,
> but sadly never did get any reply :-(
>
> https://github.com/apache/httpd/pull/494
> https://github.com/apache/httpd-tests/compare/trunk...thomasmey:httpd-tests:tests/mod-log-config
>
> maybe at least the code coverage PR and the httpd-test changes can  
> get merged, or at least looked at by somebody?
>
> Mfg
> thomas




Re: Friendly nudge: PR #429 "mod_log_config: Add log_formatter support and a JSON log_formatter"

Posted by Thomas Meyer <th...@m3y3r.de>.
On Sat, Sep 21, 2024 at 09:49:49AM -0400, Eric Covener wrote:
> On Sat, Sep 21, 2024 at 6:46 AM Thomas Meyer <th...@m3y3r.de> wrote:
> >
> > Hi all,

Hi,

> >
> > I hope you're all doing well and keeping your servers running smoothly! 🚀
> >
> > A little while ago, I submitted PR #429 to add support for log_formatter and a shiny new JSON log formatter to mod_log_config. It's been sitting quietly, waiting for a bit of love (or at least a review), but it seems everyone has been busy keeping the web safe and sound, so it hasn't gotten much attention.
> >
> > I know you're all swamped, but if anyone has a moment to take a quick look, I'd really appreciate it! The logs are patiently waiting to be formatted in style (and JSON!). If there’s anything I can do to help move it along or make it easier to review, feel free to let me know.
> >
> > Thanks a lot for your time! I'll buy the first round of virtual coffee ☕ for anyone who can help push this one through. 😉
> 
> 
> a few minor requests after paging through again:
> 
> 1. doc (mod_log_config.xml, logs.xml -- not just for the obvious
> reasons, but it helps reviewers too)
> 2. changelog entry (is the only expected change that you can pick JSON
> or are there other subtle things to declare?)
> 3. Some of the TODOS might be a little alarming to reviewers, it is
> unclear which ones are status quo for example
> 4. it's a massive pain to get started,  but test coverage in
> https://svn.apache.org/repos/asf/httpd/test/framework/trunk .  I will
> try to find some time to create a skeletal test for json and non-json
> just so the code is actually running.
> 5. replace "gruppenwechsel" in comments

I added some documentation in the manual page of mod_log_config, and removed the word gruppenwechsel.
Please see updated PR:

https://github.com/apache/httpd/pull/429/files

For testing I already did bid for some feedback regarding some changes I made, which did add
code coverage to the existing tests and add basic coverage for existing mod_log_config options,
but sadly never did get any reply :-(

https://github.com/apache/httpd/pull/494
https://github.com/apache/httpd-tests/compare/trunk...thomasmey:httpd-tests:tests/mod-log-config

maybe at least the code coverage PR and the httpd-test changes can get merged, or at least looked at by somebody?

Mfg
thomas


Re: Friendly nudge: PR #429 "mod_log_config: Add log_formatter support and a JSON log_formatter" 📝

Posted by Thomas Meyer <th...@m3y3r.de>.

Am 21. September 2024 15:49:49 MESZ schrieb Eric Covener <co...@gmail.com>:
>On Sat, Sep 21, 2024 at 6:46 AM Thomas Meyer <th...@m3y3r.de> wrote:
>>
>> Hi all,
>>
>> I hope you're all doing well and keeping your servers running smoothly! 🚀
>>
>> A little while ago, I submitted PR #429 to add support for log_formatter and a shiny new JSON log formatter to mod_log_config. It's been sitting quietly, waiting for a bit of love (or at least a review), but it seems everyone has been busy keeping the web safe and sound, so it hasn't gotten much attention.
>>
>> I know you're all swamped, but if anyone has a moment to take a quick look, I'd really appreciate it! The logs are patiently waiting to be formatted in style (and JSON!). If there’s anything I can do to help move it along or make it easier to review, feel free to let me know.
>>
>> Thanks a lot for your time! I'll buy the first round of virtual coffee ☕ for anyone who can help push this one through. 😉
>
>
>a few minor requests after paging through again:
>
>1. doc (mod_log_config.xml, logs.xml -- not just for the obvious
>reasons, but it helps reviewers too)
>2. changelog entry (is the only expected change that you can pick JSON
>or are there other subtle things to declare?)
>3. Some of the TODOS might be a little alarming to reviewers, it is


Hi,

I did update pr 429 and did fix most to-dos, but for the remaining ones I need some help and or your opinions what is maybe the best approach to handle those.

Mfg
Thomas 

>unclear which ones are status quo for example
>4. it's a massive pain to get started,  but test coverage in
>https://svn.apache.org/repos/asf/httpd/test/framework/trunk .  I will
>try to find some time to create a skeletal test for json and non-json
>just so the code is actually running.
>5. replace "gruppenwechsel" in comments

Re: Friendly nudge: PR #429 "mod_log_config: Add log_formatter support and a JSON log_formatter" 📝

Posted by Eric Covener <co...@gmail.com>.
On Sat, Sep 21, 2024 at 6:46 AM Thomas Meyer <th...@m3y3r.de> wrote:
>
> Hi all,
>
> I hope you're all doing well and keeping your servers running smoothly! 🚀
>
> A little while ago, I submitted PR #429 to add support for log_formatter and a shiny new JSON log formatter to mod_log_config. It's been sitting quietly, waiting for a bit of love (or at least a review), but it seems everyone has been busy keeping the web safe and sound, so it hasn't gotten much attention.
>
> I know you're all swamped, but if anyone has a moment to take a quick look, I'd really appreciate it! The logs are patiently waiting to be formatted in style (and JSON!). If there’s anything I can do to help move it along or make it easier to review, feel free to let me know.
>
> Thanks a lot for your time! I'll buy the first round of virtual coffee ☕ for anyone who can help push this one through. 😉


a few minor requests after paging through again:

1. doc (mod_log_config.xml, logs.xml -- not just for the obvious
reasons, but it helps reviewers too)
2. changelog entry (is the only expected change that you can pick JSON
or are there other subtle things to declare?)
3. Some of the TODOS might be a little alarming to reviewers, it is
unclear which ones are status quo for example
4. it's a massive pain to get started,  but test coverage in
https://svn.apache.org/repos/asf/httpd/test/framework/trunk .  I will
try to find some time to create a skeletal test for json and non-json
just so the code is actually running.
5. replace "gruppenwechsel" in comments