diff --git a/packages/shared/common/src/internal/events/ClientMessages.ts b/packages/shared/common/src/internal/events/ClientMessages.ts index 1d7652ba00..99e5df43c0 100644 --- a/packages/shared/common/src/internal/events/ClientMessages.ts +++ b/packages/shared/common/src/internal/events/ClientMessages.ts @@ -4,4 +4,11 @@ export default class ClientMessages { static readonly missingContextKeyNoEvent = 'Context was unspecified or had no key; event will not be sent'; + + static invalidMetricValue(badType: string) { + return ( + 'The track function was called with a non-numeric "metricValue"' + + ` (${badType}), only numeric metric values are supported.` + ); + } } diff --git a/packages/shared/sdk-client/src/LDClientImpl.events.test.ts b/packages/shared/sdk-client/src/LDClientImpl.events.test.ts index 17a705d097..eeb43a1d77 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.events.test.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.events.test.ts @@ -1,5 +1,5 @@ import { AutoEnvAttributes, clone, LDContext } from '@launchdarkly/js-sdk-common'; -import { InputIdentifyEvent } from '@launchdarkly/js-sdk-common/dist/internal'; +import { InputCustomEvent, InputIdentifyEvent } from '@launchdarkly/js-sdk-common/dist/internal'; import { basicPlatform, hasher, @@ -31,6 +31,7 @@ jest.mock('@launchdarkly/js-sdk-common', () => { const testSdkKey = 'test-sdk-key'; let ldc: LDClientImpl; let defaultPutResponse: Flags; +const carContext: LDContext = { kind: 'car', key: 'test-car' }; describe('sdk-client object', () => { beforeEach(() => { @@ -54,7 +55,6 @@ describe('sdk-client object', () => { test('identify event', async () => { defaultPutResponse['dev-test-flag'].value = false; - const carContext: LDContext = { kind: 'car', key: 'test-car' }; await ldc.identify(carContext); @@ -73,4 +73,85 @@ describe('sdk-client object', () => { }), ); }); + + it('produces track events with data', async () => { + await ldc.identify(carContext); + + ldc.track('the-event', { the: 'data' }, undefined); + expect(MockEventProcessor).toHaveBeenCalled(); + expect(ldc.eventProcessor!.sendEvent).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + kind: 'custom', + key: 'the-event', + context: expect.objectContaining({ + contexts: expect.objectContaining({ + car: { key: 'test-car' }, + }), + }), + data: { the: 'data' }, + samplingRatio: 1, + creationDate: expect.any(Number), + }), + ); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('produces track events with a metric value', async () => { + await ldc.identify(carContext); + + ldc.track('the-event', undefined, 12); + expect(MockEventProcessor).toHaveBeenCalled(); + expect(ldc.eventProcessor!.sendEvent).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + kind: 'custom', + key: 'the-event', + context: expect.objectContaining({ + contexts: expect.objectContaining({ + car: { key: 'test-car' }, + }), + }), + metricValue: 12, + samplingRatio: 1, + creationDate: expect.any(Number), + }), + ); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('produces track events with a metric value and data', async () => { + await ldc.identify(carContext); + + ldc.track('the-event', { the: 'data' }, 12); + expect(MockEventProcessor).toHaveBeenCalled(); + expect(ldc.eventProcessor!.sendEvent).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + kind: 'custom', + key: 'the-event', + context: expect.objectContaining({ + contexts: expect.objectContaining({ + car: { key: 'test-car' }, + }), + }), + metricValue: 12, + data: { the: 'data' }, + samplingRatio: 1, + creationDate: expect.any(Number), + }), + ); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('produces a warning when the metric value is non-numeric', async () => { + // @ts-ignore + await ldc.identify(carContext); + // @ts-ignore + ldc.track('the-event', { the: 'data' }, '12'); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching(/was called with a non-numeric/), + ); + }); }); diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index eef1cb12f5..42566702d7 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -402,6 +402,11 @@ export default class LDClientImpl implements LDClient { return; } + // 0 is valid, so do not truthy check the metric value + if (metricValue !== undefined && !TypeValidators.Number.is(metricValue)) { + this.logger?.warn(ClientMessages.invalidMetricValue(typeof metricValue)); + } + this.eventProcessor?.sendEvent( this.eventFactoryDefault.customEvent(key, checkedContext!, data, metricValue), ); diff --git a/packages/shared/sdk-server/__tests__/LDClient.events.test.ts b/packages/shared/sdk-server/__tests__/LDClient.events.test.ts index 3a09d688af..a80520bf2c 100644 --- a/packages/shared/sdk-server/__tests__/LDClient.events.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClient.events.test.ts @@ -5,6 +5,7 @@ import * as mocks from '@launchdarkly/private-js-mocks'; import { LDClientImpl } from '../src'; import TestData from '../src/integrations/test_data/TestData'; +import TestLogger, { LogLevel } from './Logger'; import makeCallbacks from './makeCallbacks'; const defaultUser = { key: 'user' }; @@ -14,8 +15,10 @@ describe('given a client with mock event processor', () => { let client: LDClientImpl; let events: AsyncQueue; let td: TestData; + let logger: TestLogger; beforeEach(async () => { + logger = new TestLogger(); events = new AsyncQueue(); jest .spyOn(internal.EventProcessor.prototype, 'sendEvent') @@ -30,6 +33,7 @@ describe('given a client with mock event processor', () => { mocks.basicPlatform, { updateProcessor: td.getFactory(), + logger, }, makeCallbacks(false), ); @@ -300,4 +304,54 @@ describe('given a client with mock event processor', () => { default: 'c', }); }); + + it('produces track events with data', async () => { + client.track('the-event', defaultUser, { the: 'data' }, undefined); + const e = await events.take(); + expect(e).toMatchObject({ + kind: 'custom', + key: 'the-event', + context: Context.fromLDContext(defaultUser), + data: { the: 'data' }, + }); + expect(logger.getCount(LogLevel.Warn)).toBe(0); + }); + + it('produces track events with a metric value', async () => { + client.track('the-event', defaultUser, undefined, 12); + const e = await events.take(); + expect(e).toMatchObject({ + kind: 'custom', + key: 'the-event', + context: Context.fromLDContext(defaultUser), + metricValue: 12, + }); + expect(logger.getCount(LogLevel.Warn)).toBe(0); + }); + + it('produces track events with a metric value and data', async () => { + client.track('the-event', defaultUser, { the: 'data' }, 12); + const e = await events.take(); + expect(e).toMatchObject({ + kind: 'custom', + key: 'the-event', + context: Context.fromLDContext(defaultUser), + metricValue: 12, + data: { the: 'data' }, + }); + expect(logger.getCount(LogLevel.Warn)).toBe(0); + }); + + it('produces a warning when the metric value is non-numeric', async () => { + // @ts-ignore + client.track('the-event', defaultUser, { the: 'data' }, '12'); + await events.take(); + expect(logger.getCount(LogLevel.Warn)).toBe(1); + logger.expectMessages([ + { + level: LogLevel.Warn, + matches: /was called with a non-numeric/, + }, + ]); + }); }); diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index e740d9db1d..d5c31860b8 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -714,6 +714,11 @@ export default class LDClientImpl implements LDClient { return; } + // 0 is valid, so do not truthy check the metric value + if (metricValue !== undefined && !TypeValidators.Number.is(metricValue)) { + this.logger?.warn(ClientMessages.invalidMetricValue(typeof metricValue)); + } + this.eventProcessor.sendEvent( this.eventFactoryDefault.customEvent(key, checkedContext!, data, metricValue), );