Skip to content

Commit

Permalink
Use LRUCache in scope and init new pkg for flag accessor methods
Browse files Browse the repository at this point in the history
  • Loading branch information
aliu39 committed Oct 31, 2024
1 parent bd04755 commit f494720
Show file tree
Hide file tree
Showing 26 changed files with 324 additions and 298 deletions.
41 changes: 9 additions & 32 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,14 @@ import type {
SeverityLevel,
User,
} from '@sentry/types';
import { dateTimestampInSeconds, generatePropagationContext, isPlainObject, logger, uuid4 } from '@sentry/utils';
import {
LRUMap,
dateTimestampInSeconds,
generatePropagationContext,
isPlainObject,
logger,
uuid4,
} from '@sentry/utils';

import { updateSession } from './session';
import { _getSpanForScope, _setSpanForScope } from './utils/spanOnScope';
Expand Down Expand Up @@ -99,7 +106,7 @@ class ScopeClass implements ScopeInterface {
protected _lastEventId?: string;

/** LRU cache of flags last evaluated by a feature flag provider. Used by FF integrations. */
protected _flagBuffer: FeatureFlag[];
protected _flagBuffer: LRUMap<FeatureFlag['flag'], FeatureFlag['result']>;

/** Max size of the flagBuffer */
protected _flagBufferSize: number; // TODO: make const?
Expand Down Expand Up @@ -509,36 +516,6 @@ class ScopeClass implements ScopeInterface {
return this._propagationContext;
}

/**
* @inheritDoc
*/
public getFlags(): FeatureFlag[] {
return this._flagBuffer || [];
}

/**
* @inheritDoc
*/
public insertFlag(name: string, value: boolean): void {
// Check if the flag is already in the buffer
const index = this._flagBuffer.findIndex(f => f.flag === name);

if (index !== -1) {
// Delete flag if it is in the buffer
this._flagBuffer.splice(index, 1);
} else if (this._flagBuffer.length === this._flagBufferSize) {
// If at capacity, we need to remove the earliest flag (pop from front)
// This will only happen if not a duplicate flag
this._flagBuffer.shift();
}

// Push the flag to the end of the queue
this._flagBuffer.push({
flag: name,
result: value,
});
}

/**
* @inheritDoc
*/
Expand Down
2 changes: 2 additions & 0 deletions packages/features/.eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
node_modules/
build/
39 changes: 39 additions & 0 deletions packages/features/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Note: All paths are relative to the directory in which eslint is being run, rather than the directory where this file
// lives

// ESLint config docs: https://eslint.org/docs/user-guide/configuring/

module.exports = {
extends: ['../../.eslintrc.js'],
overrides: [
{
files: ['src/**/*.ts'],
},
{
files: ['test.setup.ts', 'vitest.config.ts'],
parserOptions: {
project: ['tsconfig.test.json'],
},
rules: {
'no-console': 'off',
},
},
{
files: ['test/**/*.ts'],

rules: {
// most of these errors come from `new Promise(process.nextTick)`
'@typescript-eslint/unbound-method': 'off',
// TODO: decide if we want to enable this again after the migration
// We can take the freedom to be a bit more lenient with tests
'@typescript-eslint/no-floating-promises': 'off',
},
},
{
files: ['src/types/deprecated.ts'],
rules: {
'@typescript-eslint/naming-convention': 'off',
},
},
],
};
4 changes: 4 additions & 0 deletions packages/features/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
node_modules
/*.tgz
.eslintcache
build
21 changes: 21 additions & 0 deletions packages/features/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
MIT License

Copyright (c) 2023-2024 Functional Software, Inc. dba Sentry

Permission is hereby granted, free of charge, to any person obtaining a copy of
this software and associated documentation files (the "Software"), to deal in
the Software without restriction, including without limitation the rights to
use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
of the Software, and to permit persons to whom the Software is furnished to do
so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
Empty file added packages/features/README.md
Empty file.
73 changes: 73 additions & 0 deletions packages/features/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
{
"name": "@sentry-internal/features",
"version": "8.35.0",
"description": "Sentry SDK integration for shared feature flag internals",
"repository": "git://github.com/getsentry/sentry-javascript.git",
"homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/features",
"author": "Sentry",
"license": "MIT",
"engines": {
"node": ">=14.18"
},
"files": [
"/build/npm"
],
"main": "build/npm/cjs/index.js",
"module": "build/npm/esm/index.js",
"types": "build/npm/types/index.d.ts",
"exports": {
"./package.json": "./package.json",
".": {
"import": {
"types": "./build/npm/types/index.d.ts",
"default": "./build/npm/esm/index.js"
},
"require": {
"types": "./build/npm/types/index.d.ts",
"default": "./build/npm/cjs/index.js"
}
}
},
"typesVersions": {
"<4.9": {
"build/npm/types/index.d.ts": [
"build/npm/types-ts3.8/index.d.ts"
]
}
},
"publishConfig": {
"access": "public"
},
"dependencies": {
"@sentry/browser": "^8.35.0",
"@sentry/core": "8.35.0",
"@sentry/types": "8.35.0",
"@sentry/utils": "8.35.0"
},
"scripts": {
"build": "run-p build:transpile build:types build:bundle",
"build:transpile": "rollup -c rollup.npm.config.mjs",
"build:bundle": "rollup -c rollup.bundle.config.mjs",
"build:dev": "run-p build:transpile build:types",
"build:types": "run-s build:types:core build:types:downlevel",
"build:types:core": "tsc -p tsconfig.types.json",
"build:types:downlevel": "yarn downlevel-dts build/npm/types build/npm/types-ts3.8 --to ts3.8 && yarn node ./scripts/shim-preact-export.js",
"build:watch": "run-p build:transpile:watch build:bundle:watch build:types:watch",
"build:dev:watch": "run-p build:transpile:watch build:types:watch",
"build:transpile:watch": "yarn build:transpile --watch",
"build:bundle:watch": "yarn build:bundle --watch",
"build:types:watch": "tsc -p tsconfig.types.json --watch",
"build:tarball": "npm pack",
"circularDepCheck": "madge --circular src/index.ts",
"clean": "rimraf build sentry-internal-features-*.tgz",
"fix": "eslint . --format stylish --fix",
"lint": "eslint . --format stylish",
"test": "jest",
"test:watch": "jest --watch",
"yalc:publish": "yalc publish --push --sig"
},
"volta": {
"extends": "../../package.json"
},
"sideEffects": false
}
12 changes: 12 additions & 0 deletions packages/features/rollup.bundle.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { makeBaseBundleConfig, makeBundleConfigVariants } from '@sentry-internal/rollup-utils';

const baseBundleConfig = makeBaseBundleConfig({
bundleType: 'addon',
entrypoints: ['src/index.ts'],
licenseTitle: '@sentry-internal/features',
outputFileBase: () => 'bundles/features',
});

const builds = makeBundleConfigVariants(baseBundleConfig);

export default builds;
19 changes: 19 additions & 0 deletions packages/features/rollup.npm.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils';

export default makeNPMConfigVariants(
makeBaseNPMConfig({
hasBundles: true,
packageSpecificConfig: {
output: {
// set exports to 'named' or 'auto' so that rollup doesn't warn
exports: 'named',
// set preserveModules to false because for Replay we actually want
// to bundle everything into one file.
preserveModules:
process.env.SENTRY_BUILD_PRESERVE_MODULES === undefined
? false
: Boolean(process.env.SENTRY_BUILD_PRESERVE_MODULES),
},
},
}),
);
75 changes: 75 additions & 0 deletions packages/features/scripts/shim-preact-export.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// preact does not support more modern TypeScript versions, which breaks our users that depend on older
// TypeScript versions. To fix this, we shim the types from preact to be any and remove the dependency on preact
// for types directly. This script is meant to be run after the build/npm/types-ts3.8 directory is created.

// Path: build/npm/types-ts3.8/global.d.ts

const fs = require('fs');
const path = require('path');

/**
* This regex looks for preact imports we can replace and shim out.
*
* Example:
* import { ComponentChildren, VNode } from 'preact';
*/
const preactImportRegex = /import\s*{\s*([\w\s,]+)\s*}\s*from\s*'preact'\s*;?/;

function walk(dir) {
const files = fs.readdirSync(dir);
files.forEach(file => {
const filePath = path.join(dir, file);
const stat = fs.lstatSync(filePath);
if (stat.isDirectory()) {
walk(filePath);
} else {
if (filePath.endsWith('.d.ts')) {
const content = fs.readFileSync(filePath, 'utf8');

Check failure

Code scanning / CodeQL

Potential file system race condition High

The file may have changed since it
was checked
.
const capture = preactImportRegex.exec(content);
if (capture) {
const groups = capture[1].split(',').map(s => s.trim());

// This generates a shim snippet to replace the type imports from preact
// It generates a snippet based on the capture groups of preactImportRegex.
//
// Example:
//
// import type { ComponentChildren, VNode } from 'preact';
// becomes
// type ComponentChildren: any;
// type VNode: any;
const snippet = groups.reduce((acc, curr) => {
const searchableValue = curr.includes(' as ') ? curr.split(' as ')[1] : curr;

// look to see if imported as value, then we have to use declare const
if (content.includes(`typeof ${searchableValue}`)) {
return `${acc}declare const ${searchableValue}: any;\n`;
}

// look to see if generic type like Foo<T>
if (content.includes(`${searchableValue}<`)) {
return `${acc}type ${searchableValue}<T> = any;\n`;
}

// otherwise we can just leave as type
return `${acc}type ${searchableValue} = any;\n`;
}, '');

// we then can remove the import from preact
const newContent = content.replace(preactImportRegex, '// replaced import from preact');

// and write the new content to the file
fs.writeFileSync(filePath, snippet + newContent, 'utf8');

Check failure

Code scanning / CodeQL

Potential file system race condition High

The file may have changed since it
was checked
.
}
}
}
});
}

function run() {
// recurse through build/npm/types-ts3.8 directory
const dir = path.join('build', 'npm', 'types-ts3.8');
walk(dir);
}

run();
14 changes: 14 additions & 0 deletions packages/features/src/core/integration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/* eslint-disable @sentry-internal/sdk/no-class-field-initializers */

import * as Sentry from '@sentry/browser';
import type { IntegrationFn } from '@sentry/types';
import type { FeatureFlagOptions } from '../types';

/**
* Sentry integration for reading and modifying internal feature flag state.
*/
export const featureFlagIntegration = ((_options: FeatureFlagOptions) => {
return {
name: 'launchdarkly',
};
}) satisfies IntegrationFn;
4 changes: 4 additions & 0 deletions packages/features/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This file is used as entry point to generate the npm package and CDN bundles.

export type { FeatureFlagOptions } from './types';
export { featureFlagIntegration } from './core/integration';
1 change: 1 addition & 0 deletions packages/features/src/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export type FeatureFlagOptions = Record<string, never>;
Empty file added packages/features/test.setup.ts
Empty file.
8 changes: 8 additions & 0 deletions packages/features/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"lib": ["DOM", "ES2018"],
"module": "esnext"
},
"include": ["src/**/*.ts"]
}
15 changes: 15 additions & 0 deletions packages/features/tsconfig.test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"extends": "./tsconfig.json",

"include": ["test/**/*.ts", "vitest.config.ts", "test.setup.ts"],

"compilerOptions": {
"types": ["node"],
"esModuleInterop": true,
"allowJs": true,
"noImplicitAny": true,
"noImplicitThis": false,
"strictNullChecks": true,
"strictPropertyInitialization": false
}
}
10 changes: 10 additions & 0 deletions packages/features/tsconfig.types.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"extends": "./tsconfig.json",

"compilerOptions": {
"declaration": true,
"declarationMap": true,
"emitDeclarationOnly": true,
"outDir": "build/npm/types"
}
}
12 changes: 12 additions & 0 deletions packages/features/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { defineConfig } from 'vitest/config';

import baseConfig from '../../vite/vite.config';

export default defineConfig({
...baseConfig,
test: {
...baseConfig.test,
setupFiles: ['./test.setup.ts'],
reporters: ['default'],
},
});
Loading

0 comments on commit f494720

Please sign in to comment.