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    <miros...@zagorac.name>

What can change the nature of a man?
>From fe482c8adaa703e790b4d1995255b9754799b149 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac <mzago...@haproxy.com>
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  0x00007f8131fd5629 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  0x00007f8131fd5629 in pthread_join (threadid=140192831239936, thread_return=0x0) at pthread_join.c:45
  #1  0x00007f812f15abc7 in std::thread::join() ()
     from /tmp/haproxy-os-master/contrib/opentracing/test/libjaeger_opentracing_plugin-0.5.0.so
  #2  0x00007f812f0fb6f7 in jaegertracing::reporters::RemoteReporter::close() ()
        from /tmp/haproxy-os-master/contrib/opentracing/test/libjaeger_opentracing_plugin-0.5.0.so
  #3  0x00007f812f0b7055 in jaegertracing::reporters::CompositeReporter::close() ()
           from /tmp/haproxy-os-master/contrib/opentracing/test/libjaeger_opentracing_plugin-0.5.0.so
  #4  0x00007f812f0b9136 in jaegertracing::Tracer::Close() ()
              from /tmp/haproxy-os-master/contrib/opentracing/test/libjaeger_opentracing_plugin-0.5.0.so
  #5  0x00007f81309def32 in ot_tracer_close (tracer=0x55fb48057390) at ../../src/tracer.cpp:91
  #6  0x000055fb41785705 in ot_close (tracer=0x55fb48061168) at contrib/opentracing/src/opentracing.c:208
  #7  0x000055fb4177fc64 in flt_ot_deinit (p=<optimized out>, fconf=<optimized out>) at contrib/opentracing/src/filter.c:215
  #8  0x000055fb418bc038 in flt_deinit (proxy=proxy@entry=0x55fb4805ce50) at src/filters.c:360
  #9  0x000055fb41893ed1 in free_proxy (p=0x55fb4805ce50) at src/proxy.c:315
  #10 0x000055fb41888809 in deinit () at src/haproxy.c:2217
  #11 0x000055fb41889078 in deinit_and_exit (status=0) at src/haproxy.c:2343
  #12 0x000055fb4173d809 in main (argc=<optimized out>, argv=<optimized out>) 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, &err);
-	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)
 {
-	int retval = FLT_OT_RET_OK;
+	struct flt_ot_conf *conf = FLT_OT_DEREF(fconf, conf, NULL);
+	char               *err = NULL;
+	int                 retval = FLT_OT_RET_ERROR;
 
 	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, &err);
+		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 -
@@ -1112,7 +1120,7 @@ struct flt_ops flt_ot_ops = {
 	.init                  = flt_ot_init,
 	.deinit                = flt_ot_deinit,
 	.check                 = flt_ot_check,
-	.init_per_thread       = FLT_OT_DBG_IFDEF(flt_ot_init_per_thread, NULL),
+	.init_per_thread       = flt_ot_init_per_thread,
 	.deinit_per_thread     = FLT_OT_DBG_IFDEF(flt_ot_deinit_per_thread, NULL),
 
 	/* Stream callbacks. */
-- 
2.30.1

Reply via email to