Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 20 additions & 0 deletions src/sentry/api/endpoints/organization_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions src/sentry/snuba/ourlogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
)
11 changes: 9 additions & 2 deletions src/sentry/snuba/rpc_dataset_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -461,15 +465,18 @@ 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:
result_value = None
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]
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.

[Todo] We'd probably want to add a ... or something. Left out to keep this prototype simpler.

final_data[index][attribute] = resolved_column.process_column(result_value)

@classmethod
Expand Down
1 change: 1 addition & 0 deletions static/app/actionCreators/events.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ export type EventQuery = {
referrer?: string;
sort?: string | string[];
team?: string | string[];
truncate?: number;
};

export type TagSegment = {
Expand Down
24 changes: 17 additions & 7 deletions static/app/views/explore/logs/tables/logsTableRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<DetailsWrapper ref={isPending ? undefined : ref}>
Expand All @@ -627,7 +630,10 @@ function LogRowDetails({
<DetailsBody>
{isRegularLogResponseItem(dataRow) ? (
<LogBodyRenderer
item={getLogRowItem(OurLogKnownFieldKey.MESSAGE, dataRow, meta)}
item={{
...getLogRowItem(OurLogKnownFieldKey.MESSAGE, dataRow, meta),
value: fullMessage,
}}
extra={{
highlightTerms,
logColors,
Expand All @@ -644,7 +650,7 @@ function LogRowDetails({
}}
/>
) : (
<span>{message}</span>
<span>{fullMessage}</span>
)}
</DetailsBody>
<LogAttributeTreeWrapper>
Expand Down Expand Up @@ -695,7 +701,7 @@ function LogRowDetails({
);
}

function LogRowDetailsFilterActions({tableDataRow}: {tableDataRow: LogTableRowItem}) {
function LogRowDetailsFilterActions({fullMessage: fullMessage}: {fullMessage: string}) {
const addSearchFilter = useAddSearchFilter();
return (
<LogDetailTableActionsButtonBar>
Expand All @@ -706,7 +712,7 @@ function LogRowDetailsFilterActions({tableDataRow}: {tableDataRow: LogTableRowIt
onClick={() => {
addSearchFilter({
key: OurLogKnownFieldKey.MESSAGE,
value: tableDataRow[OurLogKnownFieldKey.MESSAGE],
value: fullMessage,
});
}}
>
Expand All @@ -719,7 +725,7 @@ function LogRowDetailsFilterActions({tableDataRow}: {tableDataRow: LogTableRowIt
onClick={() => {
addSearchFilter({
key: OurLogKnownFieldKey.MESSAGE,
value: tableDataRow[OurLogKnownFieldKey.MESSAGE],
value: fullMessage,
negated: true,
});
}}
Expand All @@ -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();

Expand All @@ -765,7 +775,7 @@ function LogRowDetailsActions({
return (
<Fragment>
{showFilterButtons ? (
<LogRowDetailsFilterActions tableDataRow={tableDataRow} />
<LogRowDetailsFilterActions fullMessage={fullMessage} />
) : (
<span />
)}
Expand Down
3 changes: 3 additions & 0 deletions static/app/views/explore/logs/useLogsQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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};
Expand Down
23 changes: 23 additions & 0 deletions static/app/views/explore/logs/useLogsQueryTruncate.spec.tsx
Original file line number Diff line number Diff line change
@@ -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);
});
});
6 changes: 6 additions & 0 deletions static/app/views/explore/logs/useLogsQueryTruncate.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import {useWindowSize} from 'sentry/utils/window/useWindowSize';

export function useLogsQueryTruncate(): number {
const {innerWidth} = useWindowSize();
return Math.max(256, innerWidth / 16);
}
74 changes: 74 additions & 0 deletions tests/sentry/snuba/test_rpc_dataset_common.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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()
Expand Down
42 changes: 42 additions & 0 deletions tests/snuba/api/endpoints/test_organization_events_ourlogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down
Loading