Re: [PATCH] MINOR: opentracing: register config file and line number on log servers

[Tim Düsterhus]

Ilya,

On 5/8/21 9:19 PM, Илья Шипицин wrote:

I've created "proof of concept" OT support for CI

https://github.com/chipitsine/haproxy/runs/2535689580

Tim, are you ok with that approach ? If yes, I will polish things a bit.


If the build time is reduced appropriately then I am fine with that. 
Please order the 'apt' dependencies alphabetically please.



I'm not very happy with unconditionally adding OT flags
https://github.com/chipitsine/haproxy/commit/dcf1812b17bd2907939eaa8d3310378d419d90c5#diff-b525f86b1f4925509959f857496dff18a9c4ed34fcc7f49357c5a6d00fb64d17R91
Tim, do you know how to add it using ${{ contains(matrix.FLAGS, 'USE_OT=1')
}} condition ?



You can simply add these to the 'flags' list within matrix.py, similarly 
to WURFL_INC and WURFL_LIB also being part of that list.


Best regards
Tim Düsterhus

Re: [PATCH] MINOR: opentracing: register config file and line number on log servers

[Илья Шипицин]

I've created "proof of concept" OT support for CI

https://github.com/chipitsine/haproxy/runs/2535689580

Tim, are you ok with that approach ? If yes, I will polish things a bit.


currently "building OT and satellites" takes 2min 11sec. seem, I will add
similar caching level as for SSL variants (i.e. checking version in "opt"
and skip build if version is there).

also, I caught an error on clang:
https://github.com/haproxy/haproxy/issues/1242

I'm not very happy with unconditionally adding OT flags
https://github.com/chipitsine/haproxy/commit/dcf1812b17bd2907939eaa8d3310378d419d90c5#diff-b525f86b1f4925509959f857496dff18a9c4ed34fcc7f49357c5a6d00fb64d17R91
Tim, do you know how to add it using ${{ contains(matrix.FLAGS, 'USE_OT=1')
}} condition ?



чт, 8 апр. 2021 г. в 14:44, Илья Шипицин :

>
>
> чт, 8 апр. 2021 г. в 14:25, Willy Tarreau :
>
>> On Wed, Apr 07, 2021 at 05:26:24PM +0500,  ??? wrote:
>> > we run "all features anebled" gcc and clang builds, for example
>> > BUG/MINOR: tools: fix parsing "us" unit for timers ·
>> > haproxy/haproxy@a683805 (github.com)
>> > <
>> https://github.com/haproxy/haproxy/runs/2275440914?check_suite_focus=true
>> >
>> >
>> > <
>> https://github.com/haproxy/haproxy/runs/2275440914?check_suite_focus=true
>> >
>> > if
>> > additional libraries are easy to install (building will increase total
>> time
>> > a lot), I'd add opentracing to those "all features" builds
>>
>> I managed to build it myself so it's reasonably accessible, however we'd
>> possibly need to cache the builds, or we'll really start to spend a lot
>> of time building dependencies.
>>
>
> That is what I meant. if build is cheap - ok.
> if packages are available - ok.
>
> other options would be either caching dependencies (github ci supports
> caches) or provisioning custom docker images for our builds
>
>
>>
>> Willy
>>
>

Re: [PATCH] MINOR: opentracing: register config file and line number on log servers

[Илья Шипицин]

чт, 8 апр. 2021 г. в 14:25, Willy Tarreau :

> On Wed, Apr 07, 2021 at 05:26:24PM +0500,  ??? wrote:
> > we run "all features anebled" gcc and clang builds, for example
> > BUG/MINOR: tools: fix parsing "us" unit for timers ·
> > haproxy/haproxy@a683805 (github.com)
> > <
> https://github.com/haproxy/haproxy/runs/2275440914?check_suite_focus=true>
> >
> > <
> https://github.com/haproxy/haproxy/runs/2275440914?check_suite_focus=true>
> > if
> > additional libraries are easy to install (building will increase total
> time
> > a lot), I'd add opentracing to those "all features" builds
>
> I managed to build it myself so it's reasonably accessible, however we'd
> possibly need to cache the builds, or we'll really start to spend a lot
> of time building dependencies.
>

That is what I meant. if build is cheap - ok.
if packages are available - ok.

other options would be either caching dependencies (github ci supports
caches) or provisioning custom docker images for our builds


>
> Willy
>

Re: [PATCH] MINOR: opentracing: register config file and line number on log servers

