Skip to content
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

Enable batching multiple events into a single POST request over buffer size if maxPostBytes setting is followed #1356

Merged
merged 4 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 0 additions & 5 deletions libraries/tracker-core/src/emitter/emitter_request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export interface EmitterRequestConfiguration {
keepalive?: boolean;
postPath?: string;
useStm?: boolean;
bufferSize?: number;
maxPostBytes?: number,
credentials?: 'omit' | 'same-origin' | 'include';
}
Expand Down Expand Up @@ -89,7 +88,6 @@ export function newEmitterRequest({
keepalive = true,
postPath = '/com.snowplowanalytics.snowplow/tp2',
useStm = true,
bufferSize,
maxPostBytes = 40000,
credentials = 'include',
}: EmitterRequestConfiguration): EmitterRequest {
Expand Down Expand Up @@ -128,9 +126,6 @@ export function newEmitterRequest({

function isFull(): boolean {
if (usePost) {
if (bufferSize !== undefined && countEvents() >= Math.max(1, bufferSize)) {
return true;
}
return countBytes() >= maxPostBytes;
} else {
return events.length >= 1;
Expand Down
2 changes: 1 addition & 1 deletion libraries/tracker-core/src/emitter/index.ts
matus-tomlein marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export interface EmitterConfigurationBase {
bufferSize?: number;
/**
* The max size a POST request can be before the tracker will force send it
* Also dictates the max size of a POST request before a batch of events is split into multiple requests
* @defaultValue 40000
*/
maxPostBytes?: number;
Expand Down Expand Up @@ -312,7 +313,6 @@ export function newEmitter({
customHeaders,
connectionTimeout,
keepalive,
bufferSize,
maxPostBytes,
useStm,
credentials,
Expand Down
11 changes: 4 additions & 7 deletions libraries/tracker-core/test/emitter/emitter_request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { newEventStorePayload } from '../../src/event_store_payload';

const newEmitterEventFromPayload = (payload: Record<string, unknown>) => {
return newEmitterEvent(newEventStorePayload({ payload }));
}
};

// MARK: - addEvent

Expand Down Expand Up @@ -89,34 +89,31 @@ test('countEvents returns the correct event count', (t) => {

// MARK: - isFull

test('isFull returns false when not reached buffer size', (t) => {
test('isFull returns false when not reached max post bytes', (t) => {
const request = newEmitterRequest({
endpoint: 'https://example.com',
maxPostBytes: 1000,
bufferSize: 2,
});

t.true(request.addEvent(newEmitterEventFromPayload({ e: 'pv', p: 'web' })));
t.false(request.isFull());
});

test('isFull returns true when reached buffer size', (t) => {
test('isFull returns false when reached buffer size and not max post bytes', (t) => {
const request = newEmitterRequest({
endpoint: 'https://example.com',
maxPostBytes: 1000,
bufferSize: 2,
});

t.true(request.addEvent(newEmitterEventFromPayload({ e: 'pv', p: 'web' })));
t.true(request.addEvent(newEmitterEventFromPayload({ e: 'pv', p: 'mob' })));
t.true(request.isFull());
t.false(request.isFull());
});

test('isFull returns true when reached max post bytes', (t) => {
const request = newEmitterRequest({
endpoint: 'https://example.com',
maxPostBytes: 10,
bufferSize: 2,
});

t.true(request.addEvent(newEmitterEventFromPayload({ e: 'pv', p: 'web' })));
Expand Down
1 change: 1 addition & 0 deletions plugins/browser-plugin-media-tracking/tests/test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('MediaTrackingPlugin', () => {
},
],
contexts: { webPage: false },
customFetch: async () => new Response(null, { status: 200 }),
});
id = `media-${idx}`;
});
Expand Down
1 change: 1 addition & 0 deletions plugins/browser-plugin-media/test/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ describe('Media Tracking API', () => {
},
],
contexts: { webPage: false },
customFetch: async () => new Response(null, { status: 200 }),
});
id = `media-${idx}`;
});
Expand Down
18 changes: 11 additions & 7 deletions trackers/node-tracker/test/tracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,13 @@ for (const eventMethod of testMethods) {
eventMethod,
bufferSize: 0,
onRequestSuccess: (batch) => {
const expected = batch[0]['e'] === 'tr' ? expectedTransaction : expectedItem;
batch.forEach((payload) => {
const expected = payload['e'] === 'tr' ? expectedTransaction : expectedItem;

checkPayload(batch[0], expected, t);
checkPayload(payload, expected, t);

requestCount--;
requestCount--;
});
if (requestCount === 0) {
resolve(batch);
}
Expand Down Expand Up @@ -472,11 +474,13 @@ for (const eventMethod of testMethods) {
endpoint,
eventMethod,
bufferSize: 0,
onRequestSuccess: ([pl]) => {
checkPayload(pl, expected, t);
count--;
onRequestSuccess: (batch) => {
batch.forEach((pl) => {
checkPayload(pl, expected, t);
count--;
});
if (count === 0) {
resolve(pl);
resolve(batch);
}
},
onRequestFailure: reject,
Expand Down
Loading