From bf85047e971add497d5c9ab72972394b1f27e887 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:11:49 -0600 Subject: [PATCH] Log events from signals (#1186) --- .changeset/shiny-boats-warn.md | 5 ++ .../signals/src/core/debug-mode/index.ts | 17 +++-- .../signals/signals/src/core/emitter/index.ts | 13 +--- .../signals/src/core/processor/processor.ts | 3 +- .../signals/src/core/signals/settings.ts | 33 ++++------ .../signals/src/core/signals/signals.ts | 11 ++-- .../signals/signals/src/lib/logger/index.ts | 49 +++++++++----- .../__tests__/normalize-url.test.ts | 4 +- .../lib/storage/__tests__/web-storage.test.ts | 65 +++++++++++++++++++ .../signals/src/lib/storage/debug-storage.ts | 29 --------- .../signals/src/lib/storage/web-storage.ts | 37 +++++++++++ .../signals/src/plugin/signals-plugin.ts | 7 +- 12 files changed, 177 insertions(+), 96 deletions(-) create mode 100644 .changeset/shiny-boats-warn.md rename packages/signals/signals/src/lib/{ => normalize-url}/__tests__/normalize-url.test.ts (93%) create mode 100644 packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts 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/.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..1f60764f6 100644 --- a/packages/signals/signals/src/core/debug-mode/index.ts +++ b/packages/signals/signals/src/core/debug-mode/index.ts @@ -17,16 +17,19 @@ export const parseDebugModeQueryString = (): boolean | undefined => { /** * This turns on advanced logging for signals! */ -export const parseSignalsLoggingAdvancedQueryString = (): - | boolean - | undefined => { +export type LogLevelOptions = 'info' | 'debug' | 'off' +export const parseSignalsLogLevel = (): LogLevelOptions | undefined => { 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 === 'string') { + 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..f4f9ba8d5 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 method call:', `analytics.${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..042515933 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,22 +114,14 @@ 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 + private storage = new WebStorage(window.sessionStorage) constructor(disableRedaction?: boolean, enableIngestion?: boolean) { - this.storage = new DebugStorage('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() @@ -140,20 +132,19 @@ export class SignalsDebugSettings { } setAllDebugging = (boolean: boolean) => { - this.storage.setDebugKey(SignalsDebugSettings.redactionKey, boolean) - this.storage.setDebugKey(SignalsDebugSettings.ingestionKey, boolean) - this.storage.setDebugKey(SignalsDebugSettings.logSignals, 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) - } - - getEnableLogSignals = (): boolean => { - return this.storage.getDebugKey(SignalsDebugSettings.logSignals) + 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 425513a7d..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 @@ -38,10 +39,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 +134,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 { + debug(boolean = true, logLevel?: LogLevelOptions): void { this.globalSettings.signalsDebug.setAllDebugging(boolean) + 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 9bb7c0367..e4f6aed23 100644 --- a/packages/signals/signals/src/lib/logger/index.ts +++ b/packages/signals/signals/src/lib/logger/index.ts @@ -1,32 +1,51 @@ -import { parseSignalsLoggingAdvancedQueryString } from '../../core/debug-mode' -import { DebugStorage } from '../storage/debug-storage' +import { + LogLevelOptions, + parseDebugModeQueryString, + parseSignalsLogLevel, +} from '../../core/debug-mode' +import { WebStorage } from '../storage/web-storage' class Logger { - private static advancedLogging = 'segment_signals_logging_advanced' + private logLevelKey = 'segment_signals_log_level' + private storage = new WebStorage(window.sessionStorage) + get logLevel(): LogLevelOptions { + return this.storage.getItem(this.logLevelKey) ?? 'off' + } - storage = new DebugStorage('sessionStorage') constructor() { - const val = parseSignalsLoggingAdvancedQueryString() - if (typeof val === 'boolean') { - this.storage.setDebugKey(Logger.advancedLogging, val) + // 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) + } else { + const debugMode = parseDebugModeQueryString() + if (debugMode === true) { + this.enableLogging('info') + } } } - private debugLoggingEnabled = (): boolean => { - return this.storage.getDebugKey(Logger.advancedLogging) + enableLogging = (type: LogLevelOptions) => { + this.storage.setItem(this.logLevelKey, type) + } + + disableLogging = () => { + this.storage.setItem(this.logLevelKey, 'off') } - enableDebugLogging = (bool = true) => { - this.storage.setDebugKey(Logger.advancedLogging, bool) + private log = (level: 'info' | 'debug', ...args: any[]): void => { + console.log(`[signals:${level}]`, ...args) } - log = (...args: any[]): void => { - console.log('[signals log]', ...args) + info = (...args: any[]): void => { + if (this.logLevel === 'info' || this.logLevel === 'debug') { + this.log('info', ...args) + } } debug = (...args: any[]): void => { - if (this.debugLoggingEnabled()) { - console.log('[signals debug]', ...args) + if (this.logLevel === 'debug') { + this.log('debug', ...args) } } } 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(() => { 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..19c1328dc --- /dev/null +++ b/packages/signals/signals/src/lib/storage/__tests__/web-storage.test.ts @@ -0,0 +1,65 @@ +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 serializing errors gracefully when setting', () => { + const key = 'testKey' + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation() + + webStorage.setItem( + key, + // @ts-ignore - intentational non-serializable value + BigInt(1) + ) + + expect(consoleWarnSpy).toHaveBeenCalledWith( + 'Storage set error', + expect.any(Object), + expect.any(Error) + ) + }) + + it('should handle JSON parsing errors gracefully when retrieving', () => { + 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 retrieval error', + expect.any(Object), + expect.any(Error) + ) + }) + }) +}) 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..f5308ca1c --- /dev/null +++ b/packages/signals/signals/src/lib/storage/web-storage.ts @@ -0,0 +1,37 @@ +export class WebStorage { + private storage: Storage + constructor(storage: Storage) { + this.storage = storage + } + + /** + * Set a json-parsable item in storage + */ + public setItem = ( + key: string, + value: T + ): void => { + try { + const item = JSON.stringify(value) + this.storage.setItem(key, item) + } catch (e) { + console.warn('Storage set error', { key, value }, e) + } + } + + /** + * Get a json-parsed item from storage + */ + public getItem = (key: string): T | undefined => { + try { + const item = this.storage.getItem(key) + if (item === null) { + return undefined + } + return JSON.parse(item) as T + } catch (e) { + console.warn('Storage retrieval error', { key }, 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..cd9b2e133 100644 --- a/packages/signals/signals/src/plugin/signals-plugin.ts +++ b/packages/signals/signals/src/plugin/signals-plugin.ts @@ -28,12 +28,11 @@ 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.enableDebugLogging() + logger.enableLogging('debug') } logger.debug(`SignalsPlugin v${version} initializing`, { @@ -89,7 +88,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) } }