Skip to content

Commit 85c0220

Browse files
authored
[FSSDK-11437] pass logger to all child components (#1028)
1 parent 1d4d290 commit 85c0220

27 files changed

+500
-85
lines changed

Diff for: lib/core/decision_service/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ export class DecisionService {
355355
reasons: [[CMAB_NOT_SUPPORTED_IN_SYNC]],
356356
});
357357
}
358-
358+
359359
const cmabPromise = this.cmabService.getDecision(configObj, user, experiment.id, decideOptions).then(
360360
(cmabDecision) => {
361361
return {

Diff for: lib/event_processor/batch_event_processor.spec.ts

+28-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
import { expect, describe, it, vi, beforeEach, afterEach, MockInstance } from 'vitest';
1717

18-
import { EventWithId, BatchEventProcessor } from './batch_event_processor';
18+
import { EventWithId, BatchEventProcessor, LOGGER_NAME } from './batch_event_processor';
1919
import { getMockSyncCache } from '../tests/mock/mock_cache';
2020
import { createImpressionEvent } from '../tests/mock/create_event';
2121
import { ProcessableEvent } from './event_processor';
@@ -40,7 +40,7 @@ const exhaustMicrotasks = async (loop = 100) => {
4040
}
4141
}
4242

43-
describe('QueueingEventProcessor', async () => {
43+
describe('BatchEventProcessor', async () => {
4444
beforeEach(() => {
4545
vi.useFakeTimers();
4646
});
@@ -49,6 +49,32 @@ describe('QueueingEventProcessor', async () => {
4949
vi.useRealTimers();
5050
});
5151

52+
it('should set name on the logger passed into the constructor', () => {
53+
const logger = getMockLogger();
54+
55+
const processor = new BatchEventProcessor({
56+
eventDispatcher: getMockDispatcher(),
57+
dispatchRepeater: getMockRepeater(),
58+
batchSize: 1000,
59+
logger,
60+
});
61+
62+
expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME);
63+
});
64+
65+
it('should set name on the logger set by setLogger', () => {
66+
const logger = getMockLogger();
67+
68+
const processor = new BatchEventProcessor({
69+
eventDispatcher: getMockDispatcher(),
70+
dispatchRepeater: getMockRepeater(),
71+
batchSize: 1000,
72+
});
73+
74+
processor.setLogger(logger);
75+
expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME);
76+
});
77+
5278
describe('start', () => {
5379
it('should log startupLogs on start', () => {
5480
const startupLogs: StartupLog[] = [

Diff for: lib/event_processor/batch_event_processor.ts

+10-1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ type EventBatch = {
6060
ids: string[],
6161
}
6262

63+
export const LOGGER_NAME = 'BatchEventProcessor';
64+
6365
export class BatchEventProcessor extends BaseService implements EventProcessor {
6466
private eventDispatcher: EventDispatcher;
6567
private closingEventDispatcher?: EventDispatcher;
@@ -80,14 +82,21 @@ export class BatchEventProcessor extends BaseService implements EventProcessor {
8082
this.closingEventDispatcher = config.closingEventDispatcher;
8183
this.batchSize = config.batchSize;
8284
this.eventStore = config.eventStore;
83-
this.logger = config.logger;
8485
this.retryConfig = config.retryConfig;
8586

8687
this.dispatchRepeater = config.dispatchRepeater;
8788
this.dispatchRepeater.setTask(() => this.flush());
8889

8990
this.failedEventRepeater = config.failedEventRepeater;
9091
this.failedEventRepeater?.setTask(() => this.retryFailedEvents());
92+
if (config.logger) {
93+
this.setLogger(config.logger);
94+
}
95+
}
96+
97+
setLogger(logger: LoggerFacade): void {
98+
this.logger = logger;
99+
this.logger.setName(LOGGER_NAME);
91100
}
92101

93102
onDispatch(handler: Consumer<LogEvent>): Fn {

Diff for: lib/event_processor/event_processor.ts

+2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { ConversionEvent, ImpressionEvent } from './event_builder/user_event'
1717
import { LogEvent } from './event_dispatcher/event_dispatcher'
1818
import { Service } from '../service'
1919
import { Consumer, Fn } from '../utils/type';
20+
import { LoggerFacade } from '../logging/logger';
2021

2122
export const DEFAULT_FLUSH_INTERVAL = 30000 // Unit is ms - default flush interval is 30s
2223
export const DEFAULT_BATCH_SIZE = 10
@@ -26,4 +27,5 @@ export type ProcessableEvent = ConversionEvent | ImpressionEvent
2627
export interface EventProcessor extends Service {
2728
process(event: ProcessableEvent): Promise<unknown>;
2829
onDispatch(handler: Consumer<LogEvent>): Fn;
30+
setLogger(logger: LoggerFacade): void;
2931
}

Diff for: lib/event_processor/forwarding_event_processor.ts

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { EventEmitter } from '../utils/event_emitter/event_emitter';
2525
import { Consumer, Fn } from '../utils/type';
2626
import { SERVICE_STOPPED_BEFORE_RUNNING } from 'error_message';
2727
import { OptimizelyError } from '../error/optimizly_error';
28+
2829
class ForwardingEventProcessor extends BaseService implements EventProcessor {
2930
private dispatcher: EventDispatcher;
3031
private eventEmitter: EventEmitter<{ dispatch: LogEvent }>;

Diff for: lib/logging/logger.ts

+12-5
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ export interface LoggerFacade {
4343
debug(message: string | Error, ...args: any[]): void;
4444
warn(message: string | Error, ...args: any[]): void;
4545
error(message: string | Error, ...args: any[]): void;
46-
child(name: string): LoggerFacade;
46+
child(name?: string): LoggerFacade;
47+
setName(name: string): void;
4748
}
4849

4950
export interface LogHandler {
@@ -84,7 +85,7 @@ type OptimizelyLoggerConfig = {
8485

8586
export class OptimizelyLogger implements LoggerFacade {
8687
private name?: string;
87-
private prefix: string;
88+
private prefix = '';
8889
private logHandler: LogHandler;
8990
private infoResolver?: MessageResolver;
9091
private errorResolver: MessageResolver;
@@ -95,11 +96,12 @@ export class OptimizelyLogger implements LoggerFacade {
9596
this.infoResolver = config.infoMsgResolver;
9697
this.errorResolver = config.errorMsgResolver;
9798
this.level = config.level;
98-
this.name = config.name;
99-
this.prefix = this.name ? `${this.name}: ` : '';
99+
if (config.name) {
100+
this.setName(config.name);
101+
}
100102
}
101103

102-
child(name: string): OptimizelyLogger {
104+
child(name?: string): OptimizelyLogger {
103105
return new OptimizelyLogger({
104106
logHandler: this.logHandler,
105107
infoMsgResolver: this.infoResolver,
@@ -109,6 +111,11 @@ export class OptimizelyLogger implements LoggerFacade {
109111
});
110112
}
111113

114+
setName(name: string): void {
115+
this.name = name;
116+
this.prefix = `${name}: `;
117+
}
118+
112119
info(message: string | Error, ...args: any[]): void {
113120
this.log(LogLevel.Info, message, args)
114121
}

Diff for: lib/odp/event_manager/odp_event_api_manager.spec.ts

+21-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
import { describe, it, expect, vi } from 'vitest';
1717

18-
import { DefaultOdpEventApiManager, eventApiRequestGenerator, pixelApiRequestGenerator } from './odp_event_api_manager';
18+
import { DefaultOdpEventApiManager, eventApiRequestGenerator, LOGGER_NAME, pixelApiRequestGenerator } from './odp_event_api_manager';
1919
import { OdpEvent } from './odp_event';
2020
import { OdpConfig } from '../odp_config';
2121

@@ -41,8 +41,28 @@ const PIXEL_URL = 'https://odp.pixel.com';
4141
const odpConfig = new OdpConfig(API_KEY, API_HOST, PIXEL_URL, []);
4242

4343
import { getMockRequestHandler } from '../../tests/mock/mock_request_handler';
44+
import { getMockLogger } from '../../tests/mock/mock_logger';
4445

4546
describe('DefaultOdpEventApiManager', () => {
47+
it('should set name on the logger passed into the constructor', () => {
48+
const logger = getMockLogger();
49+
const requestHandler = getMockRequestHandler();
50+
51+
const manager = new DefaultOdpEventApiManager(requestHandler, vi.fn(), logger);
52+
53+
expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME);
54+
});
55+
56+
it('should set name on the logger set by setLogger', () => {
57+
const logger = getMockLogger();
58+
const requestHandler = getMockRequestHandler();
59+
60+
const manager = new DefaultOdpEventApiManager(requestHandler, vi.fn());
61+
manager.setLogger(logger);
62+
63+
expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME);
64+
});
65+
4666
it('should generate the event request using the correct odp config and event', async () => {
4767
const mockRequestHandler = getMockRequestHandler();
4868
mockRequestHandler.makeRequest.mockReturnValue({

Diff for: lib/odp/event_manager/odp_event_api_manager.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export type EventDispatchResponse = {
2424
};
2525
export interface OdpEventApiManager {
2626
sendEvents(odpConfig: OdpConfig, events: OdpEvent[]): Promise<EventDispatchResponse>;
27+
setLogger(logger: LoggerFacade): void;
2728
}
2829

2930
export type EventRequest = {
@@ -34,6 +35,9 @@ export type EventRequest = {
3435
}
3536

3637
export type EventRequestGenerator = (odpConfig: OdpConfig, events: OdpEvent[]) => EventRequest;
38+
39+
export const LOGGER_NAME = 'OdpEventApiManager';
40+
3741
export class DefaultOdpEventApiManager implements OdpEventApiManager {
3842
private logger?: LoggerFacade;
3943
private requestHandler: RequestHandler;
@@ -46,9 +50,16 @@ export class DefaultOdpEventApiManager implements OdpEventApiManager {
4650
) {
4751
this.requestHandler = requestHandler;
4852
this.requestGenerator = requestDataGenerator;
49-
this.logger = logger;
53+
if (logger) {
54+
this.setLogger(logger)
55+
}
5056
}
5157

58+
setLogger(logger: LoggerFacade): void {
59+
this.logger = logger;
60+
this.logger.setName(LOGGER_NAME);
61+
}
62+
5263
async sendEvents(odpConfig: OdpConfig, events: OdpEvent[]): Promise<EventDispatchResponse> {
5364
if (events.length === 0) {
5465
return {};

Diff for: lib/odp/event_manager/odp_event_manager.spec.ts

+40-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
17-
import { DefaultOdpEventManager } from './odp_event_manager';
17+
import { DefaultOdpEventManager, LOGGER_NAME } from './odp_event_manager';
1818
import { getMockRepeater } from '../../tests/mock/mock_repeater';
1919
import { getMockLogger } from '../../tests/mock/mock_logger';
2020
import { ServiceState } from '../../service';
@@ -46,6 +46,7 @@ const makeEvent = (id: number) => {
4646
const getMockApiManager = () => {
4747
return {
4848
sendEvents: vi.fn(),
49+
setLogger: vi.fn(),
4950
};
5051
};
5152

@@ -72,6 +73,44 @@ describe('DefaultOdpEventManager', () => {
7273
expect(odpEventManager.getState()).toBe(ServiceState.New);
7374
});
7475

76+
it('should set name on the logger set using setLogger', () => {
77+
const logger = getMockLogger();
78+
79+
const manager = new DefaultOdpEventManager({
80+
repeater: getMockRepeater(),
81+
apiManager: getMockApiManager(),
82+
batchSize: 10,
83+
retryConfig: {
84+
maxRetries: 3,
85+
backoffProvider: vi.fn(),
86+
},
87+
});
88+
89+
manager.setLogger(logger);
90+
expect(logger.setName).toHaveBeenCalledWith(LOGGER_NAME);
91+
});
92+
93+
it('should pass a child logger to the event api manager when a logger is set using setLogger', () => {
94+
const logger = getMockLogger();
95+
const childLogger = getMockLogger();
96+
logger.child.mockReturnValue(childLogger);
97+
98+
const apiManager = getMockApiManager();
99+
100+
const manager = new DefaultOdpEventManager({
101+
repeater: getMockRepeater(),
102+
apiManager,
103+
batchSize: 10,
104+
retryConfig: {
105+
maxRetries: 3,
106+
backoffProvider: vi.fn(),
107+
},
108+
});
109+
110+
manager.setLogger(logger);
111+
expect(apiManager.setLogger).toHaveBeenCalledWith(childLogger);
112+
});
113+
75114
it('should stay in starting state if started with a odpIntegationConfig and not resolve or reject onRunning', async () => {
76115
const odpEventManager = new DefaultOdpEventManager({
77116
repeater: getMockRepeater(),

Diff for: lib/odp/event_manager/odp_event_manager.ts

+10
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@ import {
3434
ODP_EVENT_MANAGER_STOPPED
3535
} from 'error_message';
3636
import { OptimizelyError } from '../../error/optimizly_error';
37+
import { LoggerFacade } from '../../logging/logger';
3738

3839
export interface OdpEventManager extends Service {
3940
updateConfig(odpIntegrationConfig: OdpIntegrationConfig): void;
4041
sendEvent(event: OdpEvent): void;
42+
setLogger(logger: LoggerFacade): void;
4143
}
4244

4345
export type RetryConfig = {
@@ -53,6 +55,8 @@ export type OdpEventManagerConfig = {
5355
retryConfig: RetryConfig,
5456
};
5557

58+
export const LOGGER_NAME = 'OdpEventManager';
59+
5660
export class DefaultOdpEventManager extends BaseService implements OdpEventManager {
5761
private queue: OdpEvent[] = [];
5862
private repeater: Repeater;
@@ -73,6 +77,12 @@ export class DefaultOdpEventManager extends BaseService implements OdpEventManag
7377
this.repeater.setTask(() => this.flush());
7478
}
7579

80+
setLogger(logger: LoggerFacade): void {
81+
this.logger = logger;
82+
this.logger.setName(LOGGER_NAME);
83+
this.apiManager.setLogger(logger.child());
84+
}
85+
7686
private async executeDispatch(odpConfig: OdpConfig, batch: OdpEvent[]): Promise<unknown> {
7787
const res = await this.apiManager.sendEvents(odpConfig, batch);
7888
if (res.statusCode && !isSuccessStatusCode(res.statusCode)) {

0 commit comments

Comments
 (0)