Adds '+' character to allowed characters in baggage value

[lmonkiewicz]

This is a bit dirty way to fix it, but the alternative would be to rewrite from scratch URLDecoder, which treats + character as space. We can't use URLDecorder source code and modify it, since it is GPL licensed (at least that is how I understand it).

Maybe we could use some other implementation of URL Decoder that allows for greater customization, but nothing comes to my mind.

Fixes: #4892

[codecov]

Codecov Report

Base: 90.90% // Head: 90.98% // Increases project coverage by +0.07% 🎉

Coverage data is based on head (992e63b) compared to base (51ff803).
Patch coverage: 85.71% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #4898      +/-   ##
============================================
+ Coverage     90.90%   90.98%   +0.07%     
- Complexity     4803     4839      +36     
============================================
  Files           545      546       +1     
  Lines         14340    14439      +99     
  Branches       1383     1395      +12     
============================================
+ Hits          13036    13137     +101     
+ Misses          897      894       -3     
- Partials        407      408       +1     

Impacted Files	Coverage Δ
.../opentelemetry/api/baggage/propagation/Parser.java	97.05% <66.66%> (+1.47%)	⬆️
...elemetry/api/baggage/propagation/BaggageCodec.java	88.88% <88.88%> (ø)
...gator/DoubleExplicitBucketHistogramAggregator.java	95.71% <0.00%> (-4.29%)	⬇️
...metry/sdk/metrics/export/PeriodicMetricReader.java	87.14% <0.00%> (-2.86%)	⬇️
...va/io/opentelemetry/sdk/logs/SdkLoggerBuilder.java	100.00% <0.00%> (ø)
...a/io/opentelemetry/sdk/logs/SdkLoggerProvider.java	100.00% <0.00%> (ø)
.../io/opentelemetry/sdk/logs/LogRecordProcessor.java	85.71% <0.00%> (ø)
...a/io/opentelemetry/sdk/trace/SdkTracerBuilder.java	100.00% <0.00%> (ø)
.../io/opentelemetry/sdk/metrics/SdkMeterBuilder.java	100.00% <0.00%> (ø)
...opentelemetry/sdk/logs/NoopLogRecordProcessor.java	100.00% <0.00%> (ø)
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jack-berg]

Maybe we could use some other implementation of URL Decoder that allows for greater customization, but nothing comes to my mind.

Your interpretation is correct.

I also wondered if there is any performance benefit in checking for the presence of + before doing the replace, but this discussion indicates that the performance is improving over time, so implementation should eventually include any possible optimizations.

I think its either this approach or vendoring in some other url decoder like apache URLCodec.

[jack-berg]

This is looking good. Just a couple of minor cleanups.

[jack-berg]

Looks like value is never null. Can remove the @Nullable annotation on the value argument, and also maybe on the response. That should improve test coverage.

Same recommendation applies to private static byte[] decode(@Nullable byte[] bytes) above.

[jack-berg]

Just the one last comment about @Nullable. Thanks!