2Mrps : kudos to the team

2021-04-10 Thread Ionel GARDAIS
Hi team, list, 

If you haven't already read it, go read this blog article : 
[ 
https://www.haproxy.com/blog/haproxy-forwards-over-2-million-http-requests-per-second-on-a-single-aws-arm-instance/
 | 
https://www.haproxy.com/blog/haproxy-forwards-over-2-million-http-requests-per-second-on-a-single-aws-arm-instance/
 ] 

Such a milestone ! 
It's time to take time to thank all the great work achieved by the HAProxy team 
over the years. 
Thanks Willy for your vision and to stick with the Unix philosophy : m ake each 
program do one thing well. 
Thanks Tim, William, Aleksandar, Lukas, Christopher and all those working 
day-to-day to make HAProxy such a nice piece of code. 

*claps claps claps* 

Ionel 

--
232 avenue Napoleon BONAPARTE 92500 RUEIL MALMAISON
Capital EUR 219 300,00 - RCS Nanterre B 408 832 301 - TVA FR 09 408 832 301

Re: [PATCH] MINOR: sample: add json_string

2021-04-10 Thread Tim Düsterhus

Aleks,

On 4/10/21 12:24 AM, Aleksandar Lazic wrote:

+json_string() : string

I don't like the name. A few suggestions:

- json_query
- json_get
- json_decode


maybe json_get_string because there could be some more getter like bool, 
int, ...


The '_string' suffix does not make sense to me, because why should the 
user need to write about the expected type when using the converter? 
Samples already store their type in HAProxy and they are automatically 
casted to an appropriate type if required (i.e. there is little 
difference between a numeric string and an int).


It should be valid to do something like this.

str('{"s": "foo", "i": 1}'),json_query('$.s'),sha1,hex

and likewise

str('{"s": "foo", "i": 1}'),json_query('$.i'),add(7)


+  # get the value from the key 
kubernetes.io/serviceaccount/namespace

+  # => openshift-logging
+  http-request set-var(sess.json) 
req.hdr(Authorization),b64dec,json_string('$.kubernetes\\.io/serviceaccount/namespace') 


+ +  # get the value from the key iss
+  # => kubernetes/serviceaccount
+  http-request set-var(sess.json) 
req.hdr(Authorization),b64dec,json_string('$.iss')


I don't like that the example is that specific for Kubernetes usage. A 
more general example would be preferred, because it makes it easier to 
understand the concept.


The '$.iss' is the generic JWT field.
https://tools.ietf.org/html/rfc7519#section-4.1
"iss" (Issuer) Claim


But even a JWT is a very narrow use-case ...


But maybe I could look for a "normal" JSON sting and only JWT.



... I suggest to use something generic like my example above (with "foo" 
as a common placeholder value). Examples should explain the concept, not 
a specific use case. Users are smart enough to understand that they can 
use this to extract values from a JWT if this is what they need to do.


diff --git a/reg-tests/sample_fetches/json_string.vtc 
b/reg-tests/sample_fetches/json_string.vtc

new file mode 100644
index 0..fc387519b
--- /dev/null
+++ b/reg-tests/sample_fetches/json_string.vtc


Again, this is a converter. Move the test into the appropriate folder. 
And please make sure you understand the difference between fetches and 
converters.


Yeah the difference between fetchers and converters in not fully clear 
for me.

I think when a value is fetched from any data then it's a fetcher like this
JSON "fetcher".


The use of correct terminology is important, because everything else 
introduces confusion. It is extra important if it is used in persistent 
documentation (vs. say a discussion in IRC where it can easily be 
clarified).


The difference is explained in configuration.txt in the introduction of 
section 7 and again at the beginning of section 7.3.1:



Sample fetch methods may be combined with transformations to be applied on top
of the fetched sample (also called "converters"). These combinations form what
is called "sample expressions" and the result is a "sample".


Fetches *fetch* data from e.g. the connection and then return a *sample*.

Converters *convert* data from an existing *sample* and then return a 
new *sample*.


-


+ */
+static int sample_check_json_string(struct arg *arg, struct 
sample_conv *conv,

+   const char *file, int line, char **err)
+{
+    DPRINTF(stderr, "%s: arg->type=%d, arg->data.str.data=%ld\n",
+    __FUNCTION__,
+    arg->type, arg->data.str.data);


Debug code above.


This was intentionally. I asked my self why no Debug option is set.
This will only be printed with 'DEBUG=-DDEBUG_FULL'.
Maybe there should be a "DBUG_SAMPLES" like the other "DEBUG_*" options.


Imagine how the code and also the debug output would look if every 
converter would output several lines of debug output. Additionally 
there's not much useful information in the output here. arg->type is 
always going to be ARGT_STR, because HAProxy will automatically cast the 
argument based on the converter definition. The length of the string 
also is pretty much what you expect it to be.


There's also the 'debug' converter that effectively does what that line 
does.


Don't get me wrong. I also enjoy using 'printf' while debugging my code. 
But it is going to be removed after I finish debugging. The parts I 
touch are usually not that complex or deep in the internals that such 
output would be useful enough in case issues arise.



+    { "json_string", sample_conv_json_string, ARG1(1,STR),  
sample_check_json_string , SMP_T_STR, SMP_USE_CONST },


While testing something I also just notice that SMP_USE_CONST is 
incorrect here. I cannot apply e.g. the sha1 converter to the output of 
json_string.


Best regards
Tim Düsterhus