Now that the CLI's print context is alone in the appctx, it's possible
to refine the appctx's ctx layout so that the cli part matches exactly
a regular svcctx, and as such move the CLI context into an svcctx like
other applets. External code will still build and work because the
struct cli perfectly maps onto the struct cli_print_ctx that's located
into svc.storage. This is of course only to make a smooth transition
during 2.6 and will disappear immediately after.
A tiny change had to be applied to the opentracing addon which performs
direct accesses to the CLI's err pointer in its own print function. The
rest uses the standard cli_print_* which were the only ones that needed
a small change.
The whole "ctx.cli" struct could be tagged as deprecated so that any
possibly existing external code that relies on it will get build
warnings, and the comments in the struct are pretty clear about the
way to fix it, and the lack of future of this old API.
This reverts commit 6c9f7faa62.
This warning is no longer needed because source code can be compiled with
enabled support for OpenTracing context transfer via HAProxy variables.
This patch must be backported in 2.5.
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 |
+-----+---------------+-- .. --+-----+---------------+
The amount of memory space used to store the length of the name is 1 byte,
with a sign (the minus sign is provided for inactive records, but this is
not currently used). This means that the maximum length of the variable
name to be saved is 127 characters, which is quite enough for use in the
filter. The buffer size for such data storage is global.tune.bufsize.
This patch must be backported in 2.5.
A display of the contents of the err variable has been added to the
FLT_OT_ERR() macro, once it has been set.
This patch must be backported as far as 2.4.
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.
This patch must be backported as far as 2.4.
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
This patch must be backported as far as 2.4.
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.
This patch must be backported in 2.5.
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.
This patch must be backported as far as 2.4.
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.
This patch must be backported as far as 2.4.
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.
This patch must be backported in 2.5.
This reverts commit 560c7b874a.
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.
This patch must be backported in 2.5.
The flt_ot_var_get() function is not used anywhere and is unnecessary
in the existing implementation of the OpenTracing filter.
This patch must be backported as far as 2.4.
The flt_ot_var_unset() function is not used anywhere and is unnecessary
in the existing implementation of the OpenTracing filter.
This patch must be backported as far as 2.4.
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.
This patch must be backported as far as 2.4.
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).
This patch must be backported in 2.5.
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.
This patch must be backported as far as 2.4.
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.
Must be backported to 2.5.
There is currently a problem related to time keeping. We're mixing
the functions to perform calculations with the os-dependent code
needed to retrieve and adjust the local time.
This patch extracts from time.{c,h} the parts that are solely dedicated
to time keeping. These are the "now" or "before_poll" variables for
example, as well as the various now_*() functions that make use of
gettimeofday() and clock_gettime() to retrieve the current time.
The "tv_*" functions moved there were also more appropriately renamed
to "clock_*".
Other parts used to compute stolen time are in other files, they will
have to be picked next.
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.
At the suggestion of Willy Tarreau, the scope of the 'ot.uuid' variable was
changed from 'sess' to 'txn', so it is now limited to the transaction only.
To avoid duplicate source code, the original haproxy function is used to
generate the OpenTracing runtime context UUID.
Also, the structure flt_ot_runtime_context is simplified because the
detailed definition of UUID is removed from it (struct flt_ot_uuid),
ie the UUID is left only in the form of a string.
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.
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.
The API was extended by commit e352b9dac ("MINOR: vars: make vars_get_by_*
support an optional default value") but I didn't notice that opentracing
was using it, so it broke the build. No backport needed.
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.
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.
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.
