Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 103 additions & 64 deletions server/routers/auditLogs/queryRequestAuditLog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 <field> ... 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<string>`${sub.value}`.as("value"),
field: sql<string>`${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<string>`CAST(${sub.value} AS TEXT)`.as("value"),
field: sql<string>`${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
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
};
}

Expand Down Expand Up @@ -490,4 +529,4 @@ export async function queryRequestAuditLogs(
createHttpError(HttpCode.INTERNAL_SERVER_ERROR, "An error occurred")
);
}
}
}