diff --git a/server/routers/auditLogs/queryRequestAuditLog.ts b/server/routers/auditLogs/queryRequestAuditLog.ts index 7f4a0ec165..1270ef3416 100644 --- a/server/routers/auditLogs/queryRequestAuditLog.ts +++ b/server/routers/auditLogs/queryRequestAuditLog.ts @@ -9,7 +9,7 @@ import { import { registry } from "@server/openApi"; import { NextFunction } from "express"; import { Request, Response } from "express"; -import { eq, gt, lt, and, count, desc, inArray, isNull, or } from "drizzle-orm"; +import { eq, gt, lt, and, count, desc, inArray, isNull, or, sql } from "drizzle-orm"; import { OpenAPITags } from "@server/openApi"; import { z } from "zod"; import createHttpError from "http-errors"; @@ -294,52 +294,107 @@ async function queryUniqueFilterAttributes( const DISTINCT_LIMIT = 500; - // TODO: SOMEONE PLEASE OPTIMIZE THIS!!!!! - - // Run all queries in parallel - const [ - uniqueActors, - uniqueLocations, - uniqueHosts, - uniquePaths, - uniqueResources, - uniqueSiteResources - ] = await Promise.all([ - logsDb - .selectDistinct({ actor: requestAuditLog.actor }) + // Originally this ran 6 separate SELECT DISTINCT queries (6 round trips), + // each independently re-scanning every row in the org+time range since + // there's no index on actor/location/host/path/resourceId/siteResourceId + // individually. + // + // We still want each field's distinctness computed in the DB rather than + // pulled into the app and deduped in memory - for a busy org with e.g. + // millions of rows but only a handful of distinct paths, that would ship + // the entire matched row set across the wire just to derive a few + // values - and we still want each field's result set capped at + // DISTINCT_LIMIT like the original per-query LIMITs did. So instead of + // 6 round trips, this issues ONE query: a UNION ALL of 6 subqueries, + // each doing its own GROUP BY ... LIMIT, tagged with a `field` + // discriminator column so the flat result set can be split back apart + // below. resourceId and siteResourceId are cast to text so all 6 + // branches share a column type (required for UNION ALL) and parsed back + // to numbers afterward. + function distinctTextField(column: any, field: string) { + const sub = logsDb + .select({ value: column }) .from(requestAuditLog) - .where(baseConditions) - .limit(DISTINCT_LIMIT + 1), - logsDb - .selectDistinct({ locations: requestAuditLog.location }) - .from(requestAuditLog) - .where(baseConditions) - .limit(DISTINCT_LIMIT + 1), - logsDb - .selectDistinct({ hosts: requestAuditLog.host }) - .from(requestAuditLog) - .where(baseConditions) - .limit(DISTINCT_LIMIT + 1), - logsDb - .selectDistinct({ paths: requestAuditLog.path }) - .from(requestAuditLog) - .where(baseConditions) - .limit(DISTINCT_LIMIT + 1), - logsDb - .selectDistinct({ - id: requestAuditLog.resourceId - }) - .from(requestAuditLog) - .where(baseConditions) - .limit(DISTINCT_LIMIT + 1), - logsDb - .selectDistinct({ - id: requestAuditLog.siteResourceId + .where(and(baseConditions, sql`${column} IS NOT NULL`)) + .groupBy(column) + .limit(DISTINCT_LIMIT + 1) + .as(`sub_${field}`); + return logsDb + .select({ + value: sql`${sub.value}`.as("value"), + field: sql`${field}`.as("field") }) + .from(sub); + } + + function distinctIdField(column: any, field: string, extraCondition?: any) { + const sub = logsDb + .select({ value: column }) .from(requestAuditLog) - .where(and(baseConditions, isNull(requestAuditLog.resourceId))) + .where( + extraCondition + ? and( + baseConditions, + sql`${column} IS NOT NULL`, + extraCondition + ) + : and(baseConditions, sql`${column} IS NOT NULL`) + ) + .groupBy(column) .limit(DISTINCT_LIMIT + 1) - ]); + .as(`sub_${field}`); + return logsDb + .select({ + value: sql`CAST(${sub.value} AS TEXT)`.as("value"), + field: sql`${field}`.as("field") + }) + .from(sub); + } + + const unionedRows = await distinctTextField(requestAuditLog.actor, "actor") + .unionAll(distinctTextField(requestAuditLog.location, "location")) + .unionAll(distinctTextField(requestAuditLog.host, "host")) + .unionAll(distinctTextField(requestAuditLog.path, "path")) + .unionAll(distinctIdField(requestAuditLog.resourceId, "resourceId")) + .unionAll( + distinctIdField( + requestAuditLog.siteResourceId, + "siteResourceId", + // Mirrors the original query's isNull(resourceId) condition: + // a siteResourceId only counts when there's no resourceId. + isNull(requestAuditLog.resourceId) + ) + ); + + const uniqueActors: string[] = []; + const uniqueLocations: string[] = []; + const uniqueHosts: string[] = []; + const uniquePaths: string[] = []; + const resourceIds: number[] = []; + const siteResourceIds: number[] = []; + + for (const row of unionedRows as Array<{ value: string; field: string }>) { + switch (row.field) { + case "actor": + uniqueActors.push(row.value); + break; + case "location": + uniqueLocations.push(row.value); + break; + case "host": + uniqueHosts.push(row.value); + break; + case "path": + uniquePaths.push(row.value); + break; + case "resourceId": + resourceIds.push(Number(row.value)); + break; + case "siteResourceId": + siteResourceIds.push(Number(row.value)); + break; + } + } // TODO: for stuff like the paths this is too restrictive so lets just show some of the paths and the user needs to // refine the time range to see what they need to see @@ -348,20 +403,12 @@ async function queryUniqueFilterAttributes( // uniqueLocations.length > DISTINCT_LIMIT || // uniqueHosts.length > DISTINCT_LIMIT || // uniquePaths.length > DISTINCT_LIMIT || - // uniqueResources.length > DISTINCT_LIMIT + // resourceIds.length > DISTINCT_LIMIT // ) { // throw new Error("Too many distinct filter attributes to retrieve. Please refine your time range."); // } // Fetch resource names from main database for the unique resource IDs - const resourceIds = uniqueResources - .map((row) => row.id) - .filter((id): id is number => id !== null); - - const siteResourceIds = uniqueSiteResources - .map((row) => row.id) - .filter((id): id is number => id !== null); - let resourcesWithNames: Array<{ id: number; name: string | null }> = []; if (resourceIds.length > 0) { @@ -401,19 +448,11 @@ async function queryUniqueFilterAttributes( } return { - actors: uniqueActors - .map((row) => row.actor) - .filter((actor): actor is string => actor !== null), + actors: uniqueActors, resources: sortNamedFilterOptions(resourcesWithNames), - locations: uniqueLocations - .map((row) => row.locations) - .filter((location): location is string => location !== null), - hosts: uniqueHosts - .map((row) => row.hosts) - .filter((host): host is string => host !== null), + locations: uniqueLocations, + hosts: uniqueHosts, paths: uniquePaths - .map((row) => row.paths) - .filter((path): path is string => path !== null) }; } @@ -490,4 +529,4 @@ export async function queryRequestAuditLogs( createHttpError(HttpCode.INTERNAL_SERVER_ERROR, "An error occurred") ); } -} +} \ No newline at end of file