From 6194b63633725c3500676ec4df69797127af6a9f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 18 Oct 2023 14:55:02 +0200 Subject: [PATCH] feat(node): Remove `lru_map` dependency --- packages/deno/package.json | 4 +- packages/deno/rollup.config.js | 2 - .../deno/src/integrations/contextlines.ts | 3 +- packages/node/package.json | 1 - .../node/src/integrations/contextlines.ts | 3 +- packages/node/src/integrations/http.ts | 2 +- .../node/src/integrations/localvariables.ts | 7 +-- .../node/src/integrations/undici/index.ts | 2 +- .../test/integrations/localvariables.test.ts | 14 +---- packages/utils/src/index.ts | 1 + packages/utils/src/lru.ts | 60 +++++++++++++++++++ packages/utils/test/lru.test.ts | 41 +++++++++++++ yarn.lock | 17 ------ 13 files changed, 113 insertions(+), 44 deletions(-) create mode 100644 packages/utils/src/lru.ts create mode 100644 packages/utils/test/lru.test.ts diff --git a/packages/deno/package.json b/packages/deno/package.json index 75f8526efca4..4e875141a229 100644 --- a/packages/deno/package.json +++ b/packages/deno/package.json @@ -21,11 +21,9 @@ "@sentry/browser": "7.74.1", "@sentry/core": "7.74.1", "@sentry/types": "7.74.1", - "@sentry/utils": "7.74.1", - "lru_map": "^0.3.3" + "@sentry/utils": "7.74.1" }, "devDependencies": { - "@rollup/plugin-commonjs": "^25.0.5", "@rollup/plugin-typescript": "^11.1.5", "@types/node": "20.8.2", "rollup-plugin-dts": "^6.1.0" diff --git a/packages/deno/rollup.config.js b/packages/deno/rollup.config.js index 236501153f8b..d79b77478053 100644 --- a/packages/deno/rollup.config.js +++ b/packages/deno/rollup.config.js @@ -1,6 +1,5 @@ // @ts-check import nodeResolve from '@rollup/plugin-node-resolve'; -import commonjs from '@rollup/plugin-commonjs'; import sucrase from '@rollup/plugin-sucrase'; import { defineConfig } from 'rollup'; @@ -21,7 +20,6 @@ export default defineConfig({ nodeResolve({ extensions: ['.mjs', '.js', '.json', '.node', '.ts', '.tsx'], }), - commonjs(), sucrase({ transforms: ['typescript'] }), ], }); diff --git a/packages/deno/src/integrations/contextlines.ts b/packages/deno/src/integrations/contextlines.ts index 47cd3a09218d..ab6db13f5ef2 100644 --- a/packages/deno/src/integrations/contextlines.ts +++ b/packages/deno/src/integrations/contextlines.ts @@ -1,6 +1,5 @@ import type { Event, EventProcessor, Integration, StackFrame } from '@sentry/types'; -import { addContextToFrame } from '@sentry/utils'; -import { LRUMap } from 'lru_map'; +import { addContextToFrame, LRUMap } from '@sentry/utils'; const FILE_CONTENT_CACHE = new LRUMap(100); const DEFAULT_LINES_OF_CONTEXT = 7; diff --git a/packages/node/package.json b/packages/node/package.json index 4e2a32d8c858..9d99aecd0b5b 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -29,7 +29,6 @@ "@sentry/utils": "7.74.1", "cookie": "^0.5.0", "https-proxy-agent": "^5.0.0", - "lru_map": "^0.3.3", "tslib": "^2.4.1 || ^1.9.3" }, "devDependencies": { diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts index d1c2056c0c66..f47e31eb268a 100644 --- a/packages/node/src/integrations/contextlines.ts +++ b/packages/node/src/integrations/contextlines.ts @@ -1,7 +1,6 @@ import type { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types'; -import { addContextToFrame } from '@sentry/utils'; +import { addContextToFrame, LRUMap } from '@sentry/utils'; import { readFile } from 'fs'; -import { LRUMap } from 'lru_map'; const FILE_CONTENT_CACHE = new LRUMap(100); const DEFAULT_LINES_OF_CONTEXT = 7; diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 1f4b5c9c9224..4fd2e702bb44 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -12,11 +12,11 @@ import { fill, generateSentryTraceHeader, logger, + LRUMap, stringMatchesSomePattern, } from '@sentry/utils'; import type * as http from 'http'; import type * as https from 'https'; -import { LRUMap } from 'lru_map'; import type { NodeClient } from '../client'; import { NODE_VERSION } from '../nodeVersion'; diff --git a/packages/node/src/integrations/localvariables.ts b/packages/node/src/integrations/localvariables.ts index 39f250c23778..5ac70db4a839 100644 --- a/packages/node/src/integrations/localvariables.ts +++ b/packages/node/src/integrations/localvariables.ts @@ -1,8 +1,7 @@ /* eslint-disable max-lines */ import type { Event, EventProcessor, Exception, Hub, Integration, StackFrame, StackParser } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { logger, LRUMap } from '@sentry/utils'; import type { Debugger, InspectorNotification, Runtime, Session } from 'inspector'; -import { LRUMap } from 'lru_map'; import { NODE_VERSION } from '../nodeVersion'; import type { NodeClientOptions } from '../types'; @@ -470,8 +469,8 @@ export class LocalVariables implements Integration { } // Check if we have local variables for an exception that matches the hash - // delete is identical to get but also removes the entry from the cache - const cachedFrames = this._cachedFrames.delete(hash); + // remove is identical to get but also removes the entry from the cache + const cachedFrames = this._cachedFrames.remove(hash); if (cachedFrames === undefined) { return; diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts index 25888780a30c..ff08d1df0f65 100644 --- a/packages/node/src/integrations/undici/index.ts +++ b/packages/node/src/integrations/undici/index.ts @@ -5,10 +5,10 @@ import { dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, getSanitizedUrlString, + LRUMap, parseUrl, stringMatchesSomePattern, } from '@sentry/utils'; -import { LRUMap } from 'lru_map'; import type { NodeClient } from '../../client'; import { NODE_VERSION } from '../../nodeVersion'; diff --git a/packages/node/test/integrations/localvariables.test.ts b/packages/node/test/integrations/localvariables.test.ts index 55e179c879e6..f48666658e79 100644 --- a/packages/node/test/integrations/localvariables.test.ts +++ b/packages/node/test/integrations/localvariables.test.ts @@ -1,6 +1,6 @@ import type { ClientOptions, EventProcessor } from '@sentry/types'; +import type { LRUMap } from '@sentry/utils'; import type { Debugger, InspectorNotification } from 'inspector'; -import type { LRUMap } from 'lru_map'; import { defaultStackParser } from '../../src'; import type { DebugSession, FrameVariables } from '../../src/integrations/localvariables'; @@ -178,11 +178,7 @@ describeIf((NODE_VERSION.major || 0) >= 18)('LocalVariables', () => { expect((localVariables as unknown as LocalVariablesPrivate)._cachedFrames.size).toBe(1); - let frames: FrameVariables[] | undefined; - - (localVariables as unknown as LocalVariablesPrivate)._cachedFrames.forEach(f => { - frames = f; - }); + const frames: FrameVariables[] = (localVariables as unknown as LocalVariablesPrivate)._cachedFrames.values()[0]; expect(frames).toBeDefined(); @@ -274,11 +270,7 @@ describeIf((NODE_VERSION.major || 0) >= 18)('LocalVariables', () => { expect((localVariables as unknown as LocalVariablesPrivate)._cachedFrames.size).toBe(1); - let frames: FrameVariables[] | undefined; - - (localVariables as unknown as LocalVariablesPrivate)._cachedFrames.forEach(f => { - frames = f; - }); + const frames: FrameVariables[] = (localVariables as unknown as LocalVariablesPrivate)._cachedFrames.values()[0]; expect(frames).toBeDefined(); diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 81f4d947cd0d..5bb54c52b9cd 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -32,3 +32,4 @@ export * from './userIntegrations'; export * from './cache'; export * from './eventbuilder'; export * from './anr'; +export * from './lru'; diff --git a/packages/utils/src/lru.ts b/packages/utils/src/lru.ts new file mode 100644 index 000000000000..2a3b7bfc8ac0 --- /dev/null +++ b/packages/utils/src/lru.ts @@ -0,0 +1,60 @@ +/** A simple Least Recently Used map */ +export class LRUMap { + private readonly _cache: Map; + + public constructor(private readonly _maxSize: number) { + this._cache = new Map(); + } + + /** Get the current size of the cache */ + public get size(): number { + return this._cache.size; + } + + /** Get an entry or undefined if it was not in the cache. Re-inserts to update the recently used order */ + public get(key: K): V | undefined { + const value = this._cache.get(key); + if (value === undefined) { + return undefined; + } + // Remove and re-insert to update the order + this._cache.delete(key); + this._cache.set(key, value); + return value; + } + + /** Insert an entry and evict an older entry if we've reached maxSize */ + public set(key: K, value: V): void { + if (this._cache.size >= this._maxSize) { + // keys() returns an iterator in insertion order so keys().next() gives us the oldest key + this._cache.delete(this._cache.keys().next().value); + } + this._cache.set(key, value); + } + + /** Remove an entry and return the entry if it was in the cache */ + public remove(key: K): V | undefined { + const value = this._cache.get(key); + if (value) { + this._cache.delete(key); + } + return value; + } + + /** Clear all entries */ + public clear(): void { + this._cache.clear(); + } + + /** Get all the keys */ + public keys(): Array { + return Array.from(this._cache.keys()); + } + + /** Get all the values */ + public values(): Array { + const values: V[] = []; + this._cache.forEach(value => values.push(value)); + return values; + } +} diff --git a/packages/utils/test/lru.test.ts b/packages/utils/test/lru.test.ts new file mode 100644 index 000000000000..15f638e9cfa1 --- /dev/null +++ b/packages/utils/test/lru.test.ts @@ -0,0 +1,41 @@ +import { LRUMap } from '../src/lru'; + +describe('LRUMap', () => { + test('evicts older entries when reaching max size', () => { + const map = new LRUMap(3); + map.set('a', '1'); + map.set('b', '2'); + map.set('c', '3'); + map.set('d', '4'); + map.set('e', '5'); + + expect(map.keys()).toEqual(['c', 'd', 'e']); + }); + + test('updates last used when calling get', () => { + const map = new LRUMap(3); + map.set('a', '1'); + map.set('b', '2'); + map.set('c', '3'); + + map.get('a'); + + map.set('d', '4'); + map.set('e', '5'); + + expect(map.keys()).toEqual(['a', 'd', 'e']); + }); + + test('removes and returns entry', () => { + const map = new LRUMap(3); + map.set('a', '1'); + map.set('b', '2'); + map.set('c', '3'); + map.set('d', '4'); + map.set('e', '5'); + + expect(map.remove('c')).toEqual('3'); + + expect(map.keys()).toEqual(['d', 'e']); + }); +}); diff --git a/yarn.lock b/yarn.lock index e4e1855b1d18..5d2abce2c168 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4800,18 +4800,6 @@ magic-string "^0.25.7" resolve "^1.17.0" -"@rollup/plugin-commonjs@^25.0.5": - version "25.0.5" - resolved "https://registry.yarnpkg.com/@rollup/plugin-commonjs/-/plugin-commonjs-25.0.5.tgz#0bac8f985a5de151b4b09338847f8c7f20a28a29" - integrity sha512-xY8r/A9oisSeSuLCTfhssyDjo9Vp/eDiRLXkg1MXCcEEgEjPmLU+ZyDB20OOD0NlyDa/8SGbK5uIggF5XTx77w== - dependencies: - "@rollup/pluginutils" "^5.0.1" - commondir "^1.0.1" - estree-walker "^2.0.2" - glob "^8.0.3" - is-reference "1.2.1" - magic-string "^0.27.0" - "@rollup/plugin-json@^4.0.0", "@rollup/plugin-json@^4.1.0": version "4.1.0" resolved "https://registry.yarnpkg.com/@rollup/plugin-json/-/plugin-json-4.1.0.tgz#54e09867ae6963c593844d8bd7a9c718294496f3" @@ -19997,11 +19985,6 @@ lru-cache@^9.0.0: resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-10.0.0.tgz#b9e2a6a72a129d81ab317202d93c7691df727e61" integrity sha512-svTf/fzsKHffP42sujkO/Rjs37BCIsQVRCeNYIm9WN8rgT7ffoUnRtZCqU+6BqcSBdv8gwJeTz8knJpgACeQMw== -lru_map@^0.3.3: - version "0.3.3" - resolved "https://registry.yarnpkg.com/lru_map/-/lru_map-0.3.3.tgz#b5c8351b9464cbd750335a79650a0ec0e56118dd" - integrity sha1-tcg1G5Rky9dQM1p5ZQoOwOVhGN0= - lunr@^2.3.8: version "2.3.9" resolved "https://registry.yarnpkg.com/lunr/-/lunr-2.3.9.tgz#18b123142832337dd6e964df1a5a7707b25d35e1"