From 5748f73b37b4a66f36579a60472e7a1a27c47fd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Fri, 15 May 2026 11:41:16 -0400 Subject: [PATCH] feat(ourlogs): add truncate RPC parameter for logs events query --- .../api/endpoints/organization_events.py | 20 +++++ src/sentry/snuba/ourlogs.py | 2 + src/sentry/snuba/rpc_dataset_common.py | 11 ++- static/app/actionCreators/events.tsx | 1 + .../explore/logs/tables/logsTableRow.tsx | 24 ++++-- .../app/views/explore/logs/useLogsQuery.tsx | 3 + .../logs/useLogsQueryTruncate.spec.tsx | 23 ++++++ .../explore/logs/useLogsQueryTruncate.tsx | 6 ++ tests/sentry/snuba/test_rpc_dataset_common.py | 74 +++++++++++++++++++ .../test_organization_events_ourlogs.py | 42 +++++++++++ 10 files changed, 197 insertions(+), 9 deletions(-) create mode 100644 static/app/views/explore/logs/useLogsQueryTruncate.spec.tsx create mode 100644 static/app/views/explore/logs/useLogsQueryTruncate.tsx diff --git a/src/sentry/api/endpoints/organization_events.py b/src/sentry/api/endpoints/organization_events.py index 6c8869e27e75..6ef021678541 100644 --- a/src/sentry/api/endpoints/organization_events.py +++ b/src/sentry/api/endpoints/organization_events.py @@ -238,6 +238,16 @@ def get(self, request: Request, organization: Organization) -> Response: use_aggregate_conditions = request.GET.get("allowAggregateConditions", "1") == "1" + max_string_length: int | None = None + truncate_str = request.GET.get("truncate") + if truncate_str is not None: + try: + max_string_length = int(truncate_str) + if max_string_length < 1: + raise ValueError + except ValueError: + return Response({"detail": "truncate must be a positive integer"}, status=400) + def _data_fn( dataset_query: DatasetQuery, offset: int, @@ -601,6 +611,11 @@ def flex_time_data_fn(limit, page_token): sampling_mode=snuba_params.sampling_mode, page_token=page_token, additional_queries=additional_queries, + **( + {"max_string_length": max_string_length} + if scoped_dataset == OurLogs + else {} + ), ) return EAPPageTokenPaginator(data_fn=flex_time_data_fn), EAPPageTokenCursor @@ -621,6 +636,11 @@ def data_fn(offset, limit): config=config, sampling_mode=snuba_params.sampling_mode, additional_queries=additional_queries, + **( + {"max_string_length": max_string_length} + if scoped_dataset == OurLogs + else {} + ), ) if save_discover_dataset_decision and discover_saved_query_id: diff --git a/src/sentry/snuba/ourlogs.py b/src/sentry/snuba/ourlogs.py index 1707e73b03d7..287b7dff66a5 100644 --- a/src/sentry/snuba/ourlogs.py +++ b/src/sentry/snuba/ourlogs.py @@ -40,6 +40,7 @@ def run_table_query( search_resolver: SearchResolver | None = None, page_token: PageToken | None = None, additional_queries: AdditionalQueries | None = None, + max_string_length: int | None = None, ) -> EAPResponse: """timestamp_precise is always displayed in the UI in lieu of timestamp but since the TraceItem table isn't a DateTime64 so we need to always order by it regardless of what is actually passed to the orderby. @@ -78,6 +79,7 @@ def run_table_query( ), page_token=page_token, additional_queries=additional_queries, + max_string_length=max_string_length, ), debug=params.debug, ) diff --git a/src/sentry/snuba/rpc_dataset_common.py b/src/sentry/snuba/rpc_dataset_common.py index 37bcdc4dade7..ac99fd05217f 100644 --- a/src/sentry/snuba/rpc_dataset_common.py +++ b/src/sentry/snuba/rpc_dataset_common.py @@ -91,6 +91,7 @@ class TableQuery: page_token: PageToken | None = None additional_queries: AdditionalQueries | None = None extra_conditions: TraceItemFilter | None = None + max_string_length: int | None = None @dataclass @@ -238,10 +239,13 @@ def filter_project(cls, project: Project) -> bool: @classmethod def build_rpc_table_row_context(cls, query: TableQuery) -> dict[str, Any]: - return { + ctx: dict[str, Any] = { "project_ids": list(query.resolver.params.project_ids), "organization_id": query.resolver.params.organization_id, } + if query.max_string_length is not None: + ctx["max_string_length"] = query.max_string_length + return ctx @classmethod def get_table_rpc_request(cls, query: TableQuery) -> TableRequest: @@ -461,8 +465,9 @@ def process_column_values( final_data: SnubaData, attribute: Any, resolved_column: ResolvedColumn, - **_context_kwargs: Any, + **context_kwargs: Any, ) -> None: + max_string_length: int | None = context_kwargs.get("max_string_length") for index, result in enumerate(column_value.results): result_value: Any if result.is_null: @@ -470,6 +475,8 @@ def process_column_values( else: result_value = anyvalue_to_python(result) result_value = process_value(result_value) + if max_string_length is not None and isinstance(result_value, str): + result_value = result_value[:max_string_length] final_data[index][attribute] = resolved_column.process_column(result_value) @classmethod diff --git a/static/app/actionCreators/events.tsx b/static/app/actionCreators/events.tsx index e95ac1180698..8978acf1a682 100644 --- a/static/app/actionCreators/events.tsx +++ b/static/app/actionCreators/events.tsx @@ -165,6 +165,7 @@ export type EventQuery = { referrer?: string; sort?: string | string[]; team?: string | string[]; + truncate?: number; }; export type TagSegment = { diff --git a/static/app/views/explore/logs/tables/logsTableRow.tsx b/static/app/views/explore/logs/tables/logsTableRow.tsx index 522ade18a6ff..cdba5778ceb9 100644 --- a/static/app/views/explore/logs/tables/logsTableRow.tsx +++ b/static/app/views/explore/logs/tables/logsTableRow.tsx @@ -615,7 +615,10 @@ function LogRowDetails({ } const colSpan = fields.length + 1; // Number of dynamic fields + first cell which is always rendered. - const message = String(dataRow[OurLogKnownFieldKey.MESSAGE] ?? ''); + const fullMessage = String( + attributes[OurLogKnownFieldKey.MESSAGE] || + String(dataRow[OurLogKnownFieldKey.MESSAGE]) + ); return ( @@ -627,7 +630,10 @@ function LogRowDetails({ {isRegularLogResponseItem(dataRow) ? ( ) : ( - {message} + {fullMessage} )} @@ -695,7 +701,7 @@ function LogRowDetails({ ); } -function LogRowDetailsFilterActions({tableDataRow}: {tableDataRow: LogTableRowItem}) { +function LogRowDetailsFilterActions({fullMessage: fullMessage}: {fullMessage: string}) { const addSearchFilter = useAddSearchFilter(); return ( @@ -706,7 +712,7 @@ function LogRowDetailsFilterActions({tableDataRow}: {tableDataRow: LogTableRowIt onClick={() => { addSearchFilter({ key: OurLogKnownFieldKey.MESSAGE, - value: tableDataRow[OurLogKnownFieldKey.MESSAGE], + value: fullMessage, }); }} > @@ -719,7 +725,7 @@ function LogRowDetailsFilterActions({tableDataRow}: {tableDataRow: LogTableRowIt onClick={() => { addSearchFilter({ key: OurLogKnownFieldKey.MESSAGE, - value: tableDataRow[OurLogKnownFieldKey.MESSAGE], + value: fullMessage, negated: true, }); }} @@ -741,6 +747,10 @@ function LogRowDetailsActions({ const isFrozen = useLogsFrozenIsFrozen(); const organization = useOrganization(); const showFilterButtons = !isFrozen; + const fullMessage = String( + data?.attributes?.find(attr => attr.name === OurLogKnownFieldKey.MESSAGE)?.value || + tableDataRow[OurLogKnownFieldKey.MESSAGE] + ); const {copy} = useCopyToClipboard(); @@ -765,7 +775,7 @@ function LogRowDetailsActions({ return ( {showFilterButtons ? ( - + ) : ( )} diff --git a/static/app/views/explore/logs/useLogsQuery.tsx b/static/app/views/explore/logs/useLogsQuery.tsx index c077c4c0ed1a..4f7274037fde 100644 --- a/static/app/views/explore/logs/useLogsQuery.tsx +++ b/static/app/views/explore/logs/useLogsQuery.tsx @@ -43,6 +43,7 @@ import { OurLogKnownFieldKey, type EventsLogsResult, } from 'sentry/views/explore/logs/types'; +import {useLogsQueryTruncate} from 'sentry/views/explore/logs/useLogsQueryTruncate'; import { isRowVisibleInVirtualStream, useVirtualStreaming, @@ -98,6 +99,7 @@ function useLogsApiOptions({ const projectIds = useLogsFrozenProjectIds(); const groupBys = useQueryParamsGroupBys(); const [caseInsensitive] = useCaseInsensitivity(); + const truncate = useLogsQueryTruncate(); const search = baseSearch ? _search.copy() : _search; if (baseSearch) { @@ -139,6 +141,7 @@ function useLogsApiOptions({ referrer, sampling: highFidelity ? SAMPLING_MODE.FLEX_TIME : SAMPLING_MODE.NORMAL, caseInsensitive: caseInsensitive ? '1' : undefined, + truncate, }; const path = {organizationIdOrSlug: organization.slug}; diff --git a/static/app/views/explore/logs/useLogsQueryTruncate.spec.tsx b/static/app/views/explore/logs/useLogsQueryTruncate.spec.tsx new file mode 100644 index 000000000000..c4e01a23147c --- /dev/null +++ b/static/app/views/explore/logs/useLogsQueryTruncate.spec.tsx @@ -0,0 +1,23 @@ +import {renderHookWithProviders} from 'sentry-test/reactTestingLibrary'; + +import {useWindowSize} from 'sentry/utils/window/useWindowSize'; + +import {useLogsQueryTruncate} from './useLogsQueryTruncate'; + +jest.mock('sentry/utils/window/useWindowSize'); + +const mockUseWindowSize = jest.mocked(useWindowSize); + +describe('useLogsQueryTruncate', () => { + it('returns 256 for narrow viewports where the formula yields less', () => { + mockUseWindowSize.mockReturnValue({innerWidth: 800, innerHeight: 600}); + const {result} = renderHookWithProviders(() => useLogsQueryTruncate()); + expect(result.current).toBe(256); + }); + + it('returns the formula result for wide viewports where it exceeds 256', () => { + mockUseWindowSize.mockReturnValue({innerWidth: 6400, innerHeight: 1080}); + const {result} = renderHookWithProviders(() => useLogsQueryTruncate()); + expect(result.current).toBe(400); + }); +}); diff --git a/static/app/views/explore/logs/useLogsQueryTruncate.tsx b/static/app/views/explore/logs/useLogsQueryTruncate.tsx new file mode 100644 index 000000000000..68c032e9c41e --- /dev/null +++ b/static/app/views/explore/logs/useLogsQueryTruncate.tsx @@ -0,0 +1,6 @@ +import {useWindowSize} from 'sentry/utils/window/useWindowSize'; + +export function useLogsQueryTruncate(): number { + const {innerWidth} = useWindowSize(); + return Math.max(256, innerWidth / 16); +} diff --git a/tests/sentry/snuba/test_rpc_dataset_common.py b/tests/sentry/snuba/test_rpc_dataset_common.py index b9586755bc26..467f3ae7bef5 100644 --- a/tests/sentry/snuba/test_rpc_dataset_common.py +++ b/tests/sentry/snuba/test_rpc_dataset_common.py @@ -1,7 +1,9 @@ from datetime import datetime, timedelta, timezone +from unittest.mock import MagicMock import pytest from sentry_protos.snuba.v1.downsampled_storage_pb2 import DownsampledStorageConfig +from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeValue from sentry.search.eap.types import SearchResolverConfig from sentry.search.events.types import SnubaParams @@ -14,6 +16,78 @@ from sentry.testutils.pytest.fixtures import django_db_all +def _make_column_value(string_values: list[str]) -> MagicMock: + column_value = MagicMock() + results = [] + for val in string_values: + av = AttributeValue() + av.val_str = val + results.append(av) + column_value.results = results + return column_value + + +def _identity_column() -> MagicMock: + resolved_column = MagicMock() + resolved_column.process_column = lambda v: v + return resolved_column + + +class TestProcessColumnValuesTruncation(TestCase): + def test_truncates_long_strings(self) -> None: + long_str = "x" * 100 + final_data: list[dict] = [{}] + RPCBase.process_column_values( + _make_column_value([long_str]), + final_data, + "attr", + _identity_column(), + max_string_length=10, + ) + assert final_data[0]["attr"] == "x" * 10 + + def test_no_truncation_without_param(self) -> None: + long_str = "x" * 100 + final_data: list[dict] = [{}] + RPCBase.process_column_values( + _make_column_value([long_str]), + final_data, + "attr", + _identity_column(), + ) + assert final_data[0]["attr"] == long_str + + def test_does_not_truncate_non_string_values(self) -> None: + av = AttributeValue() + av.val_int = 42 + column_value = MagicMock() + column_value.results = [av] + final_data: list[dict] = [{}] + RPCBase.process_column_values( + column_value, + final_data, + "attr", + _identity_column(), + max_string_length=1, + ) + assert final_data[0]["attr"] == 42 + + def test_null_values_are_unchanged(self) -> None: + av = AttributeValue() + av.is_null = True + column_value = MagicMock() + column_value.results = [av] + final_data: list[dict] = [{}] + RPCBase.process_column_values( + column_value, + final_data, + "attr", + _identity_column(), + max_string_length=1, + ) + assert final_data[0]["attr"] is None + + class TestBulkTableQueries(TestCase): def setUp(self) -> None: super().setUp() diff --git a/tests/snuba/api/endpoints/test_organization_events_ourlogs.py b/tests/snuba/api/endpoints/test_organization_events_ourlogs.py index 2411162f46b7..5063afdfd348 100644 --- a/tests/snuba/api/endpoints/test_organization_events_ourlogs.py +++ b/tests/snuba/api/endpoints/test_organization_events_ourlogs.py @@ -365,6 +365,48 @@ def test_pagelimit(self) -> None: assert response.status_code == 400 assert response.data["detail"] == "Invalid per_page value. Must be between 1 and 9999." + @pytest.mark.querybuilder + def test_truncate_param(self) -> None: + log = self.create_ourlog( + {"body": "hello world"}, + timestamp=self.ten_mins_ago, + ) + self.store_eap_items([log]) + response = self.do_request( + { + "field": ["log.body"], + "project": self.project.id, + "dataset": self.dataset, + "truncate": 5, + } + ) + assert response.status_code == 200, response.content + assert response.data["data"][0]["log.body"] == "hello" + + def test_truncate_param_invalid_type(self) -> None: + response = self.do_request( + { + "field": ["log.body"], + "project": self.project.id, + "dataset": self.dataset, + "truncate": "notanumber", + } + ) + assert response.status_code == 400 + assert response.data["detail"] == "truncate must be a positive integer" + + def test_truncate_param_invalid_value(self) -> None: + response = self.do_request( + { + "field": ["log.body"], + "project": self.project.id, + "dataset": self.dataset, + "truncate": 0, + } + ) + assert response.status_code == 400 + assert response.data["detail"] == "truncate must be a positive integer" + def test_homepage_query(self) -> None: """This query matches the one made on the logs homepage so that we can be sure everything is working at least for the initial load"""