diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 5cb82bbe8..82b6aa028 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -355,7 +355,7 @@ export class DecisionService { reasons: [[CMAB_NOT_SUPPORTED_IN_SYNC]], }); } - + const cmabPromise = this.cmabService.getDecision(configObj, user, experiment.id, decideOptions).then( (cmabDecision) => { return { diff --git a/lib/event_processor/batch_event_processor.spec.ts b/lib/event_processor/batch_event_processor.spec.ts index 3f8809d18..a01a60f33 100644 --- a/lib/event_processor/batch_event_processor.spec.ts +++ b/lib/event_processor/batch_event_processor.spec.ts @@ -15,7 +15,7 @@ */ import { expect, describe, it, vi, beforeEach, afterEach, MockInstance } from 'vitest'; -import { EventWithId, BatchEventProcessor } from './batch_event_processor'; +import { EventWithId, BatchEventProcessor, LOGGER_NAME } from './batch_event_processor'; import { getMockSyncCache } from '../tests/mock/mock_cache'; import { createImpressionEvent } from '../tests/mock/create_event'; import { ProcessableEvent } from './event_processor'; @@ -40,7 +40,7 @@ const exhaustMicrotasks = async (loop = 100) => { } } -describe('QueueingEventProcessor', async () => { +describe('BatchEventProcessor', async () => { beforeEach(() => { vi.useFakeTimers(); }); @@ -49,6 +49,32 @@ describe('QueueingEventProcessor', async () => { vi.useRealTimers(); }); + it('should set name on the logger passed into the constructor', () => { + const logger = getMockLogger(); + + const processor = new BatchEventProcessor({ + eventDispatcher: getMockDispatcher(), + dispatchRepeater: getMockRepeater(), + batchSize: 1000, + logger, + }); + + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + + it('should set name on the logger set by setLogger', () => { + const logger = getMockLogger(); + + const processor = new BatchEventProcessor({ + eventDispatcher: getMockDispatcher(), + dispatchRepeater: getMockRepeater(), + batchSize: 1000, + }); + + processor.setLogger(logger); + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + describe('start', () => { it('should log startupLogs on start', () => { const startupLogs: StartupLog[] = [ diff --git a/lib/event_processor/batch_event_processor.ts b/lib/event_processor/batch_event_processor.ts index 97b4dd8f4..40282a8f3 100644 --- a/lib/event_processor/batch_event_processor.ts +++ b/lib/event_processor/batch_event_processor.ts @@ -60,6 +60,8 @@ type EventBatch = { ids: string[], } +export const LOGGER_NAME = 'BatchEventProcessor'; + export class BatchEventProcessor extends BaseService implements EventProcessor { private eventDispatcher: EventDispatcher; private closingEventDispatcher?: EventDispatcher; @@ -80,7 +82,6 @@ export class BatchEventProcessor extends BaseService implements EventProcessor { this.closingEventDispatcher = config.closingEventDispatcher; this.batchSize = config.batchSize; this.eventStore = config.eventStore; - this.logger = config.logger; this.retryConfig = config.retryConfig; this.dispatchRepeater = config.dispatchRepeater; @@ -88,6 +89,14 @@ export class BatchEventProcessor extends BaseService implements EventProcessor { this.failedEventRepeater = config.failedEventRepeater; this.failedEventRepeater?.setTask(() => this.retryFailedEvents()); + if (config.logger) { + this.setLogger(config.logger); + } + } + + setLogger(logger: LoggerFacade): void { + this.logger = logger; + this.logger.setName(LOGGER_NAME); } onDispatch(handler: Consumer): Fn { diff --git a/lib/event_processor/event_processor.ts b/lib/event_processor/event_processor.ts index f33c1a7a1..3589ce3a5 100644 --- a/lib/event_processor/event_processor.ts +++ b/lib/event_processor/event_processor.ts @@ -17,6 +17,7 @@ import { ConversionEvent, ImpressionEvent } from './event_builder/user_event' import { LogEvent } from './event_dispatcher/event_dispatcher' import { Service } from '../service' import { Consumer, Fn } from '../utils/type'; +import { LoggerFacade } from '../logging/logger'; export const DEFAULT_FLUSH_INTERVAL = 30000 // Unit is ms - default flush interval is 30s export const DEFAULT_BATCH_SIZE = 10 @@ -26,4 +27,5 @@ export type ProcessableEvent = ConversionEvent | ImpressionEvent export interface EventProcessor extends Service { process(event: ProcessableEvent): Promise; onDispatch(handler: Consumer): Fn; + setLogger(logger: LoggerFacade): void; } diff --git a/lib/event_processor/forwarding_event_processor.ts b/lib/event_processor/forwarding_event_processor.ts index 67899bddb..744ac5975 100644 --- a/lib/event_processor/forwarding_event_processor.ts +++ b/lib/event_processor/forwarding_event_processor.ts @@ -25,6 +25,7 @@ import { EventEmitter } from '../utils/event_emitter/event_emitter'; import { Consumer, Fn } from '../utils/type'; import { SERVICE_STOPPED_BEFORE_RUNNING } from 'error_message'; import { OptimizelyError } from '../error/optimizly_error'; + class ForwardingEventProcessor extends BaseService implements EventProcessor { private dispatcher: EventDispatcher; private eventEmitter: EventEmitter<{ dispatch: LogEvent }>; diff --git a/lib/logging/logger.ts b/lib/logging/logger.ts index 568bd2cac..8414d544a 100644 --- a/lib/logging/logger.ts +++ b/lib/logging/logger.ts @@ -43,7 +43,8 @@ export interface LoggerFacade { debug(message: string | Error, ...args: any[]): void; warn(message: string | Error, ...args: any[]): void; error(message: string | Error, ...args: any[]): void; - child(name: string): LoggerFacade; + child(name?: string): LoggerFacade; + setName(name: string): void; } export interface LogHandler { @@ -84,7 +85,7 @@ type OptimizelyLoggerConfig = { export class OptimizelyLogger implements LoggerFacade { private name?: string; - private prefix: string; + private prefix = ''; private logHandler: LogHandler; private infoResolver?: MessageResolver; private errorResolver: MessageResolver; @@ -95,11 +96,12 @@ export class OptimizelyLogger implements LoggerFacade { this.infoResolver = config.infoMsgResolver; this.errorResolver = config.errorMsgResolver; this.level = config.level; - this.name = config.name; - this.prefix = this.name ? `${this.name}: ` : ''; + if (config.name) { + this.setName(config.name); + } } - child(name: string): OptimizelyLogger { + child(name?: string): OptimizelyLogger { return new OptimizelyLogger({ logHandler: this.logHandler, infoMsgResolver: this.infoResolver, @@ -109,6 +111,11 @@ export class OptimizelyLogger implements LoggerFacade { }); } + setName(name: string): void { + this.name = name; + this.prefix = `${name}: `; + } + info(message: string | Error, ...args: any[]): void { this.log(LogLevel.Info, message, args) } diff --git a/lib/odp/event_manager/odp_event_api_manager.spec.ts b/lib/odp/event_manager/odp_event_api_manager.spec.ts index 316787821..04d74ea18 100644 --- a/lib/odp/event_manager/odp_event_api_manager.spec.ts +++ b/lib/odp/event_manager/odp_event_api_manager.spec.ts @@ -15,7 +15,7 @@ */ import { describe, it, expect, vi } from 'vitest'; -import { DefaultOdpEventApiManager, eventApiRequestGenerator, pixelApiRequestGenerator } from './odp_event_api_manager'; +import { DefaultOdpEventApiManager, eventApiRequestGenerator, LOGGER_NAME, pixelApiRequestGenerator } from './odp_event_api_manager'; import { OdpEvent } from './odp_event'; import { OdpConfig } from '../odp_config'; @@ -41,8 +41,28 @@ const PIXEL_URL = 'https://odp.pixel.com'; const odpConfig = new OdpConfig(API_KEY, API_HOST, PIXEL_URL, []); import { getMockRequestHandler } from '../../tests/mock/mock_request_handler'; +import { getMockLogger } from '../../tests/mock/mock_logger'; describe('DefaultOdpEventApiManager', () => { + it('should set name on the logger passed into the constructor', () => { + const logger = getMockLogger(); + const requestHandler = getMockRequestHandler(); + + const manager = new DefaultOdpEventApiManager(requestHandler, vi.fn(), logger); + + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + + it('should set name on the logger set by setLogger', () => { + const logger = getMockLogger(); + const requestHandler = getMockRequestHandler(); + + const manager = new DefaultOdpEventApiManager(requestHandler, vi.fn()); + manager.setLogger(logger); + + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + it('should generate the event request using the correct odp config and event', async () => { const mockRequestHandler = getMockRequestHandler(); mockRequestHandler.makeRequest.mockReturnValue({ diff --git a/lib/odp/event_manager/odp_event_api_manager.ts b/lib/odp/event_manager/odp_event_api_manager.ts index 23dec6274..79154b06e 100644 --- a/lib/odp/event_manager/odp_event_api_manager.ts +++ b/lib/odp/event_manager/odp_event_api_manager.ts @@ -24,6 +24,7 @@ export type EventDispatchResponse = { }; export interface OdpEventApiManager { sendEvents(odpConfig: OdpConfig, events: OdpEvent[]): Promise; + setLogger(logger: LoggerFacade): void; } export type EventRequest = { @@ -34,6 +35,9 @@ export type EventRequest = { } export type EventRequestGenerator = (odpConfig: OdpConfig, events: OdpEvent[]) => EventRequest; + +export const LOGGER_NAME = 'OdpEventApiManager'; + export class DefaultOdpEventApiManager implements OdpEventApiManager { private logger?: LoggerFacade; private requestHandler: RequestHandler; @@ -46,9 +50,16 @@ export class DefaultOdpEventApiManager implements OdpEventApiManager { ) { this.requestHandler = requestHandler; this.requestGenerator = requestDataGenerator; - this.logger = logger; + if (logger) { + this.setLogger(logger) + } } + setLogger(logger: LoggerFacade): void { + this.logger = logger; + this.logger.setName(LOGGER_NAME); + } + async sendEvents(odpConfig: OdpConfig, events: OdpEvent[]): Promise { if (events.length === 0) { return {}; diff --git a/lib/odp/event_manager/odp_event_manager.spec.ts b/lib/odp/event_manager/odp_event_manager.spec.ts index 9d16273b9..6fb5db08a 100644 --- a/lib/odp/event_manager/odp_event_manager.spec.ts +++ b/lib/odp/event_manager/odp_event_manager.spec.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; -import { DefaultOdpEventManager } from './odp_event_manager'; +import { DefaultOdpEventManager, LOGGER_NAME } from './odp_event_manager'; import { getMockRepeater } from '../../tests/mock/mock_repeater'; import { getMockLogger } from '../../tests/mock/mock_logger'; import { ServiceState } from '../../service'; @@ -46,6 +46,7 @@ const makeEvent = (id: number) => { const getMockApiManager = () => { return { sendEvents: vi.fn(), + setLogger: vi.fn(), }; }; @@ -72,6 +73,44 @@ describe('DefaultOdpEventManager', () => { expect(odpEventManager.getState()).toBe(ServiceState.New); }); + it('should set name on the logger set using setLogger', () => { + const logger = getMockLogger(); + + const manager = new DefaultOdpEventManager({ + repeater: getMockRepeater(), + apiManager: getMockApiManager(), + batchSize: 10, + retryConfig: { + maxRetries: 3, + backoffProvider: vi.fn(), + }, + }); + + manager.setLogger(logger); + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + + it('should pass a child logger to the event api manager when a logger is set using setLogger', () => { + const logger = getMockLogger(); + const childLogger = getMockLogger(); + logger.child.mockReturnValue(childLogger); + + const apiManager = getMockApiManager(); + + const manager = new DefaultOdpEventManager({ + repeater: getMockRepeater(), + apiManager, + batchSize: 10, + retryConfig: { + maxRetries: 3, + backoffProvider: vi.fn(), + }, + }); + + manager.setLogger(logger); + expect(apiManager.setLogger).toHaveBeenCalledWith(childLogger); + }); + it('should stay in starting state if started with a odpIntegationConfig and not resolve or reject onRunning', async () => { const odpEventManager = new DefaultOdpEventManager({ repeater: getMockRepeater(), diff --git a/lib/odp/event_manager/odp_event_manager.ts b/lib/odp/event_manager/odp_event_manager.ts index 75c1a632c..a076655b5 100644 --- a/lib/odp/event_manager/odp_event_manager.ts +++ b/lib/odp/event_manager/odp_event_manager.ts @@ -34,10 +34,12 @@ import { ODP_EVENT_MANAGER_STOPPED } from 'error_message'; import { OptimizelyError } from '../../error/optimizly_error'; +import { LoggerFacade } from '../../logging/logger'; export interface OdpEventManager extends Service { updateConfig(odpIntegrationConfig: OdpIntegrationConfig): void; sendEvent(event: OdpEvent): void; + setLogger(logger: LoggerFacade): void; } export type RetryConfig = { @@ -53,6 +55,8 @@ export type OdpEventManagerConfig = { retryConfig: RetryConfig, }; +export const LOGGER_NAME = 'OdpEventManager'; + export class DefaultOdpEventManager extends BaseService implements OdpEventManager { private queue: OdpEvent[] = []; private repeater: Repeater; @@ -73,6 +77,12 @@ export class DefaultOdpEventManager extends BaseService implements OdpEventManag this.repeater.setTask(() => this.flush()); } + setLogger(logger: LoggerFacade): void { + this.logger = logger; + this.logger.setName(LOGGER_NAME); + this.apiManager.setLogger(logger.child()); + } + private async executeDispatch(odpConfig: OdpConfig, batch: OdpEvent[]): Promise { const res = await this.apiManager.sendEvents(odpConfig, batch); if (res.statusCode && !isSuccessStatusCode(res.statusCode)) { diff --git a/lib/odp/odp_manager.spec.ts b/lib/odp/odp_manager.spec.ts index 26c6e82e0..376a663cf 100644 --- a/lib/odp/odp_manager.spec.ts +++ b/lib/odp/odp_manager.spec.ts @@ -16,7 +16,7 @@ import { describe, it, vi, expect } from 'vitest'; -import { DefaultOdpManager } from './odp_manager'; +import { DefaultOdpManager, LOGGER_NAME } from './odp_manager'; import { ServiceState } from '../service'; import { resolvablePromise } from '../utils/promise/resolvablePromise'; import { OdpConfig } from './odp_config'; @@ -25,6 +25,7 @@ import { ODP_USER_KEY } from './constant'; import { OptimizelySegmentOption } from './segment_manager/optimizely_segment_option'; import { OdpEventManager } from './event_manager/odp_event_manager'; import { CLIENT_VERSION, JAVASCRIPT_CLIENT_ENGINE } from '../utils/enums'; +import { getMockLogger } from '../tests/mock/mock_logger'; const keyA = 'key-a'; const hostA = 'host-a'; @@ -51,6 +52,7 @@ const getMockOdpEventManager = () => { updateConfig: vi.fn(), sendEvent: vi.fn(), makeDisposable: vi.fn(), + setLogger: vi.fn(), }; }; @@ -58,10 +60,79 @@ const getMockOdpSegmentManager = () => { return { fetchQualifiedSegments: vi.fn(), updateConfig: vi.fn(), + setLogger: vi.fn(), }; }; describe('DefaultOdpManager', () => { + describe('a logger is passed in the constructor', () => { + it('should set name on the logger passed into the constructor', () => { + const logger = getMockLogger(); + const manager = new DefaultOdpManager({ + logger, + eventManager: getMockOdpEventManager(), + segmentManager: getMockOdpSegmentManager(), + }); + + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + + it('should pass different child loggers to the eventManager and segmentManager', () => { + const logger = getMockLogger(); + const eventChildLogger = getMockLogger(); + const segmentChildLogger = getMockLogger(); + + logger.child.mockReturnValueOnce(eventChildLogger) + .mockReturnValueOnce(segmentChildLogger); + + const eventManager = getMockOdpEventManager(); + const segmentManager = getMockOdpSegmentManager(); + + const manager = new DefaultOdpManager({ + logger, + eventManager, + segmentManager, + }); + + expect(eventManager.setLogger).toHaveBeenCalledWith(eventChildLogger); + expect(segmentManager.setLogger).toHaveBeenCalledWith(segmentChildLogger); + }); + }); + + describe('setLogger method', () => { + it('should set name on the logger', () => { + const logger = getMockLogger(); + const manager = new DefaultOdpManager({ + eventManager: getMockOdpEventManager(), + segmentManager: getMockOdpSegmentManager(), + }); + + manager.setLogger(logger); + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + + it('should pass a child logger to the datafileManager', () => { + const logger = getMockLogger(); + const eventChildLogger = getMockLogger(); + const segmentChildLogger = getMockLogger(); + + logger.child.mockReturnValueOnce(eventChildLogger) + .mockReturnValueOnce(segmentChildLogger); + + const eventManager = getMockOdpEventManager(); + const segmentManager = getMockOdpSegmentManager(); + + const manager = new DefaultOdpManager({ + eventManager, + segmentManager, + }); + manager.setLogger(logger); + + expect(eventManager.setLogger).toHaveBeenCalledWith(eventChildLogger); + expect(segmentManager.setLogger).toHaveBeenCalledWith(segmentChildLogger); + }); + }); + it('should be in new state on construction', () => { const odpManager = new DefaultOdpManager({ segmentManager: getMockOdpSegmentManager(), diff --git a/lib/odp/odp_manager.ts b/lib/odp/odp_manager.ts index 6e7da8769..68a2b2c79 100644 --- a/lib/odp/odp_manager.ts +++ b/lib/odp/odp_manager.ts @@ -39,6 +39,7 @@ export interface OdpManager extends Service { sendEvent(event: OdpEvent): void; setClientInfo(clientEngine: string, clientVersion: string): void; setVuid(vuid: string): void; + setLogger(logger: LoggerFacade): void; } export type OdpManagerConfig = { @@ -48,6 +49,8 @@ export type OdpManagerConfig = { userAgentParser?: UserAgentParser; }; +export const LOGGER_NAME = 'OdpManager'; + export class DefaultOdpManager extends BaseService implements OdpManager { private configPromise: ResolvablePromise; private segmentManager: OdpSegmentManager; @@ -62,7 +65,6 @@ export class DefaultOdpManager extends BaseService implements OdpManager { super(); this.segmentManager = config.segmentManager; this.eventManager = config.eventManager; - this.logger = config.logger; this.configPromise = resolvablePromise(); @@ -80,8 +82,19 @@ export class DefaultOdpManager extends BaseService implements OdpManager { Object.entries(userAgentInfo).filter(([_, value]) => value != null && value != undefined) ); } + + if (config.logger) { + this.setLogger(config.logger); + } } + setLogger(logger: LoggerFacade): void { + this.logger = logger; + this.logger.setName(LOGGER_NAME); + this.eventManager.setLogger(logger.child()); + this.segmentManager.setLogger(logger.child()); + } + setClientInfo(clientEngine: string, clientVersion: string): void { this.clientEngine = clientEngine; this.clientVersion = clientVersion; diff --git a/lib/odp/segment_manager/odp_segment_api_manager.spec.ts b/lib/odp/segment_manager/odp_segment_api_manager.spec.ts index ad07894bd..7906f5745 100644 --- a/lib/odp/segment_manager/odp_segment_api_manager.spec.ts +++ b/lib/odp/segment_manager/odp_segment_api_manager.spec.ts @@ -19,7 +19,7 @@ import { describe, it, expect } from 'vitest'; import { ODP_USER_KEY } from '../constant'; import { getMockRequestHandler } from '../../tests/mock/mock_request_handler'; import { getMockLogger } from '../../tests/mock/mock_logger'; -import { DefaultOdpSegmentApiManager } from './odp_segment_api_manager'; +import { DefaultOdpSegmentApiManager, LOGGER_NAME } from './odp_segment_api_manager'; const API_KEY = 'not-real-api-key'; const GRAPHQL_ENDPOINT = 'https://some.example.com/graphql/endpoint'; @@ -28,7 +28,24 @@ const USER_VALUE = 'tester-101'; const SEGMENTS_TO_CHECK = ['has_email', 'has_email_opted_in', 'push_on_sale']; describe('DefaultOdpSegmentApiManager', () => { - it('should return empty list without calling api when segmentsToCheck is empty', async () => { + it('should set name on the logger passed into the constructor', () => { + const logger = getMockLogger(); + + const manager = new DefaultOdpSegmentApiManager(getMockRequestHandler(), logger); + + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + + it('should set name on the logger set by setLogger', () => { + const logger = getMockLogger(); + + const manager = new DefaultOdpSegmentApiManager(getMockRequestHandler()); + manager.setLogger(logger); + + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + + it('should return empty list without calling api when segmentsToCheck is empty', async () => { const requestHandler = getMockRequestHandler(); requestHandler.makeRequest.mockReturnValue({ abort: () => {}, diff --git a/lib/odp/segment_manager/odp_segment_api_manager.ts b/lib/odp/segment_manager/odp_segment_api_manager.ts index 1c336b298..92eeaa02e 100644 --- a/lib/odp/segment_manager/odp_segment_api_manager.ts +++ b/lib/odp/segment_manager/odp_segment_api_manager.ts @@ -20,6 +20,7 @@ import { OdpResponseSchema } from './odp_response_schema'; import { ODP_USER_KEY } from '../constant'; import { RequestHandler } from '../../utils/http_request_handler/http'; import { Response as GraphQLResponse } from '../odp_types'; +import { log } from 'console'; /** * Expected value for a qualified/valid segment */ @@ -48,15 +49,25 @@ export interface OdpSegmentApiManager { userValue: string, segmentsToCheck: string[] ): Promise; + setLogger(logger: LoggerFacade): void; } +export const LOGGER_NAME = 'OdpSegmentApiManager'; + export class DefaultOdpSegmentApiManager implements OdpSegmentApiManager { - private readonly logger?: LoggerFacade; - private readonly requestHandler: RequestHandler; + private logger?: LoggerFacade; + private requestHandler: RequestHandler; constructor(requestHandler: RequestHandler, logger?: LoggerFacade) { this.requestHandler = requestHandler; + if (logger) { + this.setLogger(logger); + } + } + + setLogger(logger: LoggerFacade): void { this.logger = logger; + this.logger.setName(LOGGER_NAME); } /** diff --git a/lib/odp/segment_manager/odp_segment_manager.spec.ts b/lib/odp/segment_manager/odp_segment_manager.spec.ts index 31598dd71..550e431b5 100644 --- a/lib/odp/segment_manager/odp_segment_manager.spec.ts +++ b/lib/odp/segment_manager/odp_segment_manager.spec.ts @@ -23,6 +23,7 @@ import { OdpConfig } from '../odp_config'; import { OptimizelySegmentOption } from './optimizely_segment_option'; import { getMockLogger } from '../../tests/mock/mock_logger'; import { getMockSyncCache } from '../../tests/mock/mock_cache'; +import { LOGGER_NAME } from './odp_segment_manager'; const API_KEY = 'test-api-key'; const API_HOST = 'https://odp.example.com'; @@ -34,6 +35,7 @@ const config = new OdpConfig(API_KEY, API_HOST, PIXEL_URL, SEGMENTS_TO_CHECK); const getMockApiManager = () => { return { fetchSegments: vi.fn(), + setLogger: vi.fn(), }; }; @@ -41,6 +43,50 @@ const userKey: ODP_USER_KEY = ODP_USER_KEY.FS_USER_ID; const userValue = 'test-user'; describe('DefaultOdpSegmentManager', () => { + describe('a logger is passed in the constructor', () => { + it('should set name on the logger passed into the constructor', () => { + const logger = getMockLogger(); + const cache = getMockSyncCache(); + const manager = new DefaultOdpSegmentManager(cache, getMockApiManager(), logger); + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + + it('should pass a child logger to the segmentApiManager', () => { + const logger = getMockLogger(); + const childLogger = getMockLogger(); + logger.child.mockReturnValue(childLogger); + + const cache = getMockSyncCache(); + const apiManager = getMockApiManager(); + const manager = new DefaultOdpSegmentManager(cache, apiManager, logger); + + expect(apiManager.setLogger).toHaveBeenCalledWith(childLogger); + }); + }); + + describe('setLogger method', () => { + it('should set name on the logger', () => { + const logger = getMockLogger(); + const cache = getMockSyncCache(); + const manager = new DefaultOdpSegmentManager(cache, getMockApiManager()); + manager.setLogger(logger); + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME); + }); + + it('should pass a child logger to the datafileManager', () => { + const logger = getMockLogger(); + const childLogger = getMockLogger(); + logger.child.mockReturnValue(childLogger); + + const cache = getMockSyncCache(); + const apiManager = getMockApiManager(); + const manager = new DefaultOdpSegmentManager(cache, apiManager); + manager.setLogger(logger); + + expect(apiManager.setLogger).toHaveBeenCalledWith(childLogger); + }); + }); + it('should return null and log error if the ODP config is not available.', async () => { const logger = getMockLogger(); const cache = getMockSyncCache(); diff --git a/lib/odp/segment_manager/odp_segment_manager.ts b/lib/odp/segment_manager/odp_segment_manager.ts index d243f2a14..8ba589dd4 100644 --- a/lib/odp/segment_manager/odp_segment_manager.ts +++ b/lib/odp/segment_manager/odp_segment_manager.ts @@ -29,8 +29,11 @@ export interface OdpSegmentManager { options?: Array ): Promise; updateConfig(config: OdpIntegrationConfig): void; + setLogger(logger: LoggerFacade): void; } +export const LOGGER_NAME = 'OdpSegmentManager'; + export class DefaultOdpSegmentManager implements OdpSegmentManager { private odpIntegrationConfig?: OdpIntegrationConfig; private segmentsCache: Cache; @@ -44,7 +47,15 @@ export class DefaultOdpSegmentManager implements OdpSegmentManager { ) { this.segmentsCache = segmentsCache; this.odpSegmentApiManager = odpSegmentApiManager; + if (logger) { + this.setLogger(logger); + } + } + + setLogger(logger: LoggerFacade): void { this.logger = logger; + this.logger.setName(LOGGER_NAME); + this.odpSegmentApiManager.setLogger(logger.child()); } /** diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index 1d41c7982..dfe708de4 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import Optimizely from '.'; import { getMockProjectConfigManager } from '../tests/mock/mock_project_config_manager'; import * as jsonSchemaValidator from '../utils/json_schema_validator'; @@ -42,6 +42,10 @@ describe('Optimizely', () => { const odpManager = extractOdpManager(createOdpManager({})); const logger = getMockLogger(); + beforeEach(() => { + vi.clearAllMocks(); + }); + it('should pass disposable options to the respective services', () => { const projectConfigManager = getMockProjectConfigManager({ initConfig: createProjectConfig(testData.getTestProjectConfig()), @@ -67,6 +71,42 @@ describe('Optimizely', () => { expect(odpManager.makeDisposable).toHaveBeenCalled(); }); + it('should set child logger to respective services', () => { + const projectConfigManager = getMockProjectConfigManager({ + initConfig: createProjectConfig(testData.getTestProjectConfig()), + }); + + const eventProcessor = getForwardingEventProcessor(eventDispatcher); + const odpManager = extractOdpManager(createOdpManager({})); + + vi.spyOn(projectConfigManager, 'setLogger'); + vi.spyOn(eventProcessor, 'setLogger'); + vi.spyOn(odpManager, 'setLogger'); + + const logger = getMockLogger(); + const configChildLogger = getMockLogger(); + const eventProcessorChildLogger = getMockLogger(); + const odpManagerChildLogger = getMockLogger(); + vi.spyOn(logger, 'child').mockReturnValueOnce(configChildLogger) + .mockReturnValueOnce(eventProcessorChildLogger) + .mockReturnValueOnce(odpManagerChildLogger); + + new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager, + jsonSchemaValidator, + logger, + eventProcessor, + odpManager, + disposable: true, + cmabService: {} as any + }); + + expect(projectConfigManager.setLogger).toHaveBeenCalledWith(configChildLogger); + expect(eventProcessor.setLogger).toHaveBeenCalledWith(eventProcessorChildLogger); + expect(odpManager.setLogger).toHaveBeenCalledWith(odpManagerChildLogger); + }); + describe('decideAsync', () => { it('should return an error decision with correct reasons if decisionService returns error', async () => { const projectConfig = createProjectConfig(getDecisionTestDatafile()); diff --git a/lib/optimizely/index.tests.js b/lib/optimizely/index.tests.js index 21209e67d..5ec683a92 100644 --- a/lib/optimizely/index.tests.js +++ b/lib/optimizely/index.tests.js @@ -9234,6 +9234,7 @@ describe('lib/optimizely', function() { onRunning: sinon.stub(), onTerminated: sinon.stub(), onDispatch: sinon.stub(), + setLogger: sinon.stub(), }; }); diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index 42e70ff48..6895fcea7 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -56,8 +56,6 @@ import { DECISION_SOURCES, DECISION_MESSAGES, FEATURE_VARIABLE_TYPES, - // DECISION_NOTIFICATION_TYPES, - // NOTIFICATION_TYPES, NODE_CLIENT_ENGINE, CLIENT_VERSION, } from '../utils/enums'; @@ -178,6 +176,13 @@ export default class Optimizely extends BaseService implements Client { this.odpManager?.makeDisposable(); } + // pass a child logger to sub-components + if (this.logger) { + this.projectConfigManager.setLogger(this.logger.child()); + this.eventProcessor?.setLogger(this.logger.child()); + this.odpManager?.setLogger(this.logger.child()); + } + let decideOptionsArray = config.defaultDecideOptions ?? []; if (!Array.isArray(decideOptionsArray)) { @@ -1280,27 +1285,22 @@ export default class Optimizely extends BaseService implements Client { /** * Returns a Promise that fulfills when this instance is ready to use (meaning - * it has a valid datafile), or has failed to become ready within a period of + * it has a valid datafile), or rejects when it has failed to become ready within a period of * time (configurable by the timeout property of the options argument), or when - * this instance is closed via the close method. + * this instance is closed via the close method before it became ready. * - * If a valid datafile was provided in the constructor, the returned Promise is - * immediately fulfilled. If an sdkKey was provided, a manager will be used to - * fetch a datafile, and the returned promise will fulfill if that fetch - * succeeds or fails before the timeout. The default timeout is 30 seconds, - * which will be used if no timeout is provided in the argument options object. + * If a static project config manager with a valid datafile was provided in the constructor, + * the returned Promise is immediately fulfilled. If a polling config manager was provided, + * it will be used to fetch a datafile, and the returned promise will fulfill if that fetch + * succeeds, or it will reject if the datafile fetch does not complete before the timeout. + * The default timeout is 30 seconds. * - * The returned Promise is fulfilled with a result object containing these - * properties: - * - success (boolean): True if this instance is ready to use with a valid - * datafile, or false if this instance failed to become - * ready or was closed prior to becoming ready. - * - reason (string=): If success is false, this is a string property with - * an explanatory message. Failure could be due to - * expiration of the timeout, network errors, - * unsuccessful responses, datafile parse errors, - * datafile validation errors, or the instance being - * closed + * The returned Promise is fulfilled with an unknown result which is not needed to + * be inspected to know that the instance is ready. If the promise is fulfilled, it + * is guaranteed that the instance is ready to use. If the promise is rejected, it + * means the instance is not ready to use, and the reason for the promise rejection + * will contain an error denoting the cause of failure. + * @param {Object=} options * @param {number|undefined} options.timeout * @return {Promise} diff --git a/lib/project_config/optimizely_config.spec.ts b/lib/project_config/optimizely_config.spec.ts index ab8d3ab5d..6e9e6747b 100644 --- a/lib/project_config/optimizely_config.spec.ts +++ b/lib/project_config/optimizely_config.spec.ts @@ -25,6 +25,7 @@ import { } from '../tests/test_data'; import { Experiment } from '../shared_types'; import { LoggerFacade } from '../logging/logger'; +import { getMockLogger } from '../tests/mock/mock_logger'; const datafile: ProjectConfig = getTestProjectConfigWithFeatures(); const typedAudienceDatafile = getTypedAudiencesConfig(); @@ -53,13 +54,7 @@ describe('Optimizely Config', () => { let optimizelySimilarExperimentkeyConfigObject: OptimizelyConfig; let projectSimilarExperimentKeyConfigObject: ProjectConfig; - const logger: LoggerFacade = { - info: vi.fn(), - debug: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - child: vi.fn().mockReturnValue(this), - }; + const logger = getMockLogger(); beforeEach(() => { projectConfigObject = createProjectConfig(cloneDeep(datafile as any)); diff --git a/lib/project_config/polling_datafile_manager.spec.ts b/lib/project_config/polling_datafile_manager.spec.ts index c8f68a1cc..a5654fa5d 100644 --- a/lib/project_config/polling_datafile_manager.spec.ts +++ b/lib/project_config/polling_datafile_manager.spec.ts @@ -15,7 +15,7 @@ */ import { describe, it, expect, vi } from 'vitest'; -import { PollingDatafileManager} from './polling_datafile_manager'; +import { LOGGER_NAME, PollingDatafileManager} from './polling_datafile_manager'; import { getMockRepeater } from '../tests/mock/mock_repeater'; import { getMockAbortableRequest, getMockRequestHandler } from '../tests/mock/mock_request_handler'; import { getMockLogger } from '../tests/mock/mock_logger'; @@ -25,7 +25,38 @@ import { ServiceState, StartupLog } from '../service'; import { getMockSyncCache, getMockAsyncCache } from '../tests/mock/mock_cache'; import { LogLevel } from '../logging/logger'; + describe('PollingDatafileManager', () => { + it('should set name on the logger passed into the constructor', () => { + const repeater = getMockRepeater(); + const requestHandler = getMockRequestHandler(); + const logger = getMockLogger(); + + const manager = new PollingDatafileManager({ + repeater, + requestHandler, + sdkKey: '123', + logger, + }); + + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME) + }); + + it('should set name on the logger set by setLogger', () => { + const repeater = getMockRepeater(); + const requestHandler = getMockRequestHandler(); + const logger = getMockLogger(); + + const manager = new PollingDatafileManager({ + repeater, + requestHandler, + sdkKey: '123', + }); + + manager.setLogger(logger); + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME) + }); + it('should log polling interval below MIN_UPDATE_INTERVAL', () => { const repeater = getMockRepeater(); const requestHandler = getMockRequestHandler(); diff --git a/lib/project_config/polling_datafile_manager.ts b/lib/project_config/polling_datafile_manager.ts index a8fb9128b..bac6adc1e 100644 --- a/lib/project_config/polling_datafile_manager.ts +++ b/lib/project_config/polling_datafile_manager.ts @@ -36,6 +36,9 @@ import { SAVED_LAST_MODIFIED_HEADER_VALUE_FROM_RESPONSE, } from 'log_message'; import { OptimizelyError } from '../error/optimizly_error'; +import { LoggerFacade } from '../logging/logger'; + +export const LOGGER_NAME = 'PollingDatafileManager'; export class PollingDatafileManager extends BaseService implements DatafileManager { private requestHandler: RequestHandler; @@ -74,12 +77,20 @@ export class PollingDatafileManager extends BaseService implements DatafileManag this.autoUpdate = autoUpdate; this.initRetryRemaining = initRetry; this.repeater = repeater; - this.logger = logger; + + if (logger) { + this.setLogger(logger); + } const urlTemplateToUse = urlTemplate || (datafileAccessToken ? DEFAULT_AUTHENTICATED_URL_TEMPLATE : DEFAULT_URL_TEMPLATE); this.datafileUrl = sprintf(urlTemplateToUse, this.sdkKey); } + + setLogger(logger: LoggerFacade): void { + this.logger = logger; + this.logger.setName(LOGGER_NAME); + } onUpdate(listener: Consumer): Fn { return this.emitter.on('update', listener); diff --git a/lib/project_config/project_config.spec.ts b/lib/project_config/project_config.spec.ts index 36ffbe89a..54aa75d97 100644 --- a/lib/project_config/project_config.spec.ts +++ b/lib/project_config/project_config.spec.ts @@ -29,20 +29,13 @@ import { FEATURE_NOT_IN_DATAFILE, UNABLE_TO_CAST_VALUE, } from 'error_message'; +import { getMockLogger } from '../tests/mock/mock_logger'; import { VariableType } from '../shared_types'; import { OptimizelyError } from '../error/optimizly_error'; -const createLogger = (...args: any) => ({ - debug: () => {}, - info: () => {}, - warn: () => {}, - error: () => {}, - child: () => createLogger(), -}); - const buildLogMessageFromArgs = (args: any[]) => sprintf(args[1], ...args.splice(2)); const cloneDeep = (obj: any) => JSON.parse(JSON.stringify(obj)); -const logger = createLogger(); +const logger = getMockLogger(); describe('createProjectConfig', () => { let configObj: ProjectConfig; @@ -283,13 +276,12 @@ describe('createProjectConfig - cmab experiments', () => { describe('getExperimentId', () => { let testData: Record; let configObj: ProjectConfig; - let createdLogger: any; + let createdLogger: ReturnType; beforeEach(function() { testData = cloneDeep(testDatafile.getTestProjectConfig()); configObj = projectConfig.createProjectConfig(cloneDeep(testData) as JSON); - createdLogger = createLogger({ logLevel: LOG_LEVEL.INFO }); - vi.spyOn(createdLogger, 'warn'); + createdLogger = getMockLogger(); }); it('should retrieve experiment ID for valid experiment key in getExperimentId', function() { @@ -334,13 +326,12 @@ describe('getLayerId', () => { describe('getAttributeId', () => { let testData: Record; let configObj: ProjectConfig; - let createdLogger: any; + let createdLogger: ReturnType; beforeEach(function() { testData = cloneDeep(testDatafile.getTestProjectConfig()); configObj = projectConfig.createProjectConfig(cloneDeep(testData) as JSON); - createdLogger = createLogger({ logLevel: LOG_LEVEL.INFO }); - vi.spyOn(createdLogger, 'warn'); + createdLogger = getMockLogger(); }); it('should retrieve attribute ID for valid attribute key in getAttributeId', function() { @@ -538,16 +529,12 @@ describe('getSendFlagDecisionsValue', () => { }); describe('getVariableForFeature', function() { - let featureManagementLogger: ReturnType; + let featureManagementLogger: ReturnType; let configObj: ProjectConfig; beforeEach(() => { - featureManagementLogger = createLogger({ logLevel: LOG_LEVEL.INFO }); + featureManagementLogger = getMockLogger(); configObj = projectConfig.createProjectConfig(testDatafile.getTestProjectConfigWithFeatures()); - vi.spyOn(featureManagementLogger, 'warn'); - vi.spyOn(featureManagementLogger, 'error'); - vi.spyOn(featureManagementLogger, 'info'); - vi.spyOn(featureManagementLogger, 'debug'); }); afterEach(() => { @@ -603,16 +590,12 @@ describe('getVariableForFeature', function() { }); describe('getVariableValueForVariation', () => { - let featureManagementLogger: ReturnType; + let featureManagementLogger: ReturnType; let configObj: ProjectConfig; beforeEach(() => { - featureManagementLogger = createLogger({ logLevel: LOG_LEVEL.INFO }); + featureManagementLogger = getMockLogger(); configObj = projectConfig.createProjectConfig(testDatafile.getTestProjectConfigWithFeatures()); - vi.spyOn(featureManagementLogger, 'warn'); - vi.spyOn(featureManagementLogger, 'error'); - vi.spyOn(featureManagementLogger, 'info'); - vi.spyOn(featureManagementLogger, 'debug'); }); afterEach(() => { @@ -695,16 +678,12 @@ describe('getVariableValueForVariation', () => { }); describe('getTypeCastValue', () => { - let featureManagementLogger: ReturnType; + let featureManagementLogger: ReturnType; let configObj: ProjectConfig; beforeEach(() => { - featureManagementLogger = createLogger({ logLevel: LOG_LEVEL.INFO }); + featureManagementLogger = getMockLogger(); configObj = projectConfig.createProjectConfig(testDatafile.getTestProjectConfigWithFeatures()); - vi.spyOn(featureManagementLogger, 'warn'); - vi.spyOn(featureManagementLogger, 'error'); - vi.spyOn(featureManagementLogger, 'info'); - vi.spyOn(featureManagementLogger, 'debug'); }); afterEach(() => { @@ -1065,7 +1044,6 @@ describe('tryCreatingProjectConfig', () => { beforeEach(() => { mockJsonSchemaValidator = vi.fn().mockReturnValue(true); vi.spyOn(configValidator, 'validateDatafile').mockReturnValue(true); - vi.spyOn(logger, 'error'); }); afterEach(() => { diff --git a/lib/project_config/project_config_manager.spec.ts b/lib/project_config/project_config_manager.spec.ts index acd8538ee..3e3236644 100644 --- a/lib/project_config/project_config_manager.spec.ts +++ b/lib/project_config/project_config_manager.spec.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { describe, it, expect, vi } from 'vitest'; -import { ProjectConfigManagerImpl } from './project_config_manager'; +import { LOGGER_NAME, ProjectConfigManagerImpl } from './project_config_manager'; import { getMockLogger } from '../tests/mock/mock_logger'; import { ServiceState } from '../service'; import * as testData from '../tests/test_data'; @@ -26,6 +26,46 @@ import { wait } from '../tests/testUtils'; const cloneDeep = (x: any) => JSON.parse(JSON.stringify(x)); describe('ProjectConfigManagerImpl', () => { + describe('a logger is passed in the constructor', () => { + it('should set name on the logger passed into the constructor', () => { + const logger = getMockLogger(); + const manager = new ProjectConfigManagerImpl({ logger }); + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME) + }); + + it('should pass a child logger to the datafileManager', () => { + const logger = getMockLogger(); + const childLogger = getMockLogger(); + logger.child.mockReturnValue(childLogger); + const datafileManager = getMockDatafileManager({}); + const datafileManagerSetLogger = vi.spyOn(datafileManager, 'setLogger'); + + const manager = new ProjectConfigManagerImpl({ logger, datafileManager }); + expect(datafileManagerSetLogger).toHaveBeenCalledWith(childLogger); + }); + }); + + describe('setLogger method', () => { + it('should set name on the logger', () => { + const logger = getMockLogger(); + const manager = new ProjectConfigManagerImpl({}); + manager.setLogger(logger); + expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME) + }); + + it('should pass a child logger to the datafileManager', () => { + const logger = getMockLogger(); + const childLogger = getMockLogger(); + logger.child.mockReturnValue(childLogger); + const datafileManager = getMockDatafileManager({}); + const datafileManagerSetLogger = vi.spyOn(datafileManager, 'setLogger'); + + const manager = new ProjectConfigManagerImpl({ datafileManager }); + manager.setLogger(logger); + expect(datafileManagerSetLogger).toHaveBeenCalledWith(childLogger); + }); + }); + it('should reject onRunning() and log error if neither datafile nor a datafileManager is passed into the constructor', async () => { const logger = getMockLogger(); const manager = new ProjectConfigManagerImpl({ logger}); diff --git a/lib/project_config/project_config_manager.ts b/lib/project_config/project_config_manager.ts index b9dbf279e..95e3fa029 100644 --- a/lib/project_config/project_config_manager.ts +++ b/lib/project_config/project_config_manager.ts @@ -46,6 +46,9 @@ export interface ProjectConfigManager extends Service { * string into project config objects. * @param {ProjectConfigManagerConfig} config */ + +export const LOGGER_NAME = 'ProjectConfigManager'; + export class ProjectConfigManagerImpl extends BaseService implements ProjectConfigManager { private datafile?: string | object; private projectConfig?: ProjectConfig; @@ -56,10 +59,19 @@ export class ProjectConfigManagerImpl extends BaseService implements ProjectConf constructor(config: ProjectConfigManagerConfig) { super(); - this.logger = config.logger; this.jsonSchemaValidator = config.jsonSchemaValidator; this.datafile = config.datafile; this.datafileManager = config.datafileManager; + + if (config.logger) { + this.setLogger(config.logger); + } + } + + setLogger(logger: LoggerFacade): void { + this.logger = logger; + this.logger.setName(LOGGER_NAME); + this.datafileManager?.setLogger(logger.child()); } start(): void { diff --git a/lib/tests/mock/mock_logger.ts b/lib/tests/mock/mock_logger.ts index f9ee207e4..e9d9cb4cf 100644 --- a/lib/tests/mock/mock_logger.ts +++ b/lib/tests/mock/mock_logger.ts @@ -17,12 +17,23 @@ import { vi } from 'vitest'; import { LoggerFacade } from '../../logging/logger'; -export const getMockLogger = () : LoggerFacade => { +type MockFn = ReturnType; +type MockLogger = { + info: MockFn; + error: MockFn; + warn: MockFn; + debug: MockFn; + child: MockFn; + setName: MockFn; +}; + +export const getMockLogger = (): MockLogger => { return { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn(), child: vi.fn().mockImplementation(() => getMockLogger()), + setName: vi.fn(), }; }; diff --git a/lib/tests/mock/mock_project_config_manager.ts b/lib/tests/mock/mock_project_config_manager.ts index 65c6268ab..931f37da1 100644 --- a/lib/tests/mock/mock_project_config_manager.ts +++ b/lib/tests/mock/mock_project_config_manager.ts @@ -50,6 +50,8 @@ export const getMockProjectConfigManager = (opt: MockOpt = {}): ProjectConfigMan }, pushUpdate: function(config: ProjectConfig) { this.listeners.forEach((listener: any) => listener(config)); + }, + setLogger: function(logger: any) { } } as any as ProjectConfigManager; };