From 9425ed488f4d31db07f0c9b1e8b2f0fb65b18c7b Mon Sep 17 00:00:00 2001 From: Miroslav Zagorac Date: Thu, 10 Jun 2021 01:23:15 +0200 Subject: [PATCH] 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 97df12e056..a4bb7fcc0f 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 2dbf5321a1..2b88a330e5 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 ot_start(struct otc_tracer *tracer, const char *cfgbuf, char **err); struct otc_span *ot_span_init(struct otc_tracer *tracer, const char *operation_name, const struct timespec *ts_steady, const struct timespec *ts_system, int ref_type, int ref_ctx_idx, const struct otc_span *ref_span, const struct otc_tag *tags, int num_tags, char **err); int ot_span_tag(struct otc_span *span, const struct otc_tag *tags, int num_tags); int ot_span_log(struct otc_span *span, const struct otc_log_field *log_fields, int num_fields); diff --git a/addons/ot/src/conf.c b/addons/ot/src/conf.c index 71db967754..d07be2c661 100644 --- a/addons/ot/src/conf.c +++ b/addons/ot/src/conf.c @@ -671,6 +671,7 @@ void flt_ot_conf_tracer_free(struct flt_ot_conf_tracer **ptr) FLT_OT_FREE((*ptr)->id); FLT_OT_FREE((*ptr)->config); + FLT_OT_FREE((*ptr)->cfgbuf); FLT_OT_FREE((*ptr)->plugin); FLT_OT_DBG(2, "- deleting acls list %s", flt_ot_list_debug(&((*ptr)->acls))); list_for_each_entry_safe(acl, aclback, &((*ptr)->acls), list) { diff --git a/addons/ot/src/filter.c b/addons/ot/src/filter.c index 6699d46137..04c4a67f13 100644 --- a/addons/ot/src/filter.c +++ b/addons/ot/src/filter.c @@ -168,11 +168,10 @@ static int flt_ot_init(struct proxy *p, struct flt_conf *fconf) /* * Initialize the OpenTracing library. - * Enable HTX streams filtering. */ - retval = ot_init(&(conf->tracer->tracer), conf->tracer->config, conf->tracer->plugin, &err); + retval = ot_init(&(conf->tracer->tracer), conf->tracer->plugin, &err); if (retval != FLT_OT_RET_ERROR) - fconf->flags |= FLT_CFG_FL_HTX; + /* Do nothing. */; else if (err != NULL) { FLT_OT_ALERT("%s", err); @@ -426,8 +425,6 @@ static int flt_ot_check(struct proxy *p, struct flt_conf *fconf) } -#ifdef DEBUG_OT - /*** * NAME * flt_ot_init_per_thread - @@ -446,14 +443,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); + + /* + * Start the OpenTracing library tracer thread. + * Enable HTX streams filtering. + */ + if (!(fconf->flags & FLT_CFG_FL_HTX)) { + retval = ot_start(conf->tracer->tracer, conf->tracer->cfgbuf, &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 +1133,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. */ diff --git a/addons/ot/src/opentracing.c b/addons/ot/src/opentracing.c index 58936d1224..b79ba29551 100644 --- a/addons/ot/src/opentracing.c +++ b/addons/ot/src/opentracing.c @@ -138,7 +138,6 @@ static void ot_mem_free(FLT_OT_DBG_ARGS(const char *func, int line, ) void *ptr) * * ARGUMENTS * tracer - - * config - * plugin - * err - * @@ -148,12 +147,12 @@ static void ot_mem_free(FLT_OT_DBG_ARGS(const char *func, int line, ) void *ptr) * RETURN VALUE * - */ -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) { char cwd[PATH_MAX], path[PATH_MAX], errbuf[BUFSIZ] = ""; int rc, retval = -1; - FLT_OT_FUNC("%p:%p \"%s\", \"%s\", %p:%p", FLT_OT_DPTR_ARGS(tracer), config, plugin, FLT_OT_DPTR_ARGS(err)); + FLT_OT_FUNC("%p:%p, \"%s\", %p:%p", FLT_OT_DPTR_ARGS(tracer), plugin, FLT_OT_DPTR_ARGS(err)); flt_ot_pools_info(); #ifdef USE_POOL_OT_SPAN_CONTEXT @@ -172,7 +171,7 @@ int ot_init(struct otc_tracer **tracer, const char *config, const char *plugin, FLT_OT_RETURN(retval); } - *tracer = otc_tracer_init(path, config, NULL, errbuf, sizeof(errbuf)); + *tracer = otc_tracer_load(path, errbuf, sizeof(errbuf)); if (*tracer == NULL) { FLT_OT_ERR("%s", (*errbuf == '\0') ? "failed to initialize tracing library" : errbuf); } else { @@ -185,6 +184,39 @@ int ot_init(struct otc_tracer **tracer, const char *config, const char *plugin, } +/*** + * NAME + * ot_start - + * + * ARGUMENTS + * tracer - + * cfgbuf - + * err - + * + * DESCRIPTION + * - + * + * RETURN VALUE + * This function does not return a value. + */ +int ot_start(struct otc_tracer *tracer, const char *cfgbuf, char **err) +{ + char errbuf[BUFSIZ] = ""; + int retval = -1; + + FLT_OT_FUNC("%p, %p, %p:%p", tracer, cfgbuf, FLT_OT_DPTR_ARGS(err)); + + if (cfgbuf == NULL) + FLT_OT_RETURN(retval); + + retval = otc_tracer_start(NULL, cfgbuf, errbuf, sizeof(errbuf)); + if (retval == -1) + FLT_OT_ERR("%s", (*errbuf == '\0') ? "failed to start tracer" : errbuf); + + FLT_OT_RETURN(retval); +} + + /*** * NAME * ot_close - diff --git a/addons/ot/src/parser.c b/addons/ot/src/parser.c index 5dec8629d4..e26ca18eee 100644 --- a/addons/ot/src/parser.c +++ b/addons/ot/src/parser.c @@ -574,7 +574,8 @@ static int flt_ot_parse_cfg_tracer(const char *file, int linenum, char **args, i */ static int flt_ot_post_parse_cfg_tracer(void) { - int retval = ERR_NONE; + char errbuf[BUFSIZ] = ""; + int retval = ERR_NONE; FLT_OT_FUNC(""); @@ -586,8 +587,13 @@ static int flt_ot_post_parse_cfg_tracer(void) if (flt_ot_current_tracer->id == NULL) FLT_OT_RETURN(retval); - if (flt_ot_current_tracer->config == NULL) + if (flt_ot_current_tracer->config == NULL) { FLT_OT_POST_PARSE_ALERT("tracer '%s' has no configuration file specified", flt_ot_current_tracer->cfg_line, flt_ot_current_tracer->id); + } else { + flt_ot_current_tracer->cfgbuf = otc_file_read(flt_ot_current_tracer->config, "#", errbuf, sizeof(errbuf)); + if (flt_ot_current_tracer->cfgbuf == NULL) + FLT_OT_POST_PARSE_ALERT("tracer '%s' %s", flt_ot_current_tracer->cfg_line, flt_ot_current_tracer->id, (*errbuf == '\0') ? "cannot load configuration file" : errbuf); + } if (flt_ot_current_tracer->plugin == NULL) FLT_OT_POST_PARSE_ALERT("tracer '%s' has no plugin library specified", flt_ot_current_tracer->cfg_line, flt_ot_current_tracer->id);