Hello Willy,

On 09/09/2021 08:43 AM, Willy Tarreau wrote:
Yes, that's where I found it but didn't know what its use was. A few
points however:
   - sess.ot.uuid will be shared by all requests on the same session,
     which is probably not what you want (e.g. for requests coming in
     H2, the last one setting it will replace all other ones' value
     while they're working with it). I think you should change its
     scope to be limited to the transaction only (txn.ot.uuid). It's
     also used in debugging outputs where it will be per-request and
     may not match what the requests will log because of this.

Thanks, i'll change that.

   - I finally found its definition here in filter.h:
       #define FTL_OT_VAR_UUID           "sess", "ot", "uuid"

     Please note the typo in the macro name (FTL instead of FLT)

Strange that there is only one such typo. :)

   - the comment in struct flt_ot_runtime_context for uuid says
     "Randomly generated UUID". We already have a function to produce a
     random UUID (ha_generate_uuid()) but flt_ot_runtime_context_init()
     makes its own from elements that at first glance look like they're
     filled from other locations (uuid.node, uuid.clock_seq etc), but in
     fact are simply aliased on the random ints. In this case better use
     the existing function and avoid duplicating the painful output format
     generation. If you need to keep a copy of the two randoms, just
     modify the existing function to pass an optional array to store
     the randoms. At first glance they're not used though.

When ot filter source was created, I didn't know what would be used and
what wouldn't.  At the time I didn't even know what opentracing was,
what it was used for and how it was done.

So, adding 'useful' things to the source, there are also those members
of the structure that were not used anywhere later (because I thought
at the time that they might be used somewhere).

Not that they bother anything, but I will remove them.  I will also use
the haproxy function to generate uuid, it's a small change in ot source.

Then I didn't use that function because it doesn't set those two uuid
64-bit numbers and it was a bit of a weird way for me to generate uuid
because at the end of the haproxy function the value of v[2] rotates
to the right by 14 bits and the value of v[3] to the left by 18 bits
which for uint64_t[2] input gave me mixed bits.

For example, if both random numbers are equal to 0x0123456789abcdef,
the result is 89abcdef-4567-4123-8def-048d159e26af, while to me the
logical result would be 89abcdef-4567-4123-8def-0123456789ab.

This result is obtained if v[2] rotates to the right by 16 bits and
v[3] to the left by 16 bits.

If all the bits are equally good random, this is somehow a more logical
result for me, because they give the correct order of the bytes of the
input data (I'm not going into the correctness of the big/little
endian way of writing data now).

   - there's quite a bunch of functions with no description at all of
     their purpose, input values, return values etc. ..

Yes, writing comments always takes useful time, but they are necessary;
at least for weirder things..  I will fill in those function
descriptions over time and send patches.

--
Zaga    <[email protected]>

What can change the nature of a man?

Reply via email to