[PATCH] CLEANUP: log: deinitialization of the log buffer in one function

[Miroslav Zagorac]

Hello all,

In several places in the source, there was the same block of code that was
used to deinitialize the log buffer.  There were even two functions that
did this, but they were called only from the code that is in the same
source file (free_tcpcheck_fmt() in src/tcpcheck.c and free_logformat_list()
in src/proxy.c - they were both static functions).

The function free_logformat_list() was moved from the file src/proxy.c to
src/log.c, and a check of the list before freeing the memory was added to
that function.

Best regards,

-- 
Miroslav Zagorac
Senior DeveloperFrom 52ef575d77a47ef7338004fbad31d8199e59c752 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 30 Jan 2024 03:14:09 +0100
Subject: [PATCH] CLEANUP: log: deinitialization of the log buffer in one
 function

In several places in the source, there was the same block of code that was
used to deinitialize the log buffer.  There were even two functions that
did this, but they were called only from the code that is in the same
source file (free_tcpcheck_fmt() in src/tcpcheck.c and free_logformat_list()
in src/proxy.c - they were both static functions).

The function free_logformat_list() was moved from the file src/proxy.c to
src/log.c, and a check of the list before freeing the memory was added to
that function.
---
 include/haproxy/log.h |  3 +++
 src/fcgi-app.c| 11 +--
 src/http_act.c| 28 
 src/http_htx.c| 26 --
 src/http_rules.c  |  9 +
 src/log.c | 16 
 src/proxy.c   | 12 
 src/tcpcheck.c| 39 +--
 src/vars.c|  9 +
 9 files changed, 43 insertions(+), 110 deletions(-)

diff --git a/include/haproxy/log.h b/include/haproxy/log.h
index 68b820734..9534eaeb2 100644
--- a/include/haproxy/log.h
+++ b/include/haproxy/log.h
@@ -64,6 +64,9 @@ void syslog_fd_handler(int fd);
 int init_log_buffers(void);
 void deinit_log_buffers(void);
 
+/* Deinitialize log buffers used for syslog messages */
+void free_logformat_list(struct list *fmt);
+
 /* build a log line for the session and an optional stream */
 int sess_build_logline(struct session *sess, struct stream *s, char *dst, size_t maxsize, struct list *list_format);
 
diff --git a/src/fcgi-app.c b/src/fcgi-app.c
index 00562f80d..dcd7fd224 100644
--- a/src/fcgi-app.c
+++ b/src/fcgi-app.c
@@ -134,16 +134,7 @@ static void fcgi_release_rule(struct fcgi_rule *rule)
 	if (!rule)
 		return;
 
-	if (!LIST_ISEMPTY(>value)) {
-		struct logformat_node *lf, *lfb;
-
-		list_for_each_entry_safe(lf, lfb, >value, list) {
-			LIST_DELETE(>list);
-			release_sample_expr(lf->expr);
-			free(lf->arg);
-			free(lf);
-		}
-	}
+	free_logformat_list(>value);
 	/* ->cond and ->name are not owned by the rule */
 	free(rule);
 }
