Reduce repeated byte-size computation in exporterhelper queueing and batching

[iblancasa]

Description

Link to tracking issue

Fixes #15276

Testing

• Added some tests and benchmarks

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.04%. Comparing base (52e6bf4) to head (6080504).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #15277   +/-   ##
=======================================
  Coverage   91.03%   91.04%           
=======================================
  Files         729      730    +1     
  Lines       48491    48527   +36     
=======================================
+ Hits        44144    44180   +36     
  Misses       3020     3020           
  Partials     1327     1327           

☔ View full report in Codecov by Harness.
📢 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.

[codspeed-hq]

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 3 untouched benchmarks

---

_{Comparing iblancasa:15276 (6080504) with main (52e6bf4)}

[jade-guiton-dd]

Looks good. I tried finding a more elegant way to do it without passing SizerTypes everywhere but it seems this is the best we can do without a big refactor (why do we have a sizer implementation in the sizer package and another one in request??)

[iblancasa]

I tried finding a more elegant way to do it without passing SizerTypes everywhere but it seems this is the best we can do without a big refactor

Yes... I spent some time trying to figure out a better way but was unable.

(why do we have a sizer implementation in the sizer package and another one in request??)

I think SizerType ended up under request during the exporterbatcher deprecation work to avoid a circular dependency; there is still a TODO to move it back once that cycle is removed.

exporter/exporterhelper/internal/request/sizer.go:11:

// TODO: Move this back to queuebatch when remove the circular dependency.

[evan-bradley]

Looks good to me. I'd like to wait a little longer for @bogdandrutu or @dmitryax to review since they're the listed code owners.

@iblancasa could you post the results of the benchmark tests on main vs. this branch? Just to make sure we're getting the speedup we're expecting.

[iblancasa]

Looks good to me. I'd like to wait a little longer for @bogdandrutu or @dmitryax to review since they're the listed code owners.

Sure.

@iblancasa could you post the results of the benchmark tests on main vs. this branch? Just to make sure we're getting the speedup we're expecting.

main:

BenchmarkLogsRequestBytesSizeRepeated-14       13304.500n ± 3%   0 B/op   0 allocs/op
BenchmarkMetricsRequestBytesSizeRepeated-14    26987.500n ± 1%   0 B/op   0 allocs/op
BenchmarkTracesRequestBytesSizeRepeated-14     16426.500n ± 2%   0 B/op   0 allocs/op
BenchmarkProfilesRequestBytesSizeRepeated-14    8413.500n ± 1%   0 B/op   0 allocs/op

This branch:

BenchmarkLogsRequestBytesSizeRepeated-14          2.660n ± 4%   0 B/op   0 allocs/op
BenchmarkMetricsRequestBytesSizeRepeated-14       2.659n ± 2%   0 B/op   0 allocs/op
BenchmarkTracesRequestBytesSizeRepeated-14        2.713n ± 1%   0 B/op   0 allocs/op
BenchmarkProfilesRequestBytesSizeRepeated-14      2.816n ± 2%   0 B/op   0 allocs/op

Comparison:

pkg: go.opentelemetry.io/collector/exporter/exporterhelper/internal/queuebatch
                                   │ main        │ branch     │ vs base │
LogsRequestBytesSizeRepeated-14      13304.500n   2.660n       -99.98% (p=0.000 n=10)
MetricsRequestBytesSizeRepeated-14   26987.500n   2.659n       -99.99% (p=0.000 n=10)
TracesRequestBytesSizeRepeated-14    16426.500n   2.713n       -99.98% (p=0.000 n=10)
geomean                              18.07µ       2.677n       -99.99%

pkg: go.opentelemetry.io/collector/exporter/exporterhelper/xexporterhelper
ProfilesRequestBytesSizeRepeated-14  8413.500n    2.816n       -99.97% (p=0.000 n=10)

[evan-bradley]

Thanks, looks great. I'll merge this by EOD Friday (EDT) if there are no other concerns.

[dmitryax]

Cannot see where we gain improvement

[iblancasa]

Results of the new benchmark:

main:

BenchmarkLogsBytesQueueBatchPipeline-14        47.81µ ± 1%   704 B/op   18 allocs/op
BenchmarkMetricsBytesQueueBatchPipeline-14     96.67µ ± 1%   704 B/op   18 allocs/op
BenchmarkTracesBytesQueueBatchPipeline-14      58.27µ ± 1%   704 B/op   18 allocs/op
BenchmarkProfilesBytesQueueBatchPipeline-14    42.31µ ± 1%   704 B/op   18 allocs/op

This branch:

  BenchmarkLogsBytesQueueBatchPipeline-14        17.36µ ± 2%   728 B/op   18 allocs/op
  BenchmarkMetricsBytesQueueBatchPipeline-14     39.07µ ± 0%   728 B/op   18 allocs/op
  BenchmarkTracesBytesQueueBatchPipeline-14      21.61µ ± 1%   728 B/op   18 allocs/op
  BenchmarkProfilesBytesQueueBatchPipeline-14    15.60µ ± 1%   729 B/op   18 allocs/op

Comparison:

  pkg: go.opentelemetry.io/collector/exporter/exporterhelper
                                    │ main      │ branch    │ vs base │
  LogsBytesQueueBatchPipeline-14      47.81µ      17.36µ      -63.68% (p=0.000 n=10)
  MetricsBytesQueueBatchPipeline-14   96.67µ      39.07µ      -59.58% (p=0.000 n=10)
  TracesBytesQueueBatchPipeline-14    58.27µ      21.61µ      -62.91% (p=0.000 n=10)
  geomean                             64.58µ      24.48µ      -62.10%

  pkg: go.opentelemetry.io/collector/exporter/exporterhelper/xexporterhelper
  ProfilesBytesQueueBatchPipeline-14  42.31µ      15.60µ      -63.13%