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

feat: add diagnostic #568

Open
wants to merge 50 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
86bd79d
feat: init with diagnostciProvider
Mercy811 Aug 24, 2023
b66a468
test: add diagnostic
Mercy811 Aug 25, 2023
cab2ecd
fix: init Destination with config
Mercy811 Aug 26, 2023
cd04c90
chore: delete comments
Mercy811 Aug 26, 2023
f2e67b1
test: add test for index
Mercy811 Aug 28, 2023
a303868
fix: update diagnostic
Mercy811 Aug 28, 2023
4521a9a
test: diagnostic
Mercy811 Aug 28, 2023
957ec26
fix: fetch
Mercy811 Aug 28, 2023
922f349
test: browser diagnostic
Mercy811 Aug 28, 2023
21dc41b
test: delete node fetch
Mercy811 Aug 28, 2023
c0ba911
fix: diagnosticProvider should be in core config
Mercy811 Sep 6, 2023
470d82b
fix: node reference error
Mercy811 Sep 6, 2023
f6796b5
test: update diagnostic tests
Mercy811 Sep 6, 2023
4fb3fca
fix: not export diagnostic event
Mercy811 Sep 6, 2023
7d15a8e
fix: not export diagnostic event
Mercy811 Sep 6, 2023
5121480
fix: add diagnostic constructor
Mercy811 Sep 7, 2023
d927a22
feat: diagnostic options
Mercy811 Sep 22, 2023
d1911b6
feat: diagnostic track
Mercy811 Sep 26, 2023
4e857cb
test: fix tests
Mercy811 Sep 27, 2023
f4893a8
feat: diagnostic track
Mercy811 Sep 26, 2023
7991120
fix: extract message constants
Mercy811 Sep 27, 2023
b5b8a48
test: add more tests for core destination
Mercy811 Sep 28, 2023
56c8af6
refactor: move to diagnostics folder
Mercy811 Oct 11, 2023
9492638
feat: change event structure
Mercy811 Oct 11, 2023
3df9ac5
test: client level tests
Mercy811 Oct 11, 2023
2b4c1f7
test: apikey
Mercy811 Oct 11, 2023
bef4da7
fix: delete duplicate diagnostic track
Mercy811 Oct 11, 2023
da7baf2
test: more client level tests
Mercy811 Oct 11, 2023
6975d5b
fix: empty commit
Mercy811 Oct 11, 2023
7d7a33c
fix: update endpoint url
Mercy811 Oct 11, 2023
4723f6b
refactor: rename
Mercy811 Oct 11, 2023
d6bac4c
fix: payload and test
Mercy811 Oct 11, 2023
7cc0639
refactor: rename to DiagnosticRequest
Mercy811 Oct 11, 2023
6cb202b
feat: diagnostic provider only accepts Diagnostic
Mercy811 Oct 12, 2023
ef5706f
test: mock diagnosticProvider
Mercy811 Oct 12, 2023
1d252d2
refactor: extract DIAGNOSTIC_MESSAGES
Mercy811 Oct 12, 2023
f72cf0e
Update packages/analytics-core/src/config.ts
Mercy811 Oct 12, 2023
3db01cd
Update packages/analytics-core/test/diagnostic.test.ts
Mercy811 Oct 12, 2023
1586520
fix: change api only when it's not set
Mercy811 Oct 12, 2023
11dcde7
fix: diagnostic not extend options
Mercy811 Oct 12, 2023
eb6d095
fix: disable BaseDiagnostic
Mercy811 Oct 12, 2023
e733169
fix: recover fulfillRequest
Mercy811 Oct 13, 2023
85cf273
fix: should set default api key for diagnostic
Mercy811 Oct 17, 2023
546b5b0
fix: time should be in minutes
Mercy811 Oct 17, 2023
9dfbe7a
fix: should diagnostic track only when api key is valid
Mercy811 Oct 17, 2023
2e373dc
test: apply recent changes
Mercy811 Oct 17, 2023
a6c34f5
chore: update dropList with retryList
Mercy811 Nov 8, 2023
d960c18
chore: use nullish coalescing
Mercy811 Nov 8, 2023
d2bbc02
refactor: extract track diagnostic when not useRetry
Mercy811 Nov 8, 2023
62aaa6f
chore(release): publish
amplitude-sdk-bot Nov 16, 2023
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
6 changes: 5 additions & 1 deletion packages/analytics-browser/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
BrowserOptions,
BrowserConfig as IBrowserConfig,
DefaultTrackingOptions,
Diagnostic,
Storage,
TrackingOptions,
TransportType,
Expand All @@ -25,6 +26,7 @@ import { parseLegacyCookies } from './cookie-migration';
import { CookieOptions } from '@amplitude/analytics-types/lib/esm/config/browser';
import { DEFAULT_IDENTITY_STORAGE, DEFAULT_SERVER_ZONE } from './constants';
import { AmplitudeBrowser } from './browser-client';
import { BrowserDiagnostic } from './diagnostics/diagnostic';

