Skip to content

Commit

Permalink
Warn and log on default value fallback dynamicconfig (#229)
Browse files Browse the repository at this point in the history
* Warn and log on default value fallback dynamicconfig

* cleanup
  • Loading branch information
tore-statsig authored Dec 20, 2022
1 parent f68d834 commit 5cee559
Show file tree
Hide file tree
Showing 13 changed files with 241 additions and 25 deletions.
4 changes: 2 additions & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
module.exports = {
roots: ['./'],
setupFilesAfterEnv: ['<rootDir>/src/__tests__/jest.setup.js'],
setupFilesAfterEnv: ['<rootDir>/src/__tests__/jest.setup.ts'],
testMatch: ['**/__tests__/**/*.test.(j|t)s', '**/?(*.)+test.(j|t)s'],
testPathIgnorePatterns: [
'<rootDir>/node_modules/',
'<rootDir>/dist/',
'<rootDir>/src/__tests__/jest.setup.js',
'<rootDir>/src/__tests__/jest.setup.ts',
],
transform: {
'^.+\\.ts$': 'ts-jest',
Expand Down
18 changes: 18 additions & 0 deletions src/DynamicConfig.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import { EvaluationDetails } from './StatsigStore';

export type OnDefaultValueFallback = (
config: DynamicConfig,
parameter: string,
defaultValueType: string,
valueType: string,
) => void;

export default class DynamicConfig {
private name: string;
public value: Record<string, any>;
private ruleID: string;
private secondaryExposures: Record<string, string>[];
private allocatedExperimentName: string;
private evaluationDetails: EvaluationDetails;
private onDefaultValueFallback: OnDefaultValueFallback | null = null;

public constructor(
configName: string,
Expand All @@ -15,13 +23,15 @@ export default class DynamicConfig {
evaluationDetails: EvaluationDetails,
secondaryExposures: Record<string, string>[] = [],
allocatedExperimentName: string = '',
onDefaultValueFallback: OnDefaultValueFallback | null = null,
) {
this.name = configName;
this.value = JSON.parse(JSON.stringify(configValue ?? {}));
this.ruleID = ruleID ?? '';
this.secondaryExposures = secondaryExposures;
this.allocatedExperimentName = allocatedExperimentName;
this.evaluationDetails = evaluationDetails;
this.onDefaultValueFallback = onDefaultValueFallback;
}

public get<T>(
Expand Down Expand Up @@ -50,6 +60,14 @@ export default class DynamicConfig {
return val as unknown as T;
}

if (this.onDefaultValueFallback != null) {
this.onDefaultValueFallback(
this,
key,
Array.isArray(defaultValue) ? 'array' : typeof defaultValue,
Array.isArray(val) ? 'array' : typeof val,
);
}
return defaultValue;
}

Expand Down
32 changes: 24 additions & 8 deletions src/StatsigClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import Diagnostics, {
DiagnosticsEvent,
DiagnosticsKey,
} from './utils/Diagnostics';
import ConsoleLogger from './utils/ConsoleLogger';

const MAX_VALUE_SIZE = 64;
const MAX_OBJ_SIZE = 2048;
Expand Down Expand Up @@ -100,6 +101,7 @@ export interface IHasStatsigInternal {
getErrorBoundary(): ErrorBoundary;
getSDKType(): string;
getSDKVersion(): string;
getConsoleLogger(): ConsoleLogger;
}

export type StatsigOverrides = {
Expand Down Expand Up @@ -175,6 +177,11 @@ export default class StatsigClient implements IHasStatsigInternal, IStatsig {
return this.identity.getSDKVersion();
}

private consoleLogger: ConsoleLogger;
public getConsoleLogger(): ConsoleLogger {
return this.consoleLogger;
}

public constructor(
sdkKey: string,
user?: StatsigUser | null,
Expand All @@ -189,6 +196,7 @@ export default class StatsigClient implements IHasStatsigInternal, IStatsig {
this.ready = false;
this.sdkKey = sdkKey;
this.options = new StatsigSDKOptions(options);
this.consoleLogger = new ConsoleLogger(this.options.getLogLevel());
StatsigLocalStorage.disabled = this.options.getDisableLocalStorage();
this.initializeDiagnostics = new Diagnostics('initialize');
this.identity = new StatsigIdentity(
Expand Down Expand Up @@ -500,11 +508,13 @@ export default class StatsigClient implements IHasStatsigInternal, IStatsig {
);
}
if (typeof eventName !== 'string' || eventName.length === 0) {
console.error('Event not logged. No valid eventName passed.');
this.consoleLogger.error(
'Event not logged. No valid eventName passed.',
);
return;
}
if (this.shouldTrimParam(eventName, MAX_VALUE_SIZE)) {
console.warn(
this.consoleLogger.info(
'eventName is too long, trimming to ' +
MAX_VALUE_SIZE +
' characters.',
Expand All @@ -515,11 +525,13 @@ export default class StatsigClient implements IHasStatsigInternal, IStatsig {
typeof value === 'string' &&
this.shouldTrimParam(value, MAX_VALUE_SIZE)
) {
console.warn('value is too long, trimming to ' + MAX_VALUE_SIZE + '.');
this.consoleLogger.info(
'value is too long, trimming to ' + MAX_VALUE_SIZE + '.',
);
value = value.substring(0, MAX_VALUE_SIZE);
}
if (this.shouldTrimParam(metadata, MAX_OBJ_SIZE)) {
console.warn('metadata is too big. Dropping the metadata.');
this.consoleLogger.info('metadata is too big. Dropping the metadata.');
metadata = { error: 'not logged due to size too large' };
}
const event = new LogEvent(eventName);
Expand Down Expand Up @@ -884,18 +896,22 @@ export default class StatsigClient implements IHasStatsigInternal, IStatsig {
return {};
}
if (this.shouldTrimParam(user.userID ?? null, MAX_VALUE_SIZE)) {
console.warn(
this.consoleLogger.info(
'User ID is too large, trimming to ' + MAX_VALUE_SIZE + 'characters',
);
user.userID = user.userID?.toString().substring(0, MAX_VALUE_SIZE);
}
if (this.shouldTrimParam(user, MAX_OBJ_SIZE)) {
user.custom = {};
if (this.shouldTrimParam(user, MAX_OBJ_SIZE)) {
console.warn('User object is too large, only keeping the user ID.');
this.consoleLogger.info(
'User object is too large, only keeping the user ID.',
);
user = { userID: user.userID };
} else {
console.warn('User object is too large, dropping the custom property.');
this.consoleLogger.info(
'User object is too large, dropping the custom property.',
);
}
}
return user;
Expand Down Expand Up @@ -925,7 +941,7 @@ export default class StatsigClient implements IHasStatsigInternal, IStatsig {
diagnostics?: Diagnostics,
): Promise<void> {
if (prefetchUsers.length > 5) {
console.warn('Cannot prefetch more than 5 users.');
this.consoleLogger.info('Cannot prefetch more than 5 users.');
}

const keyedPrefetchUsers = prefetchUsers.slice(0, 5).reduce((acc, curr) => {
Expand Down
15 changes: 15 additions & 0 deletions src/StatsigLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const APP_METRICS_PAGE_LOAD_EVENT =
const APP_METRICS_DOM_INTERACTIVE_EVENT =
INTERNAL_EVENT_PREFIX + 'app_metrics::dom_interactive_time';
const DIAGNOSTICS_EVENT = INTERNAL_EVENT_PREFIX + 'diagnostics';
const DEFAULT_VALUE_WARNING = INTERNAL_EVENT_PREFIX + 'default_value';

type FailedLogEventBody = {
events: object[];
Expand Down Expand Up @@ -248,6 +249,20 @@ export default class StatsigLogger {
this.log(configExposure);
}

public logConfigDefaultValueFallback(
user: StatsigUser | null,
message: string,
metadata: object,
): void {
const defaultValueEvent = new LogEvent(DEFAULT_VALUE_WARNING);
defaultValueEvent.setUser(user);
defaultValueEvent.setValue(message);
defaultValueEvent.setMetadata(metadata);
this.log(defaultValueEvent);
this.loggedErrors.add(message);
this.sdkInternal.getConsoleLogger().error(message);
}

public logAppError(
user: StatsigUser | null,
message: string,
Expand Down
14 changes: 14 additions & 0 deletions src/StatsigSDKOptions.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import LogEvent from './LogEvent';
import { StatsigUser } from './StatsigUser';

const DEFAULT_FEATURE_GATE_API = 'https://featuregates.org/v1/';
Expand Down Expand Up @@ -32,8 +33,15 @@ export type StatsigOptions = {
disableLocalStorage?: boolean;
initCompletionCallback?: InitCompletionCallback | null;
disableDiagnosticsLogging?: boolean;
logLevel?: LogLevel | null;
};

export enum LogLevel {
'NONE',
'INFO',
'DEBUG',
}

type BoundedNumberInput = {
default: number;
min: number;
Expand All @@ -58,6 +66,7 @@ export default class StatsigSDKOptions {
private disableLocalStorage: boolean;
private initCompletionCallback: InitCompletionCallback | null;
private disableDiagnosticsLogging: boolean;
private logLevel: LogLevel;

constructor(options?: StatsigOptions | null) {
if (options == null) {
Expand Down Expand Up @@ -103,6 +112,7 @@ export default class StatsigSDKOptions {
this.disableLocalStorage = options.disableLocalStorage ?? false;
this.initCompletionCallback = options.initCompletionCallback ?? null;
this.disableDiagnosticsLogging = options.disableDiagnosticsLogging ?? false;
this.logLevel = options?.logLevel ?? LogLevel.NONE;
}

getApi(): string {
Expand Down Expand Up @@ -169,6 +179,10 @@ export default class StatsigSDKOptions {
return this.disableDiagnosticsLogging;
}

getLogLevel(): LogLevel {
return this.logLevel;
}

private normalizeNumberInput(
input: number | undefined,
bounds: BoundedNumberInput,
Expand Down
27 changes: 27 additions & 0 deletions src/StatsigStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,9 @@ export default class StatsigStore {
this.overrides.configs[configName],
'override',
details,
[],
'',
this.onConfigDefaultValueFallback,
);
} else if (this.userValues?.dynamic_configs[configNameHash] != null) {
const rawConfigValue = this.userValues?.dynamic_configs[configNameHash];
Expand Down Expand Up @@ -572,6 +575,7 @@ export default class StatsigStore {
details,
apiConfig?.secondary_exposures,
apiConfig?.allocated_experiment_name ?? '',
this.onConfigDefaultValueFallback,
);
}

Expand Down Expand Up @@ -689,4 +693,27 @@ export default class StatsigStore {
StatsigLocalStorage.setItem(key, value);
}
}

private onConfigDefaultValueFallback(
config: DynamicConfig,
parameter: string,
defaultValueType: string,
valueType: string,
): void {
if (!this.isLoaded()) {
return;
}
this.sdkInternal.getLogger().logConfigDefaultValueFallback(
this.sdkInternal.getCurrentUser(),
`Parameter ${parameter} is a value of type ${valueType}.
Returning requested defaultValue type ${defaultValueType}`,
{
name: config.getName(),
ruleID: config.getRuleID(),
parameter,
defaultValueType,
valueType,
},
);
}
}
54 changes: 52 additions & 2 deletions src/__tests__/DynamicConfigTyped.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,45 @@ import DynamicConfig from '../DynamicConfig';
import { EvaluationReason } from '../StatsigStore';

describe('Verify behavior of DynamicConfig', () => {
let fallback: {
config: DynamicConfig;
parameter: string;
defaultValueType: string;
valueType: string;
} | null = null;

const onFallback = (
config: DynamicConfig,
parameter: string,
defaultValueType: string,
valueType: string,
) => {
fallback = {
config,
parameter,
defaultValueType,
valueType,
};
};

const expectFallback = function (
config: DynamicConfig,
parameter: string,
defaultValue: any,
valueType: string,
) {
expect(config.get(parameter, defaultValue)).toStrictEqual(defaultValue);
const defaultValueType = Array.isArray(defaultValue)
? 'array'
: typeof defaultValue;
expect(fallback).toStrictEqual({
config,
parameter,
defaultValueType: defaultValueType,
valueType: valueType,
});
fallback = null;
};
const testConfig = new DynamicConfig(
'test_config',
{
Expand All @@ -23,6 +62,9 @@ describe('Verify behavior of DynamicConfig', () => {
reason: EvaluationReason.Network,
time: Date.now(),
},
[],
'',
onFallback,
);

type TestObject = {
Expand All @@ -46,21 +88,29 @@ describe('Verify behavior of DynamicConfig', () => {
};

beforeEach(() => {
fallback = null;
expect.hasAssertions();
});

test('Test typed get', () => {
expect(testConfig.get('bool', 3)).toStrictEqual(3);
expectFallback(testConfig, 'bool', 3, 'boolean');
expect(testConfig.getValue('111', 222)).toStrictEqual(222);
// not called when default value is applied because the field is missing
expect(fallback).toBeNull();
expect(testConfig.get('numberStr2', 'test')).toStrictEqual('3.3');
expect(fallback).toBeNull();
expect(testConfig.get('boolStr1', 'test')).toStrictEqual('true');
expect(testConfig.get('numberStr2', 17)).toStrictEqual(17);
expect(fallback).toBeNull();
expectFallback(testConfig, 'numberStr2', 17, 'string');
expect(testConfig.get('arr', ['test'])).toStrictEqual([1, 2, 'three']);
expect(testConfig.get('object', ['test'])).toStrictEqual(['test']);
expect(fallback).toBeNull();
expectFallback(testConfig, 'object', ['test'], 'object');
expect(testConfig.get('object', {})).toStrictEqual({
key: 'value',
key2: 123,
});
expect(fallback).toBeNull();
});

test('Test optional type guard when runtime check succeeds', () => {
Expand Down
Loading

0 comments on commit 5cee559

Please sign in to comment.