Skip to content

fix(issues): Route adjacent-event lookups to Events dataset#114474

Merged
yuvmen merged 1 commit intomasterfrom
yuvmen/eventstore-adjacent-events-use-events-dataset
May 1, 2026
Merged

fix(issues): Route adjacent-event lookups to Events dataset#114474
yuvmen merged 1 commit intomasterfrom
yuvmen/eventstore-adjacent-events-use-events-dataset

Conversation

@yuvmen
Copy link
Copy Markdown
Member

@yuvmen yuvmen commented Apr 30, 2026

Fixes SENTRY-5NPK.

Navigating prev/next on an issue events page crashed with QueryIllegalTypeOfArgument: Argument 0 for function arrayConcat must be an array but it has type String whenever the search query filtered an ARRAY_FIELDS column (!error.type:[A,B], stack.function:[X], etc.).

Why It Started 9 Days Ago

#113234 (Apr 17) wired the issue search query into prev/next event navigation, behind a sentry-org allowlist. #113555 (Apr 21 — the day this issue first fired) flipped that to a boolean option. #114063 (Apr 27) made it the default for everyone. Before this rollout, prev/next ignored the query, so the broken SQL was never generated. The dataset asymmetry below was always there but only mattered once filter conditions started flowing through.

Why Switching to Events Is the Right Call (Not as Big as It Reads)

get_adjacent_event_ids was the last event-fetch method on the issue events page still using Dataset.Discover. The methods that load the initial event on the very same page already use Dataset.Events for error groups:

# src/sentry/models/group.py:260 — get_oldest_or_latest_event / get_recommended_event
if group.issue_category == GroupCategory.ERROR:
    dataset = Dataset.Events
else:
    dataset = Dataset.IssuePlatform

So when a user opens an issue, the first event loads through Events, and then prev/next was sending the same group's same conditions to Discover. Both datasets read from the same underlying ClickHouse errors storage; the difference is entity-level column resolution. The Discover entity tag-promotes columns like exception_stacks.type to tags.value[indexOf(tags.key, ...)] (a scalar String), which makes the existing hasAny(arrayConcat(name), array(...)) shape produced by convert_search_filter_to_snuba_query blow up. The Events entity treats those columns as the native nested arrays they are, and the same SQL parses fine.

The original justification for Discover was a comment claiming it was needed "to enable getting events across both errors and transactions, which is required when doing pagination in discover." Adjacent-event lookups always run within a single group, which is bounded to one entity, so the cross-entity Discover view isn't needed here. The stale comment is removed.

The change is scoped: only the non-transaction, non-occurrence branch of _get_dataset_for_event flips. Transactions still go to Transactions, occurrences/generic still go to IssuePlatform. Only error events change, and they now match what the rest of the page already does.

On Test Coverage

No regression test in this PR. Local devservices Snuba's discover entity treats exception_stacks.type as a real nested array column, so the production failure mode (tag-promotion to a scalar) doesn't reproduce here. The existing endpoint tests around prev/next navigation still pass.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 30, 2026
@yuvmen yuvmen force-pushed the yuvmen/eventstore-adjacent-events-use-events-dataset branch from e799916 to 6e8a2fe Compare April 30, 2026 20:23
Fixes SENTRY-5NPK. See PR description for the explanation.
@yuvmen yuvmen force-pushed the yuvmen/eventstore-adjacent-events-use-events-dataset branch from 6e8a2fe to 5c11df9 Compare April 30, 2026 20:55
@yuvmen yuvmen marked this pull request as ready for review April 30, 2026 21:48
@yuvmen yuvmen requested a review from a team as a code owner April 30, 2026 21:48
@yuvmen yuvmen merged commit 5d4fce4 into master May 1, 2026
68 checks passed
@yuvmen yuvmen deleted the yuvmen/eventstore-adjacent-events-use-events-dataset branch May 1, 2026 17:32
cleptric pushed a commit that referenced this pull request May 5, 2026
Fixes [SENTRY-5NPK](https://sentry.sentry.io/issues/SENTRY-5NPK).

Navigating prev/next on an issue events page crashed with
`QueryIllegalTypeOfArgument: Argument 0 for function arrayConcat must be
an array but it has type String` whenever the search query filtered an
`ARRAY_FIELDS` column (`!error.type:[A,B]`, `stack.function:[X]`, etc.).

**Why It Started 9 Days Ago**

#113234 (Apr 17) wired the issue search query into prev/next event
navigation, behind a sentry-org allowlist. #113555 (Apr 21 — the day
this issue first fired) flipped that to a boolean option. #114063 (Apr
27) made it the default for everyone. Before this rollout, prev/next
ignored the query, so the broken SQL was never generated. The dataset
asymmetry below was always there but only mattered once filter
conditions started flowing through.

**Why Switching to `Events` Is the Right Call (Not as Big as It Reads)**

`get_adjacent_event_ids` was the last event-fetch method on the issue
events page still using `Dataset.Discover`. The methods that load the
*initial* event on the very same page already use `Dataset.Events` for
error groups:

```python
# src/sentry/models/group.py:260 — get_oldest_or_latest_event / get_recommended_event
if group.issue_category == GroupCategory.ERROR:
    dataset = Dataset.Events
else:
    dataset = Dataset.IssuePlatform
```

So when a user opens an issue, the first event loads through `Events`,
and then prev/next was sending the same group's same conditions to
`Discover`. Both datasets read from the same underlying ClickHouse
`errors` storage; the difference is entity-level column resolution. The
`Discover` entity tag-promotes columns like `exception_stacks.type` to
`tags.value[indexOf(tags.key, ...)]` (a scalar `String`), which makes
the existing `hasAny(arrayConcat(name), array(...))` shape produced by
`convert_search_filter_to_snuba_query` blow up. The `Events` entity
treats those columns as the native nested arrays they are, and the same
SQL parses fine.

The original justification for `Discover` was a comment claiming it was
needed "to enable getting events across both errors and transactions,
which is required when doing pagination in discover." Adjacent-event
lookups always run within a single group, which is bounded to one
entity, so the cross-entity Discover view isn't needed here. The stale
comment is removed.

The change is scoped: only the non-transaction, non-occurrence branch of
`_get_dataset_for_event` flips. Transactions still go to `Transactions`,
occurrences/generic still go to `IssuePlatform`. Only error events
change, and they now match what the rest of the page already does.

**On Test Coverage**

No regression test in this PR. Local devservices Snuba's discover entity
treats `exception_stacks.type` as a real nested array column, so the
production failure mode (tag-promotion to a scalar) doesn't reproduce
here. The existing endpoint tests around prev/next navigation still
pass.
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