Skip to content

Commit 5f49770

Browse files
mjc1283mjc1283
authored
feat(event processor): Validation and defaults for event processor configuration (#346)
Summary: This PR adds validation and defaults for event batch size and flush interval. There are different defaults for the browser & node entry points: Node: batch size 10, flush interval 30000 Browser: batch size 10, flush interval 1000 I also added validation that ignores invalid values and uses these defaults. Test plan: Updated unit tests Issues: https://optimizely.atlassian.net/browse/OASIS-5163
1 parent de62473 commit 5f49770

File tree

8 files changed

+387
-46
lines changed

8 files changed

+387
-46
lines changed

‎packages/optimizely-sdk/lib/index.browser.js‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016-2017, Optimizely
2+
* Copyright 2016-2017, 2019, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -23,11 +23,15 @@ var enums = require('./utils/enums');
2323
var eventProcessor = require('@optimizely/js-sdk-event-processor');
2424
var loggerPlugin = require('./plugins/logger');
2525
var Optimizely = require('./optimizely');
26+
var eventProcessorConfigValidator = require('./utils/event_processor_config_validator');
2627

2728
var logger = logging.getLogger();
2829
logging.setLogHandler(loggerPlugin.createLogger());
2930
logging.setLogLevel(logging.LogLevel.INFO);
3031

32+
var DEFAULT_EVENT_BATCH_SIZE = 10;
33+
var DEFAULT_EVENT_FLUSH_INTERVAL = 1000; // Unit is ms, default is 1s
34+
3135
var hasRetriedEvents = false;
3236
/**
3337
* Entry point into the Optimizely Browser SDK
@@ -103,13 +107,24 @@ module.exports = {
103107

104108
config = fns.assignIn({
105109
clientEngine: enums.JAVASCRIPT_CLIENT_ENGINE,
110+
eventBatchSize: DEFAULT_EVENT_BATCH_SIZE,
111+
eventFlushInterval: DEFAULT_EVENT_FLUSH_INTERVAL,
106112
}, config, {
107113
eventDispatcher: eventDispatcher,
108114
// always get the OptimizelyLogger facade from logging
109115
logger: logger,
110116
errorHandler: logging.getErrorHandler(),
111117
});
112118

119+
if (!eventProcessorConfigValidator.validateEventBatchSize(config.eventBatchSize)) {
120+
logger.warn('Invalid eventBatchSize %s, defaulting to %s', config.eventBatchSize, DEFAULT_EVENT_BATCH_SIZE);
121+
config.eventBatchSize = DEFAULT_EVENT_BATCH_SIZE;
122+
}
123+
if (!eventProcessorConfigValidator.validateEventFlushInterval(config.eventFlushInterval)) {
124+
logger.warn('Invalid eventFlushInterval %s, defaulting to %s', config.eventFlushInterval, DEFAULT_EVENT_FLUSH_INTERVAL);
125+
config.eventFlushInterval = DEFAULT_EVENT_FLUSH_INTERVAL;
126+
}
127+
113128
return new Optimizely(config);
114129
} catch (e) {
115130
logger.error(e);

‎packages/optimizely-sdk/lib/index.browser.tests.js‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ var Optimizely = require('./optimizely');
2020
var optimizelyFactory = require('./index.browser');
2121
var packageJSON = require('../package.json');
2222
var testData = require('./tests/test_data');
23+
var eventProcessor = require('@optimizely/js-sdk-event-processor');
24+
var eventProcessorConfigValidator = require('./utils/event_processor_config_validator');
2325

2426
var chai = require('chai');
2527
var assert = chai.assert;
@@ -398,6 +400,133 @@ describe('javascript-sdk', function() {
398400
sinon.assert.calledWithExactly(logging.setLogHandler, fakeLogger);
399401
});
400402
});
403+
404+
describe('event processor configuration', function() {
405+
var eventProcessorSpy;
406+
beforeEach(function() {
407+
eventProcessorSpy = sinon.stub(eventProcessor, 'LogTierV1EventProcessor').callThrough();
408+
});
409+
410+
afterEach(function() {
411+
eventProcessor.LogTierV1EventProcessor.restore();
412+
});
413+
414+
it('should use default event flush interval when none is provided', function() {
415+
optimizelyFactory.createInstance({
416+
datafile: testData.getTestProjectConfigWithFeatures(),
417+
errorHandler: fakeErrorHandler,
418+
eventDispatcher: fakeEventDispatcher,
419+
logger: silentLogger,
420+
});
421+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
422+
flushInterval: 1000,
423+
}));
424+
});
425+
426+
describe('with an invalid flush interval', function() {
427+
beforeEach(function() {
428+
sinon.stub(eventProcessorConfigValidator, 'validateEventFlushInterval').returns(false);
429+
});
430+
431+
afterEach(function() {
432+
eventProcessorConfigValidator.validateEventFlushInterval.restore();
433+
});
434+
435+
it('should ignore the event flush interval and use the default instead', function() {
436+
optimizelyFactory.createInstance({
437+
datafile: testData.getTestProjectConfigWithFeatures(),
438+
errorHandler: fakeErrorHandler,
439+
eventDispatcher: fakeEventDispatcher,
440+
logger: silentLogger,
441+
eventFlushInterval: ['invalid', 'flush', 'interval'],
442+
});
443+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
444+
flushInterval: 1000,
445+
}));
446+
});
447+
});
448+
449+
describe('with a valid flush interval', function() {
450+
beforeEach(function() {
451+
sinon.stub(eventProcessorConfigValidator, 'validateEventFlushInterval').returns(true);
452+
});
453+
454+
afterEach(function() {
455+
eventProcessorConfigValidator.validateEventFlushInterval.restore();
456+
});
457+
458+
it('should use the provided event flush interval', function() {
459+
optimizelyFactory.createInstance({
460+
datafile: testData.getTestProjectConfigWithFeatures(),
461+
errorHandler: fakeErrorHandler,
462+
eventDispatcher: fakeEventDispatcher,
463+
logger: silentLogger,
464+
eventFlushInterval: 9000,
465+
});
466+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
467+
flushInterval: 9000,
468+
}));
469+
});
470+
});
471+
472+
it('should use default event batch size when none is provided', function() {
473+
optimizelyFactory.createInstance({
474+
datafile: testData.getTestProjectConfigWithFeatures(),
475+
errorHandler: fakeErrorHandler,
476+
eventDispatcher: fakeEventDispatcher,
477+
logger: silentLogger,
478+
});
479+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
480+
maxQueueSize: 10,
481+
}));
482+
});
483+
484+
describe('with an invalid event batch size', function() {
485+
beforeEach(function() {
486+
sinon.stub(eventProcessorConfigValidator, 'validateEventBatchSize').returns(false);
487+
});
488+
489+
afterEach(function() {
490+
eventProcessorConfigValidator.validateEventBatchSize.restore();
491+
});
492+
493+
it('should ignore the event batch size and use the default instead', function() {
494+
optimizelyFactory.createInstance({
495+
datafile: testData.getTestProjectConfigWithFeatures(),
496+
errorHandler: fakeErrorHandler,
497+
eventDispatcher: fakeEventDispatcher,
498+
logger: silentLogger,
499+
eventBatchSize: null,
500+
});
501+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
502+
maxQueueSize: 10,
503+
}));
504+
});
505+
});
506+
507+
describe('with a valid event batch size', function() {
508+
beforeEach(function() {
509+
sinon.stub(eventProcessorConfigValidator, 'validateEventBatchSize').returns(true);
510+
});
511+
512+
afterEach(function() {
513+
eventProcessorConfigValidator.validateEventBatchSize.restore();
514+
});
515+
516+
it('should use the provided event batch size', function() {
517+
optimizelyFactory.createInstance({
518+
datafile: testData.getTestProjectConfigWithFeatures(),
519+
errorHandler: fakeErrorHandler,
520+
eventDispatcher: fakeEventDispatcher,
521+
logger: silentLogger,
522+
eventBatchSize: 300,
523+
});
524+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
525+
maxQueueSize: 300,
526+
}));
527+
});
528+
});
529+
});
401530
});
402531
});
403532
});

‎packages/optimizely-sdk/lib/index.node.js‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2016-2017, Optimizely, Inc. and contributors *
2+
* Copyright 2016-2017, 2019, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -21,12 +21,15 @@ var enums = require('./utils/enums');
2121
var fns = require('./utils/fns');
2222
var jsonSchemaValidator = require('./utils/json_schema_validator');
2323
var loggerPlugin = require('./plugins/logger');
24-
2524
var Optimizely = require('./optimizely');
25+
var eventProcessorConfigValidator = require('./utils/event_processor_config_validator');
2626

2727
var logger = logging.getLogger();
2828
logging.setLogLevel(logging.LogLevel.ERROR);
2929

30+
var DEFAULT_EVENT_BATCH_SIZE = 10;
31+
var DEFAULT_EVENT_FLUSH_INTERVAL = 30000; // Unit is ms, default is 30s
32+
3033
/**
3134
* Entry point into the Optimizely Node testing SDK
3235
*/
@@ -87,7 +90,9 @@ module.exports = {
8790
config = fns.assign(
8891
{
8992
clientEngine: enums.NODE_CLIENT_ENGINE,
93+
eventBatchSize: DEFAULT_EVENT_BATCH_SIZE,
9094
eventDispatcher: defaultEventDispatcher,
95+
eventFlushInterval: DEFAULT_EVENT_FLUSH_INTERVAL,
9196
jsonSchemaValidator: jsonSchemaValidator,
9297
skipJSONValidation: false,
9398
},
@@ -99,6 +104,15 @@ module.exports = {
99104
}
100105
);
101106

107+
if (!eventProcessorConfigValidator.validateEventBatchSize(config.eventBatchSize)) {
108+
logger.warn('Invalid eventBatchSize %s, defaulting to %s', config.eventBatchSize, DEFAULT_EVENT_BATCH_SIZE);
109+
config.eventBatchSize = DEFAULT_EVENT_BATCH_SIZE;
110+
}
111+
if (!eventProcessorConfigValidator.validateEventFlushInterval(config.eventFlushInterval)) {
112+
logger.warn('Invalid eventFlushInterval %s, defaulting to %s', config.eventFlushInterval, DEFAULT_EVENT_FLUSH_INTERVAL);
113+
config.eventFlushInterval = DEFAULT_EVENT_FLUSH_INTERVAL;
114+
}
115+
102116
return new Optimizely(config);
103117
} catch (e) {
104118
logger.error(e);

‎packages/optimizely-sdk/lib/index.node.tests.js‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ var enums = require('./utils/enums');
1919
var loggerPlugin = require('./plugins/logger');
2020
var Optimizely = require('./optimizely');
2121
var optimizelyFactory = require('./index.node');
22+
var testData = require('./tests/test_data');
23+
var eventProcessor = require('@optimizely/js-sdk-event-processor');
2224

2325
var chai = require('chai');
2426
var assert = chai.assert;
@@ -92,6 +94,93 @@ describe('optimizelyFactory', function() {
9294
assert.instanceOf(optlyInstance, Optimizely);
9395
assert.equal(optlyInstance.clientVersion, '3.2.1');
9496
});
97+
98+
describe('event processor configuration', function() {
99+
var eventProcessorSpy;
100+
beforeEach(function() {
101+
eventProcessorSpy = sinon.stub(eventProcessor, 'LogTierV1EventProcessor').callThrough();
102+
});
103+
104+
afterEach(function() {
105+
eventProcessor.LogTierV1EventProcessor.restore();
106+
});
107+
108+
it('should ignore invalid event flush interval and use default instead', function() {
109+
optimizelyFactory.createInstance({
110+
datafile: testData.getTestProjectConfigWithFeatures(),
111+
errorHandler: fakeErrorHandler,
112+
eventDispatcher: fakeEventDispatcher,
113+
logger: fakeLogger,
114+
eventFlushInterval: ['invalid', 'flush', 'interval'],
115+
});
116+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
117+
flushInterval: 30000,
118+
}));
119+
});
120+
121+
it('should use default event flush interval when none is provided', function() {
122+
optimizelyFactory.createInstance({
123+
datafile: testData.getTestProjectConfigWithFeatures(),
124+
errorHandler: fakeErrorHandler,
125+
eventDispatcher: fakeEventDispatcher,
126+
logger: fakeLogger,
127+
});
128+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
129+
flushInterval: 30000,
130+
}));
131+
});
132+
133+
it('should use provided event flush interval when valid', function() {
134+
optimizelyFactory.createInstance({
135+
datafile: testData.getTestProjectConfigWithFeatures(),
136+
errorHandler: fakeErrorHandler,
137+
eventDispatcher: fakeEventDispatcher,
138+
logger: fakeLogger,
139+
eventFlushInterval: 10000,
140+
});
141+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
142+
flushInterval: 10000,
143+
}));
144+
});
145+
146+
it('should ignore invalid event batch size and use default instead', function() {
147+
optimizelyFactory.createInstance({
148+
datafile: testData.getTestProjectConfigWithFeatures(),
149+
errorHandler: fakeErrorHandler,
150+
eventDispatcher: fakeEventDispatcher,
151+
logger: fakeLogger,
152+
eventBatchSize: null,
153+
});
154+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
155+
maxQueueSize: 10,
156+
}));
157+
});
158+
159+
it('should use default event batch size when none is provided', function() {
160+
optimizelyFactory.createInstance({
161+
datafile: testData.getTestProjectConfigWithFeatures(),
162+
errorHandler: fakeErrorHandler,
163+
eventDispatcher: fakeEventDispatcher,
164+
logger: fakeLogger,
165+
});
166+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
167+
maxQueueSize: 10,
168+
}));
169+
});
170+
171+
it('should use provided event batch size when valid', function() {
172+
optimizelyFactory.createInstance({
173+
datafile: testData.getTestProjectConfigWithFeatures(),
174+
errorHandler: fakeErrorHandler,
175+
eventDispatcher: fakeEventDispatcher,
176+
logger: fakeLogger,
177+
eventBatchSize: 300,
178+
});
179+
sinon.assert.calledWithExactly(eventProcessorSpy, sinon.match({
180+
maxQueueSize: 300,
181+
}));
182+
});
183+
});
95184
});
96185
});
97186
});

