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

⚗️ [RUMF-1182] add telemetry sample rate #1510

Merged
merged 3 commits into from
Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
25 changes: 23 additions & 2 deletions packages/core/src/domain/configuration/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ describe('validateAndBuildConfiguration', () => {
expect(displaySpy).toHaveBeenCalledOnceWith('Client Token is not configured, we will not send any data.')
})

it("shouldn't display any error if the configuration is correct", () => {
validateAndBuildConfiguration({ clientToken: 'yes' })
expect(displaySpy).not.toHaveBeenCalled()
})

it('requires sampleRate to be a percentage', () => {
expect(
validateAndBuildConfiguration({ clientToken, sampleRate: 'foo' } as unknown as InitConfiguration)
Expand All @@ -44,12 +49,28 @@ describe('validateAndBuildConfiguration', () => {
validateAndBuildConfiguration({ clientToken, sampleRate: 200 } as unknown as InitConfiguration)
).toBeUndefined()
expect(displaySpy).toHaveBeenCalledOnceWith('Sample Rate should be a number between 0 and 100')
})

it("shouldn't display any error if the configuration is correct", () => {
displaySpy.calls.reset()
validateAndBuildConfiguration({ clientToken: 'yes', sampleRate: 1 })
expect(displaySpy).not.toHaveBeenCalled()
})

it('requires telemetrySampleRate to be a percentage', () => {
expect(
validateAndBuildConfiguration({ clientToken, telemetrySampleRate: 'foo' } as unknown as InitConfiguration)
).toBeUndefined()
expect(displaySpy).toHaveBeenCalledOnceWith('Telemetry Sample Rate should be a number between 0 and 100')

displaySpy.calls.reset()
expect(
validateAndBuildConfiguration({ clientToken, telemetrySampleRate: 200 } as unknown as InitConfiguration)
).toBeUndefined()
expect(displaySpy).toHaveBeenCalledOnceWith('Telemetry Sample Rate should be a number between 0 and 100')

displaySpy.calls.reset()
validateAndBuildConfiguration({ clientToken: 'yes', telemetrySampleRate: 1 })
expect(displaySpy).not.toHaveBeenCalled()
})
})

