Skip to content

[FSSDK-11437] pass logger to all child components #1028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/core/decision_service/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
30 changes: 28 additions & 2 deletions lib/event_processor/batch_event_processor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -40,7 +40,7 @@ const exhaustMicrotasks = async (loop = 100) => {
}
}

describe('QueueingEventProcessor', async () => {
describe('BatchEventProcessor', async () => {
beforeEach(() => {
vi.useFakeTimers();
});
Expand All @@ -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[] = [
Expand Down
11 changes: 10 additions & 1 deletion lib/event_processor/batch_event_processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -80,14 +82,21 @@ 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;
this.dispatchRepeater.setTask(() => this.flush());

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<LogEvent>): Fn {
Expand Down
2 changes: 2 additions & 0 deletions lib/event_processor/event_processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -26,4 +27,5 @@ export type ProcessableEvent = ConversionEvent | ImpressionEvent
export interface EventProcessor extends Service {
process(event: ProcessableEvent): Promise<unknown>;
onDispatch(handler: Consumer<LogEvent>): Fn;
setLogger(logger: LoggerFacade): void;
}
1 change: 1 addition & 0 deletions lib/event_processor/forwarding_event_processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }>;
Expand Down
17 changes: 12 additions & 5 deletions lib/logging/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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)
}
Expand Down
22 changes: 21 additions & 1 deletion lib/odp/event_manager/odp_event_api_manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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({
Expand Down
13 changes: 12 additions & 1 deletion lib/odp/event_manager/odp_event_api_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type EventDispatchResponse = {
};
export interface OdpEventApiManager {
sendEvents(odpConfig: OdpConfig, events: OdpEvent[]): Promise<EventDispatchResponse>;
setLogger(logger: LoggerFacade): void;
}

export type EventRequest = {
Expand All @@ -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;
Expand All @@ -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<EventDispatchResponse> {
if (events.length === 0) {
return {};
Expand Down
41 changes: 40 additions & 1 deletion lib/odp/event_manager/odp_event_manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -46,6 +46,7 @@ const makeEvent = (id: number) => {
const getMockApiManager = () => {
return {
sendEvents: vi.fn(),
setLogger: vi.fn(),
};
};

Expand All @@ -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(),
Expand Down
10 changes: 10 additions & 0 deletions lib/odp/event_manager/odp_event_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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;
Expand All @@ -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<unknown> {
const res = await this.apiManager.sendEvents(odpConfig, batch);
if (res.statusCode && !isSuccessStatusCode(res.statusCode)) {
Expand Down
Loading
Loading