From 19c6ffd790b4fa85c4f5e30a07a5baf8e19f1ce5 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Fri, 8 Nov 2024 14:51:09 -0600 Subject: [PATCH 01/16] refactor log levels --- .changeset/shiny-boats-warn.md | 5 ++ .../signals/src/core/debug-mode/index.ts | 20 ++++--- .../signals/signals/src/core/emitter/index.ts | 13 +---- .../signals/src/core/processor/processor.ts | 3 +- .../signals/src/core/signals/settings.ts | 6 --- .../signals/src/core/signals/signals.ts | 8 ++- .../signals/signals/src/lib/logger/index.ts | 52 +++++++++++++++---- 7 files changed, 67 insertions(+), 40 deletions(-) create mode 100644 .changeset/shiny-boats-warn.md diff --git a/.changeset/shiny-boats-warn.md b/.changeset/shiny-boats-warn.md new file mode 100644 index 000000000..38aec1e86 --- /dev/null +++ b/.changeset/shiny-boats-warn.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-signals': patch +--- + +Add signals logging for events diff --git a/packages/signals/signals/src/core/debug-mode/index.ts b/packages/signals/signals/src/core/debug-mode/index.ts index e0809d848..865addffb 100644 --- a/packages/signals/signals/src/core/debug-mode/index.ts +++ b/packages/signals/signals/src/core/debug-mode/index.ts @@ -17,16 +17,20 @@ export const parseDebugModeQueryString = (): boolean | undefined => { /** * This turns on advanced logging for signals! */ -export const parseSignalsLoggingAdvancedQueryString = (): - | boolean - | undefined => { +export type LogLevelOptions = 'info' | 'debug' | 'off' | undefined +export const parseSignalsLogLevel = (): LogLevelOptions => { const queryParams = new URLSearchParams(window.location.search) const val = - queryParams.get('segment_signals_logging_advanced') || - queryParams.get('seg_signals_logging_advanced') - if (val === 'true' || val === 'false') { - return val === 'true' + queryParams.get('segment_signals_log_level') || + queryParams.get('seg_signals_log_level') + if (val === 'info' || val === 'debug' || val === 'off') { + return val + } else if (typeof val === undefined) { + return undefined + } else { + console.error( + `Invalid signals_log_level: "${val}". Valid options are: info, debug, off` + ) } - return undefined } diff --git a/packages/signals/signals/src/core/emitter/index.ts b/packages/signals/signals/src/core/emitter/index.ts index cd7880639..c9cc705e2 100644 --- a/packages/signals/signals/src/core/emitter/index.ts +++ b/packages/signals/signals/src/core/emitter/index.ts @@ -6,21 +6,12 @@ export interface EmitSignal { emit: (signal: Signal) => void } -interface SignalEmitterSettings { - shouldLogSignals: () => boolean -} - export class SignalEmitter implements EmitSignal { private emitter = new Emitter<{ add: [Signal] }>() private listeners = new Set<(signal: Signal) => void>() - private settings?: SignalEmitterSettings - constructor(settings?: SignalEmitterSettings) { - this.settings = settings - } + emit(signal: Signal) { - if (this.settings?.shouldLogSignals()) { - logger.log('New signal:', signal.type, signal.data) - } + logger.info('New signal:', signal.type, signal.data) this.emitter.emit('add', signal) } diff --git a/packages/signals/signals/src/core/processor/processor.ts b/packages/signals/signals/src/core/processor/processor.ts index 0a2e6da9b..a1cf759cc 100644 --- a/packages/signals/signals/src/core/processor/processor.ts +++ b/packages/signals/signals/src/core/processor/processor.ts @@ -18,7 +18,8 @@ export class SignalEventProcessor { const name = methodName as MethodName const eventsCollection = analyticsMethodCalls[name] eventsCollection.forEach((args) => { - logger.debug(`analytics.${name}(...) called with args`, args) + logger.info('New signals-originated event:', name, { args }) + // @ts-ignore this.analytics[name](...args) }) diff --git a/packages/signals/signals/src/core/signals/settings.ts b/packages/signals/signals/src/core/signals/settings.ts index 322cd5a02..a649dfc40 100644 --- a/packages/signals/signals/src/core/signals/settings.ts +++ b/packages/signals/signals/src/core/signals/settings.ts @@ -114,7 +114,6 @@ export class SignalGlobalSettings { export class SignalsDebugSettings { private static redactionKey = 'segment_signals_debug_redaction_disabled' private static ingestionKey = 'segment_signals_debug_ingestion_enabled' - private static logSignals = 'segment_signals_log_signals_enabled' storage: DebugStorage constructor(disableRedaction?: boolean, enableIngestion?: boolean) { @@ -142,7 +141,6 @@ export class SignalsDebugSettings { setAllDebugging = (boolean: boolean) => { this.storage.setDebugKey(SignalsDebugSettings.redactionKey, boolean) this.storage.setDebugKey(SignalsDebugSettings.ingestionKey, boolean) - this.storage.setDebugKey(SignalsDebugSettings.logSignals, boolean) } getDisableSignalsRedaction = (): boolean => { @@ -152,8 +150,4 @@ export class SignalsDebugSettings { getEnableSignalsIngestion = (): boolean => { return this.storage.getDebugKey(SignalsDebugSettings.ingestionKey) } - - getEnableLogSignals = (): boolean => { - return this.storage.getDebugKey(SignalsDebugSettings.logSignals) - } } diff --git a/packages/signals/signals/src/core/signals/signals.ts b/packages/signals/signals/src/core/signals/signals.ts index 425513a7d..c54636c65 100644 --- a/packages/signals/signals/src/core/signals/signals.ts +++ b/packages/signals/signals/src/core/signals/signals.ts @@ -38,10 +38,7 @@ export class Signals implements ISignals { private globalSettings: SignalGlobalSettings constructor(settingsConfig: SignalsSettingsConfig = {}) { this.globalSettings = new SignalGlobalSettings(settingsConfig) - this.signalEmitter = new SignalEmitter({ - shouldLogSignals: () => - this.globalSettings.signalsDebug.getEnableLogSignals(), - }) + this.signalEmitter = new SignalEmitter() this.signalsClient = new SignalsIngestClient( this.globalSettings.ingestClient ) @@ -136,10 +133,11 @@ export class Signals implements ISignals { } /** - * Disable redaction, ingestion of signals, and other debug logging. + * Disable redaction, ingestion of signals, and other logging. */ debug(boolean = true): void { this.globalSettings.signalsDebug.setAllDebugging(boolean) + logger.enableLogging('info') } /** diff --git a/packages/signals/signals/src/lib/logger/index.ts b/packages/signals/signals/src/lib/logger/index.ts index 9bb7c0367..82ac39eb3 100644 --- a/packages/signals/signals/src/lib/logger/index.ts +++ b/packages/signals/signals/src/lib/logger/index.ts @@ -1,27 +1,61 @@ -import { parseSignalsLoggingAdvancedQueryString } from '../../core/debug-mode' +import { + parseDebugModeQueryString, + parseSignalsLogLevel, +} from '../../core/debug-mode' import { DebugStorage } from '../storage/debug-storage' class Logger { - private static advancedLogging = 'segment_signals_logging_advanced' + private static infoLogging = 'segment_signals_log_level_info' + private static debugLogging = 'segment_signals_log_level_debug' storage = new DebugStorage('sessionStorage') + constructor() { - const val = parseSignalsLoggingAdvancedQueryString() - if (typeof val === 'boolean') { - this.storage.setDebugKey(Logger.advancedLogging, val) + // if debug mode is in the query string, we want simple logging + const debugMode = parseDebugModeQueryString() + if (typeof debugMode === 'boolean') { + this.enableLogging('info') + } + + // if log level is set to 'off' / 'log' / 'debug' in the query string, we want to set the write key + const logLevel = parseSignalsLogLevel() + if (logLevel !== undefined) { + logLevel === 'off' ? this.disableLogging() : this.enableLogging(logLevel) } } + private loggingEnabled = (): boolean => { + return this.storage.getDebugKey(Logger.infoLogging) + } + private debugLoggingEnabled = (): boolean => { - return this.storage.getDebugKey(Logger.advancedLogging) + return this.storage.getDebugKey(Logger.debugLogging) } enableDebugLogging = (bool = true) => { - this.storage.setDebugKey(Logger.advancedLogging, bool) + this.storage.setDebugKey(Logger.debugLogging, bool) } - log = (...args: any[]): void => { - console.log('[signals log]', ...args) + // if debug level is enabled, info level is also enabled + enableLogging = (type: 'info' | 'debug') => { + if (type === 'info') { + this.storage.setDebugKey(Logger.infoLogging, true) + this.storage.setDebugKey(Logger.debugLogging, false) + } else if (type === 'debug') { + this.storage.setDebugKey(Logger.debugLogging, true) + this.storage.setDebugKey(Logger.infoLogging, true) + } + } + + disableLogging = () => { + this.storage.setDebugKey(Logger.infoLogging, false) + this.storage.setDebugKey(Logger.debugLogging, false) + } + + info = (...args: any[]): void => { + if (this.loggingEnabled() || this.debugLoggingEnabled()) { + console.log('[signals log]', ...args) + } } debug = (...args: any[]): void => { From 11cd71b7bb9616e8368e5224d93e0f051a8c127e Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Fri, 8 Nov 2024 15:41:48 -0600 Subject: [PATCH 02/16] wip --- .../signals/src/core/debug-mode/index.ts | 9 ++-- .../signals/src/core/signals/settings.ts | 24 ++++------- .../signals/src/core/signals/signals.ts | 5 ++- .../signals/signals/src/lib/logger/index.ts | 41 ++++++------------- .../signals/src/lib/storage/debug-storage.ts | 29 ------------- .../signals/src/lib/storage/web-storage.ts | 23 +++++++++++ .../signals/src/plugin/signals-plugin.ts | 12 +++--- 7 files changed, 57 insertions(+), 86 deletions(-) delete mode 100644 packages/signals/signals/src/lib/storage/debug-storage.ts create mode 100644 packages/signals/signals/src/lib/storage/web-storage.ts diff --git a/packages/signals/signals/src/core/debug-mode/index.ts b/packages/signals/signals/src/core/debug-mode/index.ts index 865addffb..1f60764f6 100644 --- a/packages/signals/signals/src/core/debug-mode/index.ts +++ b/packages/signals/signals/src/core/debug-mode/index.ts @@ -17,8 +17,8 @@ export const parseDebugModeQueryString = (): boolean | undefined => { /** * This turns on advanced logging for signals! */ -export type LogLevelOptions = 'info' | 'debug' | 'off' | undefined -export const parseSignalsLogLevel = (): LogLevelOptions => { +export type LogLevelOptions = 'info' | 'debug' | 'off' +export const parseSignalsLogLevel = (): LogLevelOptions | undefined => { const queryParams = new URLSearchParams(window.location.search) const val = @@ -26,11 +26,10 @@ export const parseSignalsLogLevel = (): LogLevelOptions => { queryParams.get('seg_signals_log_level') if (val === 'info' || val === 'debug' || val === 'off') { return val - } else if (typeof val === undefined) { - return undefined - } else { + } else if (typeof val === 'string') { console.error( `Invalid signals_log_level: "${val}". Valid options are: info, debug, off` ) } + return undefined } diff --git a/packages/signals/signals/src/core/signals/settings.ts b/packages/signals/signals/src/core/signals/settings.ts index a649dfc40..09fa6235a 100644 --- a/packages/signals/signals/src/core/signals/settings.ts +++ b/packages/signals/signals/src/core/signals/settings.ts @@ -5,7 +5,7 @@ import { SignalsIngestSettingsConfig } from '../client' import { SandboxSettingsConfig } from '../processor/sandbox' import { NetworkSettingsConfig } from '../signal-generators/network-gen' import { SignalsPluginSettingsConfig } from '../../types' -import { DebugStorage } from '../../lib/storage/debug-storage' +import { WebStorage } from '../../lib/storage/web-storage' export type SignalsSettingsConfig = Pick< SignalsPluginSettingsConfig, @@ -114,21 +114,15 @@ export class SignalGlobalSettings { export class SignalsDebugSettings { private static redactionKey = 'segment_signals_debug_redaction_disabled' private static ingestionKey = 'segment_signals_debug_ingestion_enabled' - storage: DebugStorage + storage: WebStorage constructor(disableRedaction?: boolean, enableIngestion?: boolean) { - this.storage = new DebugStorage('sessionStorage') + this.storage = new WebStorage(window.sessionStorage) if (typeof disableRedaction === 'boolean') { - this.storage.setDebugKey( - SignalsDebugSettings.redactionKey, - disableRedaction - ) + this.storage.setItem(SignalsDebugSettings.redactionKey, disableRedaction) } if (typeof enableIngestion === 'boolean') { - this.storage.setDebugKey( - SignalsDebugSettings.ingestionKey, - enableIngestion - ) + this.storage.setItem(SignalsDebugSettings.ingestionKey, enableIngestion) } const debugModeInQs = parseDebugModeQueryString() @@ -139,15 +133,15 @@ export class SignalsDebugSettings { } setAllDebugging = (boolean: boolean) => { - this.storage.setDebugKey(SignalsDebugSettings.redactionKey, boolean) - this.storage.setDebugKey(SignalsDebugSettings.ingestionKey, boolean) + this.storage.setItem(SignalsDebugSettings.redactionKey, boolean) + this.storage.setItem(SignalsDebugSettings.ingestionKey, boolean) } getDisableSignalsRedaction = (): boolean => { - return this.storage.getDebugKey(SignalsDebugSettings.redactionKey) + return this.storage.getItem(SignalsDebugSettings.redactionKey) ?? false } getEnableSignalsIngestion = (): boolean => { - return this.storage.getDebugKey(SignalsDebugSettings.ingestionKey) + return this.storage.getItem(SignalsDebugSettings.ingestionKey) ?? false } } diff --git a/packages/signals/signals/src/core/signals/signals.ts b/packages/signals/signals/src/core/signals/signals.ts index c54636c65..7e76ca2e2 100644 --- a/packages/signals/signals/src/core/signals/signals.ts +++ b/packages/signals/signals/src/core/signals/signals.ts @@ -15,6 +15,7 @@ import { SignalEventProcessor } from '../processor/processor' import { Sandbox, SandboxSettings } from '../processor/sandbox' import { SignalGlobalSettings, SignalsSettingsConfig } from './settings' import { logger } from '../../lib/logger' +import { LogLevelOptions } from '../debug-mode' interface ISignals { start(analytics: AnyAnalytics): Promise @@ -135,9 +136,9 @@ export class Signals implements ISignals { /** * Disable redaction, ingestion of signals, and other logging. */ - debug(boolean = true): void { + debug(boolean = true, logLevel?: LogLevelOptions): void { this.globalSettings.signalsDebug.setAllDebugging(boolean) - logger.enableLogging('info') + logger.enableLogging(logLevel ?? 'info') } /** diff --git a/packages/signals/signals/src/lib/logger/index.ts b/packages/signals/signals/src/lib/logger/index.ts index 82ac39eb3..d3792c53e 100644 --- a/packages/signals/signals/src/lib/logger/index.ts +++ b/packages/signals/signals/src/lib/logger/index.ts @@ -1,14 +1,16 @@ import { + LogLevelOptions, parseDebugModeQueryString, parseSignalsLogLevel, } from '../../core/debug-mode' -import { DebugStorage } from '../storage/debug-storage' +import { WebStorage } from '../storage/web-storage' class Logger { - private static infoLogging = 'segment_signals_log_level_info' - private static debugLogging = 'segment_signals_log_level_debug' - - storage = new DebugStorage('sessionStorage') + private logLevelKey = 'segment_signals_log_level' + get logLevel(): LogLevelOptions { + return this.storage.getItem(this.logLevelKey) ?? 'off' + } + storage = new WebStorage(window.sessionStorage) constructor() { // if debug mode is in the query string, we want simple logging @@ -24,42 +26,23 @@ class Logger { } } - private loggingEnabled = (): boolean => { - return this.storage.getDebugKey(Logger.infoLogging) - } - - private debugLoggingEnabled = (): boolean => { - return this.storage.getDebugKey(Logger.debugLogging) - } - - enableDebugLogging = (bool = true) => { - this.storage.setDebugKey(Logger.debugLogging, bool) - } - // if debug level is enabled, info level is also enabled - enableLogging = (type: 'info' | 'debug') => { - if (type === 'info') { - this.storage.setDebugKey(Logger.infoLogging, true) - this.storage.setDebugKey(Logger.debugLogging, false) - } else if (type === 'debug') { - this.storage.setDebugKey(Logger.debugLogging, true) - this.storage.setDebugKey(Logger.infoLogging, true) - } + enableLogging = (type: LogLevelOptions) => { + this.storage.setItem(this.logLevelKey, type) } disableLogging = () => { - this.storage.setDebugKey(Logger.infoLogging, false) - this.storage.setDebugKey(Logger.debugLogging, false) + this.storage.setItem(this.logLevelKey, 'off') } info = (...args: any[]): void => { - if (this.loggingEnabled() || this.debugLoggingEnabled()) { + if (this.logLevel === 'info' || this.logLevel === 'debug') { console.log('[signals log]', ...args) } } debug = (...args: any[]): void => { - if (this.debugLoggingEnabled()) { + if (this.logLevel === 'debug') { console.log('[signals debug]', ...args) } } diff --git a/packages/signals/signals/src/lib/storage/debug-storage.ts b/packages/signals/signals/src/lib/storage/debug-storage.ts deleted file mode 100644 index 55a88baa7..000000000 --- a/packages/signals/signals/src/lib/storage/debug-storage.ts +++ /dev/null @@ -1,29 +0,0 @@ -export class DebugStorage { - private storageType: 'localStorage' | 'sessionStorage' - constructor(type: 'localStorage' | 'sessionStorage') { - this.storageType = type - } - public setDebugKey = (key: string, enable: boolean): void => { - try { - if (enable) { - window[this.storageType].setItem(key, 'true') - } else { - window.sessionStorage.removeItem(key) - } - } catch (e) { - console.warn('Storage error', e) - } - } - - public getDebugKey = (key: string): boolean => { - try { - const isEnabled = Boolean(window[this.storageType].getItem(key)) - if (isEnabled) { - return true - } - } catch (e) { - console.warn('Storage error', e) - } - return false - } -} diff --git a/packages/signals/signals/src/lib/storage/web-storage.ts b/packages/signals/signals/src/lib/storage/web-storage.ts new file mode 100644 index 000000000..dbe7dc629 --- /dev/null +++ b/packages/signals/signals/src/lib/storage/web-storage.ts @@ -0,0 +1,23 @@ +export class WebStorage { + private storage: Storage + constructor(storage: Storage) { + this.storage = storage + } + + public setItem = (key: string, value: string | boolean): void => { + try { + this.storage.setItem(key, value.toString()) + } catch (e) { + console.warn('Storage error', e) + } + } + + public getItem = (key: string): T | undefined => { + try { + return (this.storage.getItem(key) as T) ?? undefined + } catch (e) { + console.warn('Storage error', e) + } + return undefined + } +} diff --git a/packages/signals/signals/src/plugin/signals-plugin.ts b/packages/signals/signals/src/plugin/signals-plugin.ts index c5f731fde..c1e41c90d 100644 --- a/packages/signals/signals/src/plugin/signals-plugin.ts +++ b/packages/signals/signals/src/plugin/signals-plugin.ts @@ -29,11 +29,8 @@ export class SignalsPlugin implements Plugin, SignalsAugmentedFunctionality { constructor(settings: SignalsPluginSettingsConfig = {}) { assertBrowserEnv() - // assign to window for debugging purposes - Object.assign(window, { SegmentSignalsPlugin: this }) - if (settings.enableDebugLogging) { - logger.enableDebugLogging() + logger.enableLogging('debug') } logger.debug(`SignalsPlugin v${version} initializing`, { @@ -57,6 +54,9 @@ export class SignalsPlugin implements Plugin, SignalsAugmentedFunctionality { networkSignalsDisallowList: settings.networkSignalsDisallowList, signalStorage: settings.signalStorage, }) + + // assign to window for debugging purposes + Object.assign(window, { SegmentSignalsPlugin: this }) } isLoaded() { @@ -89,7 +89,7 @@ export class SignalsPlugin implements Plugin, SignalsAugmentedFunctionality { /** * Enable redaction and disable ingestion of signals. Also, logs signals to the console. */ - debug(boolean = true): void { - this.signals.debug(boolean) + debug(...args: Parameters): void { + this.signals.debug(...args) } } From 05405664792f1942bbae4754e111107e4a8b5186 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Fri, 8 Nov 2024 15:43:26 -0600 Subject: [PATCH 03/16] wip --- packages/signals/signals/src/plugin/signals-plugin.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/signals/signals/src/plugin/signals-plugin.ts b/packages/signals/signals/src/plugin/signals-plugin.ts index c1e41c90d..cd9b2e133 100644 --- a/packages/signals/signals/src/plugin/signals-plugin.ts +++ b/packages/signals/signals/src/plugin/signals-plugin.ts @@ -28,6 +28,8 @@ export class SignalsPlugin implements Plugin, SignalsAugmentedFunctionality { public signals: Signals constructor(settings: SignalsPluginSettingsConfig = {}) { assertBrowserEnv() + // assign to window for debugging purposes + Object.assign(window, { SegmentSignalsPlugin: this }) if (settings.enableDebugLogging) { logger.enableLogging('debug') @@ -54,9 +56,6 @@ export class SignalsPlugin implements Plugin, SignalsAugmentedFunctionality { networkSignalsDisallowList: settings.networkSignalsDisallowList, signalStorage: settings.signalStorage, }) - - // assign to window for debugging purposes - Object.assign(window, { SegmentSignalsPlugin: this }) } isLoaded() { From e2a4489c6fa37b592ed1056d8c274f69752ca6cc Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Fri, 8 Nov 2024 15:45:54 -0600 Subject: [PATCH 04/16] wip --- packages/signals/signals/src/core/signals/settings.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/signals/signals/src/core/signals/settings.ts b/packages/signals/signals/src/core/signals/settings.ts index 09fa6235a..4b128bac3 100644 --- a/packages/signals/signals/src/core/signals/settings.ts +++ b/packages/signals/signals/src/core/signals/settings.ts @@ -114,10 +114,9 @@ export class SignalGlobalSettings { export class SignalsDebugSettings { private static redactionKey = 'segment_signals_debug_redaction_disabled' private static ingestionKey = 'segment_signals_debug_ingestion_enabled' - storage: WebStorage + private storage = new WebStorage(window.sessionStorage) constructor(disableRedaction?: boolean, enableIngestion?: boolean) { - this.storage = new WebStorage(window.sessionStorage) if (typeof disableRedaction === 'boolean') { this.storage.setItem(SignalsDebugSettings.redactionKey, disableRedaction) } From 775419290ecab7739309cdb3790d8690090b3851 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Fri, 8 Nov 2024 15:47:30 -0600 Subject: [PATCH 05/16] wip --- packages/signals/signals/src/lib/logger/index.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/signals/signals/src/lib/logger/index.ts b/packages/signals/signals/src/lib/logger/index.ts index d3792c53e..0d5bd085a 100644 --- a/packages/signals/signals/src/lib/logger/index.ts +++ b/packages/signals/signals/src/lib/logger/index.ts @@ -7,26 +7,23 @@ import { WebStorage } from '../storage/web-storage' class Logger { private logLevelKey = 'segment_signals_log_level' + private storage = new WebStorage(window.sessionStorage) get logLevel(): LogLevelOptions { return this.storage.getItem(this.logLevelKey) ?? 'off' } - storage = new WebStorage(window.sessionStorage) constructor() { - // if debug mode is in the query string, we want simple logging const debugMode = parseDebugModeQueryString() if (typeof debugMode === 'boolean') { this.enableLogging('info') } - // if log level is set to 'off' / 'log' / 'debug' in the query string, we want to set the write key const logLevel = parseSignalsLogLevel() if (logLevel !== undefined) { logLevel === 'off' ? this.disableLogging() : this.enableLogging(logLevel) } } - // if debug level is enabled, info level is also enabled enableLogging = (type: LogLevelOptions) => { this.storage.setItem(this.logLevelKey, type) } From af7d0f56eb51479a9f92e3124efa2c3b3ccc7e95 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Fri, 8 Nov 2024 15:50:31 -0600 Subject: [PATCH 06/16] wip --- .../lib/{ => normalize-url}/__tests__/normalize-url.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename packages/signals/signals/src/lib/{ => normalize-url}/__tests__/normalize-url.test.ts (93%) diff --git a/packages/signals/signals/src/lib/__tests__/normalize-url.test.ts b/packages/signals/signals/src/lib/normalize-url/__tests__/normalize-url.test.ts similarity index 93% rename from packages/signals/signals/src/lib/__tests__/normalize-url.test.ts rename to packages/signals/signals/src/lib/normalize-url/__tests__/normalize-url.test.ts index c05e75db0..22d5f0c7a 100644 --- a/packages/signals/signals/src/lib/__tests__/normalize-url.test.ts +++ b/packages/signals/signals/src/lib/normalize-url/__tests__/normalize-url.test.ts @@ -1,5 +1,5 @@ -import { setLocation } from '../../test-helpers/set-location' -import { normalizeUrl } from '../normalize-url' +import { setLocation } from '../../../test-helpers/set-location' +import { normalizeUrl } from '..' describe('normalizeUrl', () => { beforeEach(() => { From 8cfa9bac3860e138f4774edacb9d0be3b7de72ef Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Sat, 9 Nov 2024 13:43:45 -0600 Subject: [PATCH 07/16] wip --- packages/signals/signals/src/core/signals/settings.ts | 8 ++++++-- packages/signals/signals/src/lib/storage/web-storage.ts | 9 +++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/signals/signals/src/core/signals/settings.ts b/packages/signals/signals/src/core/signals/settings.ts index 4b128bac3..a3c745599 100644 --- a/packages/signals/signals/src/core/signals/settings.ts +++ b/packages/signals/signals/src/core/signals/settings.ts @@ -137,10 +137,14 @@ export class SignalsDebugSettings { } getDisableSignalsRedaction = (): boolean => { - return this.storage.getItem(SignalsDebugSettings.redactionKey) ?? false + return ( + this.storage.getBooleanItem(SignalsDebugSettings.redactionKey) ?? false + ) } getEnableSignalsIngestion = (): boolean => { - return this.storage.getItem(SignalsDebugSettings.ingestionKey) ?? false + return ( + this.storage.getBooleanItem(SignalsDebugSettings.ingestionKey) ?? false + ) } } diff --git a/packages/signals/signals/src/lib/storage/web-storage.ts b/packages/signals/signals/src/lib/storage/web-storage.ts index dbe7dc629..86af389b1 100644 --- a/packages/signals/signals/src/lib/storage/web-storage.ts +++ b/packages/signals/signals/src/lib/storage/web-storage.ts @@ -12,12 +12,17 @@ export class WebStorage { } } - public getItem = (key: string): T | undefined => { + public getItem = (key: string): string | undefined => { try { - return (this.storage.getItem(key) as T) ?? undefined + return this.storage.getItem(key) ?? undefined } catch (e) { console.warn('Storage error', e) } return undefined } + + public getBooleanItem = (key: string): boolean | undefined => { + const item = this.getItem(key) + return item === 'true' ? true : item === 'false' ? false : undefined + } } From caeda610326d02e57c4553ad881dd3df6932c94e Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Sat, 9 Nov 2024 14:15:57 -0600 Subject: [PATCH 08/16] update web-storage with test --- .../signals/src/core/signals/settings.ts | 4 +- .../lib/storage/__tests__/web-storage.test.ts | 49 +++++++++++++++++++ .../signals/src/lib/storage/web-storage.ts | 27 ++++++---- 3 files changed, 69 insertions(+), 11 deletions(-) create mode 100644 packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts diff --git a/packages/signals/signals/src/core/signals/settings.ts b/packages/signals/signals/src/core/signals/settings.ts index a3c745599..042515933 100644 --- a/packages/signals/signals/src/core/signals/settings.ts +++ b/packages/signals/signals/src/core/signals/settings.ts @@ -138,13 +138,13 @@ export class SignalsDebugSettings { getDisableSignalsRedaction = (): boolean => { return ( - this.storage.getBooleanItem(SignalsDebugSettings.redactionKey) ?? false + this.storage.getItem(SignalsDebugSettings.redactionKey) ?? false ) } getEnableSignalsIngestion = (): boolean => { return ( - this.storage.getBooleanItem(SignalsDebugSettings.ingestionKey) ?? false + this.storage.getItem(SignalsDebugSettings.ingestionKey) ?? false ) } } diff --git a/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts b/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts new file mode 100644 index 000000000..b46282724 --- /dev/null +++ b/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts @@ -0,0 +1,49 @@ +import { WebStorage } from '../web-storage' + +describe('WebStorage', () => { + let webStorage: WebStorage + + beforeEach(() => { + webStorage = new WebStorage(sessionStorage) + }) + afterEach(() => { + sessionStorage.clear() + }) + describe('getItem, setItem', () => { + it('should retrieve and parse a stored value from storage', () => { + const key = 'testKey' + const value = { foo: 'bar' } + + webStorage.setItem(key, value) + + const result = webStorage.getItem(key) + + expect(result).toEqual(value) + }) + + it('should return undefined if the key does not exist in storage', () => { + const key = 'nonexistentKey' + + const result = webStorage.getItem(key) + expect(result).toBeUndefined() + }) + + it('should handle JSON parsing errors gracefully', () => { + const key = 'testKey' + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation() + + // if somehow invalid JSON is stored in the storage + sessionStorage.setItem(key, 'invalid JSON') + + const result = webStorage.getItem(key) + + expect(result).toBeUndefined() + expect(consoleWarnSpy).toHaveBeenCalledWith( + 'Storage error', + expect.any(SyntaxError) + ) + + consoleWarnSpy.mockRestore() + }) + }) +}) diff --git a/packages/signals/signals/src/lib/storage/web-storage.ts b/packages/signals/signals/src/lib/storage/web-storage.ts index 86af389b1..67a8a98de 100644 --- a/packages/signals/signals/src/lib/storage/web-storage.ts +++ b/packages/signals/signals/src/lib/storage/web-storage.ts @@ -4,25 +4,34 @@ export class WebStorage { this.storage = storage } - public setItem = (key: string, value: string | boolean): void => { + /** + * Set a json-parsable item in storage + */ + public setItem = ( + key: string, + value: T + ): void => { try { - this.storage.setItem(key, value.toString()) + const item = JSON.stringify(value) + this.storage.setItem(key, item) } catch (e) { console.warn('Storage error', e) } } - public getItem = (key: string): string | undefined => { + /** + * Get a json-parsed item from storage + */ + public getItem = (key: string): T | undefined => { try { - return this.storage.getItem(key) ?? undefined + const item = this.storage.getItem(key) + if (item === null) { + return undefined + } + return JSON.parse(item) as T } catch (e) { console.warn('Storage error', e) } return undefined } - - public getBooleanItem = (key: string): boolean | undefined => { - const item = this.getItem(key) - return item === 'true' ? true : item === 'false' ? false : undefined - } } From 34c40381a4858df14c825478b4d288fe69eec7bb Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Sat, 9 Nov 2024 14:17:19 -0600 Subject: [PATCH 09/16] wip --- .../signals/src/lib/storage/__tests__/web-storage.test.ts | 6 +----- packages/signals/signals/src/lib/storage/web-storage.ts | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts b/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts index b46282724..6050409d0 100644 --- a/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts +++ b/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts @@ -38,11 +38,7 @@ describe('WebStorage', () => { const result = webStorage.getItem(key) expect(result).toBeUndefined() - expect(consoleWarnSpy).toHaveBeenCalledWith( - 'Storage error', - expect.any(SyntaxError) - ) - + expect(consoleWarnSpy).toHaveBeenCalledTimes(1) consoleWarnSpy.mockRestore() }) }) diff --git a/packages/signals/signals/src/lib/storage/web-storage.ts b/packages/signals/signals/src/lib/storage/web-storage.ts index 67a8a98de..c0944abe7 100644 --- a/packages/signals/signals/src/lib/storage/web-storage.ts +++ b/packages/signals/signals/src/lib/storage/web-storage.ts @@ -15,7 +15,7 @@ export class WebStorage { const item = JSON.stringify(value) this.storage.setItem(key, item) } catch (e) { - console.warn('Storage error', e) + console.warn('Storage set error', e) } } @@ -30,7 +30,7 @@ export class WebStorage { } return JSON.parse(item) as T } catch (e) { - console.warn('Storage error', e) + console.warn('Storage retrieval error', e) } return undefined } From f4900f5fdc66fa2bafeea9b5d7db2075c113b6cd Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Sat, 9 Nov 2024 14:18:11 -0600 Subject: [PATCH 10/16] wip --- packages/signals/signals/src/lib/storage/web-storage.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/signals/signals/src/lib/storage/web-storage.ts b/packages/signals/signals/src/lib/storage/web-storage.ts index c0944abe7..f5308ca1c 100644 --- a/packages/signals/signals/src/lib/storage/web-storage.ts +++ b/packages/signals/signals/src/lib/storage/web-storage.ts @@ -15,7 +15,7 @@ export class WebStorage { const item = JSON.stringify(value) this.storage.setItem(key, item) } catch (e) { - console.warn('Storage set error', e) + console.warn('Storage set error', { key, value }, e) } } @@ -30,7 +30,7 @@ export class WebStorage { } return JSON.parse(item) as T } catch (e) { - console.warn('Storage retrieval error', e) + console.warn('Storage retrieval error', { key }, e) } return undefined } From e97d2c793ceea171c40e2236353e1925bbc503dd Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Sat, 9 Nov 2024 14:19:39 -0600 Subject: [PATCH 11/16] wip --- .../lib/storage/__tests__/web-storage.test.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts b/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts index 6050409d0..3300516be 100644 --- a/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts +++ b/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts @@ -28,7 +28,23 @@ describe('WebStorage', () => { expect(result).toBeUndefined() }) - it('should handle JSON parsing errors gracefully', () => { + it('should handle JSON serializing errors gracefully when setting', () => { + const key = 'testKey' + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation() + + webStorage.setItem( + key, + // @ts-ignore + undefined + ) + + const result = webStorage.getItem(key) + + expect(result).toBeUndefined() + expect(consoleWarnSpy).toHaveBeenCalledTimes(1) + }) + + it('should handle JSON parsing errors gracefully when retrieving', () => { const key = 'testKey' const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation() @@ -39,7 +55,6 @@ describe('WebStorage', () => { expect(result).toBeUndefined() expect(consoleWarnSpy).toHaveBeenCalledTimes(1) - consoleWarnSpy.mockRestore() }) }) }) From d580e1e9820c0e1aefebf21fe23e5158f9fd56b4 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Sat, 9 Nov 2024 14:24:22 -0600 Subject: [PATCH 12/16] wip --- .../lib/storage/__tests__/web-storage.test.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts b/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts index 3300516be..19c1328dc 100644 --- a/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts +++ b/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts @@ -34,14 +34,15 @@ describe('WebStorage', () => { webStorage.setItem( key, - // @ts-ignore - undefined + // @ts-ignore - intentational non-serializable value + BigInt(1) ) - const result = webStorage.getItem(key) - - expect(result).toBeUndefined() - expect(consoleWarnSpy).toHaveBeenCalledTimes(1) + expect(consoleWarnSpy).toHaveBeenCalledWith( + 'Storage set error', + expect.any(Object), + expect.any(Error) + ) }) it('should handle JSON parsing errors gracefully when retrieving', () => { @@ -54,7 +55,11 @@ describe('WebStorage', () => { const result = webStorage.getItem(key) expect(result).toBeUndefined() - expect(consoleWarnSpy).toHaveBeenCalledTimes(1) + expect(consoleWarnSpy).toHaveBeenCalledWith( + 'Storage retrieval error', + expect.any(Object), + expect.any(Error) + ) }) }) }) From d55bf0181b755ef81f9354b6578aea922240fac1 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Sat, 9 Nov 2024 14:33:08 -0600 Subject: [PATCH 13/16] wip --- packages/signals/signals/src/lib/logger/index.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/signals/signals/src/lib/logger/index.ts b/packages/signals/signals/src/lib/logger/index.ts index 0d5bd085a..bfabfc57f 100644 --- a/packages/signals/signals/src/lib/logger/index.ts +++ b/packages/signals/signals/src/lib/logger/index.ts @@ -13,14 +13,14 @@ class Logger { } constructor() { - const debugMode = parseDebugModeQueryString() - if (typeof debugMode === 'boolean') { - this.enableLogging('info') - } - const logLevel = parseSignalsLogLevel() if (logLevel !== undefined) { logLevel === 'off' ? this.disableLogging() : this.enableLogging(logLevel) + } else { + const debugMode = parseDebugModeQueryString() + if (debugMode === true) { + this.enableLogging('info') + } } } From 73af57a74ca922b7b3cc688127b88db2a605a2aa Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Sat, 9 Nov 2024 14:33:59 -0600 Subject: [PATCH 14/16] wip --- packages/signals/signals/src/lib/logger/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/signals/signals/src/lib/logger/index.ts b/packages/signals/signals/src/lib/logger/index.ts index bfabfc57f..8a8f98ef7 100644 --- a/packages/signals/signals/src/lib/logger/index.ts +++ b/packages/signals/signals/src/lib/logger/index.ts @@ -13,6 +13,7 @@ class Logger { } constructor() { + // if log level is set in query string, use that, otherwise if debug mode is set, set log level to info const logLevel = parseSignalsLogLevel() if (logLevel !== undefined) { logLevel === 'off' ? this.disableLogging() : this.enableLogging(logLevel) From 62adb63637a1d01d068130b14e9259b998819b23 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Sat, 9 Nov 2024 14:48:28 -0600 Subject: [PATCH 15/16] wip --- packages/signals/signals/src/core/processor/processor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/signals/signals/src/core/processor/processor.ts b/packages/signals/signals/src/core/processor/processor.ts index a1cf759cc..f4f9ba8d5 100644 --- a/packages/signals/signals/src/core/processor/processor.ts +++ b/packages/signals/signals/src/core/processor/processor.ts @@ -18,7 +18,7 @@ export class SignalEventProcessor { const name = methodName as MethodName const eventsCollection = analyticsMethodCalls[name] eventsCollection.forEach((args) => { - logger.info('New signals-originated event:', name, { args }) + logger.info('New method call:', `analytics.${name}()`, args) // @ts-ignore this.analytics[name](...args) From 32725a0edbe4d1f1a0ffef0e559a6ec5da6db682 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:03:12 -0600 Subject: [PATCH 16/16] add better logging title --- packages/signals/signals/src/lib/logger/index.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/signals/signals/src/lib/logger/index.ts b/packages/signals/signals/src/lib/logger/index.ts index 8a8f98ef7..e4f6aed23 100644 --- a/packages/signals/signals/src/lib/logger/index.ts +++ b/packages/signals/signals/src/lib/logger/index.ts @@ -33,15 +33,19 @@ class Logger { this.storage.setItem(this.logLevelKey, 'off') } + private log = (level: 'info' | 'debug', ...args: any[]): void => { + console.log(`[signals:${level}]`, ...args) + } + info = (...args: any[]): void => { if (this.logLevel === 'info' || this.logLevel === 'debug') { - console.log('[signals log]', ...args) + this.log('info', ...args) } } debug = (...args: any[]): void => { if (this.logLevel === 'debug') { - console.log('[signals debug]', ...args) + this.log('debug', ...args) } } }