diff --git a/src/http_act.c b/src/http_act.c
index 260fe1d67..ebc2bcc68 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -46,17 +46,10 @@
  */
 static void release_http_action(struct act_rule *rule)
 {
-	struct logformat_node *lf, *lfb;
-
 	istfree(>arg.http.str);
 	if (rule->arg.http.re)
 		regex_free(rule->arg.http.re);
-	list_for_each_entry_safe(lf, lfb, >arg.http.fmt, list) {
-		LIST_DELETE(>list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
+	free_logformat_list(>arg.http.fmt);
 }
 
 /* Release memory allocated by HTTP actions relying on an http reply. Concretly,
@@ -1901,23 +1894,10 @@ static enum act_return http_action_set_map(struct act_rule *rule, struct proxy *
 /* Release memory allocated by an http map/acl action. */
 static void release_http_map(struct act_rule *rule)
 {
-	struct logformat_node *lf, *lfb;
-
 	free(rule->arg.map.ref);
-	list_for_each_entry_safe(lf, lfb, >arg.map.key, list) {
-		LIST_DELETE(>list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
-	if (rule->action == 1) {
-		list_for_each_entry_safe(lf, lfb, >arg.map.value, list) {
-			LIST_DELETE(>list);
-			release_sample_expr(lf->expr);
-			free(lf->arg);
-			free(lf);
-		}
-	}
+	free_logformat_list(>arg.map.key);
+	if (rule->action == 1)
+		free_logformat_list(>arg.map.value);
 }
 
 /* Parse a "add-acl", "del-acl", "set-map" or "del-map" actions. It takes one or
diff --git a/src/http_htx.c b/src/http_htx.c
index 004d3439a..954473db3 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -1117,7 +1117,6 @@ int http_str_to_htx(struct buffer *buf, struct ist raw, char **errmsg)
 
 void release_http_reply(struct http_reply *http_reply)
 {
-	struct logformat_node *lf, *lfb;
 	struct http_reply_hdr *hdr, *hdrb;
 
 	if (!http_reply)
@@ -1126,12 +1125,7 @@ void release_http_reply(struct http_reply *http_reply)
 	ha_free(_reply->ctype);
 	list_for_each_entry_safe(hdr, hdrb, _reply->hdrs, list) {
 		LIST_DELETE(>list);
-		lis

[PATCH] DOC: configuration: corrected description of keyword tune.ssl.ocsp-update.mindelay

[Miroslav Zagorac]

Hello all,

I'm not sure, but it seems to me that in the doc/configuration.txt
documentation, the paragraph related to the keyword
tune.ssl.ssl-ctx-cache-size was also copied for the description of
the keyword tune.ssl.ocsp-update.mindelay:

tune.ssl.ssl-ctx-cache-size 
  Sets the size of the cache used to store generated certificates to 
  entries. This is a LRU cache. Because generating a SSL certificate
  dynamically is expensive, they are cached. The default cache size is set to
  1000 entries.

tune.ssl.ocsp-update.mindelay 
  ..
  Sets the size of the cache used to store generated certificates to 
  entries. This is a LRU cache. Because generating a SSL certificate
  dynamically is expensive, they are cached. The default cache size is set to
  1000 entries.


If this is not the case, please ignore this patch.

Best regards,

-- 
Miroslav Zagorac
Senior DeveloperFrom dd98f4162ce15110bd71eeaf55eafe923dda3c77 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 9 Jan 2024 20:55:47 +0100
Subject: [PATCH] DOC: configuration: corrected description of keyword
 tune.ssl.ocsp-update.mindelay

Deleted the text paragraph in the description of keyword
tune.ssl.ocsp-update.mindelay, which was added in the commit 5843237
"MINOR: ssl: Add global options to modify ocsp update min/max delay",
because it was a copy of the description of tune.ssl.ssl-ctx-cache-size.
---
 doc/configuration.txt | 5 -
 1 file changed, 5 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 406a6bb01..6585288b4 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -3905,11 +3905,6 @@ tune.ssl.ocsp-update.mindelay 
   "tune.ssl.ocsp-update.maxdelay". See option "ocsp-update" for more
   information about the auto update mechanism.
 
-  Sets the size of the cache used to store generated certificates to 
-  entries. This is a LRU cache. Because generating a SSL certificate
-  dynamically is expensive, they are cached. The default cache size is set to
-  1000 entries.
-
 tune.stick-counters 
   Sets the number of stick-counters that may be tracked at the same time by a
   connection or a request via "track-sc*" actions in "tcp-request" or
-- 
2.42.0

[PATCH] MINOR: ot: logsrv struct becomes logger

[Miroslav Zagorac]

Hello all,

after the patch 18da35c "MEDIUM: tree-wide: logsrv struct becomes logger", the
OpenTracing filter can no longer be compiled in debug mode.  This patch fixes 
it.

This patch should be backported to branch 2.9.


Best regards,

-- 
Miroslav Zagorac
Senior DeveloperFrom 281b982eace44518014717038499d7002724f896 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Mon, 8 Jan 2024 12:55:11 +0100
Subject: [PATCH] MINOR: ot: logsrv struct becomes logger

Addition to commit 18da35c "MEDIUM: tree-wide: logsrv struct becomes logger",
when the OpenTracing filter is compiled in debug mode (using OT_DEBUG=1)
then logsrv should be changed to logger here as well.

This patch should be backported to branch 2.9.
---
 addons/ot/include/conf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/addons/ot/include/conf.h b/addons/ot/include/conf.h
index a4bb7fcc0..c9c4863d5 100644
--- a/addons/ot/include/conf.h
+++ b/addons/ot/include/conf.h
@@ -60,7 +60,7 @@
 #define FLT_OT_DBG_CONF_TRACER(f,a) \
 	FLT_OT_DBG(3, f FLT_OT_CONF_HDR_FMT "'%s' %p '%s' %p %u %hhu %hhu 0x%02hhx %p:%s 0x%08x %s %s %s }",\
 	   FLT_OT_CONF_HDR_ARGS(a, id), (a)->config, (a)->cfgbuf, (a)->plugin, (a)->tracer, (a)->rate_limit, (a)->flag_harderr, \
-	   (a)->flag_disabled, (a)->logging, &((a)->proxy_log), flt_ot_list_debug(&((a)->proxy_log.logsrvs)), (a)->analyzers,   \
+	   (a)->flag_disabled, (a)->logging, &((a)->proxy_log), flt_ot_list_debug(&((a)->proxy_log.loggers)), (a)->analyzers,   \
 	   flt_ot_list_debug(&((a)->acls)), flt_ot_list_debug(&((a)->ph_groups)), flt_ot_list_debug(&((a)->ph_scopes)))
 
 #define FLT_OT_DBG_CONF(f,a)  \
-- 
2.42.0

[PATCH] MINOR: properly mark the end of the CLI command in error messages

[Miroslav Zagorac]

Hello all,

this is a patch related to correctly marking the end of the CLI command in the
error message in several places in the file src/ssl_ckch.c .

-- 
Miroslav ZagoracFrom f3d8570a7872f1be4fe0e6d20d278c9397813f21 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Fri, 1 Sep 2023 22:23:27 +0200
Subject: [PATCH] MINOR: properly mark the end of the CLI command in error
 messages

In several places in the file src/ssl_ckch.c, in the message about the
incorrect use of the CLI command, the end of that CLI command is not
correctly marked with the sign ' .
---
 src/ssl_ckch.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 341531d17..07a04aa18 100644
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -2260,7 +2260,7 @@ static int cli_parse_commit_cert(char **args, char *payload, struct appctx *appc
 		return 1;
 
 	if (!*args[3])
-		return cli_err(appctx, "'commit ssl cert expects a filename\n");
+		return cli_err(appctx, "'commit ssl cert' expects a filename\n");
 
 	/* The operations on the CKCH architecture are locked so we can
 	 * manipulate ckch_store and ckch_inst */
@@ -2328,7 +2328,7 @@ static int cli_parse_set_cert(char **args, char *payload, struct appctx *appctx,
 		return 1;
 
 	if (!*args[3] || !payload)
-		return cli_err(appctx, "'set ssl cert expects a filename and a certificate as a payload\n");
+		return cli_err(appctx, "'set ssl cert' expects a filename and a certificate as a payload\n");
 
 	/* The operations on the CKCH architecture are locked so we can
 	 * manipulate ckch_store and ckch_inst */
@@ -2667,7 +2667,7 @@ static int cli_parse_set_cafile(char **args, char *payload, struct appctx *appct
 		add_cmd = 1;
 
 	if (!*args[3] || !payload)
-		return cli_err(appctx, "'set ssl ca-file expects a filename and CAs as a payload\n");
+		return cli_err(appctx, "'set ssl ca-file' expects a filename and CAs as a payload\n");
 
 	/* The operations on the CKCH architecture are locked so we can
 	 * manipulate ckch_store and ckch_inst */
@@ -2776,7 +2776,7 @@ static int cli_parse_commit_cafile(char **args, char *payload, struct appctx *ap
 		return 1;
 
 	if (!*args[3])
-		return cli_err(appctx, "'commit ssl ca-file expects a filename\n");
+		return cli_err(appctx, "'commit ssl ca-file' expects a filename\n");
 
 	/* The operations on the CKCH architecture are locked so we can
 	 * manipulate ckch_store and ckch_inst */
@@ -3368,7 +3368,7 @@ static int cli_parse_set_crlfile(char **args, char *payload, struct appctx *appc
 		return 1;
 
 	if (!*args[3] || !payload)
-		return cli_err(appctx, "'set ssl crl-file expects a filename and CRLs as a payload\n");
+		return cli_err(appctx, "'set ssl crl-file' expects a filename and CRLs as a payload\n");
 
 	/* The operations on the CKCH architecture are locked so we can
 	 * manipulate ckch_store and ckch_inst */
@@ -3471,7 +3471,7 @@ static int cli_parse_commit_crlfile(char **args, char *payload, struct appctx *a
 		return 1;
 
 	if (!*args[3])
-		return cli_err(appctx, "'commit ssl ca-file expects a filename\n");
+		return cli_err(appctx, "'commit ssl ca-file' expects a filename\n");
 
 	/* The operations on the CKCH architecture are locked so we can
 	 * manipulate ckch_store and ckch_inst */
-- 
2.42.0

Re: [PATCH] BUG/MINOR: illegal use of the malloc_trim() function if jemalloc is used

[Miroslav Zagorac]

On 22. 03. 2023. 14:33, Willy Tarreau wrote:
>> Also, in that case, when calling 'haproxy -vv', it is no longer printed that
>> malloc_trim is enabled.
> 
> I'm not sure what you mean here, because for me it's never printed that
> malloc_trim() is enabled when building with jemalloc.
> 

Hello Willy,

without the patch:

% make -j8 ADDLIB="-ljemalloc" TARGET="linux-glibc" IGNOREGIT=1
USE_GETADDRINFO=1 USE_LIBCRYPT=1 USE_LUA=1 USE_MEMORY_PROFILING=1
USE_MODULES=1 USE_NS=1 USE_OPENSSL=1 USE_PCRE=1 USE_PCRE_JIT=1 USE_TFO=1
USE_THREAD=1 USE_ZLIB=1

% ldd haproxy | grep jemalloc
libjemalloc.so.2 => /lib/x86_64-linux-gnu/libjemalloc.so.2 
(0x7f1319e3a000)

./haproxy -vv | grep trim
Support for malloc_trim() is enabled.

-- 
Zaga

What can change the nature of a man?

Re: [PATCH] BUG/MINOR: illegal use of the malloc_trim() function if jemalloc is used

[Miroslav Zagorac]

On 22. 03. 2023. 13:11, Miroslav Zagorac wrote:
> Hello all,
> 
> here is a patch that does not allow the use of malloc_trim() function if
> HAProxy is linked with the malloc library.  It was checked in some places in

.. jemalloc .., sorry for the typo.

> the source, but not everywhere.
> 
> Also, in that case, when calling 'haproxy -vv', it is no longer printed that
> malloc_trim is enabled.
> 


-- 
Miroslav Zagorac
Senior Developer

[PATCH] BUG/MINOR: illegal use of the malloc_trim() function if jemalloc is used

[Miroslav Zagorac]

Hello all,

here is a patch that does not allow the use of malloc_trim() function if
HAProxy is linked with the malloc library.  It was checked in some places in
the source, but not everywhere.

Also, in that case, when calling 'haproxy -vv', it is no longer printed that
malloc_trim is enabled.

-- 
Miroslav Zagorac
Senior DeveloperFrom 33a84d19548baf46e8fccbb58a891a3e4c6b4895 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 22 Mar 2023 12:52:19 +0100
Subject: [PATCH] BUG/MINOR: illegal use of the malloc_trim() function if
 jemalloc is used

In the event that HAProxy is linked with the jemalloc library, it is still
shown that malloc_trim() is enabled when executing "haproxy -vv":
  ..
  Support for malloc_trim() is enabled.
  ..

It's not so much a problem as it is that malloc_trim() is called in the
pat_ref_purge_range() function without any checking.

This was solved by setting the using_default_allocator variable to the
correct value in the detect_allocator() function and before calling
malloc_trim() it is checked whether the function should be called.
---
 include/haproxy/pool.h | 1 +
 src/pattern.c  | 2 +-
 src/pool.c | 9 -
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/haproxy/pool.h b/include/haproxy/pool.h
index 16146604b..a12990d24 100644
--- a/include/haproxy/pool.h
+++ b/include/haproxy/pool.h
@@ -101,6 +101,7 @@ extern int mem_poison_byte;
 /* set of POOL_DBG_* flags */
 extern uint pool_debugging;
 
+int is_trim_enabled(void);
 void *pool_get_from_os(struct pool_head *pool);
 void pool_put_to_os(struct pool_head *pool, void *ptr);
 void *pool_alloc_nocache(struct pool_head *pool);
diff --git a/src/pattern.c b/src/pattern.c
index a2557dea1..71f25a43b 100644
--- a/src/pattern.c
+++ b/src/pattern.c
@@ -2084,7 +2084,7 @@ int pat_ref_purge_range(struct pat_ref *ref, uint from, uint to, int budget)
 		HA_RWLOCK_WRUNLOCK(PATEXP_LOCK, >lock);
 
 #if defined(HA_HAVE_MALLOC_TRIM)
-	if (done) {
+	if (done && is_trim_enabled()) {
 		malloc_trim(0);
 	}
 #endif
diff --git a/src/pool.c b/src/pool.c
index 345681a0c..214d950fb 100644
--- a/src/pool.c
+++ b/src/pool.c
@@ -177,11 +177,10 @@ static void detect_allocator(void)
 
 	my_mallctl = mallctl;
 #endif
-
-	if (!my_mallctl) {
+	if (!my_mallctl)
 		my_mallctl = get_sym_curr_addr("mallctl");
-		using_default_allocator = (my_mallctl == NULL);
-	}
+
+	using_default_allocator = (my_mallctl == NULL);
 
 	if (!my_mallctl) {
 #if defined(HA_HAVE_MALLOC_TRIM)
@@ -212,7 +211,7 @@ static void detect_allocator(void)
 	}
 }
 
-static int is_trim_enabled(void)
+int is_trim_enabled(void)
 {
 	return using_default_allocator;
 }
-- 
2.37.1

Re: [EXTERNAL] Re: [PATCH 0/16] opentracing: reenable usage of vars to transmit opentracing context

[Miroslav Zagorac]

On 08.04.2022 16:24, Willy Tarreau wrote:
> My concern is essentially this one: is there any risk that a 2.5 user
> currently using opentracing would be hit by a bug introduced with this
> patch ? The code *seems* to be isolated only in the parts that are
> enabled by OT_USE_VARS but I'm asking because you know better than me.

If context transfer over variables is not used in the opentracing
configuration, the only place where "fixed" functions for variables are used
is when generating ot.uuid variable.  This variable is not generated in the
current 2.5 code because it lacks functions for working with variables.

However, given your explanation of how the thing works with backporting
functionality, we won’t break that here.

If users demand it very much, it could be considered.  Now, not to make
trouble for ourselves, we don’t need to backport it.

-- 
Miroslav Zagorac
Senior Developer

Re: [EXTERNAL] Re: [PATCH 0/16] opentracing: reenable usage of vars to transmit opentracing context

[Miroslav Zagorac]

On 08.04.2022 15:47, Willy Tarreau wrote:

Thanks, but now I'm having further questions:

- 0006-BUG-MINOR-opentracing-setting-the-return-value-in-fu.patch
- 0016-BUG-BUILD-opentracing-fixed-OT_DEFINE-variable-setti.patch
   =>  these ones are bug fixes that are independent on the series, and
  I verified that they could apply fine before the rest. Do you
  mind if I do it ? We always want to include fixes before the
  rest so that bisection is more accurate.


I don't have any problems with that.



- 0013-EXAMPLES-opentracing-refined-shell-scripts-for-testi.patch
   =>  same for this one, I'd put it just after the two fixes above.


The same here.



- 0008-DOC-opentracing-corrected-comments-in-function-descr.patch
   =>  this one, among other things, fixes the description of function
  flt_ot_smp_init() that was added as part of the series, so at
  last this part of the fix should be merged with the patch that
  introduced the function (no need to integrate an incorrect
  patch and fix it later), and the rest are fixes for the past
  so I could put it earlier in the stack, with the fixes.


Also, I agree.



- 0014-MAJOR-opentracing-reenable-usage-of-vars-to-transmit.patch
   =>  while I'm fine for 2.6, I'm really not for 2.5 without a big
  compelling reason. It's a feature addition, not a bug fix.
  Either there are currently no opentracing users in 2.5 due to
  this problem and this will serve nobody, or there are and there
  is a significant risk of breaking something for them, which would
  possibly encourage them to revert an update and keep other unrelated
  bugs. Thus is it absolutely mandatory that we backport this to 2.5 ?
  For example, is it currently working so bad that users complain and
  that it prevents them from upgrading from 2.4 to 2.5 ? That's the
  type of situation that can justify a backport of such a patch.


In my opinion this is not a feature addition but a restore of functionality 
that was temporarily disabled due to a change in working with variables (i.e. 
saving a hash instead of a name).


This should not bring any drawback to the users as this functionality has not 
worked so far in the 2.5 branch and if not used in the configuration it does 
not change anything.


Maybe some user used this functionality in version 2.4 and it prevented him 
from switching to version 2.5 because it doesn't work there?




Please do not send me another series and just let me know your opinion
about the points above, I'd like to issue 2.6-dev5 right now, ideally
with these patches, and do not want to iterate through yet another
series.


Ok.



Regarding the 2.4 series, I'm seeing 5 cleanup patches, but I haven't
checked if they were needed or not. Normally we do not backport code
cleanups in stable branches unless they are tiny, riskless and help to
get other patches backported by providing the required patch context.
One of the reasons is that some users have patches on top of these
branches and applying cleanups that are not necessary sometimes causes
them trouble to re-apply their local patches. It's unlikely that users
have patched opentracing but that's a general rule, I think you get the
idea.


These cleanup patches are not necessary for functionality, so you don't have 
to apply them.


Thank you for your help.


Best regards,

--
Miroslav Zagorac
Senior Developer

Re: [PATCH 0/16] opentracing: reenable usage of vars to transmit opentracing context

[Miroslav Zagorac]

Hello Willy,

today I tested the operation speed of the opentracing module on branch 
2.4 and 2.5 several times and unfortunately I can't confirm what I wrote 
yesterday (that the operation speed is almost doubled).


The result I get today is almost the same speed.

I don't know what the problem is, maybe I did something wrong yesterday
or Linux decided to do something different today (probably the first
one).

This means that perhaps the next paragraph should be removed from the 
description of patch 0014:


"In terms of code execution speed, this way of using context variables
is significantly faster than the previous one (when variables with names
were stored instead of hash).  For example, in the worst case, when the
rate-limit is set to 100%, the operating speed is almost doubled (this
refers to the first result in the 'addons/ot/test/README-speed-ctx'
file)."

--
Miroslav Zagorac
Senior Developer

Re: [PATCH 0/16] opentracing: reenable usage of vars to transmit opentracing context

[Miroslav Zagorac]

On 07.04.2022 20:02, Willy Tarreau wrote:

No, really, do not bother doing that. Just tell me which ones are bug
fixes that need to be backported, as we'll backport them the usual way
as part of the regular maintenance process anyway. If you tell me "those
marked bugs ought to be backported wherever they apply", that's already
sufficient, I'll add this note in the relevant commit messages and they'll
automatically flow there.


I will add to each commit a note to which branch it refers.  Branches
2.5 and 2.6 are identical in terms of patches; i still have to try how
haproxy 2.4 works with applied patches.

Please don't apply anything for now, you will get patches for 2.4 and
2.[56] branches so you don't waste time with it (some patches don't go
uncorrected).


The naive question here is, does this *need* to run *inside* the
haproxy process ? Can't this be externalized in a sidecar agent
using SPOE, peers or anything else, that would benefit from existing
libs in their native language and not require to write a wrapper ?
That was the reason SPOE was designed in the first place and maybe
it would be time that we stop wasting time writing wrappers for
ephemeral suites.


There is an implementation of opentracing that uses SPOE, but its
usability is highly questionable:

https://github.com/haproxytech/spoa-opentracing

That utility cannot transfer the opentracing context to other
microservices, which is one of the prerequisites for the operation of
the tracing service.  Also, SPOE implementation does not allow as many
events as the opentracing filter.  That utility is much slower than the
native filter implementation and it simply cannot replace the filter in
question.

--
Zaga

What can change the nature of a man?

Re: [PATCH 0/16] opentracing: reenable usage of vars to transmit opentracing context

[Miroslav Zagorac]

On 07.04.2022 18:27, Willy Tarreau wrote:

Hi Miroslav!



Hello Willi,

at first I would generally say that I did not think that some of these
patches could be backported to older versions of haproxy.  I will look
at what can be applied to older versions and send group patches for
each of them to the mailing list tomorrow (one tar file per version).


On a related note, I've just discovered this that you might be interested
in:

https://github.com/haproxy/haproxy/issues/1640

The reporter mentions that apparently OT is becoming deprecated. If you
find some info about this and if it can affect our users (depending on
how long it will take before being unsupported if at all), possibly that
we should think about a startup warning or something like this. I do
not know what it implies for it to become deprecated, if all of its
components are available in LTS distros, users will not care. The problem
is more if users have to download components from sites that risk stop
providing them.



Yes, i saw that..  after the opentracing filter in haproxy was 
implemented i looked at what should be done to add support for 
opentelemetry.


Unfortunately, although this is an upgrade of the existing telemetry
system, the API for use is not compatible.

To begin with, it is necessary to write a C wrapper for
opentelemetry-cpp, which will take considerable time.  Then it is
necessary to write a new haproxy filter (or redesign the existing
one so that it can use both API).

This is the github repository of opentelemetry API for C++:
  https://github.com/open-telemetry/opentelemetry-cpp


As for opentracing, I think it will be supported for some time because
opentelemetry is a relatively new standard and it is not designed to be
a replacement for opentracing without a change in client software.  For
new development it is recommended to use opentelemetry.

I hope I have answered at least some of the questions in some useful
way.  :)


Best regards,

--
Miroslav Zagorac
Senior Developer

[PATCH 16/16] BUG/BUILD: opentracing: fixed OT_DEFINE variable setting

[Miroslav Zagorac]

In case of using parameter 'OT_USE_VARS=1', the value of the OT_DEFINE
variable is set incorrectly (i.e. the previous value was deleted and a
new one set instead of adding new content).

--
Miroslav Zagorac
Senior Developer
>From c84aea9ff85cdbacddf2207c50b3263c23f2e238 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 9 Mar 2022 19:59:15 +0100
Subject: [PATCH 16/16] BUG/BUILD: opentracing: fixed OT_DEFINE variable
 setting

In case of using parameter 'OT_USE_VARS=1', the value of the OT_DEFINE
variable is set incorrectly (i.e. the previous value was deleted and a
new one set instead of adding new content).
---
 addons/ot/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/addons/ot/Makefile b/addons/ot/Makefile
index 0cc8c6aad..2ee74d32c 100644
--- a/addons/ot/Makefile
+++ b/addons/ot/Makefile
@@ -65,7 +65,7 @@ OPTIONS_OBJS += \
 	addons/ot/src/util.o
 
 ifneq ($(OT_USE_VARS),)
-OT_DEFINE = -DUSE_OT_VARS
+OT_DEFINE+= -DUSE_OT_VARS
 OPTIONS_OBJS += \
 	addons/ot/src/vars.o
 endif
-- 
2.30.2

[PATCH 15/16] Revert "BUILD: opentracing: display warning in case of using OT_USE_VARS at compile time"

[Miroslav Zagorac]

This reverts commit 6c9f7faa62a00a4d028704d7b11e290c83f8a49d.

This warning is no longer needed because source code can be compiled
with enabled support for OpenTracing context transfer via HAProxy
variables.

--
Miroslav Zagorac
Senior Developer
>From 068153bae147beba4f8bba3cd2c467f68d1c19ac Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 9 Mar 2022 19:57:56 +0100
Subject: [PATCH 15/16] Revert "BUILD: opentracing: display warning in case of
 using OT_USE_VARS at compile time"

This reverts commit 6c9f7faa62a00a4d028704d7b11e290c83f8a49d.

This warning is no longer needed because source code can be compiled with
enabled support for OpenTracing context transfer via HAProxy variables.
---
 addons/ot/Makefile | 2 --
 1 file changed, 2 deletions(-)

diff --git a/addons/ot/Makefile b/addons/ot/Makefile
index c7b0f8db9..0cc8c6aad 100644
--- a/addons/ot/Makefile
+++ b/addons/ot/Makefile
@@ -65,8 +65,6 @@ OPTIONS_OBJS += \
 	addons/ot/src/util.o
 
 ifneq ($(OT_USE_VARS),)
-$(warning Please do not set the OT_USE_VARS configuration variable, as the source will probably not be able to compile!  For now, this variable can only be used for experimental purposes, and is not intended for wider use.)
-
 OT_DEFINE = -DUSE_OT_VARS
 OPTIONS_OBJS += \
 	addons/ot/src/vars.o
-- 
2.30.2

[PATCH 14/16] MAJOR: opentracing: reenable usage of vars to transmit opentracing context

[Miroslav Zagorac]

Since commit 3a4bedccc ("MEDIUM: vars: replace the global name index
with a hash") the names of HAProxy variables are no longer saved, ie
their 64-bit hashes are saved instead.

This is very convenient for the HAProxy itself, but for the OpenTracing
module it is a problem because the names of the variables are important
when transferring the OpenTracing context.  Namely, this context
consists of an unknown amount of data stored in a key-value format.
The number of these data (and the name of the variable used for this
purpose) is determined with the configuration of the OpenTracing
filter, as well as with the tracer used.  The two previous sentences
seem to be in conflict, but that is only so at first glance.  The
function in the OpenTracing filter used to read the context does not
really know this, neither their number nor its name.  The only thing
that function actually knows is the prefix of the variable names used
for context transfer, and by that it could find all the necessary
data.  Of course, until the application of the above-mentioned commit.

The problem is solved in a very simple way: in a common variable that
the filter always knows its name, the names of all variables that are
the product of the OpenTracing context are saved.  The names of these
context variables can only be added to that common variable. when that
variable is no longer needed (when we no longer need context), it is
deleted.

The format for saving data to this common variable is as follows:
  +-+---+-- .. --+-+---+
  | len | variable name || len | variable name |
  +-+---+-- .. --+-+---+

This means that the maximum length of the variable name to be saved is
255 characters, which is quite enough for use in the filter.  The
buffer size for such data storage is global.tune.bufsize.

In terms of code execution speed, this way of using context variables is
significantly faster than the previous one (when variables with names
were stored instead of hash).  For example, in the worst case, when the
rate-limit is set to 100%, the operating speed is almost doubled (this
refers to the first result in the 'addons/ot/test/README-speed-ctx'
file).

--
Miroslav Zagorac
Senior Developer
>From 7d6c21e612cf0da76870506e68086ab12e3e7d5f Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Thu, 10 Mar 2022 00:03:24 +0100
Subject: [PATCH 14/16] MAJOR: opentracing: reenable usage of vars to transmit
 opentracing context

Since commit 3a4bedccc ("MEDIUM: vars: replace the global name index with
a hash") the names of HAProxy variables are no longer saved, ie their
64-bit hashes are saved instead.

This is very convenient for the HAProxy itself, but for the OpenTracing
module it is a problem because the names of the variables are important
when transferring the OpenTracing context.  Namely, this context consists
of an unknown amount of data stored in a key-value format.  The number
of these data (and the name of the variable used for this purpose) is
determined with the configuration of the OpenTracing filter, as well as
with the tracer used.  The two previous sentences seem to be in conflict,
but that is only so at first glance.  The function in the OpenTracing
filter used to read the context does not really know this, neither their
number nor its name.  The only thing that function actually knows is the
prefix of the variable names used for context transfer, and by that it
could find all the necessary data.  Of course, until the application of
the above-mentioned commit.

The problem is solved in a very simple way: in a common variable that
the filter always knows its name, the names of all variables that are the
product of the OpenTracing context are saved.  The names of these context
variables can only be added to that common variable. when that variable is
no longer needed (when we no longer need context), it is deleted.

The format for saving data to this common variable is as follows:
  +-+---+-- .. --+-+---+
  | len | variable name || len | variable name |
  +-+---+-- .. --+-+---+

This means that the maximum length of the variable name to be saved is 255
characters, which is quite enough for use in the filter.  The buffer size
for such data storage is global.tune.bufsize.

In terms of code execution speed, this way of using context variables is
significantly faster than the previous one (when variables with names were
stored instead of hash).  For example, in the worst case, when the rate-limit
is set to 100%, the operating speed is almost doubled (this refers to the
first result in the 'addons/ot/test/README-speed-ctx' file).
---
 addons/ot/include/debug.h |   1 +
 addons/ot/include/vars.h  |   8 +
 addons/ot/src/vars.c  | 516 +-
 3 files changed, 413 insertions(+), 112 deletions(-)

diff --git a/addons/ot/include/debug.h b/addons/o

[PATCH 13/16] EXAMPLES: opentracing: refined shell scripts for testing filter performance

[Miroslav Zagorac]

When calling the 'basename' command, the argument ${0} is enclosed in
quotation marks.  This is necessary if the path of the executable script
(contained in that argument) has "non-standard" elements, such as space
and the like.

Also, in the script 'test-speed.sh' the function sh_exit() has been
added for easier printing of messages at the end of execution.

--
Miroslav Zagorac
Senior Developer
>From ae45db88bc0f6e2904db54d23cfe4258af637377 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 9 Mar 2022 17:34:11 +0100
Subject: [PATCH 13/16] EXAMPLES: opentracing: refined shell scripts for
 testing filter performance

When calling the 'basename' command, the argument ${0} is enclosed in
quotation marks.  This is necessary if the path of the executable script
(contained in that argument) has "non-standard" elements, such as space
and the like.

Also, in the script 'test-speed.sh' the function sh_exit() has been added
for easier printing of messages at the end of execution.
---
 addons/ot/test/run-cmp.sh|  2 +-
 addons/ot/test/run-ctx.sh|  2 +-
 addons/ot/test/run-fe-be.sh  |  4 ++--
 addons/ot/test/run-sa.sh |  2 +-
 addons/ot/test/test-speed.sh | 37 
 5 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/addons/ot/test/run-cmp.sh b/addons/ot/test/run-cmp.sh
index 77b48bd01..8e678b7fa 100755
--- a/addons/ot/test/run-cmp.sh
+++ b/addons/ot/test/run-cmp.sh
@@ -3,7 +3,7 @@
 _ARG_HAPROXY="${1:-$(realpath -L ${PWD}/../../../haproxy)}"
_ARGS="-f cmp/haproxy.cfg"
 _LOG_DIR="_logs"
-_LOG="${_LOG_DIR}/_log-$(basename ${0} .sh)-$(date +%s)"
+_LOG="${_LOG_DIR}/_log-$(basename "${0}" .sh)-$(date +%s)"
 
 
 test -x "${_ARG_HAPROXY}" || exit 1
diff --git a/addons/ot/test/run-ctx.sh b/addons/ot/test/run-ctx.sh
index 064fa7dce..bfac61725 100755
--- a/addons/ot/test/run-ctx.sh
+++ b/addons/ot/test/run-ctx.sh
@@ -3,7 +3,7 @@
 _ARG_HAPROXY="${1:-$(realpath -L ${PWD}/../../../haproxy)}"
_ARGS="-f ctx/haproxy.cfg"
 _LOG_DIR="_logs"
-_LOG="${_LOG_DIR}/_log-$(basename ${0} .sh)-$(date +%s)"
+_LOG="${_LOG_DIR}/_log-$(basename "${0}" .sh)-$(date +%s)"
 
 
 test -x "${_ARG_HAPROXY}" || exit 1
diff --git a/addons/ot/test/run-fe-be.sh b/addons/ot/test/run-fe-be.sh
index 7e70ad618..68b250c0e 100755
--- a/addons/ot/test/run-fe-be.sh
+++ b/addons/ot/test/run-fe-be.sh
@@ -5,8 +5,8 @@ _ARG_HAPROXY="${1:-$(realpath -L ${PWD}/../../../haproxy)}"
 _ARGS_BE="-f be/haproxy.cfg"
_TIME="$(date +%s)"
 _LOG_DIR="_logs"
- _LOG_FE="${_LOG_DIR}/_log-$(basename ${0} fe-be.sh)fe-${_TIME}"
- _LOG_BE="${_LOG_DIR}/_log-$(basename ${0} fe-be.sh)be-${_TIME}"
+ _LOG_FE="${_LOG_DIR}/_log-$(basename "${0}" fe-be.sh)fe-${_TIME}"
+ _LOG_BE="${_LOG_DIR}/_log-$(basename "${0}" fe-be.sh)be-${_TIME}"
 
 
 __exit ()
diff --git a/addons/ot/test/run-sa.sh b/addons/ot/test/run-sa.sh
index e5682ea74..04a303a4f 100755
--- a/addons/ot/test/run-sa.sh
+++ b/addons/ot/test/run-sa.sh
@@ -3,7 +3,7 @@
 _ARG_HAPROXY="${1:-$(realpath -L ${PWD}/../../../haproxy)}"
_ARGS="-f sa/haproxy.cfg"
 _LOG_DIR="_logs"
-_LOG="${_LOG_DIR}/_log-$(basename ${0} .sh)-$(date +%s)"
+_LOG="${_LOG_DIR}/_log-$(basename "${0}" .sh)-$(date +%s)"
 
 
 test -x "${_ARG_HAPROXY}" || exit 1
diff --git a/addons/ot/test/test-speed.sh b/addons/ot/test/test-speed.sh
index ef2ccf061..f2ac5140e 100755
--- a/addons/ot/test/test-speed.sh
+++ b/addons/ot/test/test-speed.sh
@@ -1,11 +1,28 @@
 #!/bin/sh
 #
   _ARG_CFG="${1}"
-  _ARG_DIR="${2}"
+  _ARG_DIR="${2:-${1}}"
   _LOG_DIR="_logs"
 _HTTPD_PIDFILE="${_LOG_DIR}/thttpd.pid"
+_USAGE_MSG="usage: $(basename "${0}") cfg [dir]"
 
 
+sh_exit ()
+{
+	test -z "${2}" && {
+		echo
+		echo "Script killed!"
+	}
+
+	test -n "${1}" && {
+		echo
+		echo "${1}"
+		echo
+	}
+
+	exit ${2:-64}
+}
+
 httpd_run ()
 {
 
@@ -63,18 +80,22 @@ wrk_run ()
 }
 
 
-mkdir -p "${_LOG_DIR}" || exit 1
+command -v thttpd >/dev/null 2>&1 || sh_exit "thttpd: command not found" 5
+command -v wrk >/dev/null 2>&1|| sh_exit "wrk: command not found" 6
+
+mkdir -p "${_LOG_DIR}" || sh_exit "${_LOG_DIR}: Cannot create log directory" 1
 
 if test "${_ARG_CFG}" = "all"; then
-	${0} fe-be fe > "${_LOG_DIR}/README-speed-fe-be"
-	${0} sa sa> "${_LOG_DIR}/README-speed-sa"
-	${0} cmp cmp  > "${_LOG_DIR}/README-speed-cmp"

[PATCH 12/16] DEBUG: opentracing: display the contents of the err variable after setting

[Miroslav Zagorac]

A display of the contents of the err variable has been added to the
FLT_OT_ERR() macro, once it has been set.

--
Miroslav Zagorac
Senior Developer
>From e38e00f8eb67010cd607429ece16d39c4b9e0f73 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 8 Mar 2022 01:21:04 +0100
Subject: [PATCH 12/16] DEBUG: opentracing: display the contents of the err
 variable after setting

A display of the contents of the err variable has been added to the
FLT_OT_ERR() macro, once it has been set.
---
 addons/ot/include/define.h | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/addons/ot/include/define.h b/addons/ot/include/define.h
index d34f6b9f2..3c3e4a3e0 100644
--- a/addons/ot/include/define.h
+++ b/addons/ot/include/define.h
@@ -73,10 +73,13 @@
 	char   *p = b[__idx]; \
 	__idx = (__idx + 1) % (m)
 
-#define FLT_OT_ERR(f, ...)  \
-	do {\
-		if ((err != NULL) && (*err == NULL))\
-			(void)memprintf(err, f, ##__VA_ARGS__); \
+#define FLT_OT_ERR(f, ...) \
+	do {   \
+		if ((err != NULL) && (*err == NULL)) { \
+			(void)memprintf(err, f, ##__VA_ARGS__);\
+   \
+			FLT_OT_DBG(3, "%d err: '%s'", __LINE__, *err); \
+		}  \
 	} while (0)
 #define FLT_OT_ERR_APPEND(f, ...)   \
 	do {\
-- 
2.30.2

[PATCH 11/16] CLEANUP: opentracing: added FLT_OT_PARSE_INVALID_enum enum

[Miroslav Zagorac]

It's always good to replace "hard-coded" values with something that
looks less "hard-coded" (even though that doesn't change anything in
the code).  This is done here using FLT_OT_PARSE_INVALID_enum enum.

--
Miroslav Zagorac
Senior Developer
>From 4db45aefc8152947a6c6a97df435c7082313a7cf Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Mon, 7 Mar 2022 13:14:16 +0100
Subject: [PATCH 11/16] CLEANUP: opentracing: added FLT_OT_PARSE_INVALID_enum
 enum

It's always good to replace "hard-coded" values with something that looks
less "hard-coded" (even though that doesn't change anything in the code).
This is done here using FLT_OT_PARSE_INVALID_enum enum.
---
 addons/ot/include/parser.h | 52 ++
 addons/ot/src/parser.c | 16 ++--
 2 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/addons/ot/include/parser.h b/addons/ot/include/parser.h
index 96939916d..53e414b35 100644
--- a/addons/ot/include/parser.h
+++ b/addons/ot/include/parser.h
@@ -50,20 +50,20 @@
  * flt_ot_parse_data definition
  */
 #define FLT_OT_PARSE_TRACER_DEFINES  \
-	FLT_OT_PARSE_TRACER_DEF( ID, 0, 1, 2, 2, "ot-tracer",   " ")   \
-	FLT_OT_PARSE_TRACER_DEF(ACL, 0, 1, 3, 0, "acl", "   [flags] [operator]  ...")\
-	FLT_OT_PARSE_TRACER_DEF(LOG, 0, 1, 2, 0, "log", " { global |  [len ] [format ]  [ []] }") \
-	FLT_OT_PARSE_TRACER_DEF( CONFIG, 0, 0, 2, 2, "config",  " ")   \
-	FLT_OT_PARSE_TRACER_DEF( PLUGIN, 0, 0, 2, 2, "plugin",  " ")   \
-	FLT_OT_PARSE_TRACER_DEF( GROUPS, 0, 0, 2, 0, "groups",  "  ...")   \
-	FLT_OT_PARSE_TRACER_DEF( SCOPES, 0, 0, 2, 0, "scopes",  "  ...")   \
-	FLT_OT_PARSE_TRACER_DEF( RATE_LIMIT, 0, 0, 2, 2, "rate-limit",  " ")  \
-	FLT_OT_PARSE_TRACER_DEF( OPTION, 0, 0, 2, 2, "option",  " { disabled | dontlog-normal | hard-errors }")  \
-	FLT_OT_PARSE_TRACER_DEF(DEBUG_LEVEL, 0, 0, 2, 2, "debug-level", " ")
+	FLT_OT_PARSE_TRACER_DEF( ID, 0, CHAR, 2, 2, "ot-tracer",   " ")   \
+	FLT_OT_PARSE_TRACER_DEF(ACL, 0, CHAR, 3, 0, "acl", "   [flags] [operator]  ...")\
+	FLT_OT_PARSE_TRACER_DEF(LOG, 0, CHAR, 2, 0, "log", " { global |  [len ] [format ]  [ []] }") \
+	FLT_OT_PARSE_TRACER_DEF( CONFIG, 0, NONE, 2, 2, "config",  " ")   \
+	FLT_OT_PARSE_TRACER_DEF( PLUGIN, 0, NONE, 2, 2, "plugin",  " ")   \
+	FLT_OT_PARSE_TRACER_DEF( GROUPS, 0, NONE, 2, 0, "groups",  "  ...")   \
+	FLT_OT_PARSE_TRACER_DEF( SCOPES, 0, NONE, 2, 0, "scopes",  "  ...")   \
+	FLT_OT_PARSE_TRACER_DEF( RATE_LIMIT, 0, NONE, 2, 2, "rate-limit",  " ")  \
+	FLT_OT_PARSE_TRACER_DEF( OPTION, 0, NONE, 2, 2, "option",  " { disabled | dontlog-normal | hard-errors }")  \
+	FLT_OT_PARSE_TRACER_DEF(DEBUG_LEVEL, 0, NONE, 2, 2, "debug-level", " ")
 
 #define FLT_OT_PARSE_GROUP_DEFINES\
-	FLT_OT_PARSE_GROUP_DEF(ID, 0, 1, 2, 2, "ot-group", " ") \
-	FLT_OT_PARSE_GROUP_DEF(SCOPES, 0, 0, 2, 0, "scopes",   "  ...")
+	FLT_OT_PARSE_GROUP_DEF(ID, 0, CHAR, 2, 2, "ot-group", " ") \
+	FLT_OT_PARSE_GROUP_DEF(SCOPES, 0, NONE, 2, 0, "scopes",   "  ...")
 
 #ifdef USE_OT_VARS
 #  define FLT_OT_PARSE_SCOPE_INJECT_HELP"  [use-vars] [use-headers]"
@@ -81,16 +81,24 @@
  * so I will 

[PATCH 10/16] DEBUG: opentracing: show return values of all functions in the debug output

[Miroslav Zagorac]

If the OpenTracing filter is compiled using the 'OT_DEBUG=1' option,
then log messages are printed to stderr when the filter is running.  In
the log one can then find (among other things) the order in which the
function is called and the value that the function returns (if it is
not a void type).

Prior to applying this patch, no value returned by a function was
logged.

Log output example:
  [ 1]0.038807 [OT]: flt_ot_init_per_thread(0x56365bd45ec0, 
0x56365bd48210) {
  [ 1]0.038807 [OT]:ot_start(0x56365bd58920, 0x56365bd4e3a0, 
0x7f561acba168:(nil)) {

  [ 1]0.038807 [OT]:} = 0
  [ 1]0.038807 [OT]: } = 0

--
Miroslav Zagorac
Senior Developer
>From 0166aa33fedfa739bc678ca98af6b6a74b0e8c1e Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Fri, 4 Mar 2022 09:56:00 +0100
Subject: [PATCH 10/16] DEBUG: opentracing: show return values of all functions
 in the debug output

If the OpenTracing filter is compiled using the 'OT_DEBUG=1' option, then
log messages are printed to stderr when the filter is running.  In the log
one can then find (among other things) the order in which the function is
called and the value that the function returns (if it is not a void type).

Prior to applying this patch, no value returned by a function was logged.

Log output example:
  [ 1]0.038807 [OT]: flt_ot_init_per_thread(0x56365bd45ec0, 0x56365bd48210) {
  [ 1]0.038807 [OT]:ot_start(0x56365bd58920, 0x56365bd4e3a0, 0x7f561acba168:(nil)) {
  [ 1]0.038807 [OT]:} = 0
  [ 1]0.038807 [OT]: } = 0
---
 addons/ot/include/debug.h   |  7 +++
 addons/ot/src/cli.c | 12 ++---
 addons/ot/src/conf.c| 38 +++
 addons/ot/src/event.c   | 10 ++--
 addons/ot/src/filter.c  | 60 +++
 addons/ot/src/group.c   | 28 +--
 addons/ot/src/http.c| 14 +++---
 addons/ot/src/opentracing.c | 96 ++---
 addons/ot/src/parser.c  | 62 
 addons/ot/src/pool.c|  6 +--
 addons/ot/src/scope.c   | 26 +-
 addons/ot/src/util.c| 18 +++
 addons/ot/src/vars.c| 26 +-
 13 files changed, 205 insertions(+), 198 deletions(-)

diff --git a/addons/ot/include/debug.h b/addons/ot/include/debug.h
index 7becdf72f..6311216a6 100644
--- a/addons/ot/include/debug.h
+++ b/addons/ot/include/debug.h
@@ -39,6 +39,9 @@
 	} while (0)
 #  define FLT_OT_FUNC(f, ...)   do { FLT_OT_DBG(1, "%s(" f ") {", __func__, ##__VA_ARGS__); dbg_indent_level += 3; } while (0)
 #  define FLT_OT_RETURN(a)  do { dbg_indent_level -= 3; FLT_OT_DBG(1, "}"); return a; } while (0)
+#  define FLT_OT_RETURN_EX(a,t,f)   do { dbg_indent_level -= 3; { t _r = (a); FLT_OT_DBG(1, "} = " f, _r); return _r; } } while (0)
+#  define FLT_OT_RETURN_INT(a)  FLT_OT_RETURN_EX((a), int, "%d")
+#  define FLT_OT_RETURN_PTR(a)  FLT_OT_RETURN_EX((a), void *, "%p")
 #  define FLT_OT_DBG_IFDEF(a,b) a
 #  define FLT_OT_DBG_ARGS(a, ...)   a, ##__VA_ARGS__
 
@@ -58,8 +61,12 @@ extern struct flt_ot_debug flt_ot_debug;
 #  define FLT_OT_DBG(...)   while (0)
 #  define FLT_OT_FUNC(...)  while (0)
 #  define FLT_OT_RETURN(a)  return a
+#  define FLT_OT_RETURN_EX(a,t,f)   return a
+#  define FLT_OT_RETURN_INT(a)  return a
+#  define FLT_OT_RETURN_PTR(a)  return a
 #  define FLT_OT_DBG_IFDEF(a,b) b
 #  define FLT_OT_DBG_ARGS(...)
+#  define FLT_OT_DBG_BUF(a,b)   while (0)
 #endif /* DEBUG_OT */
 
 /*
diff --git a/addons/ot/src/cli.c b/addons/ot/src/cli.c
index 529c6874c..f9feeca15 100644
--- a/addons/ot/src/cli.c
+++ b/addons/ot/src/cli.c
@@ -97,7 +97,7 @@ static int flt_ot_cli_parse_debug(char **args, char *payload, struct appctx *app
 
 	cmn_cli_set_msg(appctx, err, msg, CLI_ST_PRINT_FREE);
 
-	FLT_OT_RETURN(retval);
+	FLT_OT_RETURN_INT(retval);
 }
 
 #endif /* DEBUG_OT */
@@ -137,7 +137,7 @@ static int flt_ot_cli_parse_disabled(char **args, char *payload, struct appctx *
 
 	cmn_cli_set_msg(appctx, NULL, msg, CLI_ST_PRINT_FREE);
 
-	FLT_OT_RETURN(retval);
+	FLT_OT_RETURN_INT(retval);
 }
 
 
@@ -175,7 +175,7 @@ static int flt_ot_cli_parse_option(char **args, char *payload, struct appctx *ap
 
 	cmn_cli_set_msg(appctx, NULL, msg, CLI_ST_PRINT_FREE);
 
-	FLT_OT_RETURN(retval);
+	FLT_OT_RETURN_INT(retval);
 }
 
 
@@ -238,7 +238,7 @@ static int flt_ot_cli_parse_logging(char **args, char *payload, struct appctx *a
 
 	cmn_cli_set_msg(appctx, err, msg, CLI_ST_PRINT_FREE);
 
-	FLT_OT_RETURN(retval);
+	FLT_OT_RETURN_INT(retval);
 }
 
 
@@ -289,7 +289,7 @@ static int flt_ot_cli_parse_rate(char **args, char *payload, struct appctx *appc
 
 	cmn_cli_set_msg(appctx, err, msg, CLI_ST_PRINT_FREE);
 
-	FLT_OT_RETURN(retval);
+	FLT_OT_RETURN_INT(retval);
 }
 
 
@@ -343,7 +343,7 @@ static int flt_ot_cli_parse_status(char **args, char *payload, struct appctx *ap
 
 	cmn_cli_set_msg(appctx, NULL, msg, CLI_ST_PRI

[PATCH 09/16] MINOR: opentracing: improved normalization of context variable names

[Miroslav Zagorac]

The flag_cpy parameter has been added to the flt_ot_normalize_name()
function, through which we can determine whether the function converts
special characters (which are part of that name) when copying a variable
name.

--
Miroslav Zagorac
Senior Developer
>From 47e4fc4b67a3275a568a2300e6ce0c61230cf0a1 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 2 Mar 2022 00:15:23 +0100
Subject: [PATCH 09/16] MINOR: opentracing: improved normalization of context
 variable names

The flag_cpy parameter has been added to the flt_ot_normalize_name()
function, through which we can determine whether the function converts
special characters (which are part of that name) when copying a variable
name.
---
 addons/ot/src/vars.c | 78 ++--
 1 file changed, 46 insertions(+), 32 deletions(-)

diff --git a/addons/ot/src/vars.c b/addons/ot/src/vars.c
index daaa14927..4a2b11dce 100644
--- a/addons/ot/src/vars.c
+++ b/addons/ot/src/vars.c
@@ -158,6 +158,7 @@ static inline struct vars *flt_ot_get_vars(struct stream *s, const char *scope)
  *   size -
  *   len  -
  *   name -
+ *   flag_cpy -
  *   err  -
  *
  * DESCRIPTION
@@ -166,11 +167,11 @@ static inline struct vars *flt_ot_get_vars(struct stream *s, const char *scope)
  * RETURN VALUE
  *   -
  */
-static int flt_ot_normalize_name(char *var_name, size_t size, int *len, const char *name, char **err)
+static int flt_ot_normalize_name(char *var_name, size_t size, int *len, const char *name, bool flag_cpy, char **err)
 {
 	int retval = 0;
 
-	FLT_OT_FUNC("%p, %zu, %p, \"%s\", %p:%p", var_name, size, len, name, FLT_OT_DPTR_ARGS(err));
+	FLT_OT_FUNC("%p, %zu, %p, \"%s\", %hhu, %p:%p", var_name, size, len, name, flag_cpy, FLT_OT_DPTR_ARGS(err));
 
 	if (!FLT_OT_STR_ISVALID(name))
 		FLT_OT_RETURN(retval);
@@ -186,33 +187,45 @@ static int flt_ot_normalize_name(char *var_name, size_t size, int *len, const ch
 	else
 		retval = -1;
 
-	/*
-	 * HAProxy does not allow the use of variable names containing '-'
-	 * or ' '.  This of course applies to HTTP header names as well.
-	 * Also, here the capital letters are converted to lowercase.
-	 */
-	while (retval != -1)
-		if (*len >= (size - 1)) {
+	if (flag_cpy) {
+		retval = strlen(name);
+		if ((*len + retval + 1) > size) {
 			FLT_OT_ERR("failed to normalize variable name, buffer too small");
 
 			retval = -1;
 		} else {
-			uint8_t ch = name[retval];
-
-			if (ch == '\0')
-break;
-			else if (ch == '-')
-ch = FLT_OT_VAR_CHAR_DASH;
-			else if (ch == ' ')
-ch = FLT_OT_VAR_CHAR_SPACE;
-			else if (isupper(ch))
-ch = ist_lc[ch];
-
-			var_name[(*len)++] = ch;
-			retval++;
+			(void)memcpy(var_name + *len, name, retval + 1);
+			*len += retval;
 		}
+	} else {
+		/*
+		 * HAProxy does not allow the use of variable names containing '-'
+		 * or ' '.  This of course applies to HTTP header names as well.
+		 * Also, here the capital letters are converted to lowercase.
+		 */
+		while (retval != -1)
+			if (*len >= (size - 1)) {
+FLT_OT_ERR("failed to normalize variable name, buffer too small");
+
+retval = -1;
+			} else {
+uint8_t ch = name[retval];
+
+if (ch == '\0')
+	break;
+else if (ch == '-')
+	ch = FLT_OT_VAR_CHAR_DASH;
+else if (ch == ' ')
+	ch = FLT_OT_VAR_CHAR_SPACE;
+else if (isupper(ch))
+	ch = ist_lc[ch];
+
+var_name[(*len)++] = ch;
+retval++;
+			}
 
-	var_name[*len] = '\0';
+		var_name[*len] = '\0';
+	}
 
 	FLT_OT_DBG(3, "var_name: \"%s\" %d/%d", var_name, retval, *len);
 
@@ -231,6 +244,7 @@ static int flt_ot_normalize_name(char *var_name, size_t size, int *len, const ch
  *   scope-
  *   prefix   -
  *   name -
+ *   flag_cpy -
  *   var_name -
  *   size -
  *   err  -
@@ -243,15 +257,15 @@ static int flt_ot_normalize_name(char *var_name, size_t size, int *len, const ch
  * RETURN VALUE
  *   -
  */
-static int flt_ot_var_name(const char *scope, const char *prefix, const char *name, char *var_name, size_t size, char **err)
+static int flt_ot_var_name(const char *scope, const char *prefix, const char *name, bool flag_cpy, char *var_name, size_t size, char **err)
 {
 	int retval = 0;
 
-	FLT_OT_FUNC("\"%s\", \"%s\", \"%s\", %p, %zu, %p:%p", scope, prefix, name, var_name, size, FLT_OT_DPTR_ARGS(err));
+	FLT_OT_FUNC("\"%s\", \"%s\", \"%s\", %hhu, %p, %zu, %p:%p", scope, prefix, name, flag_cpy, var_name, size, FLT_OT_DPTR_ARGS(err));
 
-	if (flt_ot_normalize_name(var_name, size, , scope, err) >= 0)
-		if (flt_ot_normalize_name(var_name, size, , prefix, err) >= 0)
-			(void)flt_ot_normalize_name(var_name, size, , name, err);
+	if (flt_ot_normalize_name(var_name, size, , scope, 0, err) >= 0)
+		if (flt_ot_normalize_name(var_name, size, , prefix, 0, err) >= 0)
+			(void)flt_ot_normalize_name(var_n

[PATCH 08/16] DOC: opentracing: corrected comments in function descriptions

[Miroslav Zagorac]

Several comments in the function descriptions have been corrected.

--
Miroslav Zagorac
Senior Developer
>From 0b7cd96bbda98e10108316d087fc1828b9f1f564 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 1 Mar 2022 19:18:34 +0100
Subject: [PATCH 08/16] DOC: opentracing: corrected comments in function
 descriptions

Several comments in the function descriptions have been corrected.
---
 addons/ot/src/pool.c |  2 +-
 addons/ot/src/vars.c | 17 -
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/addons/ot/src/pool.c b/addons/ot/src/pool.c
index ead53e1b8..0ee542654 100644
--- a/addons/ot/src/pool.c
+++ b/addons/ot/src/pool.c
@@ -147,7 +147,7 @@ void flt_ot_pool_free(struct pool_head *pool, void **ptr)
  *   -
  *
  * RETURN VALUE
- *   This function does not return a value.
+ *   -
  */
 struct buffer *flt_ot_trash_alloc(bool flag_clear, char **err)
 {
diff --git a/addons/ot/src/vars.c b/addons/ot/src/vars.c
index 32fd19fac..daaa14927 100644
--- a/addons/ot/src/vars.c
+++ b/addons/ot/src/vars.c
@@ -31,7 +31,8 @@
  *   scope -
  *
  * DESCRIPTION
- *   -
+ *   Function prints the contents of all variables defined for a particular
+ *   scope.
  *
  * RETURN VALUE
  *   This function does not return a value.
@@ -58,7 +59,8 @@ static void flt_ot_vars_scope_dump(struct vars *vars, const char *scope)
  *   s -
  *
  * DESCRIPTION
- *   -
+ *   Function prints the contents of all variables grouped by individual
+ *   scope.
  *
  * RETURN VALUE
  *   This function does not return a value.
@@ -95,7 +97,9 @@ void flt_ot_vars_dump(struct stream *s)
  *   data -
  *
  * DESCRIPTION
- *   -
+ *   The function initializes the value of the 'smp' structure.  If the 'data'
+ *   argument is set, then the 'sample_data' member of the 'smp' structure is
+ *   also initialized.
  *
  * RETURN VALUE
  *   This function does not return a value.
@@ -125,7 +129,8 @@ static inline void flt_ot_smp_init(struct stream *s, struct sample *smp, uint op
  *   -
  *
  * RETURN VALUE
- *   -
+ *   Returns the struct vars pointer for a stream and scope, or NULL if it does
+ *   not exist.
  */
 static inline struct vars *flt_ot_get_vars(struct stream *s, const char *scope)
 {
@@ -231,7 +236,9 @@ static int flt_ot_normalize_name(char *var_name, size_t size, int *len, const ch
  *   err  -
  *
  * DESCRIPTION
- *   -
+ *   The function initializes the value of the 'smp' structure.  If the 'data'
+ *   argument is set, then the 'sample_data' member of the 'smp' structure is
+ *   also initialized.
  *
  * RETURN VALUE
  *   -
-- 
2.30.2

[PATCH 07/16] CLEANUP: opentracing: added variable to store variable length

[Miroslav Zagorac]

The same variable should not be used to store multiple different
results, because it can be confusing.  Therefore, the var_name_len
variable has been added in several functions, in order to avoid the use
of the retval variable for several different purposes.

--
Miroslav Zagorac
Senior Developer
>From 8178840f60beae1602f8e76dd3ac9de76b017c10 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 1 Mar 2022 18:44:36 +0100
Subject: [PATCH 07/16] CLEANUP: opentracing: added variable to store variable
 length

The same variable should not be used to store multiple different results,
because it can be confusing.  Therefore, the var_name_len variable has
been added in several functions, in order to avoid the use of the retval
variable for several different purposes.
---
 addons/ot/src/vars.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/addons/ot/src/vars.c b/addons/ot/src/vars.c
index dd40e2138..32fd19fac 100644
--- a/addons/ot/src/vars.c
+++ b/addons/ot/src/vars.c
@@ -273,26 +273,26 @@ int flt_ot_var_register(const char *scope, const char *prefix, const char *name,
 {
 	struct arg arg;
 	char   var_name[BUFSIZ];
-	intretval;
+	intretval = -1, var_name_len;
 
 	FLT_OT_FUNC("\"%s\", \"%s\", \"%s\", %p:%p", scope, prefix, name, FLT_OT_DPTR_ARGS(err));
 
-	retval = flt_ot_var_name(scope, prefix, name, var_name, sizeof(var_name), err);
-	if (retval == -1)
+	var_name_len = flt_ot_var_name(scope, prefix, name, var_name, sizeof(var_name), err);
+	if (var_name_len == -1)
 		FLT_OT_RETURN(retval);
 
 	/* Set  to 0 to not release var_name memory in vars_check_arg(). */
 	(void)memset(, 0, sizeof(arg));
 	arg.type  = ARGT_STR;
 	arg.data.str.area = var_name;
-	arg.data.str.data = retval;
+	arg.data.str.data = var_name_len;
 
 	if (vars_check_arg(, err) == 0) {
 		FLT_OT_ERR_APPEND("failed to register variable '%s': %s", var_name, *err);
-
-		retval = -1;
 	} else {
 		FLT_OT_DBG(2, "variable '%s' registered", arg.data.var.name);
+
+		retval = var_name_len;
 	}
 
 	FLT_OT_RETURN(retval);
@@ -322,22 +322,22 @@ int flt_ot_var_set(struct stream *s, const char *scope, const char *prefix, cons
 {
 	struct sample smp;
 	char  var_name[BUFSIZ];
-	int   retval;
+	int   retval = -1, var_name_len;
 
 	FLT_OT_FUNC("%p, \"%s\", \"%s\", \"%s\", \"%s\", %u, %p:%p", s, scope, prefix, name, value, opt, FLT_OT_DPTR_ARGS(err));
 
-	retval = flt_ot_var_name(scope, prefix, name, var_name, sizeof(var_name), err);
-	if (retval == -1)
+	var_name_len = flt_ot_var_name(scope, prefix, name, var_name, sizeof(var_name), err);
+	if (var_name_len == -1)
 		FLT_OT_RETURN(retval);
 
 	flt_ot_smp_init(s, , opt, SMP_T_STR, value);
 
-	if (vars_set_by_name_ifexist(var_name, retval, ) == 0) {
+	if (vars_set_by_name_ifexist(var_name, var_name_len, ) == 0) {
 		FLT_OT_ERR("failed to set variable '%s'", var_name);
-
-		retval = -1;
 	} else {
 		FLT_OT_DBG(2, "variable '%s' set", var_name);
+
+		retval = var_name_len;
 	}
 
 	FLT_OT_RETURN(retval);
-- 
2.30.2

[PATCH 06/16] BUG/MINOR: opentracing: setting the return value in function flt_ot_var_set()

[Miroslav Zagorac]

Function flt_ot_var_set() did not check whether the variable was
successfully set or not.  In case of failure, the value -1 is returned.

--
Miroslav Zagorac
Senior Developer
>From 731892071c47540e2ed04d539121546194322731 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 1 Mar 2022 18:42:54 +0100
Subject: [PATCH 06/16] BUG/MINOR: opentracing: setting the return value in
 function flt_ot_var_set()

Function flt_ot_var_set() did not check whether the variable was
successfully set or not.  In case of failure, the value -1 is returned.
---
 addons/ot/src/vars.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/addons/ot/src/vars.c b/addons/ot/src/vars.c
index fa2a0bd06..dd40e2138 100644
--- a/addons/ot/src/vars.c
+++ b/addons/ot/src/vars.c
@@ -332,7 +332,13 @@ int flt_ot_var_set(struct stream *s, const char *scope, const char *prefix, cons
 
 	flt_ot_smp_init(s, , opt, SMP_T_STR, value);
 
-	vars_set_by_name_ifexist(var_name, retval, );
+	if (vars_set_by_name_ifexist(var_name, retval, ) == 0) {
+		FLT_OT_ERR("failed to set variable '%s'", var_name);
+
+		retval = -1;
+	} else {
+		FLT_OT_DBG(2, "variable '%s' set", var_name);
+	}
 
 	FLT_OT_RETURN(retval);
 }
-- 
2.30.2

[PATCH 05/16] CLEANUP: opentracing: added flt_ot_smp_init() function

[Miroslav Zagorac]

The flt_ot_smp_init() function has been added to make initializing the
sample structure easier.  The contents of the structure in question are
set in several places in the source of the OpenTracing filter, so it is
better to do this in one place.

--
Miroslav Zagorac
Senior Developer
>From 9015197281886afb6d8d23c0bbe539b63a5709c5 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 1 Mar 2022 18:41:36 +0100
Subject: [PATCH 05/16] CLEANUP: opentracing: added flt_ot_smp_init() function

The flt_ot_smp_init() function has been added to make initializing the
sample structure easier.  The contents of the structure in question are
set in several places in the source of the OpenTracing filter, so it is
better to do this in one place.
---
 addons/ot/src/vars.c | 42 +-
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/addons/ot/src/vars.c b/addons/ot/src/vars.c
index 82b089b82..fa2a0bd06 100644
--- a/addons/ot/src/vars.c
+++ b/addons/ot/src/vars.c
@@ -83,6 +83,36 @@ void flt_ot_vars_dump(struct stream *s)
 #endif /* DEBUG_OT */
 
 
+/***
+ * NAME
+ *   flt_ot_smp_init -
+ *
+ * ARGUMENTS
+ *   s-
+ *   smp  -
+ *   opt  -
+ *   type -
+ *   data -
+ *
+ * DESCRIPTION
+ *   -
+ *
+ * RETURN VALUE
+ *   This function does not return a value.
+ */
+static inline void flt_ot_smp_init(struct stream *s, struct sample *smp, uint opt, int type, const char *data)
+{
+	(void)memset(smp, 0, sizeof(*smp));
+	(void)smp_set_owner(smp, s->be, s->sess, s, opt | SMP_OPT_FINAL);
+
+	if (data != NULL) {
+		smp->data.type = type;
+
+		chunk_initstr(&(smp->data.u.str), data);
+	}
+}
+
+
 /***
  * NAME
  *   flt_ot_get_vars -
@@ -300,11 +330,7 @@ int flt_ot_var_set(struct stream *s, const char *scope, const char *prefix, cons
 	if (retval == -1)
 		FLT_OT_RETURN(retval);
 
-	(void)memset(, 0, sizeof(smp));
-	(void)smp_set_owner(, s->be, s->sess, s, opt | SMP_OPT_FINAL);
-	smp.data.type   = SMP_T_STR;
-	smp.data.u.str.area = (char *)value;
-	smp.data.u.str.data = strlen(value);
+	flt_ot_smp_init(s, , opt, SMP_T_STR, value);
 
 	vars_set_by_name_ifexist(var_name, retval, );
 
@@ -364,10 +390,8 @@ int flt_ot_vars_unset(struct stream *s, const char *scope, const char *prefix, u
 
 			FLT_OT_DBG(2, "- '%s' -> '%.*s'", var_name, (int)var->data.u.str.data, var->data.u.str.area);
 
-			(void)memset(, 0, sizeof(smp));
-			(void)smp_set_owner(, s->be, s->sess, s, opt | SMP_OPT_FINAL);
-
-			size = var_clear(var);
+			size = var_clear(var, 1);
+			flt_ot_smp_init(s, , opt, 0, NULL);
 			var_accounting_diff(vars, smp.sess, smp.strm, -size);
 
 			retval++;
-- 
2.30.2

[PATCH 04/16] CLEANUP: opentracing: removed unused function flt_ot_var_get()

[Miroslav Zagorac]

The flt_ot_var_get() function is not used anywhere and is unnecessary
in the existing implementation of the OpenTracing filter.

--
Miroslav Zagorac
Senior Developer
>From 1faf31cb0ed0cb41e89dc189cee3c753aa1cba55 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 1 Mar 2022 18:39:18 +0100
Subject: [PATCH 04/16] CLEANUP: opentracing: removed unused function
 flt_ot_var_get()

The flt_ot_var_get() function is not used anywhere and is unnecessary
in the existing implementation of the OpenTracing filter.
---
 addons/ot/include/vars.h |  1 -
 addons/ot/src/vars.c | 48 
 2 files changed, 49 deletions(-)

diff --git a/addons/ot/include/vars.h b/addons/ot/include/vars.h
index da1a04986..c683b6bc6 100644
--- a/addons/ot/include/vars.h
+++ b/addons/ot/include/vars.h
@@ -33,7 +33,6 @@ void flt_ot_vars_dump(struct stream *s);
 int  flt_ot_var_register(const char *scope, const char *prefix, const char *name, char **err);
 int  flt_ot_var_set(struct stream *s, const char *scope, const char *prefix, const char *name, const char *value, uint opt, char **err);
 int  flt_ot_vars_unset(struct stream *s, const char *scope, const char *prefix, uint opt, char **err);
-int  flt_ot_var_get(struct stream *s, const char *scope, const char *prefix, const char *name, char **value, uint opt, char **err);
 struct otc_text_map *flt_ot_vars_get(struct stream *s, const char *scope, const char *prefix, uint opt, char **err);
 
 #endif /* _OPENTRACING_VARS_H_ */
diff --git a/addons/ot/src/vars.c b/addons/ot/src/vars.c
index 1a8244c46..82b089b82 100644
--- a/addons/ot/src/vars.c
+++ b/addons/ot/src/vars.c
@@ -379,54 +379,6 @@ int flt_ot_vars_unset(struct stream *s, const char *scope, const char *prefix, u
 }
 
 
-/***
- * NAME
- *   flt_ot_var_get -
- *
- * ARGUMENTS
- *   s  -
- *   scope  -
- *   prefix -
- *   name   -
- *   value  -
- *   opt-
- *   err-
- *
- * DESCRIPTION
- *   -
- *
- * RETURN VALUE
- *   -
- */
-int flt_ot_var_get(struct stream *s, const char *scope, const char *prefix, const char *name, char **value, uint opt, char **err)
-{
-	struct sample smp;
-	char  var_name[BUFSIZ], var_value[BUFSIZ];
-	int   retval;
-
-	FLT_OT_FUNC("%p, \"%s\", \"%s\", \"%s\", %p:%p, %u, %p:%p", s, scope, prefix, name, FLT_OT_DPTR_ARGS(value), opt, FLT_OT_DPTR_ARGS(err));
-
-	retval = flt_ot_var_name(scope, prefix, name, var_name, sizeof(var_name), err);
-	if (retval == -1)
-		FLT_OT_RETURN(retval);
-
-	(void)memset(, 0, sizeof(smp));
-	(void)smp_set_owner(, s->be, s->sess, s, opt | SMP_OPT_FINAL);
-
-	if (vars_get_by_name(var_name, retval, , NULL)) {
-		retval = flt_ot_sample_to_str(&(smp.data), var_value, sizeof(var_value), err);
-		if (retval != -1)
-			FLT_OT_DBG(3, "data type %d: '%s' = '%s'", smp.data.type, var_name, var_value);
-	} else {
-		FLT_OT_ERR("failed to get variable '%s'", var_name);
-
-		retval = -1;
-	}
-
-	FLT_OT_RETURN(retval);
-}
-
-
 /***
  * NAME
  *   flt_ot_vars_get -
-- 
2.30.2

[PATCH 03/16] CLEANUP: opentracing: removed unused function flt_ot_var_unset()

[Miroslav Zagorac]

The flt_ot_var_unset() function is not used anywhere and is unnecessary
in the existing implementation of the OpenTracing filter.

--
Miroslav Zagorac
Senior Developer
>From 6f75cb4adc0d2ffa661fe861cc0986aaf99450b5 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 1 Mar 2022 18:38:37 +0100
Subject: [PATCH 03/16] CLEANUP: opentracing: removed unused function
 flt_ot_var_unset()

The flt_ot_var_unset() function is not used anywhere and is unnecessary
in the existing implementation of the OpenTracing filter.
---
 addons/ot/include/vars.h |  1 -
 addons/ot/src/vars.c | 39 ---
 2 files changed, 40 deletions(-)

diff --git a/addons/ot/include/vars.h b/addons/ot/include/vars.h
index 5ce88794a..da1a04986 100644
--- a/addons/ot/include/vars.h
+++ b/addons/ot/include/vars.h
@@ -32,7 +32,6 @@ void flt_ot_vars_dump(struct stream *s);
 #endif
 int  flt_ot_var_register(const char *scope, const char *prefix, const char *name, char **err);
 int  flt_ot_var_set(struct stream *s, const char *scope, const char *prefix, const char *name, const char *value, uint opt, char **err);
-int  flt_ot_var_unset(struct stream *s, const char *scope, const char *prefix, const char *name, uint opt, char **err);
 int  flt_ot_vars_unset(struct stream *s, const char *scope, const char *prefix, uint opt, char **err);
 int  flt_ot_var_get(struct stream *s, const char *scope, const char *prefix, const char *name, char **value, uint opt, char **err);
 struct otc_text_map *flt_ot_vars_get(struct stream *s, const char *scope, const char *prefix, uint opt, char **err);
diff --git a/addons/ot/src/vars.c b/addons/ot/src/vars.c
index f8a58d68a..1a8244c46 100644
--- a/addons/ot/src/vars.c
+++ b/addons/ot/src/vars.c
@@ -312,45 +312,6 @@ int flt_ot_var_set(struct stream *s, const char *scope, const char *prefix, cons
 }
 
 
-/***
- * NAME
- *   flt_ot_var_unset -
- *
- * ARGUMENTS
- *   s  -
- *   scope  -
- *   prefix -
- *   name   -
- *   opt-
- *   err-
- *
- * DESCRIPTION
- *   -
- *
- * RETURN VALUE
- *   -
- */
-int flt_ot_var_unset(struct stream *s, const char *scope, const char *prefix, const char *name, uint opt, char **err)
-{
-	struct sample smp;
-	char  var_name[BUFSIZ];
-	int   retval;
-
-	FLT_OT_FUNC("%p, \"%s\", \"%s\", \"%s\", %u, %p:%p", s, scope, prefix, name, opt, FLT_OT_DPTR_ARGS(err));
-
-	retval = flt_ot_var_name(scope, prefix, name, var_name, sizeof(var_name), err);
-	if (retval == -1)
-		FLT_OT_RETURN(retval);
-
-	(void)memset(, 0, sizeof(smp));
-	(void)smp_set_owner(, s->be, s->sess, s, opt | SMP_OPT_FINAL);
-
-	vars_unset_by_name_ifexist(var_name, retval, );
-
-	FLT_OT_RETURN(retval);
-}
-
-
 /***
  * NAME
  *   flt_ot_vars_unset -
-- 
2.30.2

[PATCH 02/16] MINOR: opentracing: only takes the variables lock on shared entries

[Miroslav Zagorac]

Regarding commit #61ecf2838:
  There's no point taking the variables locks for sess/txn/req/res
  contexts since these ones always run inside the same thread anyway.

--
Miroslav Zagorac
Senior Developer
>From 099ed5a7cf021d7144aff009a5baa91912375509 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 23 Feb 2022 18:15:56 +0100
Subject: [PATCH 02/16] MINOR: opentracing: only takes the variables lock on
 shared entries

Regarding commit #61ecf2838:
  There's no point taking the variables locks for sess/txn/req/res
  contexts since these ones always run inside the same thread anyway.
---
 addons/ot/src/vars.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/addons/ot/src/vars.c b/addons/ot/src/vars.c
index dd823b1a7..f8a58d68a 100644
--- a/addons/ot/src/vars.c
+++ b/addons/ot/src/vars.c
@@ -43,10 +43,10 @@ static void flt_ot_vars_scope_dump(struct vars *vars, const char *scope)
 	if (vars == NULL)
 		return;
 
-	HA_RWLOCK_RDLOCK(VARS_LOCK, &(vars->rwlock));
+	vars_rdlock(vars);
 	list_for_each_entry(var, &(vars->head), l)
 		FLT_OT_DBG(2, "'%s.%s' -> '%.*s'", scope, var->name, (int)var->data.u.str.data, var->data.u.str.area);
-	HA_RWLOCK_RDUNLOCK(VARS_LOCK, &(vars->rwlock));
+	vars_rdunlock(vars);
 }
 
 
@@ -389,7 +389,7 @@ int flt_ot_vars_unset(struct stream *s, const char *scope, const char *prefix, u
 
 	retval = 0;
 
-	HA_RWLOCK_WRLOCK(VARS_LOCK, &(vars->rwlock));
+	vars_wrlock(vars);
 	list_for_each_entry_safe(var, var_back, &(vars->head), l) {
 		FLT_OT_DBG(3, "variable cmp '%s' '%s' %d", var_prefix, var->name, var_prefix_len);
 
@@ -412,7 +412,7 @@ int flt_ot_vars_unset(struct stream *s, const char *scope, const char *prefix, u
 			retval++;
 		}
 	}
-	HA_RWLOCK_WRUNLOCK(VARS_LOCK, &(vars->rwlock));
+	vars_wrunlock(vars);
 
 	FLT_OT_RETURN(retval);
 }
@@ -501,7 +501,7 @@ struct otc_text_map *flt_ot_vars_get(struct stream *s, const char *scope, const
 	if (rc == -1)
 		FLT_OT_RETURN(retptr);
 
-	HA_RWLOCK_RDLOCK(VARS_LOCK, &(vars->rwlock));
+	vars_rdlock(vars);
 	list_for_each_entry(var, &(vars->head), l) {
 		FLT_OT_DBG(3, "variable cmp '%s' '%s' %d", var_name, var->name, rc);
 
@@ -555,7 +555,7 @@ struct otc_text_map *flt_ot_vars_get(struct stream *s, const char *scope, const
 			}
 		}
 	}
-	HA_RWLOCK_RDUNLOCK(VARS_LOCK, &(vars->rwlock));
+	vars_rdunlock(vars);
 
 	ot_text_map_show(retptr);
 
-- 
2.30.2

[PATCH 01/16] Revert "MINOR: opentracing: change the scope of the variable 'ot.uuid' from 'sess' to 'txn'"

[Miroslav Zagorac]

This reverts commit 560c7b874aef5922199e36a7f31466af323f489f.

The ot.uuid variable should have the 'sess' scope because it is created
when an OpenTracing filter is attached to a stream.  After that, the
stream processing is started and on that occasion the contexts for the
variables that have the range 'txn' and 'req' are initialized.  This
means that we cannot use variables with the specified scopes before that
point.

--
Miroslav Zagorac
Senior Developer
>From 8f777edd98512666a40ffc8abd2d00c187fe577b Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 23 Feb 2022 10:28:10 +0100
Subject: [PATCH 01/16] Revert "MINOR: opentracing: change the scope of the
 variable 'ot.uuid' from 'sess' to 'txn'"

This reverts commit 560c7b874aef5922199e36a7f31466af323f489f.

The ot.uuid variable should have the 'sess' scope because it is created
when an OpenTracing filter is attached to a stream.  After that, the
stream processing is started and on that occasion the contexts for the
variables that have the range 'txn' and 'req' are initialized.  This
means that we cannot use variables with the specified scopes before that
point.
---
 addons/ot/README-func  | 2 +-
 addons/ot/include/filter.h | 2 +-
 addons/ot/src/scope.c  | 2 +-
 addons/ot/test/be/ot.cfg   | 2 +-
 addons/ot/test/cmp/ot.cfg  | 2 +-
 addons/ot/test/ctx/ot.cfg  | 2 +-
 addons/ot/test/fe/ot.cfg   | 2 +-
 addons/ot/test/sa/ot.cfg   | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/addons/ot/README-func b/addons/ot/README-func
index a6cb21eea..273c7f928 100644
--- a/addons/ot/README-func
+++ b/addons/ot/README-func
@@ -144,7 +144,7 @@ context is created, and flags are set that define which analyzers are used.
   flt_ot_runtime_context_init() {
  flt_ot_pool_alloc() {
  }
- /* Initializing and setting the variable 'txn.ot.uuid'. */
+ /* Initializing and setting the variable 'sess.ot.uuid'. */
  if (flt_ot_var_register() != -1) {
 flt_ot_var_set() {
 }
diff --git a/addons/ot/include/filter.h b/addons/ot/include/filter.h
index 6d41b7289..c97a0cc59 100644
--- a/addons/ot/include/filter.h
+++ b/addons/ot/include/filter.h
@@ -22,7 +22,7 @@
 
 #define FLT_OT_FMT_NAME   "'" FLT_OT_OPT_NAME "' : "
 #define FLT_OT_FMT_TYPE   "'filter' : "
-#define FLT_OT_VAR_UUID   "txn", "ot", "uuid"
+#define FLT_OT_VAR_UUID   "sess", "ot", "uuid"
 #define FLT_OT_ALERT(f, ...)  ha_alert(FLT_OT_FMT_TYPE FLT_OT_FMT_NAME f "\n", ##__VA_ARGS__)
 
 #define FLT_OT_CONDITION_IF   "if"
diff --git a/addons/ot/src/scope.c b/addons/ot/src/scope.c
index 6492c2774..80b0bc21b 100644
--- a/addons/ot/src/scope.c
+++ b/addons/ot/src/scope.c
@@ -117,7 +117,7 @@ struct flt_ot_runtime_context *flt_ot_runtime_context_init(struct stream *s, str
 
 #ifdef USE_OT_VARS
 	/*
-	 * The HAProxy variable 'txn.ot.uuid' is registered here,
+	 * The HAProxy variable 'sess.ot.uuid' is registered here,
 	 * after which its value is set to runtime context UUID.
 	 */
 	if (flt_ot_var_register(FLT_OT_VAR_UUID, err) != -1)
diff --git a/addons/ot/test/be/ot.cfg b/addons/ot/test/be/ot.cfg
index 12cf0a17a..edd3f76ca 100644
--- a/addons/ot/test/be/ot.cfg
+++ b/addons/ot/test/be/ot.cfg
@@ -20,7 +20,7 @@
 ot-scope frontend_http_request
 extract "ot-ctx" use-headers
 span "HAProxy session" child-of "ot-ctx" root
-baggage "haproxy_id" var(txn.ot.uuid)
+baggage "haproxy_id" var(sess.ot.uuid)
 span "Client session" child-of "HAProxy session"
 span "Frontend HTTP request" child-of "Client session"
 tag "http.method" method
diff --git a/addons/ot/test/cmp/ot.cfg b/addons/ot/test/cmp/ot.cfg
index a16fc9ea6..21b15dd61 100644
--- a/addons/ot/test/cmp/ot.cfg
+++ b/addons/ot/test/cmp/ot.cfg
@@ -21,7 +21,7 @@
 
 ot-scope client_session_start
 span "HAProxy session" root
-baggage "haproxy_id" var(txn.ot.uuid)
+baggage "haproxy_id" var(sess.ot.uuid)
 span "Client session" child-of "HAProxy session"
 event on-client-session-start
 
diff --git a/addons/ot/test/ctx/ot.cfg b/addons/ot/test/ctx/ot.cfg
index a753f77b9..a06a4e00f 100644
--- a/addons/ot/test/ctx/ot.cfg
+++ b/addons/ot/test/ctx/ot.cfg
@@ -57,7 +57,7 @@
 ot-scope client_session_start_1
 span "HAProxy session" root
 inject "ot_ctx_1" use-headers use-vars
-baggage "haproxy_id" var(txn.ot.uuid)
+baggage "haproxy_id" var(sess.ot.uuid)
 event on-client-session-start
 
 ot-scope client_session_start_2
diff --git a/addons/ot/test/fe/ot.cfg b/addons/ot/test/f

[PATCH 0/16] opentracing: reenable usage of vars to transmit opentracing context

[Miroslav Zagorac]

Hello all,

I am sending you a set of patches that return the ability to use
haproxy variables to transfer opentracing context.


01 Revert "MINOR: opentracing: change the scope of the variable 
'ot.uuid' from 'sess' to 'txn'"

02 MINOR: opentracing: only takes the variables lock on shared entries
03 CLEANUP: opentracing: removed unused function flt_ot_var_unset()
04 CLEANUP: opentracing: removed unused function flt_ot_var_get()
05 CLEANUP: opentracing: added flt_ot_smp_init() function
06 BUG/MINOR: opentracing: setting the return value in function 
flt_ot_var_set()

07 CLEANUP: opentracing: added variable to store variable length
08 DOC: opentracing: corrected comments in function descriptions
09 MINOR: opentracing: improved normalization of context variable names
10 DEBUG: opentracing: show return values of all functions in the debug 
output

11 CLEANUP: opentracing: added FLT_OT_PARSE_INVALID_enum enum
12 DEBUG: opentracing: display the contents of the err variable after 
setting
13 EXAMPLES: opentracing: refined shell scripts for testing filter 
performance
14 MAJOR: opentracing: reenable usage of vars to transmit opentracing 
context
15 Revert "BUILD: opentracing: display warning in case of using 
OT_USE_VARS at compile time"

16 BUG/BUILD: opentracing: fixed OT_DEFINE variable setting


Here I will explain only what is most important for this set of
patches.

Since commit 3a4bedccc ("MEDIUM: vars: replace the global name index
with a hash") the names of HAProxy variables are no longer saved, ie
their 64-bit hashes are saved instead.

This is very convenient for the HAProxy itself, but for the OpenTracing
module it is a problem because the names of the variables are important
when transferring the OpenTracing context.  Namely, this context
consists of an unknown amount of data stored in a key-value format.
The number of these data (and the name of the variable used for this
purpose) is determined with the configuration of the OpenTracing
filter, as well as with the tracer used.  The two previous sentences
seem to be in conflict, but that is only so at first glance.  The
function in the OpenTracing filter used to read the context does not
really know this, neither their number nor its name.  The only thing
that function actually knows is the prefix of the variable names used
for context transfer, and by that it could find all the necessary
data.  Of course, until the application of the above-mentioned commit.

The problem is solved in a very simple way: in a common variable that
the filter always knows its name, the names of all variables that are
the product of the OpenTracing context are saved.  The names of these
context variables can only be added to that common variable. when that
variable is no longer needed (when we no longer need context), it is
deleted.

The format for saving data to this common variable is as follows:
  +-+---+-- .. --+-+---+
  | len | variable name || len | variable name |
  +-+---+-- .. --+-+---+

This means that the maximum length of the variable name to be saved is
255 characters, which is quite enough for use in the filter.  The
buffer size for such data storage is global.tune.bufsize.

In terms of code execution speed, this way of using context variables is
significantly faster than the previous one (when variables with names
were stored instead of hash).  For example, in the worst case, when the
rate-limit is set to 100%, the operating speed is almost doubled (this
refers to the first result in the 'addons/ot/test/README-speed-ctx'
file).


Best regards,

--
Miroslav Zagorac
Senior Developer

Re: [EXTERNAL] Re: [PATCH] BUILD: opentracing: display warning in case of using OT_USE_VARS at compile time

[Miroslav Zagorac]

On 12/28/2021 11:59 AM, William Lallemand wrote:

On Mon, Dec 27, 2021 at 01:22:57PM +0100, Miroslav Zagorac wrote:


On 12/27/2021 01:16 PM, Miroslav Zagorac wrote:

Hello all,

to avoid misunderstandings (like say github issue #1493) when compiling
HAproxy source with the OpenTracing filter enabled, a warning has been
added if the OT_USE_VARS variable is used.

If appropriate, please apply this commit.

Best regards.



sorry, i forgot the patch.  :(



Hi Miroslav, In which versions this patch should be backported?

Thanks



Hello William,

I think that this commit can be applied to branches 2.5 and 2.6-dev.


Best regards.

--
Miroslav Zagorac

Re: [PATCH] BUILD: opentracing: display warning in case of using OT_USE_VARS at compile time

[Miroslav Zagorac]

On 12/27/2021 01:16 PM, Miroslav Zagorac wrote:

Hello all,

to avoid misunderstandings (like say github issue #1493) when compiling
HAproxy source with the OpenTracing filter enabled, a warning has been
added if the OT_USE_VARS variable is used.

If appropriate, please apply this commit.

Best regards.



sorry, i forgot the patch.  :(

--
Miroslav Zagorac
>From 93e02c8b6d1eb14cfb16e0ba1feddf3b10a77f06 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Mon, 27 Dec 2021 12:44:07 +0100
Subject: [PATCH] BUILD: opentracing: display warning in case of using
 OT_USE_VARS at compile time

Please do not set the OT_USE_VARS configuration variable, as the source
will probably not be able to compile!  For now, this variable can only
be used for experimental purposes, and is not intended for wider use.

For further clarification, please see commit 4cb2c83f4.
---
 addons/ot/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/addons/ot/Makefile b/addons/ot/Makefile
index 0cc8c6aad..c7b0f8db9 100644
--- a/addons/ot/Makefile
+++ b/addons/ot/Makefile
@@ -65,6 +65,8 @@ OPTIONS_OBJS += \
 	addons/ot/src/util.o
 
 ifneq ($(OT_USE_VARS),)
+$(warning Please do not set the OT_USE_VARS configuration variable, as the source will probably not be able to compile!  For now, this variable can only be used for experimental purposes, and is not intended for wider use.)
+
 OT_DEFINE = -DUSE_OT_VARS
 OPTIONS_OBJS += \
 	addons/ot/src/vars.o
-- 
2.30.2

[PATCH] BUILD: opentracing: display warning in case of using OT_USE_VARS at compile time

[Miroslav Zagorac]

Hello all,

to avoid misunderstandings (like say github issue #1493) when compiling
HAproxy source with the OpenTracing filter enabled, a warning has been
added if the OT_USE_VARS variable is used.

If appropriate, please apply this commit.

Best regards.

--
Miroslav Zagorac

Re: [PATCH] BUILD: opentracing: excluded use of haproxy variables for, OpenTracing context

[Miroslav Zagorac]

Hello Tim,

On 09/11/2021 12:09 AM, Tim Düsterhus wrote:

The programming style in OT is not what I'm used to, but if I understand
it correctly then maybe istmatchi() is what you are searching for (it
matches prefixes which should be what you are doing manually)?



yes, this function could be used instead of strncasecmp().  However, it
doesn't seem optimized to me and is certainly faster than strncasecmp()
for simple cases for strings where the difference is right at the
beginning.  Already after comparing strings longer than 4 characters
(and the first 4 are the same), the function is slower than
strncasecmp() and the duration of the function increases rapidly while
the processing time of the function strncasecmp() is quite stable).

I’m not writing this to start some deeper discussion, this is purely
informative.  :)

Best regards,

--
Miroslav Zagorac
Senior Developer

Re: [PATCH 2/4] BUG/MINOR: opentracing: enable the use of http headers without a set value

[Miroslav Zagorac]

Hello Willy,

On 09/12/2021 08:19 AM, Willy Tarreau wrote:

Bah, I discovered that one after merging the first series :-/ I've
applied it on top as a cleanup with your description above as the
commit message since it's really what it's about.


It is no problem.  I apologize for the slight confusion.

--
Zaga

What can change the nature of a man?

Re: [PATCH] BUILD: opentracing: excluded use of haproxy variables for, OpenTracing context

[Miroslav Zagorac]

Hello Willy,

On 09/12/2021 08:11 AM, Willy Tarreau wrote:

thanks for working on fixing this, it's now merged. I've added a
tiny change to make sure that text_map is always initialized in
flt_ot_scope_run() because that made clang rightfully upset,
re-enabled OT in the CI since it's now OK.


Right, it's necessary, I overlooked it and the gcc didn't object.

Thanks.

--
Zaga

What can change the nature of a man?

Re: [PATCH 2/4] BUG/MINOR: opentracing: enable the use of http headers without a set value

[Miroslav Zagorac]

On 09/11/2021 12:05 AM, Miroslav Zagorac wrote:

Hello all,

the second patch from the last series of patches has been redesigned
here, the ist() function is used to set an empty string instead of
working directly with the string pointer.

I thank Tim Düsterhus for his advice.


The operation comment has been slightly corrected.
Sorry to bother you.  :)

--
Zaga

What can change the nature of a man?
>From 969f6e001a2ec9fe96ff3d148c0e673afdd1f13b Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Thu, 9 Sep 2021 01:23:42 +0200
Subject: [PATCH 2/4] BUG/MINOR: opentracing: enable the use of http headers
 without a set value

In case we transfer some data that does not have a set value via the HTTP
header, adding that header in the text map was done incorrectly.

This simple patch allows the use of HTTP headers without a set value.
---
 addons/ot/src/http.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/addons/ot/src/http.c b/addons/ot/src/http.c
index 4a12ed854..0d824ddb5 100644
--- a/addons/ot/src/http.c
+++ b/addons/ot/src/http.c
@@ -133,6 +133,17 @@ struct otc_text_map *flt_ot_http_headers_get(struct channel *chn, const char *pr
 
 v = htx_get_blk_value(htx, blk);
 
+/*
+ * In case the data of the HTTP header is not
+ * specified, v.ptr will have some non-null
+ * value and v.len will be equal to 0.  The
+ * otc_text_map_add() function will not
+ * interpret this well.  In this case v.ptr
+ * is set to an empty string.
+ */
+if (v.len == 0)
+	v = ist("");
+
 /*
  * Here, an HTTP header (which is actually part
  * of the span context is added to the text_map.
-- 
2.30.1

[PATCH 2/4] BUG/MINOR: opentracing: enable the use of http headers without a set value

[Miroslav Zagorac]

Hello all,

the second patch from the last series of patches has been redesigned
here, the ist() function is used to set an empty string instead of
working directly with the string pointer.

I thank Tim Düsterhus for his advice.

Best regards,

--
Zaga

What can change the nature of a man?
>From 661e87f9b897924d786c887d19b211816443eed5 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Thu, 9 Sep 2021 01:23:42 +0200
Subject: [PATCH 2/4] BUG/MINOR: opentracing: enable the use of http headers
 without a set value

In case we transfer some data that does not have a set value via the HTTP
header, adding that header in the text map was done incorrectly.

This simple patch allows the use of HTTP headers without a set value.
---
 addons/ot/src/http.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/addons/ot/src/http.c b/addons/ot/src/http.c
index 4a12ed854..0ed4970e5 100644
--- a/addons/ot/src/http.c
+++ b/addons/ot/src/http.c
@@ -133,6 +133,16 @@ struct otc_text_map *flt_ot_http_headers_get(struct channel *chn, const char *pr
 
 v = htx_get_blk_value(htx, blk);
 
+/*
+ * In case the data of the HTTP header is not
+ * specified, v.len will be equal to 0, and
+ * the function otc_text_map_add() will not
+ * interpret this well.  In this case v.ptr
+ * is set to an empty string.
+ */
+if (v.len == 0)
+	v = ist("");
+
 /*
  * Here, an HTTP header (which is actually part
  * of the span context is added to the text_map.
-- 
2.30.1

Re: [PATCH] BUILD: opentracing: excluded use of haproxy variables for, OpenTracing context

[Miroslav Zagorac]

Hello Tim,

On 09/10/2021 10:55 PM, Tim Düsterhus wrote:

If it's not a NULL pointer then it must be a valid pointer. I don't
quite understand why you would need to replace this by an empty constant
string then? Is this about a missing NUL terminator? Otherwise a valid
pointer with .len == 0 would be equivalent to "".



No, v.ptr is not a null pointer and points to the wrong location, v.len
is equal to 0.

For example I added to the source two calls to the printf () function,
one before the htx_get_blk_value() function call and the other after:

-
printf(">>> n = { %p '%s' %zu }, v = { %p '%s' %zu }\n", n.ptr, n.ptr, 
n.len, v.ptr, v.ptr, v.len);


v = htx_get_blk_value(htx, blk);

printf(">>> v = { %p '%s' %zu }\n", v.ptr, v.ptr, v.len);
-

I intentionally did not write the string format as '%.*s' so that the
contents can be seen.  These two headers are treated here:

1, 'ot-ctx-uberctx-haproxy_id: '
2. 'ot-ctx-uber-trace-id: 3c4069d777e89019:3c4069d777e89019:0:1'

, and this is the result:

1st header:
>>> n = { 0x56079098bec1 
'ot-ctx-uberctx-haproxy_idot-ctx-uber-trace-id3c4069d777e89019:3c4069d777e89019:0:1' 
25 }, v = { (nil) '(null)' 0 }
>>> v = { 0x56079098beda 
'ot-ctx-uber-trace-id3c4069d777e89019:3c4069d777e89019:0:1' 0 }


As can be seen from the example, v.ptr = 0x56079098beda, v.len = 0.

2nd header:
>>> n = { 0x56079098beda 
'ot-ctx-uber-trace-id3c4069d777e89019:3c4069d777e89019:0:1' 20 }, v = { 
(nil) '(null)' 0 }

>>> v = { 0x56079098beee '3c4069d777e89019:3c4069d777e89019:0:1' 37 }

Maybe I use htx_get_blk_name()/htx_get_blk_value() in this function in
an inappropriate way, so I get such a result.


Unrelated to that: Having worked quite a bit with the ist API I would
generally recommend using the helper functions in ist.h (possibly adding
new ones if they are helpful) instead of directly touching the struct.
It makes refactoring much easier, because it's easy to find the function
calls vs. accessing struct members which might be available on different
structs.

As an example in flt_ot_http_headers_get you could use an ist for prefix
and then simply use isteqi() instead of strncasecmp().


The functions strncasecmp() and isteqi() are not compatible, because
isteqi() requires that the lengths of the strings being compared are
equal, whereas in this function that is not the case.  In addition, the
prefix and len should be put in the ist structure and n.len reduced to
the size of len, this is an unnecessary complication.



 > Of course, it could look like this:
 >
 > if (v.len == 0)
 > v.ptr = "" /* or v = ist("") */;
 >

In your example I would prefer v = ist("") over v.ptr = "" and leave the
heavy lifting up to the compiler for that reason.



Ok, I will use ist("").

Best regards,

--
Zaga

What can change the nature of a man?

Re: [PATCH] BUILD: opentracing: excluded use of haproxy variables for, OpenTracing context

[Miroslav Zagorac]

On 09/10/2021 07:18 PM, Tim Düsterhus wrote:

Patch 2 looks odd to me. Is the issue that .ptr is equal to NULL and
*that* is causing issues? Then I recommend the following instead:

if (!isttest(v))
v = ist("");

this is much cleaner, because it does not attempt to cram so much logic
into that single huge line.

In any case if the issue is a NULL then you must test for that NULL
instead of testing the length.


Hello Tim,

the problem is that in this case after calling the function
htx_get_blk_value(), v.ptr is not a NULL pointer but v.len is equal
to 0, if the http header has no value.

Of course, it could look like this:

if (v.len == 0)
   v.ptr = "" /* or v = ist("") */;


Best regards,

--
Zaga

What can change the nature of a man?

[PATCH] BUILD: opentracing: excluded use of haproxy variables for, OpenTracing context

[Miroslav Zagorac]

Hello all,

I am sending 4 patches that revert to the opentracing filter function, 
with the possibility of using haproxy variables to transfer the 
opentracing context excluded.


Work with haproxy variables will be functional in the foreseeable 
future, when the necessary changes are made in the source of the 
opentracing filter.


For a more detailed explanation of the changes you can see the 
description of the attached patches.



Source build with OpenTracing filter support on github can be enabled.


Willy, thank you for your understanding and patience.

--
Zaga

What can change the nature of a man?
>From 77fb9a98a82c72bd0696412e5d04425d2431a951 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 8 Sep 2021 20:56:05 +0200
Subject: [PATCH 1/4] BUILD: opentracing: exclude the use of haproxy variables
 for the OpenTracing context

Due to a recent change in the handling of haproxy variables, their use for
OpenTracing context transfer has been excluded from the compilation process.

The use of variables can be re-enabled if the newly defined variable
OT_USE_VARS is set to 1 when calling the 'make' utility.  However,
this should not be used for now as the compilation will end in error.

This change prevents the use of haproxy variables to convey the OpenTracing
context.  This means that the 'use-vars' parameter cannot be used in the
OpenTracing filter configuration for 'inject' and 'extract' operations.

An example configuration that uses this feature is in the test/ctx
directory, while the script to run that test is test/run-ctx.sh.

Then, the 'sess.ot.uuid' variable is no longer set when initializing the
OpenTracing session.  This means that this variable can still be used in
the OpenTracing configuration, but its contents will be empty.
---
 Makefile   |  1 +
 addons/ot/Makefile |  8 +++-
 addons/ot/include/filter.h |  2 +-
 addons/ot/include/parser.h | 19 +--
 addons/ot/src/event.c  |  6 ++
 addons/ot/src/filter.c |  2 ++
 addons/ot/src/parser.c |  4 
 addons/ot/src/scope.c  | 12 ++--
 8 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index d0a7862b6..38412275c 100644
--- a/Makefile
+++ b/Makefile
@@ -107,6 +107,7 @@
 #   OT_INC : force the include path to libopentracing-c-wrapper
 #   OT_LIB : force the lib path to libopentracing-c-wrapper
 #   OT_RUNPATH : add RUNPATH for libopentracing-c-wrapper to haproxy executable
+#   OT_USE_VARS: allows the use of variables for the OpenTracing context
 #   IGNOREGIT  : ignore GIT commit versions if set.
 #   VERSION: force haproxy version reporting.
 #   SUBVERS: add a sub-version (eg: platform, model, ...).
diff --git a/addons/ot/Makefile b/addons/ot/Makefile
index ef48ce49d..0cc8c6aad 100644
--- a/addons/ot/Makefile
+++ b/addons/ot/Makefile
@@ -3,6 +3,7 @@
 # OT_INC : force the include path to libopentracing-c-wrapper
 # OT_LIB : force the lib path to libopentracing-c-wrapper
 # OT_RUNPATH : add libopentracing-c-wrapper RUNPATH to haproxy executable
+# OT_USE_VARS : allows the use of variables for the OpenTracing context
 
 OT_DEFINE=
 OT_CFLAGS=
@@ -61,8 +62,13 @@ OPTIONS_OBJS += \
 	addons/ot/src/parser.o  \
 	addons/ot/src/pool.o\
 	addons/ot/src/scope.o   \
-	addons/ot/src/util.o\
+	addons/ot/src/util.o
+
+ifneq ($(OT_USE_VARS),)
+OT_DEFINE = -DUSE_OT_VARS
+OPTIONS_OBJS += \
 	addons/ot/src/vars.o
+endif
 
 OPTIONS_CFLAGS  += $(OT_CFLAGS) -Iaddons/ot/include
 OPTIONS_LDFLAGS += $(OT_LDFLAGS)
diff --git a/addons/ot/include/filter.h b/addons/ot/include/filter.h
index 0b36354e5..c97a0cc59 100644
--- a/addons/ot/include/filter.h
+++ b/addons/ot/include/filter.h
@@ -22,7 +22,7 @@
 
 #define FLT_OT_FMT_NAME   "'" FLT_OT_OPT_NAME "' : "
 #define FLT_OT_FMT_TYPE   "'filter' : "
-#define FTL_OT_VAR_UUID   "sess", "ot", "uuid"
+#define FLT_OT_VAR_UUID   "sess", "ot", "uuid"
 #define FLT_OT_ALERT(f, ...)  ha_alert(FLT_OT_FMT_TYPE FLT_OT_FMT_NAME f "\n", ##__VA_ARGS__)
 
 #define FLT_OT_CONDITION_IF   "if"
diff --git a/addons/ot/include/parser.h b/addons/ot/include/parser.h
index 75a39cc0c..96939916d 100644
--- a/addons/ot/include/parser.h
+++ b/addons/ot/include/parser.h
@@ -65,14 +65,29 @@
 	FLT_OT_PARSE_GROUP_DEF(ID, 0, 1, 2, 2, "ot-group", " ") \
 	FLT_OT_PARSE_GROUP_DEF(SCOPES, 0, 0, 2, 0, "scopes",   "  ...")
 
+#ifdef USE_OT_VARS
+#  define FLT_OT_PARSE_SCOPE_INJECT_HELP"  [use-vars] [use-headers]"
+#  define FLT_OT_PARSE_SCOPE_EXTRACT_HELP   "  [use-vars | use-headers]"
+#else
+#  define FLT_OT_PARSE_SCOPE_INJECT_HELP"  [use-headers]"
+#  define FLT_OT_PARSE_SCOPE_EXTRACT_HELP   "  [use-headers]"
+#endif
+

Re: I just broke opentracing :-(

[Miroslav Zagorac]

Hello Willy,

On 09/09/2021 08:43 AM, Willy Tarreau wrote:

Yes, that's where I found it but didn't know what its use was. A few
points however:
   - sess.ot.uuid will be shared by all requests on the same session,
 which is probably not what you want (e.g. for requests coming in
 H2, the last one setting it will replace all other ones' value
 while they're working with it). I think you should change its
 scope to be limited to the transaction only (txn.ot.uuid). It's
 also used in debugging outputs where it will be per-request and
 may not match what the requests will log because of this.


Thanks, i'll change that.


   - I finally found its definition here in filter.h:
   #define FTL_OT_VAR_UUID   "sess", "ot", "uuid"

 Please note the typo in the macro name (FTL instead of FLT)


Strange that there is only one such typo. :)


   - the comment in struct flt_ot_runtime_context for uuid says
 "Randomly generated UUID". We already have a function to produce a
 random UUID (ha_generate_uuid()) but flt_ot_runtime_context_init()
 makes its own from elements that at first glance look like they're
 filled from other locations (uuid.node, uuid.clock_seq etc), but in
 fact are simply aliased on the random ints. In this case better use
 the existing function and avoid duplicating the painful output format
 generation. If you need to keep a copy of the two randoms, just
 modify the existing function to pass an optional array to store
 the randoms. At first glance they're not used though.


When ot filter source was created, I didn't know what would be used and
what wouldn't.  At the time I didn't even know what opentracing was,
what it was used for and how it was done.

So, adding 'useful' things to the source, there are also those members
of the structure that were not used anywhere later (because I thought
at the time that they might be used somewhere).

Not that they bother anything, but I will remove them.  I will also use
the haproxy function to generate uuid, it's a small change in ot source.

Then I didn't use that function because it doesn't set those two uuid
64-bit numbers and it was a bit of a weird way for me to generate uuid
because at the end of the haproxy function the value of v[2] rotates
to the right by 14 bits and the value of v[3] to the left by 18 bits
which for uint64_t[2] input gave me mixed bits.

For example, if both random numbers are equal to 0x0123456789abcdef,
the result is 89abcdef-4567-4123-8def-048d159e26af, while to me the
logical result would be 89abcdef-4567-4123-8def-0123456789ab.

This result is obtained if v[2] rotates to the right by 16 bits and
v[3] to the left by 16 bits.

If all the bits are equally good random, this is somehow a more logical
result for me, because they give the correct order of the bytes of the
input data (I'm not going into the correctness of the big/little
endian way of writing data now).


   - there's quite a bunch of functions with no description at all of
 their purpose, input values, return values etc. ..


Yes, writing comments always takes useful time, but they are necessary;
at least for weirder things..  I will fill in those function
descriptions over time and send patches.

--
Zaga

What can change the nature of a man?

Re: I just broke opentracing :-(

[Miroslav Zagorac]

On 09/08/2021 07:57 PM, Miroslav Zagorac wrote:

On 09/08/2021 07:42 PM, Willy Tarreau wrote:

No rush on this one, I'll let you think about it, just let me know if we
need to temporarily disable it from the CI, or if you just want a day or
two of checking before knowing if you can get a quick solution or if it
will take more time.



Thank you for understanding. For a start it would be good to exclude
ot filter from the compilation process so as not to interfere with the
development and testing of other code.

Next, I could throw out the part related to context transfer via
variables with #ifdef/#endif blocks, so that the ot filter code is
compiled without it.

After that the problem with variables could be solved without
interfering with the rest of the coding/compilation process and the
ot filter will be functional (except for the part with context transfer
via variables, but the question is whether anyone uses it).

The variable 'sess.ot.uuid' is just set, it is not used anywhere (as
far as memory serves me). I imagined that this could be used if the
user needs it, but it is not necessary.



OK, it is still used but again only as an example that it can be used
and not that it should be used.

In our examples uuid is set as one of the data from the opentracing
'baggage' data group:

test/ctx/ot.cfg:
---
   baggage "haproxy_id" var(sess.ot.uuid)
---

--
Zaga

What can change the nature of a man?

Re: I just broke opentracing :-(

[Miroslav Zagorac]

On 09/08/2021 07:42 PM, Willy Tarreau wrote:

No rush on this one, I'll let you think about it, just let me know if we
need to temporarily disable it from the CI, or if you just want a day or
two of checking before knowing if you can get a quick solution or if it
will take more time.



Thank you for understanding.  For a start it would be good to exclude
ot filter from the compilation process so as not to interfere with the
development and testing of other code.

Next, I could throw out the part related to context transfer via
variables with #ifdef/#endif blocks, so that the ot filter code is
compiled without it.

After that the problem with variables could be solved without
interfering with the rest of the coding/compilation process and the
ot filter will be functional (except for the part with context transfer
via variables, but the question is whether anyone uses it).

The variable 'sess.ot.uuid' is just set, it is not used anywhere (as
far as memory serves me).  I imagined that this could be used if the
user needs it, but it is not necessary.

--
Zaga

What can change the nature of a man?

Re: I just broke opentracing :-(

[Miroslav Zagorac]

On 09/08/2021 04:41 PM, Willy Tarreau wrote:

Hi Miroslav,

I just discovered that the changes I've made to the totally broken
variables API now broke opentracing because apparently it's using
some of the variable code's internals for its own use:

https://github.com/haproxy/haproxy/runs/3545475810

That's annoying because unbreaking the API was already extremely
complicated due to the global name table limitation that became a
feature on its own, but now I see that some upper level code outside
of vars/ is also making intimate use of that broken API (e.g. in
flt_ot_vars_unset), and I have no idea what it tries to do nor why.

Most of the breakage comes from all the undocumented functions
flt_ot_vars_* but I don't even understand why this does not use
the vars_* API that Lua, CLI and SPOE already use. Note that it
doesn't seem that the breakage is large, so most likely we can
figure together a few changes to perform to fix it, but reversing
all that is quite a pain, and even if I went through the process
of trying to build it I wouldn't know if what I'm doing works or
not :-/

The changes that broke are in the following commit range which tries
hard to minimize the visible changes:

10080716b..55f8a830d



Hello Willy,

there is a reason why i used some functions related to
reading/setting/searching variables.  If I could use the original 
haproxy functions, I wouldn't write these because duplicating the

code doesn't make sense.

What I have seen so far is that the 'name' member of the vars structure
has actually been deleted and that the var_clear() function has been
given another parameter.

Generally, in the ot filter it is possible to transfer data via a group
of variables with a common prefix, so that in one place we set all the
variables and their values ​​and in another we read all the variables
with a given prefix.  The moment we write the variables we know their
names but at the moment we read them we don't know how many there are
and their complete name, rather we take all the variables of a
particular prefix.

I need to take a closer look at how ot filter source can be redesigned
to retain functionality and use the new structure definition to store
variables.

--
Zaga

What can change the nature of a man?

[PATCH] BUILD: opentracing: fixed build when using pkg-config utility

[Miroslav Zagorac]

Hello all,

I am sending a patch that fixes building of the HAProxy in case the
system-installed opentracing c wrapper is used for the opentracing
addon.

This resolves GitHub issue #1323.

Best regards,

--
Zaga

What can change the nature of a man?
>From 5f5bffd72bb10ec80d3fe4cd1c6ea21945c2b498 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Thu, 29 Jul 2021 11:10:08 +0200
Subject: [PATCH] BUILD: opentracing: fixed build when using pkg-config utility

In case the OpenTracing C Wrapper library was installed as part of the
system (ie in a directory that pkg-config considers part of the system),
HAProxy building was not possible because in that case pkg-config did
not set the value of the OT_CFLAGS variable in the addon Makefile.

This resolves GitHub issue #1323.
---
 addons/ot/Makefile | 8 
 1 file changed, 8 insertions(+)

diff --git a/addons/ot/Makefile b/addons/ot/Makefile
index 9d9b1bfee..ef48ce49d 100644
--- a/addons/ot/Makefile
+++ b/addons/ot/Makefile
@@ -8,6 +8,7 @@ OT_DEFINE=
 OT_CFLAGS=
 OT_LDFLAGS   =
 OT_DEBUG_EXT =
+OT_PKGSTAT   =
 OTC_WRAPPER  = opentracing-c-wrapper
 
 ifneq ($(OT_DEBUG),)
@@ -16,6 +17,7 @@ OT_DEFINE= -DDEBUG_OT
 endif
 
 ifeq ($(OT_INC),)
+OT_PKGSTAT = $(shell pkg-config --exists $(OTC_WRAPPER)$(OT_DEBUG_EXT); echo $$?)
 OT_CFLAGS = $(shell pkg-config --silence-errors --cflags $(OTC_WRAPPER)$(OT_DEBUG_EXT))
 else
 ifneq ($(wildcard $(OT_INC)/$(OTC_WRAPPER)/.*),)
@@ -23,9 +25,15 @@ OT_CFLAGS = -I$(OT_INC) $(if $(OT_DEBUG),-DOTC_DBG_MEM)
 endif
 endif
 
+ifeq ($(OT_PKGSTAT),)
 ifeq ($(OT_CFLAGS),)
 $(error OpenTracing C wrapper : can't find headers)
 endif
+else
+ifneq ($(OT_PKGSTAT),0)
+$(error OpenTracing C wrapper : can't find package)
+endif
+endif
 
 ifeq ($(OT_LIB),)
 OT_LDFLAGS = $(shell pkg-config --silence-errors --libs $(OTC_WRAPPER)$(OT_DEBUG_EXT))
-- 
2.30.1

[PATCH] CLEANUP: server: a separate function for initializing the per_thr field

[Miroslav Zagorac]

Hello all,

To avoid repeating the same source code, allocating memory and
initializing the per_thr field from the server structure is
transferred to a separate function.

It seems to me that this code is only valid for the current version of 
haproxy, ie v2.4.


Best regards,

--
Zaga

What can change the nature of a man?
>From 81ec1afe2a7da947952deedb394970248df6cd8b Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 15 Jun 2021 15:33:20 +0200
Subject: [PATCH] CLEANUP: server: a separate function for initializing the
 per_thr field

To avoid repeating the same source code, allocating memory and initializing
the per_thr field from the server structure is transferred to a separate
function.
---
 include/haproxy/server.h |  1 +
 src/cfgparse.c   | 10 +-
 src/server.c | 32 ++--
 src/sink.c   | 11 +--
 4 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 74ac74053..b807a8a04 100644
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -60,6 +60,7 @@ int srv_init_addr(void);
 struct server *cli_find_server(struct appctx *appctx, char *arg);
 struct server *new_server(struct proxy *proxy);
 void free_server(struct server *srv);
+int srv_init_per_thr(struct server *srv);
 
 /* functions related to server name resolution */
 int srv_prepare_for_resolution(struct server *srv, const char *hostname);
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 906ab076f..800c61d24 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -3930,21 +3930,13 @@ int check_config_validity()
 
 	list_for_each_entry(newsrv, _list, global_list) {
 		/* initialize idle conns lists */
-		newsrv->per_thr = calloc(global.nbthread, sizeof(*newsrv->per_thr));
-		if (!newsrv->per_thr) {
+		if (srv_init_per_thr(newsrv) == -1) {
 			ha_alert("parsing [%s:%d] : failed to allocate per-thread lists for server '%s'.\n",
 			 newsrv->conf.file, newsrv->conf.line, newsrv->id);
 			cfgerr++;
 			continue;
 		}
 
-		for (i = 0; i < global.nbthread; i++) {
-			newsrv->per_thr[i].idle_conns = EB_ROOT;
-			newsrv->per_thr[i].safe_conns = EB_ROOT;
-			newsrv->per_thr[i].avail_conns = EB_ROOT;
-			MT_LIST_INIT(>per_thr[i].streams);
-		}
-
 		if (newsrv->max_idle_conns != 0) {
 			newsrv->curr_idle_thr = calloc(global.nbthread, sizeof(*newsrv->curr_idle_thr));
 			if (!newsrv->curr_idle_thr) {
diff --git a/src/server.c b/src/server.c
index 57b3feecb..2c55e320e 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4308,6 +4308,27 @@ static int srv_alloc_lb(struct server *sv, struct proxy *be)
 	return 1;
 }
 
+/* Memory allocation and initialization of the per_thr field.
+ * Returns 0 if the field has been successfully initialized, -1 on failure.
+ */
+int srv_init_per_thr(struct server *srv)
+{
+	int i;
+
+	srv->per_thr = calloc(global.nbthread, sizeof(*srv->per_thr));
+	if (!srv->per_thr)
+		return -1;
+
+	for (i = 0; i < global.nbthread; i++) {
+		srv->per_thr[i].idle_conns = EB_ROOT;
+		srv->per_thr[i].safe_conns = EB_ROOT;
+		srv->per_thr[i].avail_conns = EB_ROOT;
+		MT_LIST_INIT(>per_thr[i].streams);
+	}
+
+	return 0;
+}
+
 /* Parse a "add server" command
  * Returns 0 if the server has been successfully initialized, 1 on failure.
  */
@@ -4317,7 +4338,6 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
 	struct server *srv;
 	char *be_name, *sv_name;
 	int errcode, argc;
-	int i;
 	const int parse_flags = SRV_PARSE_DYNAMIC|SRV_PARSE_PARSE_ADDR;
 
 	usermsgs_clr("CLI");
@@ -4379,19 +4399,11 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
 		}
 	}
 
-	srv->per_thr = calloc(global.nbthread, sizeof(*srv->per_thr));
-	if (!srv->per_thr) {
+	if (srv_init_per_thr(srv) == -1) {
 		ha_alert("failed to allocate per-thread lists for server.\n");
 		goto out;
 	}
 
-	for (i = 0; i < global.nbthread; i++) {
-		srv->per_thr[i].idle_conns = EB_ROOT;
-		srv->per_thr[i].safe_conns = EB_ROOT;
-		srv->per_thr[i].avail_conns = EB_ROOT;
-		MT_LIST_INIT(>per_thr[i].streams);
-	}
-
 	if (srv->max_idle_conns != 0) {
 		srv->curr_idle_thr = calloc(global.nbthread, sizeof(*srv->curr_idle_thr));
 		if (!srv->curr_idle_thr) {
diff --git a/src/sink.c b/src/sink.c
index 6358da53a..119c4e045 100644
--- a/src/sink.c
+++ b/src/sink.c
@@ -940,7 +940,6 @@ struct sink *sink_new_from_logsrv(struct logsrv *logsrv)
 	struct sink *sink = NULL;
 	struct server *srv = NULL;
 	struct sink_forward_target *sft = NULL;
-	int i;
 
 	/* allocate new proxy to handle
 	 * forward to a stream server
@@ -972,17 +971,9 @@ struct sink *sink_new_from_logsrv(struct logsrv *logsrv)
 	HA_SPIN_INIT(>lock);
 
 	/* process per thread init */
-	srv->per_thr = calloc(global.nbthread, sizeof(*srv->per_thr));
-	if (!srv->p

Re: [PATCH] BUG/MINOR: opentracing: fixed files existence check in chroot mode

[Miroslav Zagorac]

On 06/10/2021 04:20 AM, Willy Tarreau wrote:

Thank you Miroslav. Just to be sure, is this in anyway related to the
fix or not ? We need to make sure that we maintain a smooth upgrade
path for those who provide packages.

I'll retag your patch as BUG/MEDIUM as it really addresses an initialization
issue and the fix is not that trivial. We may add the above info to the
commit message if needed.



Hello Willy,

I added the required version of the wrapper to the commit description
and changed the tag as you suggested, also dropped one parameter from
the ot_init () function because it is no longer used (after the
application of this commit).

If there is anything else, feel free to change it.  :)

Best regards,

--
Zaga

What can change the nature of a man?
>From 252a9c1f425fb3d2e37e6390174a5be4a5081a2a Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Thu, 10 Jun 2021 01:23:15 +0200
Subject: [PATCH 2/2] BUG/MEDIUM: opentracing: initialization before
 establishing daemon and/or chroot mode

This patch solves the problem reported in github issue #1204, where the
OpenTracing filter cannot communicate with the selected tracer if HAProxy
is run in daemon mode.

This commit also solves github issue #1274, where the problem manifests
itself when using the 'chroot' keyword in the HAProxy configuration.

This is solved so that the initialization of the OpenTracing plugin is
split into two operations, first the plugin (dynamic library) is loaded
before switching the HAProxy to daemon mode (or chroot) and then the
tracer thread is started.

This means that nothing is retrieved from the file system in runtime.

After applying this commit, opentracing C wrapper version 1.1.0 should be
used because the earlier version does not have separated initialization
functions.

This resolves GitHub issues #1204 and #1274.
---
 addons/ot/include/conf.h|  9 
 addons/ot/include/opentracing.h |  3 ++-
 addons/ot/src/conf.c|  1 +
 addons/ot/src/filter.c  | 35 +++--
 addons/ot/src/opentracing.c | 40 +
 addons/ot/src/parser.c  | 10 +++--
 6 files changed, 80 insertions(+), 18 deletions(-)

diff --git a/addons/ot/include/conf.h b/addons/ot/include/conf.h
index 97df12e05..a4bb7fcc0 100644
--- a/addons/ot/include/conf.h
+++ b/addons/ot/include/conf.h
@@ -57,10 +57,10 @@
 #define FLT_OT_DBG_CONF_PH(f,a) \
 	FLT_OT_DBG(3, f FLT_OT_CONF_HDR_FMT "%p }", FLT_OT_CONF_HDR_ARGS(a, id), (a)->ptr)
 
-#define FLT_OT_DBG_CONF_TRACER(f,a)   \
-	FLT_OT_DBG(3, f FLT_OT_CONF_HDR_FMT "'%s' '%s' %p %u %hhu %hhu 0x%02hhx %p:%s 0x%08x %s %s %s }", \
-	   FLT_OT_CONF_HDR_ARGS(a, id), (a)->config, (a)->plugin, (a)->tracer, (a)->rate_limit, (a)->flag_harderr,\
-	   (a)->flag_disabled, (a)->logging, &((a)->proxy_log), flt_ot_list_debug(&((a)->proxy_log.logsrvs)), (a)->analyzers, \
+#define FLT_OT_DBG_CONF_TRACER(f,a) \
+	FLT_OT_DBG(3, f FLT_OT_CONF_HDR_FMT "'%s' %p '%s' %p %u %hhu %hhu 0x%02hhx %p:%s 0x%08x %s %s %s }",\
+	   FLT_OT_CONF_HDR_ARGS(a, id), (a)->config, (a)->cfgbuf, (a)->plugin, (a)->tracer, (a)->rate_limit, (a)->flag_harderr, \
+	   (a)->flag_disabled, (a)->logging, &((a)->proxy_log), flt_ot_list_debug(&((a)->proxy_log.logsrvs)), (a)->analyzers,   \
 	   flt_ot_list_debug(&((a)->acls)), flt_ot_list_debug(&((a)->ph_groups)), flt_ot_list_debug(&((a)->ph_scopes)))
 
 #define FLT_OT_DBG_CONF(f,a)  \
@@ -155,6 +155,7 @@ struct flt_ot_conf_ph {
 struct flt_ot_conf_tracer {
 	FLT_OT_CONF_HDR(id);  /* The tracer name. */
 	char  *config;/* The OpenTracing configuration file name. */
+	char  *cfgbuf;/* The OpenTracing configuration. */
 	char  *plugin;/* The OpenTracing plugin library file name. */
 	struct otc_tracer *tracer;/* The OpenTracing tracer handle. */
 	uint32_t   rate_limit;/* [0 2^32-1] <-> [0.0 100.0] */
diff --git a/addons/ot/include/opentracing.h b/addons/ot/include/opentracing.h
index 2dbf5321a..2b88a330e 100644
--- a/addons/ot/include/opentracing.h
+++ b/addons/ot/include/opentracing.h
@@ -51,7 +51,8 @@
 void ot_text_map_show(const struct otc_text_map *text_map);
 void ot_debug(void);
 #endif
-int  ot_init(struct otc_tracer **tracer, const char *config, const char *plugin, char **err);
+int  ot_init(struct otc_tracer **tracer, const char *plugin, char **err);
+int 

Re: [PATCH] BUG/MINOR: opentracing: fixed files existence check in chroot mode

[Miroslav Zagorac]

I forgot to mention that one should take the latest version of the 
opentracing c wrapper (it is now 1.1.0).


https://github.com/haproxytech/opentracing-c-wrapper

--
Zaga

What can change the nature of a man?

Re: [PATCH] BUG/MINOR: opentracing: fixed files existence check in chroot mode

[Miroslav Zagorac]

On 06/09/2021 11:46 AM, Miroslav Zagorac wrote:

On 06/09/2021 09:10 AM, Willy Tarreau wrote:

Hi Miroslav,

On Mon, Jun 07, 2021 at 04:55:21PM +0200, Miroslav Zagorac wrote:

...
In fact, the only access to these files is achieved only once at the
beginning of the HAProxy process, in the initialization of threads.
After this initialization, no access to the file system is performed.

This resolves GitHub issue #1274.

...
Please try again to have a real initialization phase in the post_check
or wherever suits you (we can even add another hook if you need a very
special place, it's not a problem), but this thing needs to be
initialized
and to have its files loaded before chrooting. And if you still face any
issue doing that, we can discuss it to figure how to address it, but I
don't want us to paper over problems using methods that have short-term
nor long-term implications on the users. And this one definitely has.


Hello Willy,

yes, I agree with your suggestion to determine exactly where the problem
is, i.e. why the thread started by the opentracing library is killed
after chroot().

I hope it's something trivial about what I missed, because otherwise
someone needs to bury themselves in the depths of several connected
libraries written in a pretty complex C++ code.



Hello Willy,

the next two patches solve the problem satisfactorily, no more file
access after initializing the daemon mode (and/or chroot mode).

This is solved so that the initialization of the OpenTracing plugin is
split into two operations, first the plugin (dynamic library) is loaded
before switching the HAProxy to daemon mode (or chroot) and then the
tracer thread is started.

This resolves GitHub issues #1204 and #1274.

Best regards,

--
Zaga

What can change the nature of a man?
>From fd06ebf4cab6dcef649e1ca5a6f38db33ae68b26 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Thu, 10 Jun 2021 01:19:13 +0200
Subject: [PATCH 1/2] Revert "BUG/MINOR: opentracing: initialization after
 establishing daemon mode"

This reverts commit f2263435d71964d1aa3eb80df6464500696c0515.

This commit is unnecessary because although it solves the problem of using
the OpenTracing filter in daemon mode, it does not solve the same problem
if chroot is used.

The following commit related to the OpenTracing filter solves both problems
efficiently.
---
 addons/ot/src/filter.c | 48 ++
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/addons/ot/src/filter.c b/addons/ot/src/filter.c
index d5fc2c72f..6699d4613 100644
--- a/addons/ot/src/filter.c
+++ b/addons/ot/src/filter.c
@@ -156,15 +156,29 @@ static void flt_ot_return_void(const struct filter *f, char **err)
 static int flt_ot_init(struct proxy *p, struct flt_conf *fconf)
 {
 	struct flt_ot_conf *conf = FLT_OT_DEREF(fconf, conf, NULL);
-	int retval = FLT_OT_RET_OK;
+	char   *err = NULL;
+	int retval = FLT_OT_RET_ERROR;
 
 	FLT_OT_FUNC("%p, %p", p, fconf);
 
 	if (conf == NULL)
-		FLT_OT_RETURN(FLT_OT_RET_ERROR);
+		FLT_OT_RETURN(retval);
 
 	flt_ot_cli_init();
 
+	/*
+	 * Initialize the OpenTracing library.
+	 * Enable HTX streams filtering.
+	 */
+	retval = ot_init(&(conf->tracer->tracer), conf->tracer->config, conf->tracer->plugin, );
+	if (retval != FLT_OT_RET_ERROR)
+		fconf->flags |= FLT_CFG_FL_HTX;
+	else if (err != NULL) {
+		FLT_OT_ALERT("%s", err);
+
+		FLT_OT_ERR_FREE(err);
+	}
+
 	FLT_OT_RETURN(retval);
 }
 
@@ -412,6 +426,8 @@ static int flt_ot_check(struct proxy *p, struct flt_conf *fconf)
 }
 
 
+#ifdef DEBUG_OT
+
 /***
  * NAME
  *   flt_ot_init_per_thread -
@@ -430,38 +446,14 @@ static int flt_ot_check(struct proxy *p, struct flt_conf *fconf)
  */
 static int flt_ot_init_per_thread(struct proxy *p, struct flt_conf *fconf)
 {
-	struct flt_ot_conf *conf = FLT_OT_DEREF(fconf, conf, NULL);
-	char   *err = NULL;
-	int retval = FLT_OT_RET_ERROR;
+	int retval = FLT_OT_RET_OK;
 
 	FLT_OT_FUNC("%p, %p", p, fconf);
 
-	if (conf == NULL)
-		FLT_OT_RETURN(retval);
-
-	/*
-	 * Initialize the OpenTracing library.
-	 * Enable HTX streams filtering.
-	 */
-	if (conf->tracer->tracer == NULL) {
-		retval = ot_init(&(conf->tracer->tracer), conf->tracer->config, conf->tracer->plugin, );
-		if (retval != FLT_OT_RET_ERROR)
-			fconf->flags |= FLT_CFG_FL_HTX;
-		else if (err != NULL) {
-			FLT_OT_ALERT("%s", err);
-
-			FLT_OT_ERR_FREE(err);
-		}
-	} else {
-		retval = FLT_OT_RET_OK;
-	}
-
 	FLT_OT_RETURN(retval);
 }
 
 
-#ifdef DEBUG_OT
-
 /***
  * NAME
  *   flt_ot_deinit_per_thread -
@@ -1120,7 +1112,7 @@ struct flt_ops flt_ot_ops = {
 	.init  = flt_ot_init,
 	.deinit= flt_ot_deinit,
 	.check = flt_ot_check,
-	.init_per_thread   = flt_ot_init_per_thread,
+	.init_per_thread   = FLT_OT_DBG_IFDEF(flt_ot_init_per_thread

Re: [PATCH] BUG/MINOR: opentracing: fixed files existence check in chroot mode

[Miroslav Zagorac]

On 06/09/2021 09:10 AM, Willy Tarreau wrote:

Hi Miroslav,

On Mon, Jun 07, 2021 at 04:55:21PM +0200, Miroslav Zagorac wrote:

...
In fact, the only access to these files is achieved only once at the
beginning of the HAProxy process, in the initialization of threads.
After this initialization, no access to the file system is performed.

This resolves GitHub issue #1274.

...
Please try again to have a real initialization phase in the post_check
or wherever suits you (we can even add another hook if you need a very
special place, it's not a problem), but this thing needs to be initialized
and to have its files loaded before chrooting. And if you still face any
issue doing that, we can discuss it to figure how to address it, but I
don't want us to paper over problems using methods that have short-term
nor long-term implications on the users. And this one definitely has.


Hello Willy,

yes, I agree with your suggestion to determine exactly where the problem 
is, i.e. why the thread started by the opentracing library is killed 
after chroot().


I hope it's something trivial about what I missed, because otherwise 
someone needs to bury themselves in the depths of several connected 
libraries written in a pretty complex C++ code.


See you soon, with hopefully good news.  :)

Best regards,

--
Zaga

What can change the nature of a man?

Re: [PATCH] BUG/MINOR: opentracing: fixed files existence check in chroot mode

[Miroslav Zagorac]

Hello all,

I apologize for resending the patch, part of the description was written 
twice, now it's fixed


Best regards,

--
Zaga

What can change the nature of a man?
>From 4bbbe5fd3e66a37ec9703723ba22b742e7926a07 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Mon, 7 Jun 2021 16:21:31 +0200
Subject: [PATCH] BUG/MINOR: opentracing: fixed files existence check in chroot
 mode

If the 'chroot' keyword is used in the HAProxy configuration file,
HAProxy reports an error when initializing the OpenTracing API
library.

The problem is that HAProxy also executes chdir("/") during chroot
process, so the paths written in the OpenTracing configuration are
no longer correct.

This could be easily solved by writing the absolute path when using
the 'config' and 'plugin' keywords, but the problem remains that the
validity of these paths is also checked before the chroot process.

To enable the use of the absolute path of the specified files after
the chroot process, the file existence check is moved from the
configuration parser to the ot_init() function (which is executed
after the chroot/chdir process).

This may be a bit problematic because in this case the files from the
file system are retrieved in the HAProxy runtime.

In fact, the only access to these files is achieved only once at the
beginning of the HAProxy process, in the initialization of threads.
After this initialization, no access to the file system is performed.

This resolves GitHub issue #1274.
---
 addons/ot/src/opentracing.c | 11 +++
 addons/ot/src/parser.c  |  2 --
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/addons/ot/src/opentracing.c b/addons/ot/src/opentracing.c
index 58936d122..9dab708d4 100644
--- a/addons/ot/src/opentracing.c
+++ b/addons/ot/src/opentracing.c
@@ -171,6 +171,17 @@ int ot_init(struct otc_tracer **tracer, const char *config, const char *plugin,
 
 		FLT_OT_RETURN(retval);
 	}
+	else if (access(config, R_OK) == -1) {
+		FLT_OT_ERR("'%s' : %s", config, strerror(errno));
+
+		FLT_OT_RETURN(retval);
+	}
+	else if (access(path, R_OK) == -1) {
+		FLT_OT_ERR("'%s' : %s", path, strerror(errno));
+
+		FLT_OT_RETURN(retval);
+	}
+
 
 	*tracer = otc_tracer_init(path, config, NULL, errbuf, sizeof(errbuf));
 	if (*tracer == NULL) {
diff --git a/addons/ot/src/parser.c b/addons/ot/src/parser.c
index 5dec8629d..c515709cf 100644
--- a/addons/ot/src/parser.c
+++ b/addons/ot/src/parser.c
@@ -404,8 +404,6 @@ static int flt_ot_parse_cfg_file(char **ptr, const char *file, int linenum, char
 		FLT_OT_PARSE_ERR(err, "'%s' : no %s specified", flt_ot_current_tracer->id, err_msg);
 	else if (alertif_too_many_args(1, file, linenum, args, ))
 		retval |= ERR_ABORT | ERR_ALERT;
-	else if (access(args[1], R_OK) == -1)
-		FLT_OT_PARSE_ERR(err, "'%s' : %s", args[1], strerror(errno));
 	else
 		retval = flt_ot_parse_keyword(ptr, args, 0, 0, err, err_msg);
 
-- 
2.30.1

[PATCH] BUG/MINOR: opentracing: fixed files existence check in chroot mode

[Miroslav Zagorac]

Hello all,


If the 'chroot' keyword is used in the HAProxy configuration file,
HAProxy reports an error when initializing the OpenTracing API
library.

The problem is that HAProxy also executes chdir("/") during chroot
process, so the paths written in the OpenTracing configuration are
no longer correct.

This could be easily solved by writing the absolute path when using
the 'config' and 'plugin' keywords, but the problem remains that the
validity of these paths is also checked before the chroot process.

To allow the use of the absolute path of the specified files after
chroot process, the file existence check is moved from the
configuration parser to the ot_init() function (which is executed
after chroot/chdir process).

To enable the use of the absolute path of the specified files after
the chroot process, the file existence check is moved from the
configuration parser to the ot_init() function (which is executed
after the chroot/chdir process).

This may be a bit problematic because in this case the files from the
file system are retrieved in the HAProxy runtime.

In fact, the only access to these files is achieved only once at the
beginning of the HAProxy process, in the initialization of threads.
After this initialization, no access to the file system is performed.

This resolves GitHub issue #1274.


Best regards,

--
Zaga

What can change the nature of a man?
>From 50dadc20167d5d5dfa214baac031160fa9a6c612 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Mon, 7 Jun 2021 16:21:31 +0200
Subject: [PATCH] BUG/MINOR: opentracing: fixed files existence check in chroot
 mode

If the 'chroot' keyword is used in the HAProxy configuration file,
HAProxy reports an error when initializing the OpenTracing API
library.

The problem is that HAProxy also executes chdir("/") during chroot
process, so the paths written in the OpenTracing configuration are
no longer correct.

This could be easily solved by writing the absolute path when using
the 'config' and 'plugin' keywords, but the problem remains that the
validity of these paths is also checked before the chroot process.

To allow the use of the absolute path of the specified files after
chroot process, the file existence check is moved from the
configuration parser to the ot_init() function (which is executed
after chroot/chdir process).

To enable the use of the absolute path of the specified files after
the chroot process, the file existence check is moved from the
configuration parser to the ot_init() function (which is executed
after the chroot/chdir process).

This may be a bit problematic because in this case the files from the
file system are retrieved in the HAProxy runtime.

In fact, the only access to these files is achieved only once at the
beginning of the HAProxy process, in the initialization of threads.
After this initialization, no access to the file system is performed.

This resolves GitHub issue #1274.
---
 addons/ot/src/opentracing.c | 11 +++
 addons/ot/src/parser.c  |  2 --
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/addons/ot/src/opentracing.c b/addons/ot/src/opentracing.c
index 58936d122..9dab708d4 100644
--- a/addons/ot/src/opentracing.c
+++ b/addons/ot/src/opentracing.c
@@ -171,6 +171,17 @@ int ot_init(struct otc_tracer **tracer, const char *config, const char *plugin,
 
 		FLT_OT_RETURN(retval);
 	}
+	else if (access(config, R_OK) == -1) {
+		FLT_OT_ERR("'%s' : %s", config, strerror(errno));
+
+		FLT_OT_RETURN(retval);
+	}
+	else if (access(path, R_OK) == -1) {
+		FLT_OT_ERR("'%s' : %s", path, strerror(errno));
+
+		FLT_OT_RETURN(retval);
+	}
+
 
 	*tracer = otc_tracer_init(path, config, NULL, errbuf, sizeof(errbuf));
 	if (*tracer == NULL) {
diff --git a/addons/ot/src/parser.c b/addons/ot/src/parser.c
index 5dec8629d..c515709cf 100644
--- a/addons/ot/src/parser.c
+++ b/addons/ot/src/parser.c
@@ -404,8 +404,6 @@ static int flt_ot_parse_cfg_file(char **ptr, const char *file, int linenum, char
 		FLT_OT_PARSE_ERR(err, "'%s' : no %s specified", flt_ot_current_tracer->id, err_msg);
 	else if (alertif_too_many_args(1, file, linenum, args, ))
 		retval |= ERR_ABORT | ERR_ALERT;
-	else if (access(args[1], R_OK) == -1)
-		FLT_OT_PARSE_ERR(err, "'%s' : %s", args[1], strerror(errno));
 	else
 		retval = flt_ot_parse_keyword(ptr, args, 0, 0, err, err_msg);
 
-- 
2.30.1

[PATCH] BUILD/MINOR: opentracing: fixed build when using clang

[Miroslav Zagorac]

Hello,

this patch should solve the github issue #1242.

Best regards,

--
Zaga

What can change the nature of a man?
>From 5ca6a7230952f966a0eb5b0119b45d522c2c5999 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 18 May 2021 20:05:10 +0200
Subject: [PATCH] BUILD/MINOR: opentracing: fixed build when using clang

The arguments of the snprintf() function are now consistent with the
format used.

This should fix the github issue #1242.
---
 addons/ot/src/scope.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/addons/ot/src/scope.c b/addons/ot/src/scope.c
index 8305f24d2..8d70a08d6 100644
--- a/addons/ot/src/scope.c
+++ b/addons/ot/src/scope.c
@@ -116,8 +116,8 @@ struct flt_ot_runtime_context *flt_ot_runtime_context_init(struct stream *s, str
 	(void)snprintf(retptr->uuid.s, sizeof(retptr->uuid.s), "%08x-%04hx-%04hx-%04hx-%012" PRIx64,
 	   retptr->uuid.time_low,
 	   retptr->uuid.time_mid,
-	   (retptr->uuid.time_hi_and_version & UINT16_C(0xfff)) | UINT16_C(0x4000),
-	   retptr->uuid.clock_seq | UINT16_C(0x8000),
+	   (uint16_t)((retptr->uuid.time_hi_and_version & UINT16_C(0xfff)) | UINT16_C(0x4000)),
+	   (uint16_t)(retptr->uuid.clock_seq | UINT16_C(0x8000)),
 	   (uint64_t)retptr->uuid.node);
 
 	if (flt_ot_var_register(FTL_OT_VAR_UUID, err) != -1)
-- 
2.30.1

Re: [PATCH] CI: enable OpenTracing feature

[Miroslav Zagorac]

On 05/18/2021 06:49 PM, Илья Шипицин wrote:

Miroslav,


can you please keep an eye on patch from OpenTracing build failures using
clang · Issue #1242 · haproxy/haproxy (github.com)
  ?



Hello Илья,

I thought that patch was already applied..  Ok, I'll send it to the 
mailing list today.  Thank you for the notice.


Best regards,

--
Zaga

What can change the nature of a man?

[PATCH] BUILD/MINOR: opentracing: fixed compilation with filter, enabled

[Miroslav Zagorac]

Hello,

The inclusion of header files proxy.h and tools.h was added to the
addons/ot/include/include.h file.  Without this HAProxy cannot be
compiled if the OpenTracing filter is to be used.

Best regards,

--
Zaga

What can change the nature of a man?
>From 6fe243c20b938afdfa934d6c075ae154bac9a976 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 11 May 2021 19:21:54 +0200
Subject: [PATCH] BUILD/MINOR: opentracing: fixed compilation with filter
 enabled

The inclusion of header files proxy.h and tools.h was added to the
addons/ot/include/include.h file.  Without this HAProxy cannot be
compiled if the OpenTracing filter is to be used.
---
 addons/ot/include/include.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/addons/ot/include/include.h b/addons/ot/include/include.h
index 3c0d11094..f185a53b2 100644
--- a/addons/ot/include/include.h
+++ b/addons/ot/include/include.h
@@ -31,8 +31,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 
 #include "config.h"
-- 
2.30.1

Re: Opentracing span name from variable

[Miroslav Zagorac]

On 04/15/2021 04:24 PM, Andrea Bonini wrote:

Hi all,

is it possible to use a variable as span name?
for example, can i use uri as span name so in zipkin it's visible as
service:

Thanks



Hello Andrea,

the name of the span is defined statically and cannot be the content of 
a variable.  However, you can add a tag, baggage or log to the span with 
whatever dynamic content you want.


Best regards,

--
Zaga

What can change the nature of a man?

[OT PATCH 2/2] MINOR: opentracing: transfer of context names without prefix

[Miroslav Zagorac]

Hello all,

In order to enable the assignment of a context name, and yet exclude the
use of that name (prefix in this case) when extracting the context from
the HTTP header, a special character '-' has been added, which can be
specified at the beginning of the prefix.

So let's say if we look at examples of the fe-be configuration, we can
transfer the context via an HTTP header without a prefix like this:

  fe/ot.cfg:
..
span "HAProxy session"
inject "" use-headers
event on-backend-http-request

Such a context can be read in another process using a name that has a
special '-' sign at the beginning:

  be/ot.cfg:
ot-scope frontend_http_request
extract "-ot-ctx" use-headers
span "HAProxy session" child-of "-ot-ctx" root
..

This means that the context name will be '-ot-ctx' but it will not be
used when extracting data from HTTP headers.

Of course, if the context does not have a prefix set, all HTTP headers
will be inserted into the OpenTracing library as context.  All of the
above will only work correctly if that library can figure out what is
relevant to the context and what is not.

Best regards,

--
Zaga

What can change the nature of a man?
>From 6ede4cf6e83b327db132e830dd09b728057ae375 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 14 Apr 2021 11:47:28 +0200
Subject: [PATCH 2/2] MINOR: opentracing: transfer of context names without
 prefix

In order to enable the assignment of a context name, and yet exclude the
use of that name (prefix in this case) when extracting the context from
the HTTP header, a special character '-' has been added, which can be
specified at the beginning of the prefix.

So let's say if we look at examples of the fe-be configuration, we can
transfer the context via an HTTP header without a prefix like this:

  fe/ot.cfg:
..
span "HAProxy session"
inject "" use-headers
event on-backend-http-request

Such a context can be read in another process using a name that has a
special '-' sign at the beginning:

  be/ot.cfg:
ot-scope frontend_http_request
extract "-ot-ctx" use-headers
span "HAProxy session" child-of "-ot-ctx" root
..

This means that the context name will be '-ot-ctx' but it will not be
used when extracting data from HTTP headers.

Of course, if the context does not have a prefix set, all HTTP headers
will be inserted into the OpenTracing library as context.  All of the
above will only work correctly if that library can figure out what is
relevant to the context and what is not.
---
 addons/ot/include/parser.h |  1 +
 addons/ot/src/http.c   | 13 +
 2 files changed, 14 insertions(+)

diff --git a/addons/ot/include/parser.h b/addons/ot/include/parser.h
index 5e7b298d5..75a39cc0c 100644
--- a/addons/ot/include/parser.h
+++ b/addons/ot/include/parser.h
@@ -38,6 +38,7 @@
 #define FLT_OT_PARSE_SPAN_REF_CHILD "child-of"
 #define FLT_OT_PARSE_SPAN_REF_FOLLOWS   "follows-from"
 #define FLT_OT_PARSE_CTX_AUTONAME   "-"
+#define FLT_OT_PARSE_CTX_IGNORE_NAME'-'
 #define FLT_OT_PARSE_CTX_USE_HEADERS"use-headers"
 #define FLT_OT_PARSE_CTX_USE_VARS   "use-vars"
 #define FLT_OT_PARSE_OPTION_HARDERR "hard-errors"
diff --git a/addons/ot/src/http.c b/addons/ot/src/http.c
index 72b31b76f..4a12ed854 100644
--- a/addons/ot/src/http.c
+++ b/addons/ot/src/http.c
@@ -99,6 +99,19 @@ struct otc_text_map *flt_ot_http_headers_get(struct channel *chn, const char *pr
 	if (chn == NULL)
 		FLT_OT_RETURN(retptr);
 
+	/*
+	 * The keyword 'inject' allows you to define the name of the OpenTracing
+	 * context without using a prefix.  In that case all HTTP headers are
+	 * transferred because it is not possible to separate them from the
+	 * OpenTracing context (this separation is usually done via a prefix).
+	 *
+	 * When using the 'extract' keyword, the context name must be specified.
+	 * To allow all HTTP headers to be extracted, the first character of
+	 * that name must be set to FLT_OT_PARSE_CTX_IGNORE_NAME.
+	 */
+	if (FLT_OT_STR_ISVALID(prefix) && (*prefix == FLT_OT_PARSE_CTX_IGNORE_NAME))
+		prefix_len = 0;
+
 	htx = htxbuf(&(chn->buf));
 
 	for (pos = htx_get_first(htx); pos != -1; pos = htx_get_next(htx, pos)) {
-- 
2.30.1

[OT PATCH 1/2] MINOR: opentracing: correct calculation of the number of arguments in the args[]

[Miroslav Zagorac]

Hello all,

It is possible that some arguments within the configuration line are not
specified; that is, they are set to a blank string.

For example:
  keyword '' arg_2

In that case the content of the args field will be like this:
  args[0]:  'keyword'
  args[1]:  NULL pointer
  args[2]:  'arg_2'
  args[3 .. MAX_LINE_ARGS): NULL pointers

The previous way of calculating the number of arguments (as soon as a
null pointer is encountered) could not place an argument on an empty
string.

All of the above is essential for passing the OpenTracing context via
the HTTP headers (keyword 'inject'), where one of the arguments is the
context name prefix.  This way we can set an empty prefix, which is very
useful if we get context from some other process that can't add a prefix
to that data; or we want to pass the context to some process that cannot
handle the prefix of that data.

Best regards,

--
Zaga

What can change the nature of a man?
>From 0c0c0c77fe5b1a71ea639b5693eab85a917e9df0 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 14 Apr 2021 11:44:58 +0200
Subject: [PATCH 1/2] MINOR: opentracing: correct calculation of the number of
 arguments in the args[]

It is possible that some arguments within the configuration line are not
specified; that is, they are set to a blank string.

For example:
  keyword '' arg_2

In that case the content of the args field will be like this:
  args[0]:  'keyword'
  args[1]:  NULL pointer
  args[2]:  'arg_2'
  args[3 .. MAX_LINE_ARGS): NULL pointers

The previous way of calculating the number of arguments (as soon as a
null pointer is encountered) could not place an argument on an empty
string.

All of the above is essential for passing the OpenTracing context via
the HTTP headers (keyword 'inject'), where one of the arguments is the
context name prefix.  This way we can set an empty prefix, which is very
useful if we get context from some other process that can't add a prefix
to that data; or we want to pass the context to some process that cannot
handle the prefix of that data.
---
 addons/ot/src/parser.c | 17 
 addons/ot/src/util.c   | 44 +++---
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/addons/ot/src/parser.c b/addons/ot/src/parser.c
index c7522e942..5dec8629d 100644
--- a/addons/ot/src/parser.c
+++ b/addons/ot/src/parser.c
@@ -189,7 +189,7 @@ static const char *flt_ot_parse_invalid_char(const char *name, int type)
  */
 static int flt_ot_parse_cfg_check(const char *file, int linenum, char **args, const void *id, const struct flt_ot_parse_data *parse_data, size_t parse_data_size, const struct flt_ot_parse_data **pdata, char **err)
 {
-	int i, retval = ERR_NONE;
+	int i, argc, retval = ERR_NONE;
 
 	FLT_OT_FUNC("\"%s\", %d, %p, %p, %p, %zu, %p:%p, %p:%p", file, linenum, args, id, parse_data, parse_data_size, FLT_OT_DPTR_ARGS(pdata), FLT_OT_DPTR_ARGS(err));
 
@@ -197,12 +197,15 @@ static int flt_ot_parse_cfg_check(const char *file, int linenum, char **args, co
 
 	*pdata = NULL;
 
+	/* First check here if args[0] is the correct keyword. */
 	for (i = 0; (*pdata == NULL) && (i < parse_data_size); i++)
 		if (strcmp(parse_data[i].name, args[0]) == 0)
 			*pdata = parse_data + i;
 
 	if (*pdata == NULL)
 		FLT_OT_PARSE_ERR(err, "'%s' : unknown keyword", args[0]);
+	else
+		argc = flt_ot_args_count(args);
 
 	if ((retval & ERR_CODE) || (id == NULL))
 		/* Do nothing. */;
@@ -214,20 +217,16 @@ static int flt_ot_parse_cfg_check(const char *file, int linenum, char **args, co
 	 * line than is required.
 	 */
 	if (!(retval & ERR_CODE))
-		for (i = 1; i < (*pdata)->args_min; i++)
-			if (!FLT_OT_ARG_ISVALID(i))
-FLT_OT_PARSE_ERR(err, "'%s' : too few arguments (use '%s%s')", args[0], (*pdata)->name, (*pdata)->usage);
+		if (argc < (*pdata)->args_min)
+			FLT_OT_PARSE_ERR(err, "'%s' : too few arguments (use '%s%s')", args[0], (*pdata)->name, (*pdata)->usage);
 
 	/*
 	 * Checking that more arguments are specified in the configuration
 	 * line than the maximum allowed.
 	 */
-	if (!(retval & ERR_CODE) && ((*pdata)->args_max > 0)) {
-		for ( ; (i <= (*pdata)->args_max) && FLT_OT_ARG_ISVALID(i); i++);
-
-		if (i > (*pdata)->args_max)
+	if (!(retval & ERR_CODE) && ((*pdata)->args_max > 0))
+		if (argc > (*pdata)->args_max)
 			FLT_OT_PARSE_ERR(err, "'%s' : too many arguments (use '%s%s')", args[0], (*pdata)->name, (*pdata)->usage);
-	}
 
 	/* Checking that the first argument has only allowed characters. */
 	if (!(retval & ERR_CODE) && ((*pdata)->check_name > 0)) {
diff --git a/addons/ot/src/util.c b/addons/ot/src/util.c
index 3adc5a300..f6812bcc2 100644
--- a/addons/ot/src/util.c
+++ b/addons/ot/src/util.c
@@

[OT PATCH 0/2] opentracing: use of the context name without prefix

[Miroslav Zagorac]

Hello all,

the next two patches allow you to transfer the OpenTracing context via 
an HTTP headers without using a name prefix.


This is very useful when transferring context from or to some other 
process that cannot add or remove a prefix from that data.


Best regards,

--
Zaga

What can change the nature of a man?

Re: haproxy 2.4 opentracing using x-b3-traceid header

[Miroslav Zagorac]

On 04/09/2021 01:40 PM, Andrea Bonini wrote:

Hi Zaga,
thank you for the explanation. My problem is that the external service send
the span info in header as x-b3-* format (the opentracing default one).
For reading it, which extract context must i configure? if i don't
understand wrong if i want read x-b3-* header i must use a context with
empty name that is not allowed.



Hello Andrea,

in this case the ability to accept the context with an empty name should 
be added to the OpenTracing filter.  I will think about it.  If this is 
not a problem, send the header content for one such example (only those 
headers related to OpenTracing, with complete name and content).


Thank you for the question and comment.

Best regards,

--
Zaga

What can change the nature of a man?

Re: haproxy 2.4 opentracing using x-b3-traceid header

[Miroslav Zagorac]

On 04/08/2021 01:48 PM, Andrea Bonini wrote:

Hi everyone,

i have a question about opentracing addon. Is there a way for using headers
info about a trace created from an outside service?

what i want is using x-b3-traceid header from an incoming request as
traceid and use spanid as parentid

Thanks



Hello Andrea,

you are interested in propagating the span context from one service to 
another.  This can be done by setting up additional http headers in the 
service that created the tracing process that contain the span context 
which is then read in another process that wants to add its data.  This 
can be done via the ‘inject’ and ‘extract’ keywords in the opentracing 
filter configuration.  In this way they can automatically set up and 
read the contents of the headers over which the span context is transmitted.


In the test directory there is one example that uses such a mode of 
operation, where two haproxy processes are started, the first of which 
passes the span context to the second.  The example is run via the shell 
script test/run-fe-be.sh and thus starts the frontend haproxy process 
(fe) which passes the span context to the backend haproxy process (be). 
 Configurations are located in the test/fe and test/be directories.


For more explanations read the README file from the ot directory.

Best regards,

--
Zaga

What can change the nature of a man?

Re: [PATCH] MINOR: opentracing: register config file and line number on log servers

[Miroslav Zagorac]

On 04/07/2021 12:13 PM, Илья Шипицин wrote:

do you consider adding opentracing to "github actions" CI ?



Hello Илья,

I don't know how to add it because I never used it.  The filter uses 
specific libraries that are not part of the system but need to be 
installed and/or compiled independently.


Best regards,

--
Zaga

What can change the nature of a man?

[PATCH] MINOR: opentracing: register config file and line number on log servers

[Miroslav Zagorac]

Hello,

due to the modified function declaration, the opentracing filter can no 
longer be compiled.


In commit 9533a7038 new parameters have been added to the declaration
of function parse_logsrv().

This patch should be backported to all branches where the OpenTracing
filter is located.

--
Zaga

What can change the nature of a man?
>From baae10ef8a2eb487328664a3ff04475cd5d76179 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 7 Apr 2021 11:14:23 +0200
Subject: [PATCH] MINOR: opentracing: register config file and line number on
 log servers

In commit 9533a7038 new parameters have been added to the declaration
of function parse_logsrv().

This patch should be backported to all branches where the OpenTracing
filter is located.
---
 addons/ot/src/parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/addons/ot/src/parser.c b/addons/ot/src/parser.c
index b53d58d44..c7522e942 100644
--- a/addons/ot/src/parser.c
+++ b/addons/ot/src/parser.c
@@ -490,7 +490,7 @@ static int flt_ot_parse_cfg_tracer(const char *file, int linenum, char **args, i
 		}
 	}
 	else if (pdata->keyword == FLT_OT_PARSE_TRACER_LOG) {
-		if (parse_logsrv(args, &(flt_ot_current_tracer->proxy_log.logsrvs), kw_mod == KWM_NO, _log) == 0) {
+		if (parse_logsrv(args, &(flt_ot_current_tracer->proxy_log.logsrvs), kw_mod == KWM_NO, file, linenum, _log) == 0) {
 			FLT_OT_PARSE_ERR(, "'%s %s ...' : %s", args[0], args[1], err_log);
 			FLT_OT_FREE_CLEAR(err_log);
 
-- 
2.30.1

[PATCH] BUG/MINOR: opentracing: initialization after establishing daemon mode

[Miroslav Zagorac]

Hello,

This patch solves the problem reported in github issue #1204, where the 
OpenTracing filter cannot communicate with the selected tracer if 
HAProxy is run in daemon mode.


This patch should be backported to all branches where the OpenTracing 
filter is located.


--
Zaga

What can change the nature of a man?
>From fe482c8adaa703e790b4d1995255b9754799b149 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Fri, 2 Apr 2021 04:25:47 +0200
Subject: [PATCH] BUG/MINOR: opentracing: initialization after establishing
 daemon mode

This patch solves the problem reported in github issue #1204, where the
OpenTracing filter cannot communicate with the selected tracer if HAProxy
is run in daemon mode.  The author of the reported issue uses Zipkin
tracer, while in this example Jaeger tracer is used (see gdb output below).

The problem is that the OpenTracing library is initialized before HAProxy
initialize the daemon mode.  Establishing this mode kills the OpenTracing
thread, after which the correct operation of the OpenTracing filter is no
longer possible.  Also, HAProxy crashes on deinitialization of the
OpenTracing library.

The initialization of the OpenTracing library has been moved from the
flt_ot_init() function (which is started before switching the HAProxy to
daemon mode) to the flt_ot_init_per_thread() function (which is run after
switching the HAProxy to daemon mode).

Gdb output of crashed HAProxy process:

  [Thread debugging using libthread_db enabled]
  Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
  Core was generated by `../../../haproxy -f sa/haproxy.cfg'.
  Program terminated with signal SIGSEGV, Segmentation fault.
  #0  0x7f8131fd5629 in pthread_join (threadid=140192831239936, thread_return=0x0) at pthread_join.c:45
  45  pthread_join.c: No such file or directory.
  (gdb) where
  #0  0x7f8131fd5629 in pthread_join (threadid=140192831239936, thread_return=0x0) at pthread_join.c:45
  #1  0x7f812f15abc7 in std::thread::join() ()
 from /tmp/haproxy-os-master/contrib/opentracing/test/libjaeger_opentracing_plugin-0.5.0.so
  #2  0x7f812f0fb6f7 in jaegertracing::reporters::RemoteReporter::close() ()
from /tmp/haproxy-os-master/contrib/opentracing/test/libjaeger_opentracing_plugin-0.5.0.so
  #3  0x7f812f0b7055 in jaegertracing::reporters::CompositeReporter::close() ()
   from /tmp/haproxy-os-master/contrib/opentracing/test/libjaeger_opentracing_plugin-0.5.0.so
  #4  0x7f812f0b9136 in jaegertracing::Tracer::Close() ()
  from /tmp/haproxy-os-master/contrib/opentracing/test/libjaeger_opentracing_plugin-0.5.0.so
  #5  0x7f81309def32 in ot_tracer_close (tracer=0x55fb48057390) at ../../src/tracer.cpp:91
  #6  0x55fb41785705 in ot_close (tracer=0x55fb48061168) at contrib/opentracing/src/opentracing.c:208
  #7  0x55fb4177fc64 in flt_ot_deinit (p=, fconf=) at contrib/opentracing/src/filter.c:215
  #8  0x55fb418bc038 in flt_deinit (proxy=proxy@entry=0x55fb4805ce50) at src/filters.c:360
  #9  0x55fb41893ed1 in free_proxy (p=0x55fb4805ce50) at src/proxy.c:315
  #10 0x55fb4109 in deinit () at src/haproxy.c:2217
  #11 0x55fb41889078 in deinit_and_exit (status=0) at src/haproxy.c:2343
  #12 0x55fb4173d809 in main (argc=, argv=) at src/haproxy.c:3230

This patch should be backported to all branches where the OpenTracing
filter is located.
---
 contrib/opentracing/src/filter.c | 48 +++-
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/contrib/opentracing/src/filter.c b/contrib/opentracing/src/filter.c
index 6699d4613..d5fc2c72f 100644
--- a/contrib/opentracing/src/filter.c
+++ b/contrib/opentracing/src/filter.c
@@ -156,29 +156,15 @@ static void flt_ot_return_void(const struct filter *f, char **err)
 static int flt_ot_init(struct proxy *p, struct flt_conf *fconf)
 {
 	struct flt_ot_conf *conf = FLT_OT_DEREF(fconf, conf, NULL);
-	char   *err = NULL;
-	int retval = FLT_OT_RET_ERROR;
+	int retval = FLT_OT_RET_OK;
 
 	FLT_OT_FUNC("%p, %p", p, fconf);
 
 	if (conf == NULL)
-		FLT_OT_RETURN(retval);
+		FLT_OT_RETURN(FLT_OT_RET_ERROR);
 
 	flt_ot_cli_init();
 
-	/*
-	 * Initialize the OpenTracing library.
-	 * Enable HTX streams filtering.
-	 */
-	retval = ot_init(&(conf->tracer->tracer), conf->tracer->config, conf->tracer->plugin, );
-	if (retval != FLT_OT_RET_ERROR)
-		fconf->flags |= FLT_CFG_FL_HTX;
-	else if (err != NULL) {
-		FLT_OT_ALERT("%s", err);
-
-		FLT_OT_ERR_FREE(err);
-	}
-
 	FLT_OT_RETURN(retval);
 }
 
@@ -426,8 +412,6 @@ static int flt_ot_check(struct proxy *p, struct flt_conf *fconf)
 }
 
 
-#ifdef DEBUG_OT
-
 /***
  * NAME
  *   flt_ot_init_per_thread -
@@ -446,14 +430,38 @@ static int flt_ot_check(struct proxy *p, struct flt_conf *fconf)
  */
 static int flt_ot_init_per_thread(struct proxy *p, struct flt_conf *fconf)
 

Re: [PATCH] BUILD: trace: include tools.h

[Miroslav Zagorac]

On 09/25/2020 05:59 PM, Willy Tarreau wrote:

Git uses the "---" marker as an end of commit message, to put your
comments to committers below. So when I applied your patch, it dropped
this part. I've reintroduced it, but I thought you would like to know
in order not to get caught by accident on another patch if you're used
to use this :-)

Oh and by the way it also drops lines starting with "#" which tends
to annoy me as I get caught once in a while when referencing a github
issue at the beginning of a line!



Hello Willy,

thank you for the info, I made a comment for commit with 'git commit 
--amend' so I could add those lines with '---'.  When I got the patch 
with 'git format-patch -1' I was a little surprised by those multiple 
lines with '---' but I didn't remove it.


--
Zaga

What can change the nature of a man?

[PATCH] BUILD: trace: include tools.h

[Miroslav Zagorac]

Hello,

haproxy cannot be compiled on debian 9.13 if the TRACE option is used.

--
Zaga

What can change the nature of a man?
>From 97fee9d468d5bd52716fa0352a5ce9b4522730fc Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Thu, 24 Sep 2020 09:15:46 +0200
Subject: [PATCH] BUILD: trace: include tools.h

If the TRACE option is used when compiling the haproxy source,
the following error occurs:

---
src/calltrace.o: In function `make_line':
.../src/calltrace.c:204: undefined reference to `rdtsc'
src/calltrace.o: In function `calltrace':
.../src/calltrace.c:277: undefined reference to `rdtsc'
collect2: error: ld returned 1 exit status
Makefile:866: recipe for target 'haproxy' failed
---
---
 src/calltrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/calltrace.c b/src/calltrace.c
index 2208ca11b..551e8a01f 100644
--- a/src/calltrace.c
+++ b/src/calltrace.c
@@ -49,6 +49,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static FILE *log;
 static int level;
-- 
2.20.1

Re: [PATCH] MINOR: Add either(,) converter

[Miroslav Zagorac]

Hello all,

there is a small typo in the patch, if says 'iff' instead of 'iif':

---
+  Example:
+http-request set-header x-forwarded-proto %[ssl_fc,iff(https,http)]
---

--
Zaga

What can change the nature of a man?

Re: [PATCH] MINOR: Add either(,) converter

[Miroslav Zagorac]

On 09/11/2020 03:56 PM, Tim Düsterhus, WoltLab GmbH wrote:

We've had a bit of discussion regarding the naming of the converter. I
wanted to avoid calling it `if`, because then we could have stuff like this:

   http-request set-var(txn.foo) bool(1),if(bar,baz)

which can easily be confused with:

   http-request set-var(txn.foo) bool(1) if bar

Some other naming suggestions that came up:

- choice (my initial choice)
- ifor / if_or
- ifelse / if_else
- iftrue (with the  argument being optional)



Maybe something like this would be appropriate (IIF)?

https://en.wikipedia.org/wiki/IIf

--
Zaga

What can change the nature of a man?

[PATCH] BUG/MINOR: spoe: correction of setting bits for analyzer

[Miroslav Zagorac]

Hello everyone,

somehow it seems to me that this is a bug when setting bits for the 
channel analyzer in the spoe filter.. in this case the attached patch 
can be applied.


--
Zaga

What can change the nature of a man?
>From 5bbed897105cafb49cc1cf8676497a7f86ea9947 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Fri, 19 Jun 2020 22:17:17 +0200
Subject: [PATCH] BUG/MINOR: spoe: correction of setting bits for analyzer

---
 src/flt_spoe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index 49d7466ae..4790f5945 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -3187,7 +3187,7 @@ spoe_start_analyze(struct stream *s, struct filter *filter, struct channel *chn)
 		ctx->flags |= SPOE_CTX_FL_CLI_CONNECTED;
 	}
 	else {
-		if (filter->pre_analyzers & SPOE_EV_ON_TCP_RSP)
+		if (filter->pre_analyzers & AN_RES_INSPECT)
 			chn->analysers |= AN_RES_INSPECT;
 
 		if (ctx->flags & SPOE_CTX_FL_SRV_CONNECTED)
-- 
2.20.1

[PATCH] DOC: internals: Fix spelling errors in filters.txt

[Miroslav Zagorac]

Hello,

here is attached the patch that corrects several spelling errors in 
doc/internals/filters.txt document.


If the errors have already been corrected, I apologize.  :)


Best regards,

--
Zaga

What can change the nature of a man?
>From 6c40e5748a81bd56a115c7834cdf08147d312d86 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Thu, 26 Mar 2020 20:45:04 +0100
Subject: [PATCH] DOC: internals: Fix spelling errors in filters.txt

---
 doc/internals/filters.txt | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/doc/internals/filters.txt b/doc/internals/filters.txt
index cd3988fbb..5e9b58e56 100644
--- a/doc/internals/filters.txt
+++ b/doc/internals/filters.txt
@@ -54,7 +54,7 @@ places, mainly around channel analyzers. Their purpose is to allow filters to
 be involved in the data processing, from the stream creation/destruction to
 the data forwarding. Depending of what it should do, a filter can implement all
 or part of these callbacks. For now, existing callbacks are focused on
-streams. But futur improvements could enlarge filters scope. For example, it
+streams. But future improvements could enlarge filters scope. For example, it
 could be useful to handle events at the connection level.
 
 In HAProxy configuration file, a filter is declared in a proxy section, except
@@ -84,7 +84,7 @@ filters are also chained, frontend ones called first. Even if the filters
 processing is serialized, each filter will bahave as it was alone (unless it was
 developed to be aware of other filters). For all that, some constraints are
 imposed to filters, especially when data exchanged between the client and the
-server are processed. We will dicuss again these constraints when we will tackle
+server are processed. We will discuss again these constraints when we will tackle
 the subject of writing a filter.
 
 
@@ -122,11 +122,11 @@ The list of available filters is reported by 'haproxy -vv':
 Multiple filter lines can be used in a proxy section to chain filters. Filters
 will be called in the declaration order.
 
-Some filters can support implicit declarartions in certain circumstances
+Some filters can support implicit declarations in certain circumstances
 (without the filter line). This is not recommended for new features but are
 useful for existing ones moved in a filter, for backward compatibility
-reasons. Implicit declarartions are supported when there is only one filter used
-on a proxy. When several filters are used, explicit declarartions are mandatory.
+reasons. Implicit declarations are supported when there is only one filter used
+on a proxy. When several filters are used, explicit declarations are mandatory.
 The HTTP compression filter is one of these filters. Alone, using 'compression'
 keywords is enough to use it. But when at least a second filter is used, a
 filter line must be added.
@@ -283,7 +283,7 @@ the structure 'stream', the field 'strm_flt' is the state of all filter
 instances attached to a stream:
 
 /*
- * Structure reprensenting the "global" state of filters attached to a
+ * Structure representing the "global" state of filters attached to a
  * stream.
  */
 struct strm_flt {
@@ -302,7 +302,7 @@ Filter instances attached to a stream are stored in the field
 'strm_flt.filters', each instance is of type 'struct filter *':
 
 /*
- * Structure reprensenting a filter instance attached to a stream
+ * Structure representing a filter instance attached to a stream
  *
  * 2D-Array fields are used to store info per channel. The first index
  * stands for the request channel, and the second one for the response
@@ -659,7 +659,7 @@ For example:
 The main purpose of filters is to take part in the channels analyzing. To do so,
 there is 2 callbacks, 'flt_ops.channel_pre_analyze' and
 'flt_ops.channel_post_analyze', called respectively before and after each
-analyzer attached to a channel, execpt analyzers responsible for the data
+analyzer attached to a channel, except analyzers responsible for the data
 parsing/forwarding (TCP or HTTP data). Concretely, on the request channel, these
 callbacks could be called before following analyzers:
 
-- 
2.20.1

[PATCH] CLEANUP: remove unused code in 'my_ffsl/my_flsl' functions

[Miroslav Zagorac]

It seems to me that shifting the variable 'a' at the end of the 
functions is of no use.  Maybe there is something I don't see here, in 
which case I apologize.  :)


--
Zaga

What can change the nature of a man?
>From 772c6517400e45c6e07c7f0fd733d9cbd68c65da Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Sun, 8 Mar 2020 16:32:20 +0100
Subject: [PATCH] CLEANUP: remove unused code in 'my_ffsl/my_flsl' functions

Shifting the variable 'a' one bit to the right has no effect on the
result of the functions.
---
 include/common/standard.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/common/standard.h b/include/common/standard.h
index d8bccbafb..103a07e5f 100644
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -902,7 +902,6 @@ static inline unsigned int my_ffsl(unsigned long a)
 		cnt += 2;
 	}
 	if (!(a & 0x1)) {
-		a >>= 1;
 		cnt += 1;
 	}
 #endif /* x86_64 */
@@ -946,7 +945,6 @@ static inline unsigned int my_flsl(unsigned long a)
 		cnt += 2;
 	}
 	if (a & 0x2) {
-		a >>= 1;
 		cnt += 1;
 	}
 #endif /* x86_64 */
-- 
2.20.1

Re: mirroragent (haproxy)

[Miroslav Zagorac]

On 03/04/2020 07:48 AM, Tuktamyshev Ainur wrote:

Dear Christopher

Configuration`s and info about haproxy and spoa version in attachment.


   * If possible, info about the mirror requests

Requests coming to haproxy should go to a specific IP, as well as duplicated in 
our parser located on another IP address.
On the parser’s machine, we don’t see any incoming requests from the SPOA, even 
when removing tcpdump.
The connection between the servers is stable.




Hello,

instead of 'messages mirrormessage' and 'spoe-message mirrormessage' you 
should use 'messages mirror' and 'spoe-message mirror' in the mirror.cfg 
file.


--
Zaga

What can change the nature of a man?

Re: haproxy 2.1.2 and 2.1.3

[Miroslav Zagorac]

On 02/21/2020 12:32 PM, Tim Düsterhus wrote:

Miroslav,

I order to save you duplicated effort in helping I'd like to let you
know that Marcel also created a GitHub issue upon request from Ilya:

https://github.com/haproxy/haproxy/issues/512


Hello Tim,

ah.. okay, i missed that. Thank you.  :)


Best regards,

--
Zaga

What can change the nature of a man?

Re: haproxy 2.1.2 and 2.1.3

[Miroslav Zagorac]

On 02/21/2020 11:42 AM, marcel.klooster...@kpn.com wrote:

Hi,

It's not working on test-server also.
Test-server is a solaris zone on T7 with 11.4 sru 950, production is a solaris 
zone on T8 with 11.4 sru 950.



You can start gdb like this:

% gdb -core core-file program

, and in your case it would look like this:

% gdb -core core_sz4203_haproxy_0_0_1582273479_18461 /usr/local/sbin/haproxy

After that, in gdb program you write command 'where' (without quotes),
which will print backtrace of all stack frames.  It will work better if 
you have debug symbols included in the program you want to debug.  :)


--
Zaga

What can change the nature of a man?

Re: [PATCH v4] BUG/MINOR: dns: allow 63 char in hostname

[Miroslav Zagorac]

On 01/28/2020 12:02 AM, Baptiste wrote:

On Sun, Jan 26, 2020 at 7:53 PM William Dauchy  wrote:


hostname were limited to 62 char, which is not RFC1035 compliant;
- the parsing loop should stop when above max label char
- fix len label test where d[i] was wrongly used
- simplify the whole function to avoid using two extra char* variable

this should fix github issue #387
 ...


This patch is "approved".
Willy, you can apply.

Baptiste



Hello,

whether in this function is sufficient to check the length of the label 
and its contents (uppercase and lowercase letters, numbers and hyphen) 
or whether RFC1035 should be followed where it states the following:


"The labels must follow the rules for ARPANET host names.  They must
start with a letter, end with a letter or digit, and have as interior
characters only letters, digits, and hyphen.  There are also some
restrictions on the length.  Labels must be 63 characters or less."

--
Zaga

What can change the nature of a man?

Re: [PATCH] BUG/MINOR: dns: allow 63 char in hostname

[Miroslav Zagorac]

On 01/26/2020 12:39 AM, William Dauchy wrote:

hostname were limited to 62 char, which is not RFC1035 compliant;
simply remove equality in condition as counter is initialised at zero.
(also simplify brackets for readability)

this should github issue #387

Signed-off-by: William Dauchy
---
  src/dns.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dns.c b/src/dns.c
index eefd8d0dc..8b3a0927e 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1497,7 +1497,7 @@ int dns_hostname_validation(const char *string, char 
**err)
d++;
}

-   if ((i>= DNS_MAX_LABEL_SIZE)&&  (d[i] != '.')) {
+   if (i>  DNS_MAX_LABEL_SIZE&&  d[i] != '.') {
if (err)
*err = DNS_LABEL_TOO_LONG;
return 0;


Hello William,

i think that patch does not correct the bug in that function because in 
the loop above the variable i and pointer d increase at the same time.


This means that *d should be written instead of d[i].

--
Zaga

What can change the nature of a man?

[PATCH] BUG/MINOR: WURFL: fix send_log() function arguments

[Miroslav Zagorac]

Hello,

this patch fixes the segmentation fault in WURFL device detection code.

The problem occurs when character combinations that represent the 
special formating codes used in the vfprintf() function are found within 
the user agent data.


For example, by running the command 'curl -A"%s s% s%" ...', the user 
agent string is set to '%s s% s%', causing the HAProxy to crash.


This only happens if the HAProxy is compiled with the option -DWURFL_DEBUG.

This patch could be backported in every version supporting the 
ScientiaMobile's WURFL. (as far as 1.7)


--
Miroslav Zagorac
>From 87bc2f08062218f9790135a1899d06936aa7178c Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Mon, 14 Oct 2019 17:15:56 +0200
Subject: [PATCH] BUG/MINOR: WURFL: fix send_log() function arguments

If the user agent data contains text that has special characters that
are used to format the output from the vfprintf() function, haproxy
crashes.  String "%s %s %s" may be used as an example.

% curl -A "%s %s %s" localhost:10080/index.html
curl: (52) Empty reply from server

haproxy log:
:WURFL-test.clireq[00c7:]: GET /index.html HTTP/1.1
:WURFL-test.clihdr[00c7:]: host: localhost:10080
:WURFL-test.clihdr[00c7:]: user-agent: %s %s %s
:WURFL-test.clihdr[00c7:]: accept: */*
segmentation fault (core dumped)

gdb 'where' output:
#0  strlen () at ../sysdeps/x86_64/strlen.S:106
#1  0x7f7c014a8da8 in _IO_vfprintf_internal (s=s@entry=0x7ffc808fe750, format=,
format@entry=0x7ffc808fe9c0 "WURFL: retrieve header request returns [%s %s %s]\n",
ap=ap@entry=0x7ffc808fe8b8) at vfprintf.c:1637
#2  0x7f7c014cfe89 in _IO_vsnprintf (
string=0x55cb772c34e0 "WURFL: retrieve header request returns [(null) %s %s %s  B,w\313U",
maxlen=,
format=format@entry=0x7ffc808fe9c0 "WURFL: retrieve header request returns [%s %s %s]\n",
args=args@entry=0x7ffc808fe8b8) at vsnprintf.c:114
#3  0x55cb758f898f in send_log (p=p@entry=0x0, level=level@entry=5,
format=format@entry=0x7ffc808fe9c0 "WURFL: retrieve header request returns [%s %s %s]\n")
at src/log.c:1477
#4  0x55cb75845e0b in ha_wurfl_log (
message=message@entry=0x55cb75989460 "WURFL: retrieve header request returns [%s]\n") at src/wurfl.c:47
#5  0x55cb7584614a in ha_wurfl_retrieve_header (header_name=, wh=0x7ffc808fec70)
at src/wurfl.c:763

In case WURFL (actually HAProxy) is not compiled with debug option
enabled (-DWURFL_DEBUG), this bug does not come to light.

This patch could be backported in every version supporting
the ScientiaMobile's WURFL. (as far as 1.7)
---
 src/wurfl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wurfl.c b/src/wurfl.c
index 1e702c029..47968e212 100644
--- a/src/wurfl.c
+++ b/src/wurfl.c
@@ -44,7 +44,7 @@ inline static void ha_wurfl_log(char * message, ...)
 	va_start(argp, message);
 	vsnprintf(logbuf, sizeof(logbuf), message, argp);
 	va_end(argp);
-	send_log(NULL, LOG_NOTICE, logbuf, NULL);
+	send_log(NULL, LOG_NOTICE, "%s", logbuf);
 }
 #else
 inline static void ha_wurfl_log(char * message, ...)
-- 
2.20.1