On Wed, Sep 08, 2021 at 07:23:59PM +0200, Miroslav Zagorac wrote: > On 09/08/2021 06:46 PM, Willy Tarreau wrote: > > I have no doubt about this, what I mean is that once such needs are > > identified, as much as possible we should try to move these parts into > > the proper location (vars), or if really needed, that must be clearly > > explained around the code so that someone else may try to figure what > > to change there. For example I've seen a function that parses scope > > names and turns them list heads, that's the perfect example of something > > that ought to have been adapted into vars.c if what was provided was not > > sufficient, because since then new scopes were added without anyone > > noticing they were listed there, so that function was not updated (there > > I doubt anyone cares but conceptually it's a problem). > > It is clear to me.. at the time when the code for the ot filter was > being developed, it was easier for me to write the functions I was > missing (ie the content I was missing in the existing functions) and > then see what to do with it .. and it remained like that after > completion. At the time, I didn't think that something more > significant could change in working with variables one day.
I know how we end up like this, I'm not saying that in bad terms, just as something to be careful about in the future. The codebase is huge nowadays and no single person knows even 10% of it anymore, so if we continue to create hacks or temporary stuff in various corners it's unmaintainable and breaks all over every time someone tries to fix a design problem. And that's already what happened with these shitty variables that have been causing us some gray hair for a few years now. > I was wrong, obviously. No worries. > > > 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. > > > > Hmmm that may become quite problematic then since we don't have that > > name anymore :-/ Are these variables used only for internal stuff in > > OT or are they used to exchange with the rest of the configuration ? > > In opentracing there is a context of data, it is a group of data that > is transferred from one place where they are used to another place. > > This data is defined as a key:value, where the use of haproxy variables > suited me perfectly. The problem is that it is not known in advance > what the keys of this data are and how much of this data is, this is > set by the user in the opentracing configuration (which has nothing to > do with haproxy). OK thanks for explaining! Now I understand indeed why you reused variables for this. > The operations that handle this data are 'inject' and 'extract', the > first writes all the data from the opentracing context to the haproxy > variable and the second extracts the data from the haproxy variable to > the opentracing context. > > During the 'inject' operation, it is known which are the variables (or > their names) because they are generated from the 'key' part of the > opentracing data. > > During the 'extract' operation this is not known, ie only the prefix of > the variable under which one of the 'inject' operations of that > variable is written is known. Got it. It's just as if you'd serialize a blob that it extracted on the other side, in some sort. > > Do you have a concrete example of a use case for such prefixes on > > receipt ? For example if you're dumping all variables from your current > > request, maybe that's enough. Or if you need prefixes for dumping all > > variables of a particular scope, we could proceed differently. > > Context transfer is conceived in two ways, one is via variables (if > transferred within the same haproxy process) and the other is via http > header (if transferred between different processes, there does not have > to be haproxy as one of the processes). > > An example of this is the configuration from the test/ctx directory, > more precisely the test/ctx/ot.cfg file. The only explicit variable I'm seeing there is sess.ot.uuid, so I guess that the rest is totally transparent, and that if needed we can arrange so that sess.ot.uuid is always created on new sessions. > An example of context transfer via http header is in the test/fe and > test/be directories where data is transferred from the frontend process > via the http header to the backend process, which can then add > opentracing data to the trace initiated in the frontend process. > > In principle, context transfer via haproxy variables can be omitted > because this data can also be transferred via opentracing process > without interfering with its context, because this process is started > and completed in the same haproxy program and does not need to be > exposed to the outside. OK. > But maybe someone needs it; I implemented it because it can be done and > not because someone might use it. Thus maybe that could just be placed at a single place into the OT context using simple accessors ? > On the other hand, the transfer of context between processes via http > headers is mandatory. > > I don't know at all if i was able to clarify some things related to the > use of variables and the use of opentracing context. It's much clearer now, thank you! > > > 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. > > > > Just thinking loud, if you absolutely need to enumerate a list of > > variables when setting them, maybe it could be possible to enumerate > > their names into another well-known one and look up that one first on > > the other side? > > It is possible, it is not a bad idea. I'll think about it, thanks for > the advice. > > I'm sorry that ot filter code caused these compilation problems. Don't worry. I was annoyed first because I spent several days attacking the problem under various angles several 4 times, restarting from scratch each time I discovered some brown paper bag over the global names table, and now that everything worked fine I discovered the problem, realizing that I almost never build OT due to the dependencies that are annoying to deal with on a daily basis. And where I thought "ah crap, I didn't notice it, I must update it to use the updated API", I discovered that it appeared to dig much deeper than all other ones, and that it was really not a nice surprise. 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. Cheers, Willy
