Skip to content

Commit 8b096ee

Browse files
heiskrCopilot
andauthored
Tighten code comments in src/audit-logs and src/webhooks (#63461)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c52b57f-2c29-4aa1-8233-99d0d6e13571
1 parent 9ee5672 commit 8b096ee

22 files changed

Lines changed: 101 additions & 416 deletions

‎src/audit-logs/lib/deduplicate.ts‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,15 @@ import type {
1313
// this write-path module doesn't pull in the request-path lib/index.ts.
1414
const AUDIT_LOG_DATA_DIR = 'src/audit-logs/data'
1515

16-
// Builds the deduplicated "shared" format (entries pool + fields pool +
17-
// version index) from in-memory audit log data and writes it to disk.
18-
//
19-
// Output is deterministic: versions and pages are iterated in sorted order so
20-
// that the same input always produces byte-identical files. This lets the sync
21-
// pipeline (sync.ts) and the standalone rebuild script (rebuild-dedup.ts)
22-
// produce the exact same files, so the two can never drift apart.
16+
// Sort versions and pages so sync.ts and rebuild-dedup.ts write byte-identical
17+
// shared files from the same audit log data.
2318
export async function writeDeduplicatedAuditLogData(
2419
auditLogData: VersionedAuditLogData,
2520
): Promise<void> {
2621
console.log(`\n▶️ Writing deduplicated audit log data...\n`)
2722

28-
// Build fields pool: unique fields arrays
2923
const fieldsPool: string[][] = []
30-
const fieldsMap = new Map<string, number>() // JSON key → index
24+
const fieldsMap = new Map<string, number>()
3125

3226
function getFieldsIndex(fields: string[] | undefined): number | undefined {
3327
if (!fields || fields.length === 0) return undefined
@@ -39,9 +33,8 @@ export async function writeDeduplicatedAuditLogData(
3933
return index
4034
}
4135

42-
// Build entries pool: unique events (with fields replaced by index)
4336
const entriesPool: DeduplicatedAuditLogEntry[] = []
44-
const entriesMap = new Map<string, number>() // JSON key → index
37+
const entriesMap = new Map<string, number>()
4538

4639
function getEntryIndex(event: AuditLogEventT): number {
4740
const fieldsIndex = getFieldsIndex(event.fields)
@@ -61,8 +54,6 @@ export async function writeDeduplicatedAuditLogData(
6154
return index
6255
}
6356

64-
// Build version index. Iterate versions and pages in sorted order so the
65-
// output is deterministic regardless of the input's insertion order.
6657
const versionIndex: AuditLogVersionIndex = {}
6758
let totalEntries = 0
6859

‎src/audit-logs/lib/index.ts‎

Lines changed: 17 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,13 @@ export const AUDIT_LOG_DATA_DIR = 'src/audit-logs/data'
2222
const auditLogEventsCache = new Map<string, Map<string, AuditLogEventT[]>>()
2323
const categorizedAuditLogEventsCache = new Map<string, Map<string, CategorizedEvents>>()
2424

25-
// Shared dedup data, loaded once and shared across all versions.
2625
let sharedEntries: DeduplicatedAuditLogEntry[] | null = null
2726
let sharedFieldsPool: string[][] | null = null
2827
let sharedVersionIndex: AuditLogVersionIndex | null = null
29-
let sharedFormatAvailable: boolean | null = null // null = not checked yet
28+
let sharedFormatAvailable: boolean | null = null
3029

31-
// A missing shared-format file is expected (per-version files are the fallback),
32-
// but a corrupt or unparseable file should fail loudly rather than silently
33-
// degrade to the per-version files and hide bad generated data.
30+
// Missing shared-format files fall back to per-version files; corrupt shared
31+
// data must fail loudly so generated data problems stay visible.
3432
function isFileNotFoundError(err: unknown): boolean {
3533
if (!(err instanceof Error) || !('code' in err)) return false
3634
const code = (err as NodeJS.ErrnoException).code
@@ -49,18 +47,17 @@ function loadSharedFormat(): boolean {
4947
sharedVersionIndex = readCompressedJsonFileFallback(
5048
path.join(AUDIT_LOG_DATA_DIR, 'version-index.json'),
5149
) as AuditLogVersionIndex
52-
// Freeze pool data so reconstructed events (which return references into
53-
// these pools) can't be mutated by downstream code and leak across versions.
50+
// Freeze reused field arrays so downstream mutations cannot leak across reconstructed events.
5451
Object.freeze(sharedEntries)
5552
Object.freeze(sharedFieldsPool)
5653
for (const fields of sharedFieldsPool) Object.freeze(fields)
5754
sharedFormatAvailable = true
5855
} catch (err) {
5956
if (isFileNotFoundError(err)) {
60-
// Shared files don't exist, so fall back to per-version files silently.
57+
// Missing shared files use per-version files silently.
6158
sharedFormatAvailable = false
6259
} else {
63-
// Corrupt JSON, schema mismatch, and so on. Surface it instead of hiding it.
60+
// Corrupt shared data fails loudly instead of hiding behind the per-version fallback.
6461
console.error('Failed to load shared audit log dedup format (corrupt data?):', err)
6562
throw err
6663
}
@@ -117,11 +114,9 @@ export type TitleResolutionContext = Context & {
117114
redirects: Record<string, string>
118115
}
119116

120-
// Memoizes resolved reference links by their input string. The resolved markdown
121-
// only depends on the link string plus the pages/redirects indexes, and those
122-
// indexes are process-wide singletons that only change on deploy (a fresh
123-
// process). Without this, the audit-log pages re-render ~500 page titles on every
124-
// request (~90–150ms of repeated work). See docs-engineering#6650.
117+
// Cache reference-link rendering by input string because pages and redirects are
118+
// process-wide deploy-time indexes. Audit-log pages otherwise re-render about
119+
// 500 titles on each request, adding about 90 to 150 ms of warm-server work.
125120
const referenceLinksMarkdownCache = new Map<string, Promise<string>>()
126121

127122
export function resolveReferenceLinksToMarkdown(
@@ -154,7 +149,6 @@ async function computeReferenceLinksToMarkdown(
154149
try {
155150
const page = findPage(link, context.pages, context.redirects)
156151
if (page) {
157-
// Create a minimal context for rendering the title
158152
const renderContext = {
159153
currentLanguage: 'en',
160154
currentVersion: 'free-pro-team@latest',
@@ -164,11 +158,9 @@ async function computeReferenceLinksToMarkdown(
164158
const title = await page.renderProp('title', renderContext, { textOnly: true })
165159
markdownLinks.push(`[${title}](${link})`)
166160
} else {
167-
// If we can't resolve the link, use the original URL
168161
markdownLinks.push(link)
169162
}
170163
} catch (error) {
171-
// If resolution fails, use the original URL
172164
console.warn(
173165
`Failed to resolve title for link: ${link}`,
174166
error instanceof Error
@@ -202,7 +194,6 @@ async function resolveReferenceLinksToTitles(
202194
try {
203195
const page = findPage(link, context.pages, context.redirects)
204196
if (page) {
205-
// Create a minimal context for rendering the title
206197
const renderContext = {
207198
currentLanguage: 'en',
208199
currentVersion: 'free-pro-team@latest',
@@ -212,11 +203,9 @@ async function resolveReferenceLinksToTitles(
212203
const title = await page.renderProp('title', renderContext, { textOnly: true })
213204
titles.push(title)
214205
} else {
215-
// If we can't resolve the link, use the original URL
216206
titles.push(link)
217207
}
218208
} catch (error) {
219-
// If resolution fails, use the original URL
220209
console.warn(
221210
`Failed to resolve title for link: ${link}`,
222211
error instanceof Error
@@ -232,22 +221,10 @@ async function resolveReferenceLinksToTitles(
232221
return titles.join(', ')
233222
}
234223

235-
// get audit log event data for the requested page and version
236-
//
237-
// returns an array of event objects that look like this:
238-
//
239-
// [
240-
// {
241-
// action: 'account.billing_date_change',
242-
// description: 'event description',
243-
// docs_reference_links: 'event reference links'
244-
// },
245-
// ]
246224
export function getAuditLogEvents(page: string, version: string): AuditLogEventT[] {
247225
const openApiVersion = getOpenApiVersion(version)
248226

249-
// If the data isn't cached for an entire version or a particular page, read
250-
// the data from the shared dedup format or fall back to per-version JSON files
227+
// Prefer shared dedup data, but keep per-version JSON as the compatibility fallback.
251228
if (!auditLogEventsCache.has(openApiVersion)) {
252229
auditLogEventsCache.set(openApiVersion, new Map())
253230
}
@@ -256,7 +233,6 @@ export function getAuditLogEvents(page: string, version: string): AuditLogEventT
256233
if (events) {
257234
auditLogEventsCache.get(openApiVersion)?.set(page, events)
258235
} else {
259-
// Fall back to per-version JSON file
260236
const auditLogFileName = path.join(AUDIT_LOG_DATA_DIR, openApiVersion, `${page}.json`)
261237
auditLogEventsCache
262238
.get(openApiVersion)
@@ -265,27 +241,14 @@ export function getAuditLogEvents(page: string, version: string): AuditLogEventT
265241
}
266242

267243
const auditLogEvents = auditLogEventsCache.get(openApiVersion)?.get(page)
268-
// If an event doesn't yet have a description (value will be empty string or
269-
// "N/A"), then we don't show the event.
244+
// Hide events whose upstream description is missing or marked N/A.
270245
const filteredAuditLogEvents = auditLogEvents?.filter(
271246
(event) => event.description !== 'N/A' && event.description !== '',
272247
)
273248

274249
return filteredAuditLogEvents || []
275250
}
276251

277-
// get categorized audit log event data for the requested page and version
278-
//
279-
// Events are grouped by category; the category is the first part of the event
280-
// action (e.g. category is `repo` for the `repo.create` event) so we extract
281-
// the categories and then group events under those categories and return an
282-
// object that looks like this:
283-
//
284-
// {
285-
// git: [ [Object], [Object] ],
286-
// repo: [ [Object] ],
287-
// user: [ [Object], [Object] ]
288-
// }
289252
export function getCategorizedAuditLogEvents(page: string, version: string): CategorizedEvents {
290253
const events = getAuditLogEvents(page, version)
291254
const openApiVersion = getOpenApiVersion(version)
@@ -362,24 +325,10 @@ export async function filterByAllowlistValues({
362325
return [...minimalEvents, ...currentEvents]
363326
}
364327

365-
// Filters audit log events based on allowlist values and processes an
366-
// event's supported GHES versions.
367-
//
368-
// Mutates `currentGhesEvents` and updates it with any new filtered for audit
369-
// log events, the object maps GHES versions to page events for that version e.g.:
370-
//
371-
// {
372-
// ghes-3.10': {
373-
// organization: [...],
374-
// enterprise: [...],
375-
// user: [...],
376-
// },
377-
// ghes-3.11': {
378-
// organization: [...],
379-
// enterprise: [...],
380-
// user: [...],
381-
// },
382-
// }
328+
// Mutates currentGhesEvents so each supported GHES version maps the requested
329+
// page to filtered events. It ignores upstream GHES versions that docs no
330+
// longer supports because nightly syncs would otherwise recreate deprecated
331+
// src/audit-logs/data/ghes-X.Y directories.
383332
export async function filterAndUpdateGhesDataByAllowlistValues({
384333
eventsToCheck,
385334
allowListValue,
@@ -401,11 +350,6 @@ export async function filterAndUpdateGhesDataByAllowlistValues({
401350
}) {
402351
if (!currentGhesEvents) currentGhesEvents = {}
403352

404-
// Upstream `audit-log-allowlists/data/schema.json` lags docs's deprecation
405-
// schedule, so events still list `ghes` keys for versions we've already
406-
// dropped from `supported` in `enterprise-server-releases.ts`. Without this
407-
// filter, the nightly sync would re-add `src/audit-logs/data/ghes-X.Y/`
408-
// dirs for those deprecated versions. See docs-engineering#6562.
409353
const supportedGhesVersionSet = new Set(supportedGhesVersions)
410354

411355
const seenByGhesVersion = new Map()
@@ -454,18 +398,6 @@ export async function filterAndUpdateGhesDataByAllowlistValues({
454398
}
455399
}
456400

457-
// we need to initialize as we go to build up the `minimalEvents`
458-
// object that we'll return which will contain the GHES events for
459-
// each versions + page type combos e.g. when processing GHES events
460-
// for the organization events page we'll end up with something like
461-
// this:
462-
//
463-
// {
464-
// 'ghes-3.10': { organization: [Array] },
465-
// 'ghes-3.11': { organization: [Array] },
466-
// 'ghes-3.8': { organization: [Array] },
467-
// 'ghes-3.9': { organization: [Array] }
468-
// }
469401
if (!currentGhesEvents[fullGhesVersion]) {
470402
currentGhesEvents[fullGhesVersion] = {}
471403
currentGhesEvents[fullGhesVersion][auditLogPage] = []
@@ -495,17 +427,15 @@ function categorizeEvents(events: AuditLogEventT[]) {
495427
return categorizedEvents
496428
}
497429

430+
// The generic API-only description is wrong for api.request, so it gets its own.
431+
// The schema has no field to identify it, so match on the action name.
498432
function processAndGetEventDescription(
499433
event: AuditLogEventT,
500434
allowlists: string[],
501435
pipelineConfig: PipelineConfig,
502436
) {
503437
let description = event.description
504438

505-
// api.request is a unique event because it's an api_only event but is the only
506-
// one of these events where the description we append isn't correct so we
507-
// have to account for it separately. There's not yet anything in the schema
508-
// we can hook onto to treat it differently.
509439
if (
510440
(allowlists.includes('org_api_only') || allowlists.includes('business_api_only')) &&
511441
event.action !== 'api.request'

‎src/audit-logs/scripts/rebuild-dedup.ts‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,8 @@
1-
// Rebuilds the deduplicated "shared" audit log format from the per-version page
2-
// files already on disk: src/audit-logs/data/shared/entries.json and
3-
// fields-pool.json, plus src/audit-logs/data/version-index.json.
4-
//
5-
// Unlike `sync-audit-log`, this does NOT fetch from github/audit-log-allowlists
6-
// and needs no GITHUB_TOKEN. Use it to regenerate the shared files when the
7-
// per-version JSON files have changed but the shared files are stale, for
8-
// example on an older audit-log-pipeline PR that predates the dedup format.
9-
// That is what makes the `deduplication` test fail.
10-
//
11-
// npm run rebuild-audit-log-dedup
1+
// Rebuilds src/audit-logs/data/shared/entries.json, fields-pool.json, and
2+
// src/audit-logs/data/version-index.json from per-version JSON already on disk.
3+
// It does not fetch github/audit-log-allowlists and does not need GITHUB_TOKEN.
4+
// Use it when src/audit-logs/data/shared or version-index.json is stale.
5+
// Run with npm run rebuild-audit-log-dedup.
126
import { existsSync, readdirSync, readFileSync, statSync } from 'fs'
137
import path from 'path'
148

@@ -17,7 +11,6 @@ import type { AuditLogEventT, VersionedAuditLogData } from '../types'
1711

1812
const AUDIT_LOG_DATA_DIR = 'src/audit-logs/data'
1913

20-
// Directories under the data dir that are not per-version event data.
2114
const NON_VERSION_DIRS = new Set(['shared'])
2215

2316
function loadAuditLogDataFromDisk(): VersionedAuditLogData {

0 commit comments

Comments
 (0)