baggage: Improve performance

[cdvr1993]

Several of our microservices during profiling report a considerable cost coming from the regex that validate the key and the value (in the range of 3-6%). For instance:

After checking the regex I saw that we can easily replace that to a simple for loop implementation. All tests are passing, but hopefully didn't miss a condition.

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/baggage
cpu: AMD EPYC 7B13
             │ /tmp/old.txt │            /tmp/new.txt             │
             │    sec/op    │   sec/op     vs base                │
New-96          1.394µ ± 5%   1.382µ ± 7%        ~ (p=0.190 n=10)
NewMember-96   307.10n ± 1%   39.66n ± 1%  -87.08% (p=0.000 n=10)
Parse-96        4.575µ ± 6%   1.037µ ± 7%  -77.33% (p=0.000 n=10)
geomean         1.251µ        384.5n       -69.26%

             │ /tmp/old.txt  │             /tmp/new.txt             │
             │     B/op      │    B/op     vs base                  │
New-96          835.0 ± 0%     835.0 ± 0%        ~ (p=1.000 n=10) ¹
NewMember-96    0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
Parse-96       1108.0 ± 0%     832.0 ± 0%  -24.91% (p=0.000 n=10)
geomean                    ²                -9.11%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

             │ /tmp/old.txt  │             /tmp/new.txt             │
             │   allocs/op   │ allocs/op   vs base                  │
New-96          16.00 ± 0%     16.00 ± 0%        ~ (p=1.000 n=10) ¹
NewMember-96    0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
Parse-96       11.000 ± 0%     7.000 ± 0%  -36.36% (p=0.000 n=10)
geomean                    ²               -13.99%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: cdvr1993 / name: Cristian Velazquez (eddc838, d657b76, 903d51c, 62539fb, 3f3e54f, b2470c6, 6f8aea4, bb39a11, 7503e88, c0da462, 97a1986, a8c3d1a, 8f1d45b, 89a7be3, 14e7c0f)
• ✅ login: pellared / name: Robert Pająk (416cb6a, ff7e331, fba4b9e, 6dc6ce1, ad00a12, a80a261, 6b5de85, 00be1c9, ef7173b, c5728bb, 412bf43)

[codecov]

Codecov Report

Merging #4743 (412bf43) into main (0c1c434) will increase coverage by 0.0%.
The diff coverage is 100.0%.

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #4743   +/-   ##
=====================================
  Coverage   82.0%   82.1%           
=====================================
  Files        225     225           
  Lines      18242   18325   +83     
=====================================
+ Hits       14976   15059   +83     
  Misses      2981    2981           
  Partials     285     285           

Files	Coverage Δ
baggage/baggage.go	99.2% <100.0%> (+0.2%)	⬆️

[pellared]

Please fix the build (lint) failures.

[pellared]

Can you please run all benchmarks for the whole package? The changes should also affect BenchmarkNew and BenchmarkParse.

[MrAlias]

Thank you the contribution.

I agree with @pellared's comments in that this looks like a partial solution. There is coupling with other functionality that this does not address. I do not think these changes should be accepted as they currently are presented.

[pellared]

I did my best in reviewing the PR. I made some IMO trivial (editorial) changes directly. I also have a bigger suggestion on how parsePropertyInternal could be implemented to make it more readable and maintainable.

[pellared]

Approving.

However, I would appreciate adding some more comments to the code as suggested in #4743 (comment) and #4743 (comment).

Thank you for your contribution 👍

[cdvr1993]

hi everyone, just checking in, is there anything else missing to merge this? Also, I couldn't find it, what is the process for releasing? Do you release on a schedule or is it on-demand?