> Hi!
>
> As discussed few weeks ago in issue #2856 , this is a PR
> that adds two useful options to the log-forward section.
>
> ```
> option dontparselog
> Enables HAProxy to relay syslog messages
> without attempting to parse and
> restructure them, useful for
> forwarding messages that may not conform to
> traditional formats.
> This option should be used with the format raw setting on
> destination log targets to ensure the original message content is
> preserved.
>
> option assume-rfc6587-ntf
> Directs HAProxy to
> treat incoming TCP log streams always as using
> non-transparent
> framing. This option simplifies the framing logic and ensures
> consistent handling of messages, particularly useful when dealing with
> improperly formed starting characters.
> ```
>
> Thanks a lot
> in advance for taking this into consideration.
> Best,
> Rober
Hi Rober,
Thanks for your contribution.
Here are some remarks:
> diff --git a/src/log.c b/src/log.c
> index 90b5645cb2e6..234f83e2426f 100644
> --- a/src/log.c
> +++ b/src/log.c
> @@ -5376,6 +5376,24 @@ void app_log(struct list *loggers, struct buffer *tag,
> int level, const char *fo
>
> __send_log(NULL, loggers, tag, level, logline, data_len,
> default_rfc5424_sd_log_format, 2);
> }
> +
> +/*
> + * This function sets up the initial state for a log message by preparing
> + * the buffer, setting default values for the log level and facility, and
> + * initializing metadata fields. It is used before parsing or constructing
> + * a log message to ensure all fields are in a known state.
> + */
> +void prepare_log_message(char *buf, size_t buflen, int *level, int *facility,
idendation issue below
> +
> struct ist *metadata, char **message, size_t *size)
> +{
> + *level = *facility = -1;
> +
> + *message = buf;
> + *size = buflen;
> +
> + memset(metadata, 0, LOG_META_FIELDS*sizeof(struct ist));
> +}
> @@ -6203,6 +6206,34 @@ int cfg_parse_log_forward(const char *file, int
> linenum, char **args, int kwm)
> }
> cfg_log_forward->timeout.client = MS_TO_TICKS(timeout);
> }
> + else if (strcmp(args[0], "option") == 0) {
> + int optnum;
> +
> + if (*(args[1]) == '\0') {
> + ha_alert("parsing [%s:%d]: '%s' expects an option
> name.\n",
> + file, linenum, args[0]);
> + err_code |= ERR_ALERT | ERR_FATAL;
identation issue below
> + goto out;
> + }
> +
> +
> + for (optnum = 0; cfg_opts_log[optnum].name; optnum++) {
> + if (strcmp(args[1], cfg_opts_log[optnum].name) == 0) {
> + if (cfg_opts_log[optnum].cap == PR_CAP_NONE) {
> + ha_alert("parsing [%s:%d]: option
> '%s' is not supported due to build options.\n",
identation issue below
> + file, linenum, cfg_opts2[optnum].name);
> + err_code |= ERR_ALERT | ERR_FATAL;
> + goto out;
> + }
> + if (alertif_too_many_args_idx(0, 1, file,
> linenum, args, &err_code))
> + goto out;
> + if (warnifnotcap(cfg_log_forward,
> cfg_opts_log[optnum].cap, file, linenum, args[1], NULL)) {
> + err_code |= ERR_WARN;
> + goto out;
> + }
> + cfg_log_forward->options_log |=
> cfg_opts_log[optnum].val;
> + }
> + }
> + }
> diff --git a/src/proxy.c b/src/proxy.c
> index a1143fc68631..9000f63b751e 100644
> --- a/src/proxy.c
> +++ b/src/proxy.c
> @@ -134,6 +134,13 @@ const struct cfg_opt cfg_opts2[] =
> { NULL, 0, 0, 0 }
> };
>
> +/* proxy->options_log */
> +const struct cfg_opt cfg_opts_log[] = {
identation issue below
> + {"dontparselog", PR_OL_DONTPARSELOG,
> PR_CAP_FE, 0, PR_MODE_SYSLOG },
> + {"assume-rfc6587-ntf", PR_OL_ASSUME_RFC6587_NTF, PR_CAP_FE, 0,
> PR_MODE_SYSLOG },
> + { NULL, 0, 0, 0 }
> +};
> +
> /* Helper function to resolve a single sticking rule after config parsing.
> * Returns 1 for success and 0 for failure
> */
> diff --git a/src/log.c b/src/log.c
> index 234f83e2426f..af9c4c704e94 100644
> --- a/src/log.c
> +++ b/src/log.c
> @@ -5759,7 +5759,8 @@ void syslog_fd_handler(int fd)
>
> prepare_log_message(buf->area, buf->data, &level,
> &facility, metadata, &message, &size);
>
> - parse_log_message(buf->area, buf->data, &level,
> &facility, metadata, &message, &size);
> + if (!(l->bind_conf->frontend->options_log &
> PR_OL_DONTPARSELOG))
I would rather have "struct proxy *frontend = strm_fe(s);" and use that
frontend pointer to access options instead of l->bind_conf->frontend.
> + parse_log_message(buf->area, buf->data,
> &level, &facility, metadata, &message, &size);
Well, aside from indentation issues, I'm fine with the first patch.
Regarding the second one, the doc and behavior look good to me, However
I don't have an opinion regarding the "proxy->options_log" options
dedicated to log-forward proxies. Since log-forward section only
supports keywords explicitly handled in cfg_parse_log_forward() (so
proxy->options and proxy->options2 are currently unused), I'm wondering
if we could re-use the same memory area, either have PR_O_* flags that
have a different meaning when set on log-forward proxy, or share the
same memory area by leveraging an union for options_log member. Also it
looks like cfg_opts_log[] is a bit overkill because only the name->flag
value association is used. I'll let other give their opinion on that point.
Thanks :)
Aurelien
