diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 8d8ed44e1..4a23eba7e 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -420,7 +420,7 @@ takes: | Comma-separated fixed values (`--scope`, `--features`) | `choiceListParser(values)` (or `(values, parse)`) | the next item of the list, leaving out the items already in it (`org:read,org:w` → `org:read,org:write`) | | Platform | `parsePlatform` | platform identifiers, common ones first | | Something that already exists in Sentry, as a flag value (`--environment`, `--release`, `--team`, `--owner`, `--integration`, `--app`, `--dsn`, `--workflow`, `--project`, `issue tags `) | `parseEnvironment`, `parseEnvironmentList`, `parseRelease`, `parseTeam`, `parseOwner` (`team:`), `parseIntegration`, `parseSentryApp`, `parseAppLinkField` (`name=value`), `parseDsn`, `parseWorkflowList`, `parseProjectSlug`, `parseProjectList`, `parseTagKey` | cached entities of the org the command's target names, else of every org, each once; the `*List` parsers complete the last item of a comma-separated list | -| Field or aggregate of a dataset (`explore --field`/`--sort`, `dashboard widget --group-by`/`--query`/`--sort`, `alert metrics --aggregate`) | `fieldParser`, `fieldOrAggregateParser`, `aggregateParser` (`fn(field)`), or `aggregateShorthandParser` (`fn:field`), given a `DatasetCompletion`: what completes on each dataset (start from `DATASET_FIELDS`), the command's `--dataset` aliases, and its default dataset | the fields of the dataset the `--dataset` typed names (else the default; every dataset when there is none): cached attribute keys of spans, logs, and trace metrics, cached issue tag keys for errors, or a fixed set (replays); then its aggregate functions, from `aggregateFunctionsOf` in `types/dashboard.ts`, as `fn()`, and `fn(field)` once a name is typed in full (`p95` → `p95(span.duration)`). A word starting with `-` completes flag names, so a descending sort's `-` is typed after completing | +| Field or aggregate of a dataset (`explore --field`/`--sort`, `dashboard widget --group-by`/`--query`/`--sort`, `alert metrics --aggregate`) | `fieldParser`, `fieldOrAggregateParser`, `aggregateParser` (`fn(field)`), or `aggregateShorthandParser` (`fn:field`), given a `DatasetCompletion`: what completes on each dataset (start from `DATASET_FIELDS`), the command's `--dataset` aliases, and its default dataset | the fields of the dataset the `--dataset` typed names (else the default; every dataset when there is none): cached attribute keys of spans, logs, and trace metrics, cached issue tag keys for errors, or a fixed set (replays), matched one `.` segment at a time (`span.d` finds `span.duration`, never `span.op`); then its whole aggregates (`aggregates`), and its aggregate functions as `fn()`, and `fn(field)` once a name is typed in full (`p95` → `p95(span.duration)`). Widgets take the functions of `aggregateFunctionsOf` in `types/dashboard.ts`; `alert metrics --aggregate` takes the narrower ones metric alerts accept (`METRIC_ALERT_FIELD_COMPLETION`). A word starting with `-` completes flag names, so a descending sort's `-` is typed after completing | | Issue as a flag value (`issue merge --into`) | `parseIssueReference` | what `ISSUE_TARGET` completes | | Git ref, commit, or repository (`--base-ref`, `--head-sha`, `--repo`, `--commit REPO@SHA`, `set-commits --from`) | `parseVcsRef` (branch and tag names as the remote knows them), `parseGitRef` (refs git resolves locally), `parseGitCommit`, `parseGitRepo` (`owner/repo` of the remotes), `parseRepoCommitList` | read from git in the shell's directory; nothing outside a work tree | | A structured value with its own grammar (`issue archive --until`, `api `) | `parseArchiveUntil`, `parseApiEndpoint` | the grammar: `forever`, `auto`, `10x`, `10x/5m`, …; API schema paths one segment at a time, with cached orgs and projects for their placeholders | diff --git a/packages/cli/src/commands/alert/mutation-utils.ts b/packages/cli/src/commands/alert/mutation-utils.ts index 059522dd7..d5c0cb7b6 100644 --- a/packages/cli/src/commands/alert/mutation-utils.ts +++ b/packages/cli/src/commands/alert/mutation-utils.ts @@ -2,10 +2,9 @@ * Shared validation and parsing helpers for alert mutation commands (create/edit). */ -import { - DATASET_FIELDS, - type DatasetCompletion, - type DatasetFields, +import type { + DatasetCompletion, + DatasetFields, } from "../../lib/completion-target.js"; import { ValidationError } from "../../lib/errors.js"; import { @@ -20,18 +19,78 @@ export const ISSUE_MATCH_MODES = ["all", "any"] as const; /** Deprecated dataset names that now route to `spans` with `is_transaction:true`. */ const TRANSACTION_DATASET_NAMES = new Set(["transaction", "transactions"]); +/* + * Aggregates metric alerts accept, from getsentry/sentry (`@sentry/api` + * types detector `data_sources` as `Array`, so it has none): + * - Spans and logs (EAP) skip column checks for the functions in + * `EAP_FUNCTIONS` (`sentry.incidents.logic`); the subscription then + * resolves them with the dataset's EAP definitions, so logs, which have + * no formulas (`sentry.search.eap.ourlogs.aggregates`), take only the + * plain aggregates. + * - Errors take a function of no column or of `user`, `release`, `dist` + * (`SUPPORTED_COLUMNS`, `TRANSLATABLE_COLUMNS` in the same module). + * - Crash rate takes only `percentage(sessions_crashed|users_crashed, + * sessions|users)` (`CRASH_RATE_ALERT_AGGREGATE_RE` in + * `sentry.snuba.entity_subscription`). + */ + +/** `EAP_FUNCTIONS` that logs also define. */ +const LOG_ALERT_FUNCTIONS = [ + "count", + "count_unique", + "avg", + "p50", + "p75", + "p90", + "p95", + "p99", + "max", + "min", + "sum", +] as const; + +/** `EAP_FUNCTIONS`, all of which spans define. */ +const SPAN_ALERT_FUNCTIONS = [ + ...LOG_ALERT_FUNCTIONS, + "p100", + "epm", + "eps", + "failure_count", + "failure_rate", + "apdex", + "user_misery", +] as const; + +const ERROR_ALERT_FIELDS: DatasetFields = { + aggregates: [ + "count()", + "count_unique(user)", + "count_unique(release)", + "count_unique(dist)", + ], +}; + +const CRASH_RATE_ALERT_FIELDS: DatasetFields = { + aggregates: [ + "percentage(sessions_crashed,sessions)", + "percentage(users_crashed,users)", + ], +}; + /** - * What `--aggregate` completes, by the dataset `--dataset` names. Crash - * rate (`metrics`, `sessions`) aggregates are not fields of a dataset, so - * nothing completes for them; without `--dataset`, every dataset's. + * What `--aggregate` completes, by the dataset `--dataset` names: the + * aggregates metric alerts accept on it, narrower than a widget's. Without + * `--dataset`, every dataset's. */ export const METRIC_ALERT_FIELD_COMPLETION: DatasetCompletion = { datasets: { - errors: DATASET_FIELDS.errors, - events: DATASET_FIELDS.errors, - spans: DATASET_FIELDS.spans, - logs: DATASET_FIELDS.logs, - } satisfies Partial>, + errors: ERROR_ALERT_FIELDS, + events: ERROR_ALERT_FIELDS, + spans: { entity: "span-attribute", functions: SPAN_ALERT_FUNCTIONS }, + logs: { entity: "log-attribute", functions: LOG_ALERT_FUNCTIONS }, + metrics: CRASH_RATE_ALERT_FIELDS, + sessions: CRASH_RATE_ALERT_FIELDS, + } satisfies Record, aliases: { ...METRIC_ALERT_DATASET_ALIASES, ...Object.fromEntries( diff --git a/packages/cli/src/lib/complete.ts b/packages/cli/src/lib/complete.ts index ef7c09fbb..830832afb 100644 --- a/packages/cli/src/lib/complete.ts +++ b/packages/cli/src/lib/complete.ts @@ -45,7 +45,7 @@ import { silenceReadonlyHandling } from "./db/resilient.js"; import { getEnv } from "./env.js"; import type { GitCompletion } from "./git.js"; import { ValidationError } from "./errors.js"; -import { fuzzyMatch } from "./fuzzy.js"; +import { fuzzyMatch, fuzzyMatchSegments } from "./fuzzy.js"; import type { CommandCompletions } from "./introspect.js"; import { COMMON_PLATFORMS, VALID_PLATFORMS } from "./platforms.js"; @@ -940,9 +940,11 @@ function datasetFieldsOf(slot: CompleterContext): DatasetFields[] { } /** - * The fields of `datasets` that match the partial: cached entities of the - * org the command's target names (else of every org), then fixed fields, - * each once. + * The fields of `datasets` that match the partial, one `.` segment at a + * time ({@link fuzzyMatchSegments}: `span.d` finds `span.duration`, never + * `span.op`): cached entities of the org the command's target names (else + * of every org), most recently seen first when nothing is typed, then + * fixed fields, each once. */ async function matchFields( partial: string, @@ -950,32 +952,48 @@ async function matchFields( datasets: readonly DatasetFields[], ): Promise { const found = new Map(); + const add = (value: string, description?: string) => { + if (!found.has(value)) { + found.set(value, description); + } + }; const orgs = datasets.some(({ entity }) => entity) ? await flagOrgs(slot.positionals) : []; for (const { entity, fields = [] } of datasets) { - const cached = entity ? matchCachedEntities(entity, partial, orgs) : []; - for (const { value, label } of cached) { - if (!found.has(value)) { - found.set(value, label); + const labels = new Map(); + for (const { value, label } of entity + ? getCachedCompletionEntities(entity, orgs) + : []) { + if (!labels.has(value)) { + labels.set(value, label); } } - for (const value of fuzzyMatch(partial, fields, { mode: "search" })) { - if (!found.has(value)) { - found.set(value, undefined); - } + const cached = + partial === "" + ? [...labels.keys()] + : fuzzyMatchSegments(partial, [...labels.keys()]); + for (const value of cached.slice(0, MAX_ENTITY_COMPLETIONS)) { + add(value, labels.get(value)); + } + for (const value of fuzzyMatchSegments(partial, fields)) { + add(value); } } return [...found].map(([value, description]) => ({ value, description })); } -/** The aggregate function names of `datasets`, each once. */ -function functionsOf(datasets: readonly DatasetFields[]): string[] { - return [...new Set(datasets.flatMap(({ functions = [] }) => functions))]; +/** The aggregate function names, or whole aggregates, of `datasets`, each once. */ +function unionOf( + datasets: readonly DatasetFields[], + key: "functions" | "aggregates", +): string[] { + return [...new Set(datasets.flatMap((dataset) => dataset[key] ?? []))]; } /** - * Complete an aggregate call. Before the `(`, the function names that + * Complete an aggregate call: first the whole aggregates of the dataset + * that start with the partial. Before the `(`, the function names that * start with the partial, as `fn()`; a name typed in full also completes * `fn(field)` for each field, so the shell inserts `fn(` and lists them. * After the `(`, `fn(field)` for the fields that match. @@ -989,7 +1007,10 @@ async function completeAggregates( options: { fields: boolean }, ): Promise { const datasets = datasetFieldsOf(slot); - const functions = functionsOf(datasets); + const functions = unionOf(datasets, "functions"); + const fixed = unionOf(datasets, "aggregates") + .filter((value) => value.startsWith(partial)) + .map((value) => ({ value })); const calls = async (name: string, argument: string) => [ ...(argument === "" ? [{ value: `${name}()` }] : []), ...(await matchFields(argument, slot, datasets)).map( @@ -999,13 +1020,17 @@ async function completeAggregates( const open = partial.indexOf("("); if (open !== -1) { const name = partial.slice(0, open); - return functions.includes(name) - ? await calls(name, partial.slice(open + 1)) - : []; + return [ + ...fixed, + ...(functions.includes(name) + ? await calls(name, partial.slice(open + 1)) + : []), + ]; } const names = functions.filter((name) => name.startsWith(partial)); return [ ...(options.fields ? await matchFields(partial, slot, datasets) : []), + ...fixed, ...(names.includes(partial) ? await calls(partial, "") : []), ...names .filter((name) => name !== partial) @@ -1024,7 +1049,7 @@ async function completeShorthandAggregates( slot: CompleterContext, ): Promise { const datasets = datasetFieldsOf(slot); - const functions = functionsOf(datasets); + const functions = unionOf(datasets, "functions"); const colon = partial.indexOf(":"); if (colon === -1) { return functions diff --git a/packages/cli/src/lib/completion-target.ts b/packages/cli/src/lib/completion-target.ts index 4ae387571..562699430 100644 --- a/packages/cli/src/lib/completion-target.ts +++ b/packages/cli/src/lib/completion-target.ts @@ -195,6 +195,8 @@ export type DatasetFields = { fields?: readonly string[]; /** Its aggregate function names */ functions?: readonly string[]; + /** Whole aggregates it takes that are not a function of one field */ + aggregates?: readonly string[]; }; /** diff --git a/packages/cli/src/lib/fuzzy.ts b/packages/cli/src/lib/fuzzy.ts index 6041fc0b6..65a8f9eaa 100644 --- a/packages/cli/src/lib/fuzzy.ts +++ b/packages/cli/src/lib/fuzzy.ts @@ -79,6 +79,39 @@ export type FuzzyMatchOptions = { mode?: "search"; }; +/** + * Score one candidate as {@link fuzzyMatch} ranks it: lower is better. + * + * @returns The score, or `undefined` when the candidate does not match + */ +function matchScore( + partial: string, + candidate: string, + search: boolean, +): number | undefined { + const lowerPartial = partial.toLowerCase(); + const lowerCandidate = candidate.toLowerCase(); + if (lowerCandidate === lowerPartial) { + return EXACT_SCORE; + } + if (lowerCandidate.startsWith(lowerPartial)) { + return PREFIX_SCORE; + } + if (lowerCandidate.includes(lowerPartial)) { + return CONTAINS_SCORE; + } + if (search && partial.length < 3) { + return; + } + const dist = editDistance( + lowerPartial, + lowerCandidate, + search ? Math.floor(partial.length / 3) : undefined, + ); + const threshold = Math.max(2, Math.floor(partial.length / 3)); + return dist <= threshold ? FUZZY_BASE_SCORE + dist : undefined; +} + /** * Rank candidates by relevance to a partial input string. * @@ -115,29 +148,12 @@ export function fuzzyMatch( return opts?.maxResults ? sorted.slice(0, opts.maxResults) : sorted; } - const lowerPartial = partial.toLowerCase(); - const threshold = Math.max(2, Math.floor(partial.length / 3)); const search = opts?.mode === "search"; const scored: { candidate: string; score: number }[] = []; - for (const candidate of candidates) { - const lowerCandidate = candidate.toLowerCase(); - - if (lowerCandidate === lowerPartial) { - scored.push({ candidate, score: EXACT_SCORE }); - } else if (lowerCandidate.startsWith(lowerPartial)) { - scored.push({ candidate, score: PREFIX_SCORE }); - } else if (lowerCandidate.includes(lowerPartial)) { - scored.push({ candidate, score: CONTAINS_SCORE }); - } else if (!search || partial.length >= 3) { - const dist = editDistance( - lowerPartial, - lowerCandidate, - search ? Math.floor(partial.length / 3) : undefined, - ); - if (dist <= threshold) { - scored.push({ candidate, score: FUZZY_BASE_SCORE + dist }); - } + const score = matchScore(partial, candidate, search); + if (score !== undefined) { + scored.push({ candidate, score }); } } @@ -148,3 +164,55 @@ export function fuzzyMatch( const results = scored.map((s) => s.candidate); return opts?.maxResults ? results.slice(0, opts.maxResults) : results; } + +/** + * Rank candidates made of `.`-joined segments (`span.duration`) + * by an unfinished input, one segment at a time: the input's Nth segment + * is matched against the candidate's Nth, as {@link fuzzyMatch} matches in + * search mode. A candidate needs a segment for each one typed, and can + * have more after them, so `span.d` finds `span.duration` and + * `span.description` but never `span.op`, and `span` finds every `span.*`. + * + * When `partial` is empty, all candidates are returned (sorted + * alphabetically). + * + * @param partial - The partial input to match against + * @param candidates - The list of candidate strings + * @returns Candidates sorted by their summed segment scores, then + * alphabetically + */ +export function fuzzyMatchSegments( + partial: string, + candidates: readonly string[], +): string[] { + if (partial === "") { + return [...candidates].sort(); + } + const typed = partial.split("."); + const scored: { candidate: string; score: number }[] = []; + for (const candidate of candidates) { + const segments = candidate.split("."); + if (segments.length < typed.length) { + continue; + } + let score = 0; + for (const [index, segment] of typed.entries()) { + const segmentScore = + segment === "" + ? PREFIX_SCORE + : matchScore(segment, segments[index] ?? "", true); + if (segmentScore === undefined) { + score = Number.POSITIVE_INFINITY; + break; + } + score += segmentScore; + } + if (Number.isFinite(score)) { + scored.push({ candidate, score }); + } + } + scored.sort( + (a, b) => a.score - b.score || a.candidate.localeCompare(b.candidate), + ); + return scored.map((s) => s.candidate); +} diff --git a/packages/cli/test/lib/complete.test.ts b/packages/cli/test/lib/complete.test.ts index 2dc474394..0345d5af4 100644 --- a/packages/cli/test/lib/complete.test.ts +++ b/packages/cli/test/lib/complete.test.ts @@ -1318,7 +1318,27 @@ describe("getCompletions: fields and aggregates of the --dataset given", () => { ); }); - test("alert --aggregate completes calls, not bare fields", async () => { + test("inside fn(, fields match one segment at a time", async () => { + await seedOrgs([{ slug: "acme", name: "Acme" }]); + cacheCompletionEntities("span-attribute", "acme", [ + { value: "span.op", label: "string" }, + { value: "span.duration", label: "number" }, + ]); + const field = ["explore", "acme/", "-d", "spans", "-F"]; + + // The typed segment never matches past the candidate's: `d` is not `op` + expect(await values(field, "avg(span.d")).toEqual(["avg(span.duration)"]); + // A typo within the segment being typed still matches + expect(await values(field, "avg(span.duraton")).toEqual([ + "avg(span.duration)", + ]); + expect((await values(field, "avg(span.")).sort()).toEqual([ + "avg(span.duration)", + "avg(span.op)", + ]); + }); + + test("alert --aggregate completes the calls metric alerts accept", async () => { await seedFields(); const create = ["alert", "metrics", "create", "acme/", "--dataset", "span"]; @@ -1328,16 +1348,50 @@ describe("getCompletions: fields and aggregates of the --dataset given", () => { "p95()", "p99()", ]); + expect(await values([...create, "--aggregate"], "fail")).toEqual([ + "failure_count()", + "failure_rate()", + ]); expect(await values([...create, "--aggregate"], "avg(span.d")).toEqual([ "avg(span.duration)", ]); - // Crash rate aggregates are not fields of a dataset - expect( - await values( - ["alert", "metrics", "create", "--dataset", "sessions", "--aggregate"], - "", - ), - ).toEqual([]); + // Not an alert function, though widgets take it + expect(await values([...create, "--aggregate"], "any")).toEqual([]); + }); + + test("alert --aggregate narrows to each dataset's aggregates", async () => { + await seedFields(); + const aggregate = (dataset: string) => [ + "alert", + "metrics", + "create", + "acme/", + "--dataset", + dataset, + "--aggregate", + ]; + + // Logs have no formulas: no epm, failure_rate, or p100 + expect(await values(aggregate("logs"), "p10")).toEqual([]); + expect(await values(aggregate("log"), "e")).toEqual([]); + // Errors take no column but user, release, dist, not their tag keys + expect(await values(aggregate("errors"), "")).toEqual([ + "count()", + "count_unique(user)", + "count_unique(release)", + "count_unique(dist)", + ]); + expect(await values(aggregate("errors"), "count_unique(u")).toEqual([ + "count_unique(user)", + ]); + // Crash rate takes only crash-free percentages + expect(await values(aggregate("sessions"), "")).toEqual([ + "percentage(sessions_crashed,sessions)", + "percentage(users_crashed,users)", + ]); + expect(await values(aggregate("metrics"), "percentage(u")).toEqual([ + "percentage(users_crashed,users)", + ]); }); test("dashboard widgets complete shorthand aggregates and group-by columns", async () => { diff --git a/packages/cli/test/lib/fuzzy.test.ts b/packages/cli/test/lib/fuzzy.test.ts index 3513d603f..b562ca1d0 100644 --- a/packages/cli/test/lib/fuzzy.test.ts +++ b/packages/cli/test/lib/fuzzy.test.ts @@ -8,7 +8,11 @@ import { array, assert as fcAssert, property, string } from "fast-check"; import { describe, expect, test } from "vitest"; -import { fuzzyMatch, levenshtein } from "../../src/lib/fuzzy.js"; +import { + fuzzyMatch, + fuzzyMatchSegments, + levenshtein, +} from "../../src/lib/fuzzy.js"; import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; // Short strings to keep Levenshtein computation fast in property tests @@ -190,6 +194,56 @@ describe("fuzzyMatch: search mode", () => { }); }); +describe("fuzzyMatchSegments", () => { + const fields = [ + "span.op", + "span.duration", + "span.description", + "http.method", + "span", + ]; + + test("matches the typed segment within the candidate's, never past it", () => { + expect(fuzzyMatchSegments("span.d", fields)).toEqual([ + "span.description", + "span.duration", + ]); + expect(fuzzyMatchSegments("span.d", ["span.op"])).toEqual([]); + }); + + test("tolerates a typo within a segment", () => { + expect(fuzzyMatchSegments("span.duraton", fields)).toEqual([ + "span.duration", + ]); + expect(fuzzyMatchSegments("spn.duration", fields)).toEqual([ + "span.duration", + ]); + }); + + test("a segment typed in full matches the candidates under it", () => { + expect(fuzzyMatchSegments("span", fields)).toEqual([ + "span", + "span.description", + "span.duration", + "span.op", + ]); + expect(fuzzyMatchSegments("span.", fields)).toEqual([ + "span.description", + "span.duration", + "span.op", + ]); + }); + + test("needs a candidate segment for each typed one", () => { + expect(fuzzyMatchSegments("span.op.x", fields)).toEqual([]); + expect(fuzzyMatchSegments("duration", fields)).toEqual([]); + }); + + test("returns every candidate, sorted, for an empty partial", () => { + expect(fuzzyMatchSegments("", ["b.a", "a.b"])).toEqual(["a.b", "b.a"]); + }); +}); + describe("property: fuzzyMatch", () => { test("exact candidate is always in the result", () => { const candidatesArb = array(shortStringArb, {