Hi Nathan,

On Tue, Aug 20, 2024 at 04:19:35PM +0000, Nathan Wehrman wrote:
> Since we just implemented the TCP CLF format I thought it would be a quick
> and easy time to add a couple of new environment variables.  It's a
> very small and simple patch but if a person needed this and it wasn't
> available then it's very frustrating so I thought I'd just get it done.

Thank you!

> If I missed
> anything then please let me know but I think it's just these 4 new lines.

There were three minor issues that I corrected:

> From 7cc48e41c54c6a59da62362f576397b46d09ec14 Mon Sep 17 00:00:00 2001
> From: Nathan Wehrman <nwehr...@haproxy.com>
> Date: Tue, 20 Aug 2024 09:11:34 -0700
> Subject: MINOR: Created env variables for http and tcp clf formats
                ^^^^
Usually we place a subsystem name here so that it's easier to figure
what this is about when reviewing long series of commits (typically
during backports or bisect sessions), it gives a bit of context and
usually helps the reader spot something without having to entirely
read hundreds of lines. There's nothing strict at all, you can see
stuff like "h2", "mworker" or "quic" for example. Usually what's purely
related to the config is tagged with "config" or "cfgparse" when it's
more specific to the parser. Here I'll add "config" since it's mostly
something that improves the config abilities.

> --- a/doc/configuration.txt
> +++ b/doc/configuration.txt
> @@ -927,6 +927,10 @@ file, or could be inherited by a program (See 3.7. 
> Programs):
>    defined in section 8.2.3 "HTTP log format". It can be used to override the
>    default log format without having to copy the whole original definition.
>  
> +* HAPROXY_HTTP_CLF_LOG_FMT: contains the value of the default HTTP CLF log 
> format as
                                                                             
^^^^^^^^^^
> +  defined in section 8.2.3 "HTTP log format". It can be used to override the
> +  default log format without having to copy the whole original definition.

For the doc we wap the lines at 80 chars so that they render correctly in
default window sizes. I've re-justified the 3 lines.

> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -1188,8 +1188,10 @@ static int read_cfg(char *progname)
>        * unset them after the list_for_each_entry loop.
>        */
>       setenv("HAPROXY_HTTP_LOG_FMT", default_http_log_format, 1);
> -     setenv("HAPROXY_HTTPS_LOG_FMT", default_https_log_format, 1);
> +     setenv("HAPROXY_HTTP_CLF_LOG_FMT", clf_http_log_format, 1);
> +    setenv("HAPROXY_HTTPS_LOG_FMT", default_https_log_format, 1);
>       setenv("HAPROXY_TCP_LOG_FMT", default_tcp_log_format, 1);
> +     setenv("HAPROXY_TCP_CLF_LOG_FMT", clf_tcp_log_format, 1);

You seem to be having an issue with your editor's tab handling, because
there was a similar one in your previous patch. The line about
"HAPROXY_HTTPS_LOG_FMT" had its leading tab replaced with 4 spaces. I
suspect you're viewing with a tab size of 4 and maybe you manually fixed
a mistake on the wrong line and the editor turned your tabs to spaces.

It's worth checking the output of "git diff" to avoid such things. Also
if you add:

   [color]
        ui = true

To your global git config (e.g. "git config color.ui true" or
"git config --global color.ui true"), git diff will then also highlight
trailing spaces/tabs and such annoying changes on invisible chars.

Now applied, thank you!
willy


Reply via email to