docs: add request size limitation#782
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the OTLP specification documentation to define request size/body limits for OTLP/gRPC and OTLP/HTTP, aiming to reduce memory-exhaustion risk from oversized payloads.
Changes:
- Documented an OTLP/gRPC request message size limit recommendation (4 MiB) and client behavior on oversize errors.
- Documented an OTLP/HTTP request body size limit recommendation (20 MiB) and recommended HTTP 413 handling semantics.
- Added an Unreleased changelog entry for the documentation update.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/specification.md | Adds normative guidance for gRPC request message size limits and HTTP request body limits. |
| CHANGELOG.md | Records the docs change in the Unreleased “Added” section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@open-telemetry/profiling-maintainers do you think 32MiB request size (uncompressed) is large enough that we practically never hit this limit? If it is not large enough is there such number or we think that legitimate profiles can be arbitrarily large? If there is no practical limit for profiles do we feel comfortable with the approach that there is a known default limitation for payloads (e.g. 32MiB) and all profile emitters are expected to split collected profiles to keep individual payloads under this default limit? |
This comment was marked as resolved.
This comment was marked as resolved.
florianl
left a comment
There was a problem hiding this comment.
do you think 32MiB request size (uncompressed) is large enough that we practically never hit this limit? If it is not large enough is there such number or we think that legitimate profiles can be arbitrarily large? If there is no practical limit for profiles do we feel comfortable with the approach that there is a known default limitation for payloads (e.g. 32MiB) and all profile emitters are expected to split collected profiles to keep individual payloads under this default limit?
Since this limit is configurable, environments using larger profiles can still increase it. However, for most environments, 32 MiB appears to be a reasonable default.
Have you done any testing? Do we have some numbers? Could you e.g.. setup the https://github.com/open-telemetry/opentelemetry-ebpf-profiler for all services (including e.g. Prometheus, Jaeger) on https://github.com/open-telemetry/opentelemetry-demo/? Also waiting on @felixge with (from #782 (comment)):
|
I think this only works as a strategy if users have control over receiver's configuration, which may not always be the case. My preference would be one the following approaches, in the order of priority:
It is important that interoperability works by default, without requiring configuration. Configuration can be used to fine tune performance (e.g. if larger payloads result in better overall compression and memory usage is acceptable). |
felixge
left a comment
There was a problem hiding this comment.
Sorry for the delay, but I'm still planning to share some hard data here. Hope to be able to post it later this week.
|
@open-telemetry/collector-maintainers given that collector is primary OTLP server in the ecosystem, can you review? |
tigrannajaryan
left a comment
There was a problem hiding this comment.
Blocking temporarily until we get more data for profiles.
| compression, to avoid overwhelming the server. It is RECOMMENDED to use 32 MiB | ||
| as the default limit. Implementations MAY allow this limit to be configured. If | ||
| the limit is exceeded, the client MUST NOT make the request and SHOULD record | ||
| the fact that the request was discarded. |
There was a problem hiding this comment.
There was a discussion during the SIG meeting regarding splitting the request instead of only dropping. I think nothing is against adding later adding a sentence like:
In such scenario, the discarded request MAY be spitted into smaller requests that are below this limit.
Per #781 (comment)
Add request size limitation for HTTP body and gRPC messages to mitigate memory usage risks.
Reference: https://cwe.mitre.org/data/definitions/789.html
The values are taken from
otlpreceiver defaults#782 (comment)4 MiB32 MiB20 MiB32 MiBOTel implementations: