Repository navigation
fix(cli): Sort every Events dataset through a shared resolver - #1521
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
I think what we did was to actually fix the count properly instead of warning and we should fix this everywhere using a centralized mechanism. Research that and redo this PR accordingly. |
All Events datasets accept sort as long as the sort column is selected. The 400 that made logs, metrics, and errors dashboard widgets look unsortable was "sort column not selected". Move sort resolution into @sentry/toolkit-core/events-sort and use it from explore, dashboard widgets, and MCP search-events, so grouped queries always send a stable sort and --limit returns a real top N. Fixes #1477
4ea8a39 to
5da7743
Compare
Port parseFunction and the aggregate/equation helpers from the Sentry frontend's Discover fields module into @sentry/toolkit-core/events-fields, and use them for sort resolution and explore's aggregate checks instead of a parentheses heuristic.
|
Perfect. Now let's refactor any other code that might benefit from this new parser. Also check all Sentry issues in the CLI project for API 400 errors that might get fixed by this and list them here for automatic resolution. |
CodeQL flagged the frontend's /^(\w+)\((.*)\)$/ and /(\w+)\((.*)\)/g patterns as polynomial on adversarial input. Do the same matching with linear scans, check parity against the frontend patterns on every short field, and add getColumnsAndAggregates and isTracemetricsAggregate.
Replace parenthesis heuristics with parseFunction, isAggregateFieldOrEquation, and getColumnsAndAggregates in explore, dashboard widget validation, MCP search-events, and the MCP explorer URL builders. The numeric aggregate field check now reads the first argument, so percentile(span.duration,0.95) no longer warns.
|
Refactor (90b91d9). These callers now use the shared parser instead of checking for parentheses:
aa0cb43 replaces the frontend regexes with linear scans to fix the CodeQL polynomial-regex alert. A test checks that they give the same answers as the frontend regexes on every field up to 5 characters, including newlines. Tests pass: toolkit-core 73, CLI explore/dashboard 411, MCP 1822. Sentry
If you want these errors to group on their own, the fix would be to fingerprint explore |
Fixes #1477
Sentry accepts
sorton every Events dataset. It only rejects a sort whose column isn't selected: RPC datasets return "orderby must also be in the selected columns or groupby", and the errorsevents-statstop-events path returns "Cannot sort by a field that is not selected." That 400 is why the CLI decided logs, metrics, and errors dashboard widgets were unsortable, and why it either dropped the sort or warned about it. Without a sort, grouped results come back in no particular order, so--limitreturns an arbitrary subset, and offset pagination duplicates and skips rows (getsentry/cli#1519).This follows the getsentry/cli#1520 approach of making sort work instead of warning, and does it in one shared place:
@sentry/toolkit-core/events-sort.resolveEventsSortkeeps an explicit sort, otherwise defaults grouped queries to their first aggregate descending, and adds the sort column tofieldwhen it's missing. It never adds a non-aggregate column to an aggregate query, because that would change the GROUP BY. The callers:sentry explore: sorts every dataset, including logs and metrics. The hand-curatedSORTABLE_DATASETSlist and the ignore-sort warning are removed, which also takes care of explore: canonical source for sortable dataset list (errors/discover) cli#1523.search_events: its privateaugmentFieldsWithSortis replaced by the sharedensureSortInFields. The MCP behavior doesn't change.I checked against the
sentryorg:sort=-count()returns rows in descending order on errors, spans, and logs, and on tracemetrics with a matching aggregate. Each dataset 400s only when the sort column isn't selected. On errors,events-statstop events works once the aggregate is added tofield.CLI
log list,span list, andtrace listkeep their fixed sorts, which their fixed field lists already select.via Junior system actor
event.--
View Junior Session [Sentry]