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.
This reverts commit f2263435d7.
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.
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.
The presence of this field causes a long dependency chain because almost
everyone includes global-t.h, and vars include sample_data which include
some system includes as well as HTTP parts.
There is absolutely no reason for having the process-wide variables in
the global struct, let's just move them into vars.c and vars.h. This
reduces from ~190k to ~170k the preprocessed output of version.c.
Currently the stats filling function knows nothing about the caller's
needs, so let's pass the STAT_* flags so that it can adapt to the
requester's constraints.
The current "ADD" vs "ADDQ" is confusing because when thinking in terms
of appending at the end of a list, "ADD" naturally comes to mind, but
here it does the opposite, it inserts. Several times already it's been
incorrectly used where ADDQ was expected, the latest of which was a
fortunate accident explained in 6fa922562 ("CLEANUP: stream: explain
why we queue the stream at the head of the server list").
Let's use more explicit (but slightly longer) names now:
LIST_ADD -> LIST_INSERT
LIST_ADDQ -> LIST_APPEND
LIST_ADDED -> LIST_INLIST
LIST_DEL -> LIST_DELETE
The same is true for MT_LISTs, including their "TRY" variant.
LIST_DEL_INIT keeps its short name to encourage to use it instead of the
lazier LIST_DELETE which is often less safe.
The change is large (~674 non-comment entries) but is mechanical enough
to remain safe. No permutation was performed, so any out-of-tree code
can easily map older names to new ones.
The list doc was updated.
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.
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.
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.
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:🧵: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.
This one is the last optional module to build with haproxy, so let's move
it to addons/. It was renamed to "ot" as it was the only one whose USE_*
option did not match the directory name, now this is consistent.
Few changes were required, only the Makefile, and doc were adjusted, as
the directory was already self-contained and relocatable.
Both the source file and the dummy library are now at the same place.
Maybe the build howto could be moved there as well to make things even
cleaner.
The Makefile, MAINTAINERS, doc, and vtest matrix were updated.
Both the source file and the dummy library are now at the same place.
Maybe the build howto could be moved there as well to make things even
cleaner.
The Makefile, MAINTAINERS, doc, github build matrix, coverity checks
and travis CI's build were updated.
Now it's much cleaner, both 51d.c and the dummy library live together and
are easier to spot and maintain. The build howto probably ought to be moved
there as well. Makefile, docs and MAINTAINERS were updated, as well as
the github CI's build matrix, travis CI's, and coverity checks.
Let's start to better organize the addons by moving promex there (and
with an easier directory name). The makefile and maintainers files were
updated, as well as the CI's build matrix.
Miroslav Zagorac
Willy Tarreau