// Exported for testing purposes only. Do not expose to public interface.
export class BrowserConfig extends Config implements IBrowserConfig {
Expand Down Expand Up @@ -75,9 +77,10 @@ export class BrowserConfig extends Config implements IBrowserConfig {
},
public transport: 'fetch' | 'xhr' | 'beacon' = 'fetch',
public useBatch: boolean = false,
public diagnosticProvider: Diagnostic = new BrowserDiagnostic(),
userId?: string,
) {
super({ apiKey, storageProvider, transportProvider: createTransport(transport) });
super({ apiKey, storageProvider, transportProvider: createTransport(transport), diagnosticProvider });
this._cookieStorage = cookieStorage;
this.deviceId = deviceId;
this.lastEventId = lastEventId;
Expand Down Expand Up @@ -247,6 +250,7 @@ export const useBrowserConfig = async (
trackingOptions,
options.transport,
options.useBatch,
options.diagnosticProvider,
userId,
);
};
Expand Down
10 changes: 10 additions & 0 deletions packages/analytics-browser/src/diagnostics/diagnostic.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { BaseDiagnostic } from '@amplitude/analytics-core';

export class BrowserDiagnostic extends BaseDiagnostic {
isDisabled = false;

async flush(): Promise<void> {
await fetch(this.serverUrl, this.requestPayloadBuilder(this.queue));
await super.flush();
}
}
186 changes: 185 additions & 1 deletion packages/analytics-browser/test/browser-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { AmplitudeBrowser } from '../src/browser-client';
import * as core from '@amplitude/analytics-core';
import * as Config from '../src/config';
import * as CookieMigration from '../src/cookie-migration';
import { UserSession } from '@amplitude/analytics-types';
import { Status, UserSession } from '@amplitude/analytics-types';
import {
CookieStorage,
FetchTransport,
Expand Down Expand Up @@ -261,6 +261,190 @@ describe('browser-client', () => {
}).promise;
expect(webAttributionPluginPlugin).toHaveBeenCalledTimes(1);
});

