From f5ab2a9b5e9ef2b04727aee91de5e43c99bc0161 Mon Sep 17 00:00:00 2001 From: Hans Ott Date: Fri, 16 May 2025 10:37:57 +0200 Subject: [PATCH] Remove compressedTimings It's a bit much per operation --- library/agent/Agent.ts | 10 +-- library/agent/InspectionStatistics.test.ts | 93 ++-------------------- library/agent/InspectionStatistics.ts | 90 +-------------------- library/agent/api/Event.ts | 5 -- library/helpers/percentiles.test.ts | 70 ---------------- library/helpers/percentiles.ts | 35 -------- library/sources/Lambda.test.ts | 13 --- 7 files changed, 10 insertions(+), 306 deletions(-) delete mode 100644 library/helpers/percentiles.test.ts delete mode 100644 library/helpers/percentiles.ts diff --git a/library/agent/Agent.ts b/library/agent/Agent.ts index 2846a4d6f..dc296cf93 100644 --- a/library/agent/Agent.ts +++ b/library/agent/Agent.ts @@ -52,10 +52,7 @@ export class Agent { ); private routes: Routes = new Routes(200); private rateLimiter: RateLimiter = new RateLimiter(5000, 120 * 60 * 1000); - private statistics = new InspectionStatistics({ - maxPerfSamplesInMemory: 5000, - maxCompressedStatsInMemory: 100, - }); + private statistics = new InspectionStatistics(); private middlewareInstalled = false; private attackLogger = new AttackLogger(1000); @@ -350,12 +347,10 @@ export class Agent { const now = performance.now(); const diff = now - this.lastHeartbeat; const shouldSendHeartbeat = diff > this.sendHeartbeatEveryMS; - const hasCompressedStats = this.statistics.hasCompressedStats(); const canSendInitialStats = !this.serviceConfig.hasReceivedAnyStats() && !this.statistics.isEmpty(); const shouldReportInitialStats = - !this.reportedInitialStats && - (hasCompressedStats || canSendInitialStats); + !this.reportedInitialStats && canSendInitialStats; if (shouldSendHeartbeat || shouldReportInitialStats) { this.heartbeat(); @@ -544,7 +539,6 @@ export class Agent { } async flushStats(timeoutInMS: number) { - this.statistics.forceCompress(); await this.sendHeartbeat(timeoutInMS); } diff --git a/library/agent/InspectionStatistics.test.ts b/library/agent/InspectionStatistics.test.ts index fc218b3a0..262b8cf8a 100644 --- a/library/agent/InspectionStatistics.test.ts +++ b/library/agent/InspectionStatistics.test.ts @@ -5,10 +5,7 @@ import { InspectionStatistics } from "./InspectionStatistics"; t.test("it resets stats", async () => { const clock = FakeTimers.install(); - const stats = new InspectionStatistics({ - maxPerfSamplesInMemory: 50, - maxCompressedStatsInMemory: 5, - }); + const stats = new InspectionStatistics(); stats.onInspectedCall({ withoutContext: false, @@ -28,7 +25,6 @@ t.test("it resets stats", async () => { interceptorThrewError: 0, withoutContext: 0, total: 1, - compressedTimings: [], }, }, startedAt: 0, @@ -65,10 +61,7 @@ t.test("it keeps track of amount of calls", async () => { const maxPerfSamplesInMemory = 50; const maxCompressedStatsInMemory = 5; - const stats = new InspectionStatistics({ - maxPerfSamplesInMemory: maxPerfSamplesInMemory, - maxCompressedStatsInMemory: maxCompressedStatsInMemory, - }); + const stats = new InspectionStatistics(); t.same(stats.getStats(), { sinks: {}, @@ -101,7 +94,6 @@ t.test("it keeps track of amount of calls", async () => { interceptorThrewError: 0, withoutContext: 0, total: 1, - compressedTimings: [], }, }, startedAt: 0, @@ -133,7 +125,6 @@ t.test("it keeps track of amount of calls", async () => { interceptorThrewError: 0, withoutContext: 1, total: 2, - compressedTimings: [], }, }, startedAt: 0, @@ -159,7 +150,6 @@ t.test("it keeps track of amount of calls", async () => { interceptorThrewError: 1, withoutContext: 1, total: 3, - compressedTimings: [], }, }, startedAt: 0, @@ -191,7 +181,6 @@ t.test("it keeps track of amount of calls", async () => { interceptorThrewError: 1, withoutContext: 1, total: 4, - compressedTimings: [], }, }, startedAt: 0, @@ -223,7 +212,6 @@ t.test("it keeps track of amount of calls", async () => { interceptorThrewError: 1, withoutContext: 1, total: 5, - compressedTimings: [], }, }, startedAt: 0, @@ -237,8 +225,6 @@ t.test("it keeps track of amount of calls", async () => { }, }); - t.same(stats.hasCompressedStats(), false); - clock.tick(1000); for (let i = 0; i < maxPerfSamplesInMemory; i++) { @@ -251,7 +237,6 @@ t.test("it keeps track of amount of calls", async () => { }); } - t.same(stats.hasCompressedStats(), true); t.same(stats.getStats(), { sinks: { mongodb: { @@ -262,19 +247,6 @@ t.test("it keeps track of amount of calls", async () => { interceptorThrewError: 1, withoutContext: 1, total: 55, - compressedTimings: [ - { - averageInMS: 2.1719999999999997, - percentiles: { - "50": 2.1, - "75": 3.4000000000000004, - "90": 4.1000000000000005, - "95": 4.4, - "99": 4.6000000000000005, - }, - compressedAt: 1000, - }, - ], }, }, startedAt: 0, @@ -288,9 +260,6 @@ t.test("it keeps track of amount of calls", async () => { }, }); - // @ts-expect-error Stats is private - t.ok(stats.stats.mongodb.durations.length < maxPerfSamplesInMemory); - for ( let i = 0; i < maxPerfSamplesInMemory * maxCompressedStatsInMemory * 2; @@ -305,22 +274,13 @@ t.test("it keeps track of amount of calls", async () => { }); } - t.same( - // @ts-expect-error Stats is private - stats.stats.mongodb.compressedTimings.length, - maxCompressedStatsInMemory - ); - clock.uninstall(); }); t.test("it keeps track of requests", async () => { const clock = FakeTimers.install(); - const stats = new InspectionStatistics({ - maxPerfSamplesInMemory: 50, - maxCompressedStatsInMemory: 5, - }); + const stats = new InspectionStatistics(); t.same(stats.getStats(), { sinks: {}, @@ -402,53 +362,10 @@ t.test("it keeps track of requests", async () => { clock.uninstall(); }); -t.test("it force compresses stats", async () => { - const clock = FakeTimers.install(); - - const stats = new InspectionStatistics({ - maxPerfSamplesInMemory: 50, - maxCompressedStatsInMemory: 5, - }); - - t.same(stats.getStats(), { - sinks: {}, - startedAt: 0, - requests: { - total: 0, - aborted: 0, - attacksDetected: { - total: 0, - blocked: 0, - }, - }, - }); - - stats.onRequest(); - - stats.onInspectedCall({ - withoutContext: false, - sink: "mongodb", - blocked: false, - durationInMs: 0.1, - attackDetected: false, - }); - - t.same(stats.hasCompressedStats(), false); - - stats.forceCompress(); - - t.same(stats.hasCompressedStats(), true); - - clock.uninstall(); -}); - t.test("it keeps track of aborted requests", async () => { const clock = FakeTimers.install(); - const stats = new InspectionStatistics({ - maxPerfSamplesInMemory: 50, - maxCompressedStatsInMemory: 5, - }); + const stats = new InspectionStatistics(); stats.onAbortedRequest(); @@ -464,4 +381,6 @@ t.test("it keeps track of aborted requests", async () => { }, }, }); + + clock.uninstall(); }); diff --git a/library/agent/InspectionStatistics.ts b/library/agent/InspectionStatistics.ts index ad2076bd5..597c62211 100644 --- a/library/agent/InspectionStatistics.ts +++ b/library/agent/InspectionStatistics.ts @@ -1,18 +1,6 @@ -import { percentiles } from "../helpers/percentiles"; - -type SinkCompressedTimings = { - averageInMS: number; - percentiles: Record; - compressedAt: number; -}; - type SinkStats = { withoutContext: number; total: number; - // array where we accumulate durations for each sink-request (e.g. mysql.query) - durations: number[]; - // array where we put compressed blocks of stats - compressedTimings: SinkCompressedTimings[]; interceptorThrewError: number; attacksDetected: { total: number; @@ -25,8 +13,6 @@ type SinkStatsWithoutTimings = Omit; export class InspectionStatistics { private startedAt = Date.now(); private stats: Record = {}; - private readonly maxPerfSamplesInMemory: number; - private readonly maxCompressedStatsInMemory: number; private requests: { total: number; aborted: number; @@ -36,23 +22,6 @@ export class InspectionStatistics { }; } = { total: 0, aborted: 0, attacksDetected: { total: 0, blocked: 0 } }; - constructor({ - maxPerfSamplesInMemory, - maxCompressedStatsInMemory, - }: { - maxPerfSamplesInMemory: number; - maxCompressedStatsInMemory: number; - }) { - this.maxPerfSamplesInMemory = maxPerfSamplesInMemory; - this.maxCompressedStatsInMemory = maxCompressedStatsInMemory; - } - - hasCompressedStats() { - return Object.values(this.stats).some( - (sinkStats) => sinkStats.compressedTimings.length > 0 - ); - } - isEmpty() { return ( this.requests.total === 0 && @@ -94,7 +63,6 @@ export class InspectionStatistics { }, interceptorThrewError: sinkStats.interceptorThrewError, withoutContext: sinkStats.withoutContext, - compressedTimings: sinkStats.compressedTimings, }; } @@ -110,8 +78,6 @@ export class InspectionStatistics { this.stats[sink] = { withoutContext: 0, total: 0, - durations: [], - compressedTimings: [], interceptorThrewError: 0, attacksDetected: { total: 0, @@ -121,48 +87,6 @@ export class InspectionStatistics { } } - private compressPerfSamples(sink: string) { - /* c8 ignore start */ - if (!this.stats[sink]) { - return; - } - - if (this.stats[sink].durations.length === 0) { - return; - } - /* c8 ignore stop */ - - const timings = this.stats[sink].durations; - const averageInMS = - timings.reduce((acc, curr) => acc + curr, 0) / timings.length; - - const [p50, p75, p90, p95, p99] = percentiles( - [50, 75, 90, 95, 99], - timings - ); - - this.stats[sink].compressedTimings.push({ - averageInMS, - percentiles: { - "50": p50, - "75": p75, - "90": p90, - "95": p95, - "99": p99, - }, - compressedAt: Date.now(), - }); - - if ( - this.stats[sink].compressedTimings.length > - this.maxCompressedStatsInMemory - ) { - this.stats[sink].compressedTimings.shift(); - } - - this.stats[sink].durations = []; - } - interceptorThrewError(sink: string) { this.ensureSinkStats(sink); this.stats[sink].total += 1; @@ -188,6 +112,8 @@ export class InspectionStatistics { sink, blocked, attackDetected, + // Let's remove later + // eslint-disable-next-line @typescript-eslint/no-unused-vars durationInMs, withoutContext, }: { @@ -206,12 +132,6 @@ export class InspectionStatistics { return; } - if (this.stats[sink].durations.length >= this.maxPerfSamplesInMemory) { - this.compressPerfSamples(sink); - } - - this.stats[sink].durations.push(durationInMs); - if (attackDetected) { this.stats[sink].attacksDetected.total += 1; if (blocked) { @@ -219,10 +139,4 @@ export class InspectionStatistics { } } } - - forceCompress() { - for (const sink in this.stats) { - this.compressPerfSamples(sink); - } - } } diff --git a/library/agent/api/Event.ts b/library/agent/api/Event.ts index f06c842f3..f97f4bc7b 100644 --- a/library/agent/api/Event.ts +++ b/library/agent/api/Event.ts @@ -72,11 +72,6 @@ type MonitoredSinkStats = { interceptorThrewError: number; withoutContext: number; total: number; - compressedTimings: { - averageInMS: number; - percentiles: Record; - compressedAt: number; - }[]; }; type Heartbeat = { diff --git a/library/helpers/percentiles.test.ts b/library/helpers/percentiles.test.ts deleted file mode 100644 index b30263b2d..000000000 --- a/library/helpers/percentiles.test.ts +++ /dev/null @@ -1,70 +0,0 @@ -import * as t from "tap"; -import { percentiles } from "./percentiles"; - -function generateArray( - length: number, - fn: (value: unknown, index: number) => number -) { - return Array.from({ length: length }).map(fn); -} - -function generateArraySimple(length: number) { - return generateArray(length, (v, i) => i + 1); -} - -function shuffleArray(arr: number[]) { - return arr.sort(() => 0.5 - Math.random()); -} - -const stubsSimple = [ - { percentile: 0, list: shuffleArray(generateArraySimple(100)), result: 1 }, - { percentile: 25, list: shuffleArray(generateArraySimple(100)), result: 25 }, - { percentile: 50, list: shuffleArray(generateArraySimple(100)), result: 50 }, - { percentile: 75, list: shuffleArray(generateArraySimple(100)), result: 75 }, - { - percentile: 100, - list: shuffleArray(generateArraySimple(100)), - result: 100, - }, - { - percentile: 75, - list: shuffleArray( - generateArraySimple(100).concat(generateArraySimple(30)) - ), - result: 68, - }, -]; - -t.test("percentile of simple values", async (t) => { - stubsSimple.forEach((stub) => { - t.same( - percentiles([stub.percentile], stub.list), - [stub.result], - JSON.stringify(stub) - ); - }); -}); - -t.test("percentile with negative values", async (t) => { - t.same(percentiles([50], shuffleArray([-1, -2, -3, -4, -5])), [-3]); - t.same(percentiles([50], shuffleArray([7, 6, -1, -2, -3, -4, -5])), [-2]); -}); - -t.test("array of percentiles", async (t) => { - t.same( - percentiles([0, 25, 50, 75, 100], shuffleArray(generateArraySimple(100))), - [1, 25, 50, 75, 100] - ); -}); - -t.test("throw an error if less than 0", async (t) => { - t.throws(() => percentiles([-1], [1])); -}); - -t.test("throw an error if grater than 100", async (t) => { - t.throws(() => percentiles([101], [1])); -}); - -t.test("empty list", async (t) => { - t.throws(() => percentiles([50], [])); -}); diff --git a/library/helpers/percentiles.ts b/library/helpers/percentiles.ts deleted file mode 100644 index 1c4a0107e..000000000 --- a/library/helpers/percentiles.ts +++ /dev/null @@ -1,35 +0,0 @@ -type NumberList = Array; - -export function percentiles(percentiles: number[], list: NumberList): number[] { - if (list.length === 0) { - throw new Error("List should not be empty"); - } - - percentiles.forEach((p) => { - if (p < 0) { - throw new Error( - `Expect percentile to be >= 0 but given "${p}" and its type is "${typeof p}".` - ); - } - - if (p > 100) { - throw new Error( - `Expect percentile to be <= 100 but given "${p}" and its type is "${typeof p}".` - ); - } - }); - - const sortedList: number[] = Array.from(list).sort((a, b) => a - b); - - return percentiles.map((p) => getPercentileValue(p, sortedList)); -} - -function getPercentileValue(p: number, list: number[]): number { - if (p === 0) { - return list[0]; - } - - const kIndex = Math.ceil(list.length * (p / 100)) - 1; - - return list[kIndex]; -} diff --git a/library/sources/Lambda.test.ts b/library/sources/Lambda.test.ts index 91e3c1a68..9cf404e24 100644 --- a/library/sources/Lambda.test.ts +++ b/library/sources/Lambda.test.ts @@ -279,19 +279,6 @@ t.test("it sends heartbeat after first and every 10 minutes", async () => { }, interceptorThrewError: 0, withoutContext: 0, - compressedTimings: [ - { - averageInMS: 0.09999999999999981, - percentiles: { - 50: 0.1, - 75: 0.1, - 90: 0.1, - 95: 0.1, - 99: 0.1, - }, - compressedAt: 60 * 1000 * 10 + 1, - }, - ], }, }, startedAt: 0,