Enforce OTLP request size limits#8446
Conversation
|
280870d to
6720aae
Compare
|
@jkwatson Thanks — I pushed a follow-up that fixes the NullAway compile failure in the OTLP internal exporters and applies the naming nits to use constants and public API parameter names. |
jack-berg
left a comment
There was a problem hiding this comment.
A few small comments, but looks pretty good! Thanks for working on this!
afad5b0 to
3cf2f56
Compare
|
Fixed the remaining Spotless failure in |
jack-berg
left a comment
There was a problem hiding this comment.
Looks like there are still some build failures from static analysis.
Couple more nits to fix along with the build, but looks good to me
17cec67 to
2e40dc0
Compare
|
Rebased this branch onto current Follow-up fixes in the latest push:
Local verification completed with:
|
|
This PR has review comments. Review suggestions, whether from maintainers or automated reviewers, aren't always correct or required. Please evaluate each comment on its merits, then make sure each thread has a clear outcome. For example, link to the commit if you applied a suggestion, explain why it wasn't applied, or ask a follow-up question. Automation flags a PR for human review once every review thread has a reply or is marked as resolved. Status across open PRs is visible on the pull request dashboard. |
2e40dc0 to
d69235b
Compare
…y-java into issue-8444-otlp-message-limits
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8446 +/- ##
============================================
- Coverage 91.61% 91.61% -0.01%
- Complexity 10316 10339 +23
============================================
Files 1013 1013
Lines 27278 27347 +69
Branches 3203 3206 +3
============================================
+ Hits 24991 25053 +62
- Misses 1560 1568 +8
+ Partials 727 726 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@open-telemetry/java-approvers - heads up - this PR adds a default OTLP message / request body size of 64mb, where none currently exists. You could make the argument that this is a breaking behavior change, and that we should proceed but only once there is a env var / declarative config property to configure a different limit. (Ive opened open-telemetry/opentelemetry-configuration#695 to add declarative config schema to configure this) Personally, I think the lack of current limit is a bug and adding a default of 64mb is a generous limit few will hit. |
Fixes #8444.
Why
opentelemetry-proto#782now recommends that OTLP exporters enforce configurable outbound request/message size limits.What
docs/apidiffs/current_vs_latest/opentelemetry-exporter-otlp.txtfor the new public builder methodsTesting
I was not able to complete local Gradle verification on this Windows machine because Gradle / the Kotlin compiler daemon repeatedly failed to start due to paging-file / metaspace limits before compilation finished. The changes are pushed for CI verification.