describe('diagnostic', () => {
test('should init diagnostic with default api key', async () => {
await client.init(apiKey).promise;
expect(client.config.diagnosticProvider.apiKey).toBe(apiKey);
});

test('should not diagnostic track when 200', async () => {
const transportProvider = {
send: jest.fn().mockImplementationOnce(() => {
return Promise.resolve({
status: Status.Success,
statusCode: 200,
});
}),
};

await client.init(apiKey, {
defaultTracking: false,
}).promise;
const diagnosticTrack = jest.spyOn(client.config.diagnosticProvider, 'track');
client.config.transportProvider = transportProvider;
await client.track('event_type', { userId: 'user_0' }).promise;

expect(diagnosticTrack).toHaveBeenCalledTimes(0);
});

test.each([null, new Error()])('should diagnostic track when 0 unexpected error', async (res) => {
const transportProvider = {
send: jest.fn().mockImplementationOnce(() => {
return Promise.resolve(res);
}),
};

await client.init(apiKey, {
defaultTracking: false,
}).promise;
const diagnosticTrack = jest.spyOn(client.config.diagnosticProvider, 'track');
client.config.transportProvider = transportProvider;
justin-fiedler marked this conversation as resolved.
Show resolved Hide resolved
await client.track('event_type', { userId: 'user_0' }).promise;

expect(diagnosticTrack).toHaveBeenCalledTimes(1);
expect(diagnosticTrack).toHaveBeenCalledWith(1, 0, core.DIAGNOSTIC_MESSAGES.UNEXPECTED_ERROR);
});

test('should diagnostic track when flush and non 200', async () => {
const transportProvider = {
send: jest.fn().mockImplementationOnce(() => {
return Promise.resolve({
status: Status.Failed,
statusCode: 500,
});
}),
};

await client.init(apiKey, {
defaultTracking: false,
}).promise;
const diagnosticTrack = jest.spyOn(client.config.diagnosticProvider, 'track');
client.config.transportProvider = transportProvider;
client.track('event_type', { userId: 'user_0' });
await client.flush().promise;

expect(diagnosticTrack).toHaveBeenCalledTimes(1);
expect(diagnosticTrack).toHaveBeenCalledWith(1, 0, core.DIAGNOSTIC_MESSAGES.UNEXPECTED_ERROR);
});

test.each([
['api_key', undefined, core.DIAGNOSTIC_MESSAGES.INVALID_OR_MISSING_FIELDS],
[undefined, { time: [0] }, core.DIAGNOSTIC_MESSAGES.EVENT_ERROR],
])(
'should diagnostic track when 400 invalid response',
async (missingField, eventsWithInvalidFields, message) => {
const transportProvider = {
send: jest.fn().mockImplementationOnce(() => {
return Promise.resolve({
status: Status.Invalid,
statusCode: 400,
body: {
error: 'error',
missingField: missingField,
eventsWithInvalidFields: eventsWithInvalidFields,
eventsWithMissingFields: {},
eventsWithInvalidIdLengths: {},
silencedEvents: [],
},
});
}),
};

await client.init(apiKey, {
defaultTracking: false,
}).promise;
const diagnosticTrack = jest.spyOn(client.config.diagnosticProvider, 'track');
client.config.transportProvider = transportProvider;
await client.track('event_type', { userId: 'user_0' }).promise;

expect(diagnosticTrack).toHaveBeenCalledTimes(1);
expect(diagnosticTrack).toHaveBeenCalledWith(1, 400, message);
},
);

test('should diagnostic track when 429 rate limit when flush', async () => {
const transportProvider = {
send: jest.fn().mockImplementationOnce(() => {
return Promise.resolve({
status: Status.RateLimit,
statusCode: 429,
body: {
exceededDailyQuotaUsers: { user_0: 1 },
exceededDailyQuotaDevices: {},
throttledEvents: [],
justin-fiedler marked this conversation as resolved.
Show resolved Hide resolved
},
});
}),
};

await client.init(apiKey, {
defaultTracking: false,
}).promise;
const diagnosticTrack = jest.spyOn(client.config.diagnosticProvider, 'track');
client.config.transportProvider = transportProvider;
client.track('event_type', { userId: 'user_0' });
// flush() calls destination.flush(useRetry: false)
await client.flush().promise;

expect(diagnosticTrack).toHaveBeenCalledTimes(1);
expect(diagnosticTrack).toHaveBeenCalledWith(1, 429, core.DIAGNOSTIC_MESSAGES.EXCEEDED_DAILY_QUOTA);
});

test('should diagnostic track when 413', async () => {
const transportProvider = {
send: jest.fn().mockImplementationOnce(() => {
return Promise.resolve({
status: Status.PayloadTooLarge,
statusCode: 413,
body: {
error: 'error',
},
});
}),
};

await client.init(apiKey, {
defaultTracking: false,
}).promise;
const diagnosticTrack = jest.spyOn(client.config.diagnosticProvider, 'track');
client.config.transportProvider = transportProvider;
await client.track('event_type', { userId: 'user_0' }).promise;

expect(diagnosticTrack).toHaveBeenCalledTimes(1);
expect(diagnosticTrack).toHaveBeenCalledWith(1, 413, core.DIAGNOSTIC_MESSAGES.PAYLOAD_TOO_LARGE);
});

test('should diagnostic track when 500 hit max retries', async () => {
const transportProvider = {
send: jest.fn().mockImplementationOnce(() => {
return Promise.resolve({
status: Status.Invalid,
statusCode: 400,
body: {
error: 'error',
missingField: '',
eventsWithInvalidFields: {},
eventsWithMissingFields: {},
eventsWithInvalidIdLengths: {},
silencedEvents: [],
},
});
}),
};

await client.init(apiKey, {
defaultTracking: false,
flushMaxRetries: 1,
}).promise;
const diagnosticTrack = jest.spyOn(client.config.diagnosticProvider, 'track');
client.config.transportProvider = transportProvider;
await client.track('event_type', { userId: 'user_0' }).promise;

expect(diagnosticTrack).toHaveBeenCalledTimes(1);
expect(diagnosticTrack).toHaveBeenCalledWith(1, 500, core.DIAGNOSTIC_MESSAGES.EXCEEDED_MAX_RETRY);
});
});
});

describe('getUserId', () => {
Expand Down
9 changes: 9 additions & 0 deletions packages/analytics-browser/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { SendBeaconTransport } from '../src/transports/send-beacon';
import { uuidPattern } from './helpers/constants';
import { DEFAULT_IDENTITY_STORAGE, DEFAULT_SERVER_ZONE } from '../src/constants';
import { AmplitudeBrowser } from '../src/browser-client';
import { BrowserDiagnostic } from '../src/diagnostics/diagnostic';

describe('config', () => {
const someUUID: string = expect.stringMatching(uuidPattern) as string;
Expand Down Expand Up @@ -73,13 +74,19 @@ describe('config', () => {
transport: 'fetch',
transportProvider: new FetchTransport(),
useBatch: false,
diagnosticProvider: new BrowserDiagnostic({ apiKey }),
});
});

test('shoud return _optOut', () => {
const config = new Config.BrowserConfig(apiKey);
expect(config.optOut).toBe(false);
});

test('should set default api key for diagnostic provider', () => {
const config = new Config.BrowserConfig(apiKey);
expect(config.diagnosticProvider.apiKey).toBe(apiKey);
});
});

