Skip to content

tele(taskbroker): check zlib compression viability #91693

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.

Sign up for GitHub

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
May 15, 2025
Merged

tele(taskbroker): check zlib compression viability #91693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/sentry/options/defaults.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3107,6 +3107,12 @@
)

# Taskbroker flags
register(
"taskworker.try_compress.profile_metrics",
default=0.0,
type=Float,
flags=FLAG_AUTOMATOR_MODIFIABLE,
)

register(
"taskworker.route.overrides",
Expand Down
23 changes: 20 additions & 3 deletions src/sentry/profiles/consumers/process/factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import random
from base64 import b64encode
from collections.abc import Iterable, Mapping

Expand All @@ -10,15 +11,31 @@
from sentry import options
from sentry.processing.backpressure.arroyo import HealthChecker, create_backpressure_step
from sentry.profiles.task import process_profile_task
from sentry.utils import metrics


def process_message(message: Message[KafkaPayload]) -> None:
sampled = is_sampled(message.payload.headers)

if sampled or options.get("profiling.profile_metrics.unsampled_profiles.enabled"):
process_profile_task.delay(
payload=b64encode(message.payload.value).decode("utf-8"), sampled=sampled
)
b64encoded = b64encode(message.payload.value).decode("utf-8")
process_profile_task.delay(payload=b64encoded, sampled=sampled)
metrics.distribution("profiling.profile_metrics.uncompressed_bytes", len(b64encoded))

if random.random() < options.get("taskworker.try_compress.profile_metrics"):
import time
import zlib

start_time = time.perf_counter()
metrics.distribution(
"profiling.profile_metrics.compressed_bytes",
len(b64encode(zlib.compress(message.payload.value))),
Copy link
Member

Choose a reason for hiding this comment

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

So zipping a base64 encoded msgpack payload 🤔 Wouldn't zlib.compress() return bytes which also need to be b64 encoded, as not all bytes can be json encoded.

Should we be zipping the msgpack, and then b64 encoding the zip? That might entirely defeat the benefits of zip though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I did not know that message.payload.value is a msgpack payload. Why do we b64 encode it?

Ideally we should zip the bytes and then b64 encode the zipped values.

Copy link
Member

Choose a reason for hiding this comment

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

We currently have to b64 encode the msgpack, as not all bytes can be json encoded 😢

Copy link
Member

Choose a reason for hiding this comment

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

I think I got myself mixed up here 😵 . I see now that you're doing b64(zip(msgpack)) which is as good as we can get for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

all good

)
end_time = time.perf_counter()
metrics.distribution(
"profiling.profile_metrics.compression_time",
end_time - start_time,
)


class ProcessProfileStrategyFactory(ProcessingStrategyFactory[KafkaPayload]):
Expand Down
4 changes: 4 additions & 0 deletions tests/sentry/processing/backpressure/test_checking.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"backpressure.checking.interval": 5,
"backpressure.monitoring.enabled": True,
"backpressure.status_ttl": 60,
"taskworker.try_compress.profile_metrics": False,
}
)
def test_backpressure_unhealthy_profiles():
Expand All @@ -53,6 +54,7 @@ def test_backpressure_unhealthy_profiles():
"backpressure.checking.interval": 5,
"backpressure.monitoring.enabled": False,
"backpressure.status_ttl": 60,
"taskworker.try_compress.profile_metrics": False,
}
)
def test_bad_config():
Expand All @@ -67,6 +69,7 @@ def test_bad_config():
"backpressure.checking.interval": 5,
"backpressure.monitoring.enabled": True,
"backpressure.status_ttl": 60,
"taskworker.try_compress.profile_metrics": False,
}
)
def test_backpressure_healthy_profiles(process_profile_task):
Expand Down Expand Up @@ -138,6 +141,7 @@ def test_backpressure_healthy_events(preprocess_event):
{
"backpressure.checking.enabled": False,
"backpressure.checking.interval": 5,
"taskworker.try_compress.profile_metrics": False,
}
)
def test_backpressure_not_enabled(process_profile_task):
Expand Down
Loading