Validate members once, in NewMember

[dmathieu]

This is an alternative attempt to #2505 at at improving the number of calls to validate() in baggage.Member, calling it only when the member is added to the baggage.
Right now, we call it twice. Once within baggage.NewMember, and another time within baggage.New.

Closes #2498

[codecov]

Codecov Report

Merging #2522 (b3d6bb7) into main (c5776cc) will increase coverage by 0.0%.
The diff coverage is 97.6%.

@@          Coverage Diff          @@
##            main   #2522   +/-   ##
=====================================
  Coverage   76.0%   76.0%           
=====================================
  Files        174     174           
  Lines      12190   12206   +16     
=====================================
+ Hits        9266    9286   +20     
+ Misses      2679    2675    -4     
  Partials     245     245           

Impacted Files	Coverage Δ
baggage/baggage.go	97.3% <97.6%> (+0.1%)	⬆️
exporters/jaeger/jaeger.go	93.5% <0.0%> (+0.8%)	⬆️
sdk/trace/batch_span_processor.go	82.1% <0.0%> (+0.9%)	⬆️

[dmathieu]

I have added an isEmpty, which is used to check whether the provided member and property are empty or not, so they don't need revalidation but also can't be added to their parent struct if they are empty.

[MadVikingGod]

So as it stands, this change has a net negative effect on parsing. For this to proceed, you will have to do a bit of profiling and improve what is already here.

name         old time/op    new time/op    delta
New-4          3.15µs ± 5%    3.12µs ± 3%     ~     (p=0.489 n=9+9)
NewMember-4     363ns ± 5%     360ns ± 3%     ~     (p=0.605 n=9+9)
Parse-4        6.29µs ± 6%    7.40µs ± 4%  +17.61%  (p=0.000 n=10+9)

name         old alloc/op   new alloc/op   delta
New-4            832B ± 0%      832B ± 0%     ~     (all equal)
NewMember-4     0.00B          0.00B          ~     (all equal)
Parse-4        1.35kB ± 0%    1.43kB ± 0%   +5.94%  (p=0.000 n=10+10)

name         old allocs/op  new allocs/op  delta
New-4            16.0 ± 0%      16.0 ± 0%     ~     (all equal)
NewMember-4      0.00           0.00          ~     (all equal)
Parse-4          19.0 ± 0%      20.0 ± 0%   +5.26%  (p=0.000 n=10+10)

[dmathieu]

So, I've done two changes:

• New actually needs to call hasData and not call validate(). If we call validate, with hasData being true, we rerun the validation (hence no performance improvement in your benchstat).
• We need to check the regex themselves and not use NewMember in parseMember. Doing the latter reruns the validation, both on the member and on the properties, hence the performance loss.

Here is my benchstat with the latest changes:

name          old time/op    new time/op    delta
New-12          2.45µs ± 1%    1.22µs ± 2%  -50.29%  (p=0.000 n=10+10)
NewMember-12     288ns ± 1%     290ns ± 2%     ~     (p=0.315 n=9+10)
Parse-12        4.92µs ± 1%    4.94µs ± 1%     ~     (p=0.589 n=10+9)

name          old alloc/op   new alloc/op   delta
New-12            832B ± 0%      832B ± 0%     ~     (all equal)
NewMember-12     0.00B          0.00B          ~     (all equal)
Parse-12        1.36kB ± 0%    1.36kB ± 0%     ~     (p=0.875 n=10+10)

name          old allocs/op  new allocs/op  delta
New-12            16.0 ± 0%      16.0 ± 0%     ~     (all equal)
NewMember-12      0.00           0.00          ~     (all equal)
Parse-12          19.0 ± 0%      19.0 ± 0%     ~     (all equal)

[MadVikingGod]

I don't see any further problems with this approach.