describe('cookie options', () => {
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/domain/configuration/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface InitConfiguration {
clientToken: string
beforeSend?: GenericBeforeSendCallback | undefined
sampleRate?: number | undefined
telemetrySampleRate?: number | undefined
silentMultipleInit?: boolean | undefined

// transport options
Expand Down Expand Up @@ -56,6 +57,7 @@ export interface Configuration extends TransportConfiguration {
beforeSend: GenericBeforeSendCallback | undefined
cookieOptions: CookieOptions
sampleRate: number
telemetrySampleRate: number
service: string | undefined
silentMultipleInit: boolean

Expand All @@ -81,6 +83,11 @@ export function validateAndBuildConfiguration(initConfiguration: InitConfigurati
return
}

if (initConfiguration.telemetrySampleRate !== undefined && !isPercentage(initConfiguration.telemetrySampleRate)) {
display.error('Telemetry Sample Rate should be a number between 0 and 100')
return
}

// Set the experimental feature flags as early as possible, so we can use them in most places
updateExperimentalFeatures(initConfiguration.enableExperimentalFeatures)

Expand All @@ -90,6 +97,7 @@ export function validateAndBuildConfiguration(initConfiguration: InitConfigurati
initConfiguration.beforeSend && catchUserErrors(initConfiguration.beforeSend, 'beforeSend threw an error:'),
cookieOptions: buildCookieOptions(initConfiguration),
sampleRate: initConfiguration.sampleRate ?? 100,
telemetrySampleRate: initConfiguration.telemetrySampleRate ?? 20,
service: initConfiguration.service,
silentMultipleInit: !!initConfiguration.silentMultipleInit,

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Context } from '../../tools/context'
import { display } from '../../tools/display'
import type { Configuration } from '../configuration'
import { updateExperimentalFeatures, resetExperimentalFeatures } from '../configuration'
import type { InternalMonitoring, MonitoringMessage } from './internalMonitoring'
Expand All @@ -8,11 +9,13 @@ import {
resetInternalMonitoring,
startInternalMonitoring,
callMonitored,
setDebugMode,
} from './internalMonitoring'
import type { TelemetryEvent } from './telemetryEvent.types'

const configuration: Partial<Configuration> = {
maxInternalMonitoringMessagesPerPage: 7,
telemetrySampleRate: 100,
}

describe('internal monitoring', () => {
Expand Down Expand Up @@ -189,6 +192,52 @@ describe('internal monitoring', () => {
})
})

describe('setDebug', () => {
let displaySpy: jasmine.Spy

beforeEach(() => {
displaySpy = spyOn(display, 'error')
})

afterEach(() => {
resetInternalMonitoring()
resetExperimentalFeatures()
})

it('when not called, should not display error', () => {
startInternalMonitoring(configuration as Configuration)

callMonitored(() => {
throw new Error('message')
})

expect(displaySpy).not.toHaveBeenCalled()
})

it('when called, should display error', () => {
startInternalMonitoring(configuration as Configuration)
setDebugMode(true)

callMonitored(() => {
throw new Error('message')
})

expect(displaySpy).toHaveBeenCalled()
})

it('when called and telemetry not sampled, should display error', () => {
updateExperimentalFeatures(['telemetry'])
startInternalMonitoring({ ...configuration, telemetrySampleRate: 0 } as Configuration)
setDebugMode(true)

callMonitored(() => {
throw new Error('message')
})

expect(displaySpy).toHaveBeenCalled()
})
})

describe('new telemetry', () => {
let internalMonitoring: InternalMonitoring
let notifySpy: jasmine.Spy<(event: TelemetryEvent & Context) => void>
Expand Down Expand Up @@ -246,6 +295,30 @@ describe('internal monitoring', () => {

expect(oldNotifySpy.calls.mostRecent().args[0].foo).toEqual('bar')
})

it('should notify when sampled', () => {
spyOn(Math, 'random').and.callFake(() => 0)
internalMonitoring = startInternalMonitoring({ ...configuration, telemetrySampleRate: 50 } as Configuration)
internalMonitoring.telemetryEventObservable.subscribe(notifySpy)

callMonitored(() => {
throw new Error('message')
})

expect(notifySpy).toHaveBeenCalled()
})

it('should not notify when not sampled', () => {
spyOn(Math, 'random').and.callFake(() => 1)
internalMonitoring = startInternalMonitoring({ ...configuration, telemetrySampleRate: 50 } as Configuration)
internalMonitoring.telemetryEventObservable.subscribe(notifySpy)

callMonitored(() => {
throw new Error('message')
})

expect(notifySpy).not.toHaveBeenCalled()
})
})

describe('when disabled', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Context } from '../../tools/context'
import { display } from '../../tools/display'
import { toStackTraceString } from '../../tools/error'
import { assign, combine, jsonStringify } from '../../tools/utils'
import { assign, combine, jsonStringify, performDraw } from '../../tools/utils'
import type { Configuration } from '../configuration'
import { computeStackTrace } from '../tracekit'
import { Observable } from '../../tools/observable'
Expand Down Expand Up @@ -38,7 +38,8 @@ const monitoringConfiguration: {
debugMode?: boolean
maxMessagesPerPage: number
sentMessageCount: number
} = { maxMessagesPerPage: 0, sentMessageCount: 0 }
telemetryEnabled: boolean
} = { maxMessagesPerPage: 0, sentMessageCount: 0, telemetryEnabled: false }

let onInternalMonitoringMessageCollected: ((message: MonitoringMessage) => void) | undefined

Expand All @@ -48,9 +49,11 @@ export function startInternalMonitoring(configuration: Configuration): InternalM
const monitoringMessageObservable = new Observable<MonitoringMessage>()
const telemetryEventObservable = new Observable<TelemetryEvent & Context>()

monitoringConfiguration.telemetryEnabled = performDraw(configuration.telemetrySampleRate)

onInternalMonitoringMessageCollected = (message: MonitoringMessage) => {
monitoringMessageObservable.notify(withContext(message))
if (isExperimentalFeatureEnabled('telemetry')) {
if (isExperimentalFeatureEnabled('telemetry') && monitoringConfiguration.telemetryEnabled) {
telemetryEventObservable.notify(toTelemetryEvent(message))
}
}
Expand Down Expand Up @@ -113,6 +116,7 @@ export function startFakeInternalMonitoring() {

export function resetInternalMonitoring() {
onInternalMonitoringMessageCollected = undefined
monitoringConfiguration.debugMode = undefined
}

export function monitored<T extends (...params: any[]) => unknown>(
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/lib/framework/createTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ import { createMockServerApp } from './serverApps/mock'
const DEFAULT_RUM_CONFIGURATION = {
applicationId: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee',
clientToken: 'token',
telemetrySampleRate: 100,
enableExperimentalFeatures: [],
}

const DEFAULT_LOGS_CONFIGURATION = {
clientToken: 'token',
telemetrySampleRate: 100,
}

export function createTest(title: string) {
Expand Down