Skip to content

feat(ourlogs): add truncate RPC parameter for logs events query#116008

Open
JoshuaKGoldberg wants to merge 7 commits into
masterfrom
logs-query-truncate-backend
Open

feat(ourlogs): add truncate RPC parameter for logs events query#116008
JoshuaKGoldberg wants to merge 7 commits into
masterfrom
logs-query-truncate-backend

Conversation

@JoshuaKGoldberg
Copy link
Copy Markdown
Member

@JoshuaKGoldberg JoshuaKGoldberg commented May 21, 2026

Adds a truncate query parameter to the organization events endpoint for the OurLogs dataset. When provided, string column values are truncated at the RPC layer to the specified length. This can reduce response payload size nicely when there are huge log fields, such as -but not limited to- message.

Split out of #115638. #116009 has the corresponding frontend changes.

Closes LOGS-816.

Adds a `truncate` query parameter to the organization events endpoint
that truncates string column values at the RPC layer for the OurLogs
dataset, reducing payload size for wide viewports.
@JoshuaKGoldberg JoshuaKGoldberg requested review from a team as code owners May 21, 2026 14:46
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 21, 2026

LOGS-816

"organizations:dynamic-sampling",
"organizations:on-demand-metrics-extraction",
"organizations:on-demand-metrics-extraction-widgets",
"organizations:on-demand-metrics-extraction-experimental",
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🤔 some kind of merge oddity, will remove

Comment thread src/sentry/snuba/rpc_dataset_common.py Outdated
@JoshuaKGoldberg JoshuaKGoldberg changed the title feat(ourlogs): add truncate RPC parameter for logs events query feat(ourlogs): add truncate RPC parameter for logs events query May 21, 2026
result_value = process_value(result_value)

# Note: post-query truncation may not be our preferred method long-term.
# We may want to set up a function that filters/truncates at the EAP side.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For my own learning, why is this not the preferred approach for this PR, time/scope?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Please enjoy this 84-reply thread 😄 https://sentry.slack.com/archives/C08CR80T3RB/p1778786746769669

(Kevan and Will can speak to this better than me)

truncate_str = request.GET.get("truncate")
if truncate_str is not None:
try:
max_string_length = int(truncate_str)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ahh ok, this one does cast int here, so at least backend side is protected, sweet

Copy link
Copy Markdown
Contributor

@adrianviquez adrianviquez left a comment

Choose a reason for hiding this comment

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

small q, otherwise 👍

Comment thread tests/snuba/api/endpoints/test_organization_events_ourlogs.py
Copy link
Copy Markdown
Contributor

@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.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 215fa68. Configure here.

Comment thread tests/snuba/api/endpoints/test_organization_events_ourlogs.py Outdated
Comment thread src/sentry/snuba/rpc_dataset_common.py
Copy link
Copy Markdown
Member

@wmak wmak left a comment

Choose a reason for hiding this comment

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

Small comments but overall lgtm

additional_queries=additional_queries,
**(
{"max_string_length": max_string_length}
if scoped_dataset == OurLogs
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we either error at the API level when this is passed on non logs datasets, or just enable it everywhere?

sampling_mode=snuba_params.sampling_mode,
page_token=page_token,
additional_queries=additional_queries,
**(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you just pass this directly to run_table_query instead of a kwargs or put it in the config?

assert response.status_code == 400
assert response.data["detail"] == "Invalid per_page value. Must be between 1 and 9999."

@pytest.mark.querybuilder
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this test needs to be part of the smoke suite

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.

3 participants