describe('useBrowserConfig', () => {
Expand Down Expand Up @@ -128,6 +135,7 @@ describe('config', () => {
transport: 'fetch',
transportProvider: new FetchTransport(),
useBatch: false,
diagnosticProvider: new BrowserDiagnostic({ apiKey }),
});
expect(getTopLevelDomain).toHaveBeenCalledTimes(1);
});
Expand Down Expand Up @@ -213,6 +221,7 @@ describe('config', () => {
transport: 'fetch',
transportProvider: new FetchTransport(),
useBatch: false,
diagnosticProvider: new BrowserDiagnostic({ apiKey }),
});
});

Expand Down
47 changes: 47 additions & 0 deletions packages/analytics-browser/test/diagnostic.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { DIAGNOSTIC_ENDPOINT, DIAGNOSTIC_METADATA_TYPE } from '@amplitude/analytics-core';
import { BrowserDiagnostic } from '../src/diagnostics/diagnostic';

describe('Diagnostic', () => {
test('should fetch', async () => {
const diagnostic = new BrowserDiagnostic();
const fetchMock = jest.fn().mockResolvedValueOnce({} as Response);
Date.now = jest.fn().mockReturnValue(1697583342266);
global.fetch = fetchMock;

diagnostic.track(5, 400, 'test message 0');
diagnostic.track(10, 500, 'test message 1');
await diagnostic.flush();

expect(fetchMock).toHaveBeenCalledTimes(1);
Mercy811 marked this conversation as resolved.
Show resolved Hide resolved
expect(fetchMock).toHaveBeenCalledWith(DIAGNOSTIC_ENDPOINT, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({
api_key: '',
omni_metrics: [
{
metadata_type: DIAGNOSTIC_METADATA_TYPE,
library: 'amplitude-ts',
accounting_time_min: Math.floor(1697583342266 / 60 / 1000),
response_code: 400,
trigger: 'test message 0',
action: 'drop events',
event_count: 5,
},
{
metadata_type: DIAGNOSTIC_METADATA_TYPE,
library: 'amplitude-ts',
accounting_time_min: Math.floor(1697583342266 / 60 / 1000),
response_code: 500,
trigger: 'test message 1',
action: 'drop events',
event_count: 10,
},
],
}),
});
fetchMock.mockRestore();
});
});
6 changes: 6 additions & 0 deletions packages/analytics-browser/test/helpers/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ export const createConfigurationMock = (options?: Partial<BrowserConfig>) => {
},
trackingSessionEvents: false,
userId: undefined,
diagnosticProvider: {
isDisabled: false,
serverUrl: undefined,
apiKey: undefined,
track: jest.fn(),
},
...options,
};
};
10 changes: 10 additions & 0 deletions packages/analytics-core/src/config.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
Event,
Config as IConfig,
Diagnostic,
Logger as ILogger,
LogLevel,
Storage,
Expand All @@ -18,6 +19,7 @@ import {
} from './constants';

import { Logger } from './logger';
import { BaseDiagnostic } from './diagnostics/diagnostic';

export const getDefaultConfig = () => ({
flushMaxRetries: 12,
Expand All @@ -30,6 +32,7 @@ export const getDefaultConfig = () => ({
serverUrl: AMPLITUDE_SERVER_URL,
serverZone: 'US' as ServerZoneType,
useBatch: false,
diagnosticProvider: new BaseDiagnostic(),
Mercy811 marked this conversation as resolved.
Show resolved Hide resolved
});

export class Config implements IConfig {
Expand All @@ -48,6 +51,7 @@ export class Config implements IConfig {
transportProvider: Transport;
storageProvider?: Storage<Event[]>;
useBatch: boolean;
diagnosticProvider: Diagnostic;

protected _optOut = false;
get optOut() {
Expand Down Expand Up @@ -75,6 +79,12 @@ export class Config implements IConfig {
this.storageProvider = options.storageProvider;
this.transportProvider = options.transportProvider;
this.useBatch = options.useBatch ?? defaultConfig.useBatch;

this.diagnosticProvider = options.diagnosticProvider ?? defaultConfig.diagnosticProvider;
if (!this.diagnosticProvider.apiKey) {
this.diagnosticProvider.apiKey = this.apiKey;
}

this.loggerProvider.enable(this.logLevel);

const serverConfig = createServerConfig(options.serverUrl, options.serverZone, options.useBatch);
Expand Down
Loading
Loading