[Willy Tarreau]

On Wed, Apr 07, 2021 at 05:26:24PM +0500,  ??? wrote:
> we run "all features anebled" gcc and clang builds, for example
> BUG/MINOR: tools: fix parsing "us" unit for timers ·
> haproxy/haproxy@a683805 (github.com)
> 
> 
> 
> if
> additional libraries are easy to install (building will increase total time
> a lot), I'd add opentracing to those "all features" builds

I managed to build it myself so it's reasonably accessible, however we'd
possibly need to cache the builds, or we'll really start to spend a lot
of time building dependencies.

Willy

Re: [PATCH] MINOR: opentracing: register config file and line number on log servers

[Willy Tarreau]

Hi Miroslav!

On Wed, Apr 07, 2021 at 11:32:16AM +0200, Miroslav Zagorac wrote:
> Hello,
> 
> due to the modified function declaration, the opentracing filter can no
> longer be compiled.
> 
> In commit 9533a7038 new parameters have been added to the declaration
> of function parse_logsrv().

Oops, indeed, now merged, thanks!

> This patch should be backported to all branches where the OpenTracing
> filter is located.

Not needed as the patch which broke it is only in 2.4.

Thanks!
Willy

Re: [PATCH] MINOR: opentracing: register config file and line number on log servers

[Илья Шипицин]

On Wed, Apr 7, 2021, 2:16 PM Miroslav Zagorac  wrote:

> On 04/07/2021 12:13 PM, Илья Шипицин wrote:
> > do you consider adding opentracing to "github actions" CI ?
> >
>
> Hello Илья,
>
> I don't know how to add it because I never used it.  The filter uses
> specific libraries that are not part of the system but need to be
> installed and/or compiled independently.
>

we run "all features anebled" gcc and clang builds, for example
BUG/MINOR: tools: fix parsing "us" unit for timers ·
haproxy/haproxy@a683805 (github.com)


if
additional libraries are easy to install (building will increase total time
a lot), I'd add opentracing to those "all features" builds

>
> Best regards,
>
> --
> Zaga
>
> What can change the nature of a man?
>

Re: [PATCH] MINOR: opentracing: register config file and line number on log servers

[Miroslav Zagorac]

On 04/07/2021 12:13 PM, Илья Шипицин wrote:

do you consider adding opentracing to "github actions" CI ?



Hello Илья,

I don't know how to add it because I never used it.  The filter uses 
specific libraries that are not part of the system but need to be 
installed and/or compiled independently.


Best regards,

--
Zaga

What can change the nature of a man?

Re: [PATCH] MINOR: opentracing: register config file and line number on log servers

[Илья Шипицин]

do you consider adding opentracing to "github actions" CI ?

ср, 7 апр. 2021 г. в 14:35, Miroslav Zagorac :

> Hello,
>
> due to the modified function declaration, the opentracing filter can no
> longer be compiled.
>
> 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.
>
> --
> Zaga
>
> What can change the nature of a man?
>

[PATCH] MINOR: opentracing: register config file and line number on log servers

[Miroslav Zagorac]

Hello,

due to the modified function declaration, the opentracing filter can no 
longer be compiled.


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.

--
Zaga

What can change the nature of a man?
>From baae10ef8a2eb487328664a3ff04475cd5d76179 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Wed, 7 Apr 2021 11:14:23 +0200
Subject: [PATCH] MINOR: opentracing: register config file and line number on
 log servers

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.
---
 addons/ot/src/parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/addons/ot/src/parser.c b/addons/ot/src/parser.c
index b53d58d44..c7522e942 100644
--- a/addons/ot/src/parser.c
+++ b/addons/ot/src/parser.c
@@ -490,7 +490,7 @@ static int flt_ot_parse_cfg_tracer(const char *file, int linenum, char **args, i
 		}
 	}
 	else if (pdata->keyword == FLT_OT_PARSE_TRACER_LOG) {
-		if (parse_logsrv(args, &(flt_ot_current_tracer->proxy_log.logsrvs), kw_mod == KWM_NO, _log) == 0) {
+		if (parse_logsrv(args, &(flt_ot_current_tracer->proxy_log.logsrvs), kw_mod == KWM_NO, file, linenum, _log) == 0) {
 			FLT_OT_PARSE_ERR(, "'%s %s ...' : %s", args[0], args[1], err_log);
 			FLT_OT_FREE_CLEAR(err_log);
 
-- 
2.30.1