Skip to content

fix: oversize payload causing universal instrumentation failures#1067

Merged
joeyzhao2018 merged 6 commits intomainfrom
SLES-2722
Mar 9, 2026
Merged

fix: oversize payload causing universal instrumentation failures#1067
joeyzhao2018 merged 6 commits intomainfrom
SLES-2722

Conversation

@joeyzhao2018
Copy link
Contributor

@joeyzhao2018 joeyzhao2018 commented Mar 5, 2026

Instead of passing the whole Request into the spawned task and calling extract_request_body (which consumes the request and early-returns on failure), the fix:

  1. Splits the request upfront with request.into_parts() — headers are preserved regardless of body extraction outcome
  2. Attempts body buffering via Bytes::from_request
  3. Falls back to Bytes::new() on failure (e.g. oversized payload) with a warn! log
  4. Always calls universal_instrumentation_end — trace context, span finalization, and status code extraction proceed with degraded (empty) payload rather than being silently
    dropped

The FromRequest import was added to the module-level axum imports to support calling Bytes::from_request directly.

@joeyzhao2018 joeyzhao2018 changed the title add test to show the issue fix: oversize payload causing universal instrumentation failures Mar 5, 2026
joeyzhao2018 and others added 3 commits March 9, 2026 13:37
When the response payload exceeds the 6MB DefaultBodyLimit,
extract_request_body fails inside the spawned task, causing an early
return that skips universal_instrumentation_end entirely — dropping
trace context, span finalization, and status code extraction.

Fix by splitting the request into parts upfront (preserving headers)
and falling back to an empty body when buffering fails, so processing
always continues with a degraded payload. Update the test to verify
the graceful degradation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joeyzhao2018 joeyzhao2018 marked this pull request as ready for review March 9, 2026 17:44
@joeyzhao2018 joeyzhao2018 requested a review from a team as a code owner March 9, 2026 17:44

use crate::{
http::{extract_request_body, headers_to_map},
http::{extract_request_body_or_empty, headers_to_map},
Copy link
Contributor

Choose a reason for hiding this comment

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

So if you're using extract_request_body_or_empty, what's going on with extract_request_body? Is that ever used again? Do we need to update this new behavior in previous usages or?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are a few other usages for extract_request_body

  1. let (_, body) = match extract_request_body(request).await {
  2. let (parts, body) = match extract_request_body(request).await {
  3. let (parts, body) = match extract_request_body(req).await {
  4. let (parts, body) = match extract_request_body(request).await {
  5. let (parts, body) = match extract_request_body(request).await {

I actually don't have enough background knowledge to say if those cases can follow the same pattern...Do you think we should make all of them just log the error and accept empty body?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, fair enough, let's keep it as is for this one and let the other ones handle it as they expect it

Copy link
Contributor

@duncanista duncanista left a comment

Choose a reason for hiding this comment

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

LGTM

@joeyzhao2018 joeyzhao2018 merged commit dd17d4a into main Mar 9, 2026
50 checks passed
@joeyzhao2018 joeyzhao2018 deleted the SLES-2722 branch March 9, 2026 19:12
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.

3 participants