Skip to content

Test skip processing headers#301

Merged
fpacifici merged 5 commits intomainfrom
fpacifici/exclude_headers
Apr 23, 2026
Merged

Test skip processing headers#301
fpacifici merged 5 commits intomainfrom
fpacifici/exclude_headers

Conversation

@fpacifici
Copy link
Copy Markdown
Collaborator

BUilt on top of #299

One of the main throughput issues we observed in the tests mentioned in
#299 is that headers are
really expensive to pass to and from rust. Most of the issue seems
related with https://github.com/getsentry/streams/blob/main/sentry_streams/src/messages.rs#L47-L82.
Specifically this code is executed every time python references
headers.

This is a test to confirm the impact by removing all the headers related
logic.
All messages are created with empty headers, the header fetching logic
is never called.
I left the support for headers in the rust code because the consumer
to test relies on a header filter so I cannto remove it entirely.

This change is suppsoed to be reverted after we perform a production test

@fpacifici fpacifici requested a review from a team as a code owner April 22, 2026 21:43
@fpacifici fpacifici changed the base branch from main to fpacifici/optimize_messages April 22, 2026 21:43
@fpacifici fpacifici force-pushed the fpacifici/exclude_headers branch from 485b7d2 to 0c9a2f3 Compare April 22, 2026 22:00
@fpacifici fpacifici force-pushed the fpacifici/exclude_headers branch from 0c9a2f3 to fb8d2bb Compare April 23, 2026 15:40
@fpacifici fpacifici changed the base branch from fpacifici/optimize_messages to main April 23, 2026 15:40
@@ -110,7 +110,7 @@ def __init__(
schema: Optional[str] = None,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The PyMessage and PyRawMessage constructors accept a headers parameter but silently ignore it, always initializing headers to an empty list.
Severity: MEDIUM

Suggested Fix

The headers parameter should be used to initialize the self._headers attribute in the PyMessage and PyRawMessage constructors. If the intention is to permanently remove header handling, the headers parameter should be removed from the constructor signatures and all call sites to prevent silent data loss.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry_streams/sentry_streams/pipeline/message.py#L110

Potential issue: The constructors for `PyMessage` and `PyRawMessage` accept a `headers`
parameter but the implementation ignores it, always initializing the internal `_headers`
attribute to an empty list. This means any headers passed when creating these message
objects are silently discarded. This violates the API contract and can lead to
unexpected behavior or silent failures in any code that relies on headers being
preserved across processing steps, such as custom filters or monitoring logic.

Did we get this right? 👍 / 👎 to inform future reviews.

@fpacifici fpacifici merged commit 7b984bf into main Apr 23, 2026
22 checks passed
@fpacifici fpacifici deleted the fpacifici/exclude_headers branch April 23, 2026 17:01
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