Skip to content

fix(otlp): use body.len() for body_size and fix empty-traces check#1093

Draft
lym953 wants to merge 3 commits intoyiming.luo/drop-traces-2from
yiming.luo/drop-traces-3
Draft

fix(otlp): use body.len() for body_size and fix empty-traces check#1093
lym953 wants to merge 3 commits intoyiming.luo/drop-traces-2from
yiming.luo/drop-traces-3

Conversation

@lym953
Copy link
Contributor

@lym953 lym953 commented Mar 9, 2026

Summary

  • size_of_val(&traces) returned the 24-byte stack size of the Vec regardless of content, so body_size was always a meaningless constant and the empty-trace guard (if body_size == 0) never fired
  • Replace with body.len() (raw OTLP wire bytes) for body_size and traces.is_empty() for the empty-trace guard

Test plan

  • test_v1_traces_empty_request_returns_error: verifies that an OTLP request with no spans returns 500 (previously the guard never fired)
  • test_v1_traces_body_size_equals_request_body_len: verifies that body_size passed to process_traces equals body.len()

🤖 Generated with Claude Code

@lym953 lym953 changed the base branch from main to yiming.luo/drop-traces-2 March 9, 2026 20:37
@lym953
Copy link
Contributor Author

lym953 commented Mar 9, 2026

@duncanista In your PR #654 there's a commit use size_of_val for payload, instead of .encoded_len(). Do you remember why?

lym953 and others added 3 commits March 9, 2026 16:51
size_of_val(&traces) returned the 24-byte stack size of the Vec regardless of
content, so body_size was always a meaningless constant and the empty-trace
guard never fired. Use body.len() for the actual OTLP wire byte count and
traces.is_empty() for the empty-trace guard.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lym953 lym953 force-pushed the yiming.luo/drop-traces-3 branch from c651cfe to 4fa9dca Compare March 9, 2026 20:53
@duncanista
Copy link
Contributor

@lym953 I remember it was because it was "more correct", I think after using prost, the method for len wasn't available on the message itself, it would crash

@lym953
Copy link
Contributor Author

lym953 commented Mar 10, 2026

@duncanista Thanks. From my test, I didn't see any problem using prost::Message::encoded_len. Appreciate review: `#1091

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.

2 participants