Repository navigation
Conversation
Pull request dashboard statusWaiting on the author · refreshed 2026-10-14 05:33 UTC Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):
Status above doesn't look right?
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8945 +/- ##
=========================================
Coverage 90.65% 90.66%
- Complexity 10887 10889 +2
=========================================
Files 1015 1015
Lines 29731 29731
Branches 3876 3877 +1
=========================================
+ Hits 26953 26955 +2
Misses 1860 1860
+ Partials 918 916 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
psx95
left a comment
There was a problem hiding this comment.
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
|
@psx95 Thanks for the review. Moving it into the internal builders makes sense to me, and it would match how One catch: the public setters reject 0 before the internal builder sees it ( I'll wait for @jack-berg / @jkwatson to weigh in and update the PR either way. |
Fixes #8940
Description
max_request_size: "A value of 0 indicates no limit (infinity) and is not recommended."OtlpDeclarativeConfigUtilpassed 0 tosetMaxRequestBodySize/setMaxRequestMessageSize, which only accept positive values, so SDK creation failed withDeclarativeConfigException.Long.MAX_VALUE, astimeout: 0already is. The limit is only used insize > maxchecks, so this turns the check off.Testing done
otlp_http max_request_size 0andotlp_grpc max_request_size 0cases toSpanExporterFactoryTest,MetricExporterFactoryTestandLogRecordExporterFactoryTest.DeclarativeConfigException(... must be positive). With the fix, all 41 tests in these classes pass../gradlew :exporters:otlp:all:check :sdk-extensions:declarative-config:checkpassed (1190 and 546 tests). No apidiff change (internal package).