Skip to content

feat(otlphttp): configure response body size limit - #8847

Open
jstar0 wants to merge 7 commits into
open-telemetry:mainfrom
jstar0:fix/otlphttp-response-body-limit
Open

feat(otlphttp): configure response body size limit#8847
jstar0 wants to merge 7 commits into
open-telemetry:mainfrom
jstar0:fix/otlphttp-response-body-limit

Conversation

@jstar0

@jstar0 jstar0 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #8819.

  • Add WithMaxResponseBodySize to the OTLP/HTTP trace, metric, and log exporters.
  • Keep the finite 4 MiB default response-body limit and apply it after decompression.
  • Treat non-positive option values as invalid and retain the default instead of disabling the safety limit.
  • Use bounded reads that detect oversized responses without integer-overflow edge cases.
  • Keep the generated gRPC configuration free of HTTP-only response-body settings.

Verification

  • go test ./... passes in the trace, metric, and log OTLP/HTTP exporter modules.
  • make precommit completed generation, formatting/lint, README validation, and module consistency checks.
  • The repository-wide race run reached the pre-existing bridge/opentracing gRPC test, which failed locally with an EOF while reading the server preface; no changed package failed.

@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.5%. Comparing base (9ba91a2) to head (4b985e9).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #8847   +/-   ##
=====================================
  Coverage   88.5%   88.5%           
=====================================
  Files        333     333           
  Lines      21101   21129   +28     
=====================================
+ Hits       18687   18718   +31     
+ Misses      2414    2411    -3     
Files with missing lines Coverage Δ
exporters/otlp/otlplog/otlploghttp/client.go 92.3% <100.0%> (+1.3%) ⬆️
exporters/otlp/otlplog/otlploghttp/config.go 96.3% <100.0%> (+0.1%) ⬆️
...tlpmetric/otlpmetricgrpc/internal/oconf/options.go 94.3% <100.0%> (ø)
exporters/otlp/otlpmetric/otlpmetrichttp/client.go 92.7% <100.0%> (+1.4%) ⬆️
exporters/otlp/otlpmetric/otlpmetrichttp/config.go 90.0% <100.0%> (+0.5%) ⬆️
...tlpmetric/otlpmetrichttp/internal/oconf/options.go 96.8% <100.0%> (+0.1%) ⬆️
...trace/otlptracegrpc/internal/otlpconfig/options.go 94.5% <ø> (ø)
exporters/otlp/otlptrace/otlptracehttp/client.go 89.5% <100.0%> (+1.1%) ⬆️
...trace/otlptracehttp/internal/otlpconfig/options.go 93.1% <100.0%> (+0.2%) ⬆️
exporters/otlp/otlptrace/otlptracehttp/options.go 100.0% <100.0%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MrAlias MrAlias left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR adds the requested option consistently across trace, metric, and log OTLP/HTTP exporters, preserves the 4 MiB default for ordinary configurations, updates generated configuration, and adds focused tests.

Comment thread exporters/otlp/otlptrace/otlptracehttp/client.go Outdated
Comment thread CHANGELOG.md Outdated
@jstar0

jstar0 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

Heads-up while refreshing this branch: current main now ships its own response-body limit (package-level maxResponseBodySize var, enforced via http.MaxBytesReader with MaxBytesError handling, from the changes around #8876). That limit is not user-configurable — the variable exists so tests can override it — while this PR adds WithMaxResponseBodySize as a per-exporter option.

The two overlap, so before I rebase I'd like to align on the intended shape:

  1. Compose them (my preference): keep main's package var as the default source, and let WithMaxResponseBodySize override it per exporter. This preserves TestResponseBodySizeLimit's var-override pattern unchanged and adds the configurability this PR (and feat(otlphttp): configure response body size limit #8819) ask for.
  2. Var-only: if maintainers prefer the non-configurable cap as final, this PR becomes redundant for the option parts and I can close it in favor of main's implementation.

Happy to rebase under either decision — the option plumbing (oconf + trace/metric/log exporters) is already in place on this branch.

Compose WithMaxResponseBodySize with the package-level 4 MiB default,
keep every response-body read on http.MaxBytesReader including MaxInt64,
and name the full exporter modules in the changelog.

Signed-off-by: King Star <mcxin.y@gmail.com>
@jstar0

jstar0 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Merged current main and composed the two limits as described above:

  • Package-level maxResponseBodySize remains the default (tests can still override it).
  • WithMaxResponseBodySize overrides that default per exporter.
  • Every response-body read, including WithMaxResponseBodySize(math.MaxInt64), goes through http.MaxBytesReader.
  • Changelog entry now names the three HTTP exporter modules.

Verified with go test in otlptracehttp, otlpmetrichttp, otlploghttp, plus their otlpconfig/oconf packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(otlphttp): configure response body size limit

2 participants