[thswlsqls]

Fixes #8940

Description

• The declarative config schema (v1.2.0) says for max_request_size: "A value of 0 indicates no limit (infinity) and is not recommended." Setting it to 0 made SDK creation fail, because the OTLP builders only accepted positive values.
• Following the zero value guidance from Add zero value guidance to the knowledge base #8986, the seven public setMaxRequestBodySize / setMaxRequestMessageSize setters (OTLP HTTP and gRPC span, metric and log exporters, and the gRPC profiles exporter) now accept 0 and document it as no limit. Negative values are still rejected.
• HttpExporterBuilder and GrpcExporterBuilder map 0 to Long.MAX_VALUE, like they already do for a 0 timeout. The limit is only used in size > max checks, so nothing is allocated up front.
• Declarative config gets this for free, so OtlpDeclarativeConfigUtil passes the value through unchanged.
• Context: the option was added in Enforce OTLP request size limits #8446; the meaning of 0 was agreed in feat: add OTLP request and response size limits opentelemetry-configuration#700.

Testing done

• AbstractHttpTelemetryExporterTest / AbstractGrpcTelemetryExporterTest: setMaxRequestSize(0) is now accepted, -1 still throws ("must be non-negative"), and the new requestBodySizeLimitDisabledWithZero / requestMessageSizeLimitDisabledWithZero tests export successfully with 0.
• Declarative config factory tests: kept the max_request_size 0 cases and added negative cases for otlp_http and otlp_grpc in SpanExporterFactoryTest, MetricExporterFactoryTest and LogRecordExporterFactoryTest.
• With the 0 → Long.MAX_VALUE mapping reverted, the new zero tests fail; with it, they pass.
• ./gradlew :exporters:otlp:all:check :exporters:otlp:profiles:check :exporters:otlp:testing-internal:check :sdk-extensions:declarative-config:check passed. No apidiff change (only argument checks and Javadoc changed).

[opentelemetry-pr-dashboard]

Pull request dashboard status

Merged · refreshed 2026-11-05 18:14 UTC

Status above doesn't look right?

• Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.37%. Comparing base (1acb73a) to head (30659ac).

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #8945   +/-   ##
=========================================
  Coverage     90.37%   90.37%           
- Complexity    11012    11015    +3     
=========================================
  Files          1020     1020           
  Lines         30119    30120    +1     
  Branches       3932     3933    +1     
=========================================
+ Hits          27219    27220    +1     
  Misses         1941     1941           
  Partials        959      959           

☔ 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.

[psx95]

Could we instead make this change in the internal exporter builders instead?

For instance, we could make this change in here in HttpExporterBuilder.

The handling of 0 value for timeout is also done here. This would make the behavior of configuring SDK programmatically vs via declarative config consistent.

The config options listed in exporter spec do not explicitly state how to handle 0, but I think the spec might just be outdated.

Perhaps other maintainers would want to chime in here?
cc: @jack-berg , @jkwatson

[thswlsqls]

@psx95 Thanks for the review. Moving it into the internal builders makes sense to me, and it would match how setTimeout treats 0.

One catch: the public setters reject 0 before the internal builder sees it (checkArgument(maxRequestBodySizeBytes > 0, ...) in OtlpHttpSpanExporterBuilder.setMaxRequestBodySize, and the same in the other five OTLP builders). So this would also mean relaxing that check to >= 0 and documenting 0 as "no limit" on the stable builders. That's why I kept the change in the declarative config layer.

I'll wait for @jack-berg / @jkwatson to weigh in and update the PR either way.

[jack-berg]

and it would match how setTimeout treats 0.
One catch: the public setters reject 0 before the internal builder sees it (checkArgument(maxRequestBodySizeBytes > 0, ...) in OtlpHttpSpanExporterBuilder.setMaxRequestBodySize, and the same in the other five OTLP builders). So this would also mean relaxing that check to >= 0 and documenting 0 as "no limit" on the stable builders. That's why I kept the change in the declarative config layer.

So we're inconsistent:

• 0 value is tolerated in setTimeout, and triggers infinite semantics
• 0 is not tolerated in setMaxRequestBodySize, and throws

We can relax the more restrictive APIs, but we can't make the more relaxed APIs more restrictive because doing so is a breaking change.

I'm ok with relaxing the APIs for consistency, but I want to make sure that we can come up with standard guidance for this that makes sense in all the cases, which we can add to the knowledge base. In particular, I'm thinking about cases like BatchSpanProcessorBuilder#setMaxQueueSize. On first glance, it seems like another case where you can assign the 0 value to mean infinite, but you can't / shouldn't initialize a queue with Integer.MAX_VALUE, so tolerating 0 would be a foot gun. What is the litmus test that determines whether a property should allow 0 to be set or not?

[jack-berg]

Adding a policy around this here: #8986

[jkwatson]

should we be erroring if someone passes in a negative?

[thswlsqls]

@jkwatson Thanks. It already errors: a negative value reaches the public setter, whose checkArgument rejects it, so building the exporter fails.

After the rework the setters check >= 0, so negatives keep failing. I'll add a negative case to the declarative config tests to keep that covered.

[thswlsqls]

@jkwatson Added negative max_request_size cases for otlp_http and otlp_grpc to the span, metric and log exporter factory tests in 30659ac.

The PR changed quite a bit since your approval, so another look would be appreciated.

[jack-berg]

We've got consensus on #8986, so let's go ahead with this. I agree with @psx95's comment about doing this programmatic config layer - i.e. changing Otlp{Protocol}{Signal}ExporterBuilder and the underlying {Protocol}ExporterBuilder to relax the constraints and translate 0 to to Long.MAX_VALUE. This approach is most aligned with #8986 which tries to standardize behavior at the programmatic API level. Declarative config would get 0 handling for free.

[thswlsqls]

@jack-berg Thanks, I'll rework the PR that way.

The seven public setMaxRequestBodySize / setMaxRequestMessageSize setters will accept 0, and HttpExporterBuilder / GrpcExporterBuilder will map it to Long.MAX_VALUE, the same way they already handle a 0 timeout. I'll drop the special case from OtlpDeclarativeConfigUtil and document and test the 0 behavior on the builders, per #8986.

[thswlsqls]

@jack-berg Reworked in 30659ac: the seven setters now accept 0, HttpExporterBuilder / GrpcExporterBuilder map it to Long.MAX_VALUE, and the OtlpDeclarativeConfigUtil special case is gone. The new requestBodySizeLimitDisabledWithZero / requestMessageSizeLimitDisabledWithZero tests export with 0 and fail without the mapping.

[otelbot]

Thank you for your contribution @thswlsqls! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.