[confighttp] Reuse zstd reader objects

[ethercrow]

Description

Allocating zstd reader objects on every incoming requests leads to a lot of byte buffers being allocated and collected. The compression library recommends reusing readers to avoid these allocations.

Link to tracking issue

Fixes #11824

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ethercrow / name: Dmitry Ivanov (f3d4394, a8bbbdf, 505ee1c, 609e89a, 0419397)
• ✅ login: codeboten / name: Alex Boten (19d8a15)

[codecov]

Codecov Report

❌ Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.92%. Comparing base (8b73477) to head (19d8a15).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
config/confighttp/compression.go	85.00%	2 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (85.00%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13396      +/-   ##
==========================================
- Coverage   91.95%   91.92%   -0.04%     
==========================================
  Files         529      529              
  Lines       31446    31461      +15     
==========================================
+ Hits        28917    28921       +4     
- Misses       1998     2006       +8     
- Partials      531      534       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[codeboten]

Thanks @ethercrow! The change looks good, can you post a benchmark to show the performance before and after this change?

[ethercrow]

Sorry, I have never run this code. My problem turned out to be in zstd decoding in grpc, which is handled by another library entirely. I was just assuming that the only place in the collector codebase involving zstd decoding must be the culprit.

[codeboten]

@ethercrow ah thanks for the update... is this change no longer necessary then?

[ethercrow]

I think it's still a good change to make.

I will probably do some real world testing in future, when I have some collectors communicating through otlphttp. Just can't promise any specific date.

[jmacd]

Nice!

Note that https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/internal/otelarrow/compression/zstd has re-use, too, good to have.

[codeboten]

Ran some benchmarks before and after the change, change looks good:

goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/collector/config/confighttp
cpu: Apple M3 Max
                                │    /tmp/a    │             /tmp/b              │
                                │    sec/op    │    sec/op     vs base           │
ServerWithDecompression/test-14   170.2µ ± ∞ ¹   120.3µ ± ∞ ¹  ~ (p=0.333 n=2) ²

                                │    /tmp/a     │              /tmp/b              │
                                │     B/op      │     B/op       vs base           │
ServerWithDecompression/test-14   14.18Mi ± ∞ ¹   11.44Mi ± ∞ ¹  ~ (p=0.333 n=2) ²

                                │   /tmp/a    │             /tmp/b             │
                                │  allocs/op  │  allocs/op   vs base           │
ServerWithDecompression/test-14   262.5 ± ∞ ¹   254.5 ± ∞ ¹  ~ (p=0.333 n=2) ²