Skip to content

Commit

Permalink
🐛 [RUMF-1372] Should not set rule_psr if it is not defined by the u…
Browse files Browse the repository at this point in the history
…ser (#1705)

* 🐛 [RUMF-1372] Should not set `rule_psr` if it is not defined by the user

* Update packages/rum-core/src/domain/tracing/tracer.ts

Co-authored-by: Bastien Caudan <[email protected]>

* getRulePsr should return 0 if set + test

* fix unique test running

* use core isNumber method

Co-authored-by: Bastien Caudan <[email protected]>
  • Loading branch information
liywjl and bcaudan authored Aug 30, 2022
1 parent 0e0e738 commit 17da547
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 7 deletions.
4 changes: 2 additions & 2 deletions packages/rum-core/src/domain/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ describe('validateAndBuildRumConfiguration', () => {
})

describe('tracingSampleRate', () => {
it('defaults to 100 if the option is not provided', () => {
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.tracingSampleRate).toBe(100)
it('defaults to undefined if the option is not provided', () => {
expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.tracingSampleRate).toBeUndefined()
})

it('is set to provided value', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/rum-core/src/domain/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export interface RumConfiguration extends Configuration {
// Built from init configuration
actionNameAttribute: string | undefined
allowedTracingOrigins: Array<string | RegExp>
tracingSampleRate: number
tracingSampleRate: number | undefined
excludedActivityUrls: Array<string | RegExp>
applicationId: string
defaultPrivacyLevel: DefaultPrivacyLevel
Expand Down Expand Up @@ -104,7 +104,7 @@ export function validateAndBuildRumConfiguration(
actionNameAttribute: initConfiguration.actionNameAttribute,
premiumSampleRate: premiumSampleRate ?? 100,
allowedTracingOrigins: initConfiguration.allowedTracingOrigins ?? [],
tracingSampleRate: initConfiguration.tracingSampleRate ?? 100,
tracingSampleRate: initConfiguration.tracingSampleRate,
excludedActivityUrls: initConfiguration.excludedActivityUrls ?? [],
trackInteractions: !!initConfiguration.trackInteractions || trackFrustrations,
trackFrustrations,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,55 @@ describe('resourceCollection', () => {
const traceInfo = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd!
expect(traceInfo.rule_psr).toEqual(0.6)
})

it('should not define rule_psr if tracingSampleRate is undefined', () => {
setupBuilder = setup().beforeBuild(({ lifeCycle }) => {
startResourceCollection(
lifeCycle,
validateAndBuildRumConfiguration({
clientToken: 'xxx',
applicationId: 'xxx',
})!
)
})

const { lifeCycle, rawRumEvents } = setupBuilder.build()
lifeCycle.notify(
LifeCycleEventType.REQUEST_COMPLETED,
createCompletedRequest({
traceSampled: true,
spanId: new TraceIdentifier(),
traceId: new TraceIdentifier(),
})
)
const traceInfo = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd!
expect(traceInfo.rule_psr).toBeUndefined()
})

it('should define rule_psr to 0 if tracingSampleRate is set to 0', () => {
setupBuilder = setup().beforeBuild(({ lifeCycle }) => {
startResourceCollection(
lifeCycle,
validateAndBuildRumConfiguration({
clientToken: 'xxx',
applicationId: 'xxx',
tracingSampleRate: 0,
})!
)
})

const { lifeCycle, rawRumEvents } = setupBuilder.build()
lifeCycle.notify(
LifeCycleEventType.REQUEST_COMPLETED,
createCompletedRequest({
traceSampled: true,
spanId: new TraceIdentifier(),
traceId: new TraceIdentifier(),
})
)
const traceInfo = (rawRumEvents[0].rawRumEvent as RawRumResourceEvent)._dd!
expect(traceInfo.rule_psr).toEqual(0)
})
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
toServerDuration,
relativeToClocks,
assign,
isNumber,
} from '@datadog/browser-core'
import type { RumConfiguration } from '../../configuration'
import type { RumPerformanceEntry, RumPerformanceResourceTiming } from '../../../browser/performanceCollection'
Expand Down Expand Up @@ -167,5 +168,5 @@ function toPerformanceEntryRepresentation(entry: RumPerformanceEntry): Performan
* @returns number between 0 and 1 which represents tracing sample rate
*/
function getRulePsr(configuration: RumConfiguration) {
return configuration.tracingSampleRate / 100
return isNumber(configuration.tracingSampleRate) ? configuration.tracingSampleRate / 100 : undefined
}
4 changes: 2 additions & 2 deletions packages/rum-core/src/domain/tracing/tracer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getOrigin, matchList, objectEntries, shallowClone, performDraw } from '@datadog/browser-core'
import { getOrigin, matchList, objectEntries, shallowClone, performDraw, isNumber } from '@datadog/browser-core'
import type { RumConfiguration } from '../configuration'
import type {
RumFetchCompleteContext,
Expand Down Expand Up @@ -93,7 +93,7 @@ function injectHeadersIfTracingAllowed(

context.traceId = new TraceIdentifier()
context.spanId = new TraceIdentifier()
context.traceSampled = performDraw(configuration.tracingSampleRate)
context.traceSampled = !isNumber(configuration.tracingSampleRate) || performDraw(configuration.tracingSampleRate)
inject(makeTracingHeaders(context.traceId, context.spanId, context.traceSampled))
}

Expand Down

0 comments on commit 17da547

Please sign in to comment.