From f12d0f63c68b7d30e99fb7c57fad8adc5cb8b4b5 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 14 Mar 2024 15:43:33 -0300 Subject: [PATCH 1/8] refactor: Extract "HttpStats" utulity class --- lib/util/http/index.ts | 25 +-- lib/util/stats.spec.ts | 218 ++++++++++++++++++++++++++- lib/util/stats.ts | 130 ++++++++++++++++ lib/workers/repository/index.ts | 5 +- lib/workers/repository/stats.spec.ts | 148 ------------------ lib/workers/repository/stats.ts | 74 --------- 6 files changed, 363 insertions(+), 237 deletions(-) delete mode 100644 lib/workers/repository/stats.spec.ts delete mode 100644 lib/workers/repository/stats.ts diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index c58279d4210ccc..09f2bf191aca5d 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -1,3 +1,4 @@ +/* eslint-disable import/order */ import merge from 'deepmerge'; import got, { Options, RequestError } from 'got'; import type { SetRequired } from 'type-fest'; @@ -26,10 +27,10 @@ import type { HttpRequestOptions, HttpResponse, InternalHttpOptions, - RequestStats, } from './types'; // TODO: refactor code to remove this (#9651) import './legacy'; +import { HttpStats, type HttpRequestStatsDataPoint } from '../stats'; export { RequestError as HttpError }; @@ -76,6 +77,8 @@ function applyDefaultHeaders(options: Options): void { }; } +type QueueStatsData = Pick; + // Note on types: // options.requestType can be either 'json' or 'buffer', but `T` should be // `Buffer` in the latter case. @@ -84,7 +87,7 @@ function applyDefaultHeaders(options: Options): void { async function gotTask( url: string, options: SetRequired, - requestStats: Omit, + queueStats: QueueStatsData, ): Promise> { logger.trace({ url, options }, 'got request'); @@ -119,9 +122,13 @@ async function gotTask( throw error; } finally { - const httpRequests = memCache.get('http-requests') || []; - httpRequests.push({ ...requestStats, duration, statusCode }); - memCache.set('http-requests', httpRequests); + HttpStats.write({ + method: options.method, + url, + reqMs: duration, + queueMs: queueStats.queueMs, + status: statusCode, + }); } } @@ -236,12 +243,8 @@ export class Http { } const startTime = Date.now(); const httpTask: GotTask = () => { - const queueDuration = Date.now() - startTime; - return gotTask(url, options, { - method: options.method, - url, - queueDuration, - }); + const queueMs = Date.now() - startTime; + return gotTask(url, options, { queueMs }); }; const throttle = this.getThrottle(url); diff --git a/lib/util/stats.spec.ts b/lib/util/stats.spec.ts index 1d273e3c221947..9859dc916abd30 100644 --- a/lib/util/stats.spec.ts +++ b/lib/util/stats.spec.ts @@ -1,6 +1,11 @@ import { logger } from '../../test/util'; import * as memCache from './cache/memory'; -import { LookupStats, PackageCacheStats, makeTimingReport } from './stats'; +import { + HttpStats, + LookupStats, + PackageCacheStats, + makeTimingReport, +} from './stats'; describe('util/stats', () => { beforeEach(() => { @@ -223,4 +228,215 @@ describe('util/stats', () => { }); }); }); + + describe('HttpStats', () => { + it('returns empty report', () => { + const res = HttpStats.getReport(); + expect(res).toEqual({ + allRequests: [], + requestsByHost: {}, + statsByHost: {}, + totalRequests: 0, + urlCounts: {}, + }); + }); + + it('writes data points', () => { + HttpStats.write({ + method: 'GET', + url: 'https://example.com/foo', + reqMs: 100, + queueMs: 10, + status: 200, + }); + HttpStats.write({ + method: 'GET', + url: 'https://example.com/foo', + reqMs: 200, + queueMs: 20, + status: 200, + }); + HttpStats.write({ + method: 'GET', + url: 'https://example.com/bar', + reqMs: 400, + queueMs: 40, + status: 200, + }); + HttpStats.write({ + method: 'GET', + url: 'https://example.com/foo', + reqMs: 800, + queueMs: 80, + status: 404, + }); + HttpStats.write({ + method: 'GET', + url: '', + reqMs: 100, + queueMs: 100, + status: 400, + }); + + const res = HttpStats.getReport(); + + expect(res).toEqual({ + allRequests: [ + 'GET https://example.com/bar 200 400 40', + 'GET https://example.com/foo 200 100 10', + 'GET https://example.com/foo 200 200 20', + 'GET https://example.com/foo 404 800 80', + ], + requestsByHost: { + 'example.com': [ + { + method: 'GET', + queueMs: 40, + reqMs: 400, + status: 200, + url: 'https://example.com/bar', + }, + { + method: 'GET', + queueMs: 10, + reqMs: 100, + status: 200, + url: 'https://example.com/foo', + }, + { + method: 'GET', + queueMs: 20, + reqMs: 200, + status: 200, + url: 'https://example.com/foo', + }, + { + method: 'GET', + queueMs: 80, + reqMs: 800, + status: 404, + url: 'https://example.com/foo', + }, + ], + }, + statsByHost: { + 'example.com': { + count: 4, + queueAvgMs: 38, + queueMaxMs: 80, + queueMedianMs: 40, + reqAvgMs: 375, + reqMaxMs: 800, + reqMedianMs: 400, + }, + }, + totalRequests: 4, + urlCounts: { + 'https://example.com/bar (GET, 200)': 1, + 'https://example.com/foo (GET, 200)': 2, + 'https://example.com/foo (GET, 404)': 1, + }, + }); + }); + + it('logs report', () => { + HttpStats.write({ + method: 'GET', + url: 'https://example.com/foo', + reqMs: 100, + queueMs: 10, + status: 200, + }); + HttpStats.write({ + method: 'GET', + url: 'https://example.com/foo', + reqMs: 200, + queueMs: 20, + status: 200, + }); + HttpStats.write({ + method: 'GET', + url: 'https://example.com/bar', + reqMs: 400, + queueMs: 40, + status: 200, + }); + HttpStats.write({ + method: 'GET', + url: 'https://example.com/foo', + reqMs: 800, + queueMs: 80, + status: 404, + }); + + HttpStats.report(); + + expect(logger.logger.trace).toHaveBeenCalledTimes(1); + const [traceData, traceMsg] = logger.logger.trace.mock.calls[0]; + expect(traceMsg).toBe('HTTP full stats'); + expect(traceData).toEqual({ + allRequests: [ + 'GET https://example.com/bar 200 400 40', + 'GET https://example.com/foo 200 100 10', + 'GET https://example.com/foo 200 200 20', + 'GET https://example.com/foo 404 800 80', + ], + requestsByHost: { + 'example.com': [ + { + method: 'GET', + queueMs: 40, + reqMs: 400, + status: 200, + url: 'https://example.com/bar', + }, + { + method: 'GET', + queueMs: 10, + reqMs: 100, + status: 200, + url: 'https://example.com/foo', + }, + { + method: 'GET', + queueMs: 20, + reqMs: 200, + status: 200, + url: 'https://example.com/foo', + }, + { + method: 'GET', + queueMs: 80, + reqMs: 800, + status: 404, + url: 'https://example.com/foo', + }, + ], + }, + }); + + expect(logger.logger.debug).toHaveBeenCalledTimes(1); + const [debugData, debugMsg] = logger.logger.debug.mock.calls[0]; + expect(debugMsg).toBe('HTTP stats'); + expect(debugData).toEqual({ + statsByHost: { + 'example.com': { + count: 4, + queueAvgMs: 38, + queueMaxMs: 80, + queueMedianMs: 40, + reqAvgMs: 375, + reqMaxMs: 800, + reqMedianMs: 400, + }, + }, + totalRequests: 4, + urlCounts: { + 'https://example.com/bar (GET, 200)': 1, + 'https://example.com/foo (GET, 200)': 2, + 'https://example.com/foo (GET, 404)': 1, + }, + }); + }); + }); }); diff --git a/lib/util/stats.ts b/lib/util/stats.ts index 719431f315c9da..25e1451a2a6d43 100644 --- a/lib/util/stats.ts +++ b/lib/util/stats.ts @@ -1,5 +1,6 @@ import { logger } from '../logger'; import * as memCache from './cache/memory'; +import { parseUrl } from './url'; type LookupStatsData = Record; @@ -103,3 +104,132 @@ export class PackageCacheStats { logger.debug(report, 'Package cache statistics'); } } + +export interface HttpRequestStatsDataPoint { + method: string; + url: string; + reqMs: number; + queueMs: number; + status: number; +} + +interface HostStatsData { + count: number; + reqAvgMs: number; + reqMedianMs: number; + reqMaxMs: number; + queueAvgMs: number; + queueMedianMs: number; + queueMaxMs: number; +} + +interface HttpStatsCollection { + urlCounts: Record; + allRequests: string[]; + requestsByHost: Record; + statsByHost: Record; + totalRequests: number; +} + +export class HttpStats { + static write(data: HttpRequestStatsDataPoint): void { + const httpRequests = + memCache.get('http-requests') ?? []; + httpRequests.push(data); + memCache.set('http-requests', httpRequests); + } + + static getDataPoints(): HttpRequestStatsDataPoint[] { + const httpRequests = + memCache.get('http-requests') ?? []; + + // istanbul ignore next: sorting is hard and not worth testing + httpRequests.sort((a, b) => { + if (a.url < b.url) { + return -1; + } + + if (a.url > b.url) { + return 1; + } + + return 0; + }); + + return httpRequests; + } + + static getReport(): HttpStatsCollection { + const dataPoints = HttpStats.getDataPoints(); + + const urlCounts: Record = {}; + const allRequests: string[] = []; + const requestsByHost: Record = {}; + + for (const dataPoint of dataPoints) { + const { url, reqMs, queueMs, status } = dataPoint; + const method = dataPoint.method.toUpperCase(); + + const parsedUrl = parseUrl(url); + if (!parsedUrl) { + logger.debug({ url }, 'Failed to parse URL during stats reporting'); + continue; + } + const { hostname, origin, pathname } = parsedUrl; + const baseUrl = `${origin}${pathname}`; + + const urlKey = `${baseUrl} (${method}, ${status})`; + urlCounts[urlKey] ??= 0; + urlCounts[urlKey] += 1; + + allRequests.push(`${method} ${url} ${status} ${reqMs} ${queueMs}`); + + requestsByHost[hostname] ??= []; + requestsByHost[hostname].push(dataPoint); + } + + const statsByHost: Record = {}; + + let totalRequests = 0; + for (const [hostname, requests] of Object.entries(requestsByHost)) { + const count = requests.length; + totalRequests += count; + + const reqTimes = requests.map((r) => r.reqMs); + const queueTimes = requests.map((r) => r.queueMs); + + const reqReport = makeTimingReport(reqTimes); + const queueReport = makeTimingReport(queueTimes); + + statsByHost[hostname] = { + count, + reqAvgMs: reqReport.avgMs, + reqMedianMs: reqReport.medianMs, + reqMaxMs: reqReport.maxMs, + queueAvgMs: queueReport.avgMs, + queueMedianMs: queueReport.medianMs, + queueMaxMs: queueReport.maxMs, + }; + } + + return { + urlCounts, + allRequests, + requestsByHost, + statsByHost, + totalRequests, + }; + } + + static report(): void { + const { + urlCounts, + allRequests, + requestsByHost, + statsByHost, + totalRequests, + } = HttpStats.getReport(); + logger.trace({ allRequests, requestsByHost }, 'HTTP full stats'); + logger.debug({ urlCounts, statsByHost, totalRequests }, 'HTTP stats'); + } +} diff --git a/lib/workers/repository/index.ts b/lib/workers/repository/index.ts index 09394691788da7..01c0658eb3e79b 100644 --- a/lib/workers/repository/index.ts +++ b/lib/workers/repository/index.ts @@ -19,7 +19,7 @@ import { clearDnsCache, printDnsStats } from '../../util/http/dns'; import * as queue from '../../util/http/queue'; import * as throttle from '../../util/http/throttle'; import { addSplit, getSplits, splitInit } from '../../util/split'; -import { LookupStats, PackageCacheStats } from '../../util/stats'; +import { HttpStats, LookupStats, PackageCacheStats } from '../../util/stats'; import { setBranchCache } from './cache'; import { extractRepoProblems } from './common'; import { ensureDependencyDashboard } from './dependency-dashboard'; @@ -32,7 +32,6 @@ import { ensureOnboardingPr } from './onboarding/pr'; import { extractDependencies, updateRepo } from './process'; import type { ExtractResult } from './process/extract-update'; import { ProcessResult, processResult } from './result'; -import { printRequestStats } from './stats'; // istanbul ignore next export async function renovateRepository( @@ -126,7 +125,7 @@ export async function renovateRepository( const splits = getSplits(); logger.debug(splits, 'Repository timing splits (milliseconds)'); PackageCacheStats.report(); - printRequestStats(); + HttpStats.report(); LookupStats.report(); printDnsStats(); clearDnsCache(); diff --git a/lib/workers/repository/stats.spec.ts b/lib/workers/repository/stats.spec.ts deleted file mode 100644 index ac850dd6383d6c..00000000000000 --- a/lib/workers/repository/stats.spec.ts +++ /dev/null @@ -1,148 +0,0 @@ -import { logger } from '../../../test/util'; -import type { Logger } from '../../logger/types'; -import * as memCache from '../../util/cache/memory'; -import { printRequestStats } from './stats'; - -const log = logger.logger as jest.Mocked; - -describe('workers/repository/stats', () => { - beforeEach(() => { - memCache.init(); - }); - - describe('printRequestStats()', () => { - it('runs', () => { - memCache.set('http-requests', [ - { - method: 'get', - url: 'https://api.github.com/api/v3/user', - duration: 100, - queueDuration: 0, - statusCode: 200, - }, - { - method: 'post', - url: 'https://api.github.com/graphql', - duration: 130, - queueDuration: 0, - statusCode: 401, - }, - { - method: 'post', - url: 'https://api.github.com/graphql', - duration: 150, - queueDuration: 0, - statusCode: 200, - }, - { - method: 'post', - url: 'https://api.github.com/graphql', - duration: 20, - queueDuration: 10, - statusCode: 200, - }, - { - method: 'get', - url: 'https://api.github.com/api/v3/repositories', - duration: 500, - queueDuration: 0, - statusCode: 500, - }, - { - method: 'get', - url: 'https://auth.docker.io', - duration: 200, - queueDuration: 0, - statusCode: 401, - }, - ]); - expect(printRequestStats()).toBeUndefined(); - expect(log.trace).toHaveBeenCalledOnce(); - expect(log.debug).toHaveBeenCalledTimes(1); - expect(log.trace.mock.calls[0][0]).toMatchInlineSnapshot(` - { - "allRequests": [ - "GET https://api.github.com/api/v3/repositories 500 500 0", - "GET https://api.github.com/api/v3/user 200 100 0", - "POST https://api.github.com/graphql 401 130 0", - "POST https://api.github.com/graphql 200 150 0", - "POST https://api.github.com/graphql 200 20 10", - "GET https://auth.docker.io 401 200 0", - ], - "requestHosts": { - "api.github.com": [ - { - "duration": 500, - "method": "get", - "queueDuration": 0, - "statusCode": 500, - "url": "https://api.github.com/api/v3/repositories", - }, - { - "duration": 100, - "method": "get", - "queueDuration": 0, - "statusCode": 200, - "url": "https://api.github.com/api/v3/user", - }, - { - "duration": 130, - "method": "post", - "queueDuration": 0, - "statusCode": 401, - "url": "https://api.github.com/graphql", - }, - { - "duration": 150, - "method": "post", - "queueDuration": 0, - "statusCode": 200, - "url": "https://api.github.com/graphql", - }, - { - "duration": 20, - "method": "post", - "queueDuration": 10, - "statusCode": 200, - "url": "https://api.github.com/graphql", - }, - ], - "auth.docker.io": [ - { - "duration": 200, - "method": "get", - "queueDuration": 0, - "statusCode": 401, - "url": "https://auth.docker.io", - }, - ], - }, - } - `); - expect(log.debug.mock.calls[0][0]).toMatchInlineSnapshot(` - { - "hostStats": { - "api.github.com": { - "queueAvgMs": 2, - "requestAvgMs": 180, - "requestCount": 5, - }, - "auth.docker.io": { - "queueAvgMs": 0, - "requestAvgMs": 200, - "requestCount": 1, - }, - }, - "totalRequests": 6, - "urls": { - "https://api.github.com/api/v3/repositories (GET,500)": 1, - "https://api.github.com/api/v3/user (GET,200)": 1, - "https://api.github.com/graphql (POST,200)": 2, - "https://api.github.com/graphql (POST,401)": 1, - "https://auth.docker.io (GET,401)": 1, - }, - } - `); - }); - }); -}); diff --git a/lib/workers/repository/stats.ts b/lib/workers/repository/stats.ts deleted file mode 100644 index 692dfd67a50798..00000000000000 --- a/lib/workers/repository/stats.ts +++ /dev/null @@ -1,74 +0,0 @@ -import URL from 'node:url'; -import { logger } from '../../logger'; -import * as memCache from '../../util/cache/memory'; -import type { RequestStats } from '../../util/http/types'; - -export function printRequestStats(): void { - const httpRequests = memCache.get('http-requests'); - // istanbul ignore next - if (!httpRequests) { - return; - } - httpRequests.sort((a, b) => { - if (a.url === b.url) { - return 0; - } - if (a.url < b.url) { - return -1; - } - return 1; - }); - const allRequests: string[] = []; - const requestHosts: Record = {}; - const rawUrls: Record = {}; - for (const httpRequest of httpRequests) { - const { method, url, duration, queueDuration, statusCode } = httpRequest; - const [baseUrl] = url.split('?'); - // put method last for better sorting - const urlKey = `${baseUrl} (${method.toUpperCase()},${statusCode})`; - if (rawUrls[urlKey]) { - rawUrls[urlKey] += 1; - } else { - rawUrls[urlKey] = 1; - } - allRequests.push( - `${method.toUpperCase()} ${url} ${statusCode} ${duration} ${queueDuration}`, - ); - const { hostname } = URL.parse(url); - - // istanbul ignore if: TODO: fix types (#9610) - if (!hostname) { - return; - } - requestHosts[hostname] = requestHosts[hostname] || []; - requestHosts[hostname].push(httpRequest); - } - const urls: Record = {}; - // Sort urls for easier reading - for (const url of Object.keys(rawUrls).sort()) { - urls[url] = rawUrls[url]; - } - logger.trace({ allRequests, requestHosts }, 'full stats'); - type HostStats = { - requestCount: number; - requestAvgMs: number; - queueAvgMs: number; - }; - const hostStats: Record = {}; - let totalRequests = 0; - for (const [hostname, requests] of Object.entries(requestHosts)) { - const requestCount = requests.length; - totalRequests += requestCount; - const requestSum = requests - .map(({ duration }) => duration) - .reduce((a, b) => a + b, 0); - const requestAvgMs = Math.round(requestSum / requestCount); - - const queueSum = requests - .map(({ queueDuration }) => queueDuration) - .reduce((a, b) => a + b, 0); - const queueAvgMs = Math.round(queueSum / requestCount); - hostStats[hostname] = { requestCount, requestAvgMs, queueAvgMs }; - } - logger.debug({ urls, hostStats, totalRequests }, 'http statistics'); -} From 98da47d41da5c5d9639872a51aed8f5a0806c537 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 14 Mar 2024 15:56:19 -0300 Subject: [PATCH 2/8] Fix lint --- lib/util/http/index.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index 09f2bf191aca5d..b4b6e4d2b6b9d0 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -30,7 +30,7 @@ import type { } from './types'; // TODO: refactor code to remove this (#9651) import './legacy'; -import { HttpStats, type HttpRequestStatsDataPoint } from '../stats'; +import { type HttpRequestStatsDataPoint, HttpStats } from '../stats'; export { RequestError as HttpError }; @@ -135,10 +135,7 @@ async function gotTask( export class Http { private options?: GotOptions; - constructor( - protected hostType: string, - options: HttpOptions = {}, - ) { + constructor(protected hostType: string, options: HttpOptions = {}) { const retryLimit = process.env.NODE_ENV === 'test' ? 0 : 2; this.options = merge(options, { context: { hostType }, From 0ee1888a308c3041ecb0f224dda8d8377efcd049 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 14 Mar 2024 16:01:34 -0300 Subject: [PATCH 3/8] Fix prettier --- lib/util/http/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index b4b6e4d2b6b9d0..623358abb07006 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -135,7 +135,10 @@ async function gotTask( export class Http { private options?: GotOptions; - constructor(protected hostType: string, options: HttpOptions = {}) { + constructor( + protected hostType: string, + options: HttpOptions = {}, + ) { const retryLimit = process.env.NODE_ENV === 'test' ? 0 : 2; this.options = merge(options, { context: { hostType }, From 251dcfe6624f38a46bc1f56f4f8ca2556b4645f6 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 14 Mar 2024 16:38:32 -0300 Subject: [PATCH 4/8] Fix eslint --- lib/util/http/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index 623358abb07006..107f4726344928 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -1,4 +1,3 @@ -/* eslint-disable import/order */ import merge from 'deepmerge'; import got, { Options, RequestError } from 'got'; import type { SetRequired } from 'type-fest'; @@ -12,6 +11,7 @@ import { getCache } from '../cache/repository'; import { clone } from '../clone'; import { hash } from '../hash'; import { type AsyncResult, Result } from '../result'; +import { type HttpRequestStatsDataPoint, HttpStats } from '../stats'; import { resolveBaseUrl } from '../url'; import { applyAuthorization, removeAuthorization } from './auth'; import { hooks } from './hooks'; @@ -30,7 +30,6 @@ import type { } from './types'; // TODO: refactor code to remove this (#9651) import './legacy'; -import { type HttpRequestStatsDataPoint, HttpStats } from '../stats'; export { RequestError as HttpError }; From 6a015bd33c145afba5f17d129974aa252058f30f Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 15 Mar 2024 08:20:29 -0300 Subject: [PATCH 5/8] Refactor field names and structure --- lib/util/stats.spec.ts | 94 +++++++++++++++++++++++++----------------- lib/util/stats.ts | 74 +++++++++++++++++---------------- 2 files changed, 94 insertions(+), 74 deletions(-) diff --git a/lib/util/stats.spec.ts b/lib/util/stats.spec.ts index 9859dc916abd30..3cca1e79fabaf1 100644 --- a/lib/util/stats.spec.ts +++ b/lib/util/stats.spec.ts @@ -233,11 +233,11 @@ describe('util/stats', () => { it('returns empty report', () => { const res = HttpStats.getReport(); expect(res).toEqual({ - allRequests: [], - requestsByHost: {}, - statsByHost: {}, - totalRequests: 0, - urlCounts: {}, + hostRequests: {}, + hosts: {}, + rawRequests: [], + requests: 0, + urls: {}, }); }); @@ -281,13 +281,7 @@ describe('util/stats', () => { const res = HttpStats.getReport(); expect(res).toEqual({ - allRequests: [ - 'GET https://example.com/bar 200 400 40', - 'GET https://example.com/foo 200 100 10', - 'GET https://example.com/foo 200 200 20', - 'GET https://example.com/foo 404 800 80', - ], - requestsByHost: { + hostRequests: { 'example.com': [ { method: 'GET', @@ -319,7 +313,7 @@ describe('util/stats', () => { }, ], }, - statsByHost: { + hosts: { 'example.com': { count: 4, queueAvgMs: 38, @@ -330,11 +324,25 @@ describe('util/stats', () => { reqMedianMs: 400, }, }, - totalRequests: 4, - urlCounts: { - 'https://example.com/bar (GET, 200)': 1, - 'https://example.com/foo (GET, 200)': 2, - 'https://example.com/foo (GET, 404)': 1, + rawRequests: [ + 'GET https://example.com/bar 200 400 40', + 'GET https://example.com/foo 200 100 10', + 'GET https://example.com/foo 200 200 20', + 'GET https://example.com/foo 404 800 80', + ], + requests: 5, + urls: { + 'https://example.com/bar': { + GET: { + '200': 1, + }, + }, + 'https://example.com/foo': { + GET: { + '200': 2, + '404': 1, + }, + }, }, }); }); @@ -375,13 +383,13 @@ describe('util/stats', () => { const [traceData, traceMsg] = logger.logger.trace.mock.calls[0]; expect(traceMsg).toBe('HTTP full stats'); expect(traceData).toEqual({ - allRequests: [ + rawRequests: [ 'GET https://example.com/bar 200 400 40', 'GET https://example.com/foo 200 100 10', 'GET https://example.com/foo 200 200 20', 'GET https://example.com/foo 404 800 80', ], - requestsByHost: { + hostRequests: { 'example.com': [ { method: 'GET', @@ -418,25 +426,35 @@ describe('util/stats', () => { expect(logger.logger.debug).toHaveBeenCalledTimes(1); const [debugData, debugMsg] = logger.logger.debug.mock.calls[0]; expect(debugMsg).toBe('HTTP stats'); - expect(debugData).toEqual({ - statsByHost: { - 'example.com': { - count: 4, - queueAvgMs: 38, - queueMaxMs: 80, - queueMedianMs: 40, - reqAvgMs: 375, - reqMaxMs: 800, - reqMedianMs: 400, + expect(debugData).toEqual( + { + "hosts": { + "example.com": { + "count": 4, + "queueAvgMs": 38, + "queueMaxMs": 80, + "queueMedianMs": 40, + "reqAvgMs": 375, + "reqMaxMs": 800, + "reqMedianMs": 400, + }, }, - }, - totalRequests: 4, - urlCounts: { - 'https://example.com/bar (GET, 200)': 1, - 'https://example.com/foo (GET, 200)': 2, - 'https://example.com/foo (GET, 404)': 1, - }, - }); + "requests": 4, + "urls": { + "https://example.com/bar": { + "GET": { + "200": 1, + }, + }, + "https://example.com/foo": { + "GET": { + "200": 2, + "404": 1, + }, + }, + }, + } + ); }); }); }); diff --git a/lib/util/stats.ts b/lib/util/stats.ts index 25e1451a2a6d43..3f883fa154c6a7 100644 --- a/lib/util/stats.ts +++ b/lib/util/stats.ts @@ -123,12 +123,18 @@ interface HostStatsData { queueMaxMs: number; } +// url -> method -> status -> count +type UrlHttpStat = Record>>; + interface HttpStatsCollection { - urlCounts: Record; - allRequests: string[]; - requestsByHost: Record; - statsByHost: Record; - totalRequests: number; + // debug data + urls: UrlHttpStat; + hosts: Record; + requests: number; + + // trace data + rawRequests: string[]; + hostRequests: Record; } export class HttpStats { @@ -162,9 +168,11 @@ export class HttpStats { static getReport(): HttpStatsCollection { const dataPoints = HttpStats.getDataPoints(); - const urlCounts: Record = {}; - const allRequests: string[] = []; - const requestsByHost: Record = {}; + const requests = dataPoints.length; + + const urls: UrlHttpStat = {}; + const rawRequests: string[] = []; + const hostRequests: Record = {}; for (const dataPoint of dataPoints) { const { url, reqMs, queueMs, status } = dataPoint; @@ -178,30 +186,29 @@ export class HttpStats { const { hostname, origin, pathname } = parsedUrl; const baseUrl = `${origin}${pathname}`; - const urlKey = `${baseUrl} (${method}, ${status})`; - urlCounts[urlKey] ??= 0; - urlCounts[urlKey] += 1; + urls[baseUrl] ??= {}; + urls[baseUrl][method] ??= {}; + urls[baseUrl][method][status] ??= 0; + urls[baseUrl][method][status] += 1; - allRequests.push(`${method} ${url} ${status} ${reqMs} ${queueMs}`); + rawRequests.push(`${method} ${url} ${status} ${reqMs} ${queueMs}`); - requestsByHost[hostname] ??= []; - requestsByHost[hostname].push(dataPoint); + hostRequests[hostname] ??= []; + hostRequests[hostname].push(dataPoint); } - const statsByHost: Record = {}; + const hosts: Record = {}; - let totalRequests = 0; - for (const [hostname, requests] of Object.entries(requestsByHost)) { - const count = requests.length; - totalRequests += count; + for (const [hostname, dataPoints] of Object.entries(hostRequests)) { + const count = dataPoints.length; - const reqTimes = requests.map((r) => r.reqMs); - const queueTimes = requests.map((r) => r.queueMs); + const reqTimes = dataPoints.map((r) => r.reqMs); + const queueTimes = dataPoints.map((r) => r.queueMs); const reqReport = makeTimingReport(reqTimes); const queueReport = makeTimingReport(queueTimes); - statsByHost[hostname] = { + hosts[hostname] = { count, reqAvgMs: reqReport.avgMs, reqMedianMs: reqReport.medianMs, @@ -213,23 +220,18 @@ export class HttpStats { } return { - urlCounts, - allRequests, - requestsByHost, - statsByHost, - totalRequests, + urls, + rawRequests, + hostRequests, + hosts, + requests, }; } static report(): void { - const { - urlCounts, - allRequests, - requestsByHost, - statsByHost, - totalRequests, - } = HttpStats.getReport(); - logger.trace({ allRequests, requestsByHost }, 'HTTP full stats'); - logger.debug({ urlCounts, statsByHost, totalRequests }, 'HTTP stats'); + const { urls, rawRequests, hostRequests, hosts, requests } = + HttpStats.getReport(); + logger.trace({ rawRequests, hostRequests }, 'HTTP full stats'); + logger.debug({ urls, hosts, requests }, 'HTTP stats'); } } From d5eed26a497bfbfe4d3ec477ea96495c25e1080b Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 15 Mar 2024 08:27:09 -0300 Subject: [PATCH 6/8] Use word "statistics" consistently + prettier fix --- lib/util/stats.spec.ts | 50 ++++++++++++++++++++---------------------- lib/util/stats.ts | 4 ++-- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/lib/util/stats.spec.ts b/lib/util/stats.spec.ts index 3cca1e79fabaf1..e69be32a3d4f08 100644 --- a/lib/util/stats.spec.ts +++ b/lib/util/stats.spec.ts @@ -425,36 +425,34 @@ describe('util/stats', () => { expect(logger.logger.debug).toHaveBeenCalledTimes(1); const [debugData, debugMsg] = logger.logger.debug.mock.calls[0]; - expect(debugMsg).toBe('HTTP stats'); - expect(debugData).toEqual( - { - "hosts": { - "example.com": { - "count": 4, - "queueAvgMs": 38, - "queueMaxMs": 80, - "queueMedianMs": 40, - "reqAvgMs": 375, - "reqMaxMs": 800, - "reqMedianMs": 400, - }, + expect(debugMsg).toBe('HTTP statistics'); + expect(debugData).toEqual({ + hosts: { + 'example.com': { + count: 4, + queueAvgMs: 38, + queueMaxMs: 80, + queueMedianMs: 40, + reqAvgMs: 375, + reqMaxMs: 800, + reqMedianMs: 400, }, - "requests": 4, - "urls": { - "https://example.com/bar": { - "GET": { - "200": 1, - }, + }, + requests: 4, + urls: { + 'https://example.com/bar': { + GET: { + '200': 1, }, - "https://example.com/foo": { - "GET": { - "200": 2, - "404": 1, - }, + }, + 'https://example.com/foo': { + GET: { + '200': 2, + '404': 1, }, }, - } - ); + }, + }); }); }); }); diff --git a/lib/util/stats.ts b/lib/util/stats.ts index 3f883fa154c6a7..6508f49d834551 100644 --- a/lib/util/stats.ts +++ b/lib/util/stats.ts @@ -231,7 +231,7 @@ export class HttpStats { static report(): void { const { urls, rawRequests, hostRequests, hosts, requests } = HttpStats.getReport(); - logger.trace({ rawRequests, hostRequests }, 'HTTP full stats'); - logger.debug({ urls, hosts, requests }, 'HTTP stats'); + logger.trace({ rawRequests, hostRequests }, 'HTTP full statistics'); + logger.debug({ urls, hosts, requests }, 'HTTP statistics'); } } From 0a63c4bbb1dc0dbe7b7e9c9a0446b94b79c9d0ce Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 15 Mar 2024 08:28:56 -0300 Subject: [PATCH 7/8] Fix --- lib/util/stats.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/util/stats.spec.ts b/lib/util/stats.spec.ts index e69be32a3d4f08..4814b2dfddea7e 100644 --- a/lib/util/stats.spec.ts +++ b/lib/util/stats.spec.ts @@ -5,9 +5,9 @@ import { LookupStats, PackageCacheStats, makeTimingReport, -} from './stats'; +} from './statistics'; -describe('util/stats', () => { +describe('util/statistics', () => { beforeEach(() => { memCache.init(); }); @@ -381,7 +381,7 @@ describe('util/stats', () => { expect(logger.logger.trace).toHaveBeenCalledTimes(1); const [traceData, traceMsg] = logger.logger.trace.mock.calls[0]; - expect(traceMsg).toBe('HTTP full stats'); + expect(traceMsg).toBe('HTTP full statistics'); expect(traceData).toEqual({ rawRequests: [ 'GET https://example.com/bar 200 400 40', From 8e9295fefa6451b55903cf85fc9dacc81cba953a Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 15 Mar 2024 08:31:27 -0300 Subject: [PATCH 8/8] Fix test --- lib/util/stats.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/util/stats.spec.ts b/lib/util/stats.spec.ts index 4814b2dfddea7e..f23bfe35d6e3a1 100644 --- a/lib/util/stats.spec.ts +++ b/lib/util/stats.spec.ts @@ -5,9 +5,9 @@ import { LookupStats, PackageCacheStats, makeTimingReport, -} from './statistics'; +} from './stats'; -describe('util/statistics', () => { +describe('util/stats', () => { beforeEach(() => { memCache.init(); });