‎packages/optimizely-sdk/lib/optimizely/index.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ var FEATURE_VARIABLE_TYPES = enums.FEATURE_VARIABLE_TYPES;
3838
var DECISION_NOTIFICATION_TYPES = enums.DECISION_NOTIFICATION_TYPES;
3939
var NOTIFICATION_TYPES = enums.NOTIFICATION_TYPES;
4040

41-
var DEFAULT_EVENT_MAX_QUEUE_SIZE = 1;
42-
var DEFAULT_EVENT_FLUSH_INTERVAL = 5000;
4341
var DEFAULT_ONREADY_TIMEOUT = 30000;
4442

4543
/**
@@ -110,8 +108,8 @@ function Optimizely(config) {
110108

111109
this.eventProcessor = new eventProcessor.LogTierV1EventProcessor({
112110
dispatcher: this.eventDispatcher,
113-
flushInterval: config.eventFlushInterval !== undefined ? config.eventFlushInterval : DEFAULT_EVENT_FLUSH_INTERVAL,
114-
maxQueueSize: config.eventBatchSize !== undefined ? config.eventBatchSize : DEFAULT_EVENT_MAX_QUEUE_SIZE,
111+
flushInterval: config.eventFlushInterval,
112+
maxQueueSize: config.eventBatchSize,
115113
notificationCenter: this.notificationCenter,
116114
});
117115
this.eventProcessor.start();

0 commit comments

Comments
 (0)