Skip to content

fix(search): map bare duration to transaction.duration in search parser#114473

Merged
BYK merged 1 commit intomasterfrom
byk/fix-bare-duration-search-500
Apr 30, 2026
Merged

fix(search): map bare duration to transaction.duration in search parser#114473
BYK merged 1 commit intomasterfrom
byk/fix-bare-duration-search-500

Conversation

@BYK
Copy link
Copy Markdown
Member

@BYK BYK commented Apr 30, 2026

Summary

  • Adds "duration" to SKIP_FILTER_RESOLUTION in constants.py so bare duration in search filters resolves to tags[duration] instead of the internal ClickHouse column
  • Fixes a 500 Internal Server Error when users write duration:>3s instead of transaction.duration:>3s

Root Cause

Bare duration is an internal Snuba column name (UInt32, milliseconds) for transaction.duration. It is NOT in duration_keys or numeric_keys, so the search parser correctly treats it as a text filter — but resolve_column then resolves it to the raw ClickHouse column via DATASET_FIELDS, creating a string-vs-numeric mismatch that crashes Snuba with a 500.

When a user writes duration:>3s:

  1. The PEG grammar matches it as a duration_filter node
  2. is_duration_key("duration") returns False → falls through to text filter handling
  3. The value becomes the string ">3s" (operator + value concatenated)
  4. resolve_column("duration") finds it in DATASET_FIELDS[Transactions] as the raw UInt32 column
  5. This produces WHERE duration = '>3s' — string vs UInt32 mismatch → Snuba 500

Fix

Add "duration" to SKIP_FILTER_RESOLUTION (alongside the existing total_count and total_transaction_duration entries). This forces default_filter_converter to resolve it as tags[duration] instead of the internal column — harmless (no results from a nonexistent tag) instead of a 500.

@BYK BYK requested a review from a team as a code owner April 30, 2026 18:54
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 30, 2026
@BYK BYK requested a review from JoshFerge April 30, 2026 18:55
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.

I don't think this is the right fix, can you link me the 500? I can take a look. But duration alone like this should be treated as the tag duration rather than the transaction.duration

Copy link
Copy Markdown
Member Author

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Thanks for the review @wmak! You're right — duration should be treated as a tag, not remapped.

Here's the 500: https://sentry.sentry.io/issues/7452286129/

The root cause trace:

  1. duration:>3s hits the PEG grammar as a duration_filter node
  2. is_duration_key("duration")False (only transaction.duration is in duration_keys)
  3. Falls through to text filter: SearchFilter("duration", "=", ">3s")
  4. In default_filter_converter, self.resolve_column("duration") at base.py:1319 resolves to the raw ClickHouse UInt32 column (via DATASET_FIELDS[Transactions] match at snuba.py:1640) instead of tags[duration]
  5. This produces WHERE duration = '>3s' — type mismatch → SchemaValidationError → 500

So the real bug is in step 4: resolve_column("duration") should NOT resolve to the internal column when called from the search filter path. The DATASET_FIELDS check at snuba.py:1639-1642 is too eager — it exposes internal column names to user queries.

I'll revert the key_mappings approach and look at the correct fix. Should this be addressed in resolve_column (preventing internal-only names from being exposed), or somewhere in the builder's filter resolution path?

Bare `duration` is an internal Snuba column name (UInt32) for
`transaction.duration`.  When users write `duration:>3s`, the
search parser correctly treats it as a text filter (since
`duration` is not in `duration_keys`), but `resolve_column`
resolves it to the raw ClickHouse column via `DATASET_FIELDS`,
producing a string-vs-numeric mismatch that crashes Snuba
with a 500.

Add `duration` to `SKIP_FILTER_RESOLUTION` so it resolves to
`tags[duration]` in filter context — harmless (no results)
instead of a 500.

Reverts the `key_mappings` approach from the previous commit
per reviewer feedback: bare `duration` should be a tag lookup,
not remapped to `transaction.duration`.
@BYK BYK force-pushed the byk/fix-bare-duration-search-500 branch from 494f1f6 to 14d48fd Compare April 30, 2026 19:05
@BYK BYK requested a review from wmak April 30, 2026 19:11
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 1 potential issue.

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 14d48fd. Configure here.

Comment thread src/sentry/search/events/constants.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.

lgtm

@BYK BYK merged commit dcd1dbb into master Apr 30, 2026
64 checks passed
@BYK BYK deleted the byk/fix-bare-duration-search-500 branch April 30, 2026 20:01
cleptric pushed a commit that referenced this pull request May 5, 2026
…parser (#114473)

## Summary

- Adds `"duration"` to `SKIP_FILTER_RESOLUTION` in `constants.py` so
bare `duration` in search filters resolves to `tags[duration]` instead
of the internal ClickHouse column
- Fixes a 500 Internal Server Error when users write `duration:>3s`
instead of `transaction.duration:>3s`

## Root Cause

Bare `duration` is an internal Snuba column name (UInt32, milliseconds)
for `transaction.duration`. It is NOT in `duration_keys` or
`numeric_keys`, so the search parser correctly treats it as a text
filter — but `resolve_column` then resolves it to the raw ClickHouse
column via `DATASET_FIELDS`, creating a string-vs-numeric mismatch that
crashes Snuba with a 500.

When a user writes `duration:>3s`:
1. The PEG grammar matches it as a `duration_filter` node
2. `is_duration_key("duration")` returns `False` → falls through to text
filter handling
3. The value becomes the string `">3s"` (operator + value concatenated)
4. `resolve_column("duration")` finds it in
`DATASET_FIELDS[Transactions]` as the raw UInt32 column
5. This produces `WHERE duration = '>3s'` — string vs UInt32 mismatch →
Snuba 500

## Fix

Add `"duration"` to `SKIP_FILTER_RESOLUTION` (alongside the existing
`total_count` and `total_transaction_duration` entries). This forces
`default_filter_converter` to resolve it as `tags[duration]` instead of
the internal column — harmless (no results from a nonexistent tag)
instead of a 500.
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