Skip to content

feat(taskworker): Make process_profile worker support uncompressed payloads #95692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 16, 2025

Conversation

enochtangg
Copy link
Member

@enochtangg enochtangg commented Jul 16, 2025

The taskworker producer layer now supports zstd compression. This means that the zlib task level compression is no longer necessary here. In order to remove, this PR updates the worker code to take an optional parameter which decides whether or not the payload should be zlib decompressed. In a subsequent PR, the producer code will be updated to send uncompressed payloads

@enochtangg enochtangg requested a review from a team as a code owner July 16, 2025 18:29
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 16, 2025
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Payload Decoding Error Handling Regression

The decode_payload function introduces inconsistent error handling. The uncompressed payload path (compressed_profile=False) lacks try/catch blocks for b64decode and msgpack.unpackb operations. Consequently, failures in these operations will not be logged or tracked with metrics, unlike the compressed path which retains this error handling. This regression can lead to unhandled exceptions and hinders debugging.

src/sentry/profiles/task.py#L115-L117

raise
else:
return msgpack.unpackb(b64decode(encoded.encode("utf-8")), use_list=False)

Fix in CursorFix in Web


Was this report helpful? Give feedback by reacting with 👍 or 👎

Copy link

codecov bot commented Jul 16, 2025

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/profiles/task.py 63.63% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #95692      +/-   ##
==========================================
+ Coverage   75.45%   83.88%   +8.43%     
==========================================
  Files       10563    10563              
  Lines      608872   608874       +2     
  Branches    23913    23913              
==========================================
+ Hits       459394   510738   +51344     
+ Misses     149091    97749   -51342     
  Partials      387      387              

Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Looks good. Inflight tasks won't have compressed_profile set, but the default value maintains backwards compatibility, and in a future deploy we can disable task layer compression.

@enochtangg enochtangg merged commit 869baf4 into master Jul 16, 2025
67 checks passed
@enochtangg enochtangg deleted the make-process-profile-support-uncompressed branch July 16, 2025 19:11
enochtangg added a commit that referenced this pull request Jul 17, 2025
The process_profiles worker code now supports uncompressed parameter
payloads (#95692). This PR is
responsible for removing double compression by changing the
process_profile call site to send uncompressed tasks to taskworker as
taskworker already handles zstd compression in its platform.
cursor bot pushed a commit that referenced this pull request Jul 18, 2025
…yloads (#95692)

The taskworker producer layer now supports zstd compression. This means
that the zlib task level compression is no longer necessary here. In
order to remove, this PR updates the worker code to take an optional
parameter which decides whether or not the payload should be zlib
decompressed. In a subsequent PR, the producer code will be updated to
send uncompressed payloads
cursor bot pushed a commit that referenced this pull request Jul 18, 2025
The process_profiles worker code now supports uncompressed parameter
payloads (#95692). This PR is
responsible for removing double compression by changing the
process_profile call site to send uncompressed tasks to taskworker as
taskworker already handles zstd compression in its platform.
andrewshie-sentry pushed a commit that referenced this pull request Jul 21, 2025
…yloads (#95692)

The taskworker producer layer now supports zstd compression. This means
that the zlib task level compression is no longer necessary here. In
order to remove, this PR updates the worker code to take an optional
parameter which decides whether or not the payload should be zlib
decompressed. In a subsequent PR, the producer code will be updated to
send uncompressed payloads
andrewshie-sentry pushed a commit that referenced this pull request Jul 21, 2025
The process_profiles worker code now supports uncompressed parameter
payloads (#95692). This PR is
responsible for removing double compression by changing the
process_profile call site to send uncompressed tasks to taskworker as
taskworker already handles zstd compression in its platform.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants