Skip to content

Commit

Permalink
πŸ‘·[RUMF-1170] Tweak build env (#1354)
Browse files Browse the repository at this point in the history
* πŸ› fix source code executed outside jasmine spec

* πŸ‘· switch build env strategy

* ♻️ use new build env variables

* πŸ‘Œ use DefinePlugin

* πŸ‘Œ remove @ts-ignore

* πŸ‘Œ keep build env only for unit test
  • Loading branch information
bcaudan authored Feb 24, 2022
1 parent 04eb20e commit 59100a2
Show file tree
Hide file tree
Showing 28 changed files with 151 additions and 194 deletions.
1 change: 0 additions & 1 deletion LICENSE-3rdparty.csv
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ dev,prettier,MIT,Copyright James Long and contributors
dev,puppeteer,Apache-2.0,Copyright 2017 Google Inc.
dev,replace-in-file,MIT,Copyright 2015-2019 Adam Reis
dev,sinon,BSD-3-Clause,Copyright 2010-2017 Christian Johansen
dev,string-replace-loader,MIT,Copyright 2015 Valentyn Barmashyn
dev,terser-webpack-plugin,MIT,Copyright JS Foundation and other contributors
dev,ts-loader,MIT,Copyright 2015 TypeStrong
dev,ts-node,MIT,Copyright 2014 Blake Embrey
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@
"prettier": "2.5.1",
"replace-in-file": "6.3.2",
"sinon": "9.2.4",
"string-replace-loader": "3.1.0",
"terser-webpack-plugin": "5.1.1",
"ts-loader": "8.0.18",
"ts-node": "9.1.1",
Expand Down
5 changes: 3 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
"sideEffects": false,
"scripts": {
"build": "run-p build:cjs build:esm",
"build:cjs": "rm -rf cjs && tsc -p tsconfig.cjs.json",
"build:esm": "rm -rf esm && tsc -p tsconfig.esm.json"
"build:cjs": "rm -rf cjs && tsc -p tsconfig.cjs.json && yarn replace-build-env cjs",
"build:esm": "rm -rf esm && tsc -p tsconfig.esm.json && yarn replace-build-env esm",
"replace-build-env": "node ../../scripts/replace-build-env.js"
},
"dependencies": {
"tslib": "^1.10.0"
Expand Down
21 changes: 5 additions & 16 deletions packages/core/src/boot/init.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { setDebugMode } from '../domain/internalMonitoring'
import { catchUserErrors } from '../tools/catchUserErrors'

export function makePublicApi<T>(
buildEnv: BuildEnv,
stub: T
): T & { onReady(callback: () => void): void; version: string } {
// replaced at build time
declare const __BUILD_ENV__SDK_VERSION__: string

export function makePublicApi<T>(stub: T): T & { onReady(callback: () => void): void; version: string } {
const publicApi = {
...stub,

version: buildEnv.sdkVersion,
version: __BUILD_ENV__SDK_VERSION__,

// This API method is intentionally not monitored, since the only thing executed is the
// user-provided 'callback'. All SDK usages executed in the callback should be monitored, and
Expand Down Expand Up @@ -37,14 +37,3 @@ export function defineGlobal<Global, Name extends keyof Global>(global: Global,
existingGlobalVariable.q.forEach((fn) => catchUserErrors(fn, 'onReady callback threw an error:')())
}
}

export enum BuildMode {
RELEASE = 'release',
CANARY = 'canary',
E2E_TEST = 'e2e-test',
}

export interface BuildEnv {
buildMode: BuildMode
sdkVersion: string
}
35 changes: 13 additions & 22 deletions packages/core/src/domain/configuration/configuration.spec.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,18 @@
import type { RumEvent } from '../../../../rum-core/src'
import type { BuildEnv } from '../../boot/init'
import { BuildMode } from '../../boot/init'
import { display } from '../../tools/display'
import type { InitConfiguration } from './configuration'
import { validateAndBuildConfiguration } from './configuration'
import { isExperimentalFeatureEnabled, updateExperimentalFeatures } from './experimentalFeatures'

describe('validateAndBuildConfiguration', () => {
const clientToken = 'some_client_token'
const buildEnv: BuildEnv = {
buildMode: BuildMode.RELEASE,
sdkVersion: 'some_version',
}

beforeEach(() => {
updateExperimentalFeatures([])
})

it('updates experimental feature flags', () => {
validateAndBuildConfiguration({ clientToken, enableExperimentalFeatures: ['foo'] }, buildEnv)
validateAndBuildConfiguration({ clientToken, enableExperimentalFeatures: ['foo'] })
expect(isExperimentalFeatureEnabled('foo')).toBeTrue()
})

Expand All @@ -30,62 +24,59 @@ describe('validateAndBuildConfiguration', () => {
})

it('requires the InitConfiguration to be defined', () => {
expect(validateAndBuildConfiguration(undefined as unknown as InitConfiguration, buildEnv)).toBeUndefined()
expect(validateAndBuildConfiguration(undefined as unknown as InitConfiguration)).toBeUndefined()
expect(displaySpy).toHaveBeenCalledOnceWith('Client Token is not configured, we will not send any data.')
})

it('requires clientToken to be defined', () => {
expect(validateAndBuildConfiguration({} as unknown as InitConfiguration, buildEnv)).toBeUndefined()
expect(validateAndBuildConfiguration({} as unknown as InitConfiguration)).toBeUndefined()
expect(displaySpy).toHaveBeenCalledOnceWith('Client Token is not configured, we will not send any data.')
})

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

displaySpy.calls.reset()
expect(
validateAndBuildConfiguration({ clientToken, sampleRate: 200 } as unknown as InitConfiguration, buildEnv)
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", () => {
validateAndBuildConfiguration({ clientToken: 'yes', sampleRate: 1 }, buildEnv)
validateAndBuildConfiguration({ clientToken: 'yes', sampleRate: 1 })
expect(displaySpy).not.toHaveBeenCalled()
})
})

describe('cookie options', () => {
it('should not be secure nor crossSite by default', () => {
const configuration = validateAndBuildConfiguration({ clientToken }, buildEnv)!
const configuration = validateAndBuildConfiguration({ clientToken })!
expect(configuration.cookieOptions).toEqual({ secure: false, crossSite: false })
})

it('should be secure when `useSecureSessionCookie` is truthy', () => {
const configuration = validateAndBuildConfiguration({ clientToken, useSecureSessionCookie: true }, buildEnv)!
const configuration = validateAndBuildConfiguration({ clientToken, useSecureSessionCookie: true })!
expect(configuration.cookieOptions).toEqual({ secure: true, crossSite: false })
})

it('should be secure and crossSite when `useCrossSiteSessionCookie` is truthy', () => {
const configuration = validateAndBuildConfiguration({ clientToken, useCrossSiteSessionCookie: true }, buildEnv)!
const configuration = validateAndBuildConfiguration({ clientToken, useCrossSiteSessionCookie: true })!
expect(configuration.cookieOptions).toEqual({ secure: true, crossSite: true })
})

it('should have domain when `trackSessionAcrossSubdomains` is truthy', () => {
const configuration = validateAndBuildConfiguration(
{ clientToken, trackSessionAcrossSubdomains: true },
buildEnv
)!
const configuration = validateAndBuildConfiguration({ clientToken, trackSessionAcrossSubdomains: true })!
expect(configuration.cookieOptions).toEqual({ secure: false, crossSite: false, domain: jasmine.any(String) })
})
})

describe('beforeSend', () => {
it('should be undefined when beforeSend is missing on user configuration', () => {
const configuration = validateAndBuildConfiguration({ clientToken }, buildEnv)!
const configuration = validateAndBuildConfiguration({ clientToken })!
expect(configuration.beforeSend).toBeUndefined()
})

Expand All @@ -95,7 +86,7 @@ describe('validateAndBuildConfiguration', () => {
return false
}
}
const configuration = validateAndBuildConfiguration({ clientToken, beforeSend }, buildEnv)!
const configuration = validateAndBuildConfiguration({ clientToken, beforeSend })!
expect(configuration.beforeSend!({ view: { url: '/foo' } }, {})).toBeFalse()
expect(configuration.beforeSend!({ view: { url: '/bar' } }, {})).toBeUndefined()
})
Expand All @@ -105,7 +96,7 @@ describe('validateAndBuildConfiguration', () => {
const beforeSend = () => {
throw myError
}
const configuration = validateAndBuildConfiguration({ clientToken, beforeSend }, buildEnv)!
const configuration = validateAndBuildConfiguration({ clientToken, beforeSend })!
const displaySpy = spyOn(display, 'error')
expect(configuration.beforeSend!(null, {})).toBeUndefined()
expect(displaySpy).toHaveBeenCalledWith('beforeSend threw an error:', myError)
Expand Down
10 changes: 3 additions & 7 deletions packages/core/src/domain/configuration/configuration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { BuildEnv } from '../../boot/init'
import type { CookieOptions } from '../../browser/cookie'
import { getCurrentSite } from '../../browser/cookie'
import { catchUserErrors } from '../../tools/catchUserErrors'
Expand Down Expand Up @@ -71,10 +70,7 @@ export interface Configuration extends TransportConfiguration {
maxMessageSize: number
}

export function validateAndBuildConfiguration(
initConfiguration: InitConfiguration,
buildEnv: BuildEnv
): Configuration | undefined {
export function validateAndBuildConfiguration(initConfiguration: InitConfiguration): Configuration | undefined {
if (!initConfiguration || !initConfiguration.clientToken) {
display.error('Client Token is not configured, we will not send any data.')
return
Expand All @@ -85,11 +81,11 @@ export function validateAndBuildConfiguration(
return
}

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

return {
...computeTransportConfiguration(initConfiguration, buildEnv),
...computeTransportConfiguration(initConfiguration),

beforeSend:
initConfiguration.beforeSend && catchUserErrors(initConfiguration.beforeSend, 'beforeSend threw an error:'),
Expand Down
9 changes: 5 additions & 4 deletions packages/core/src/domain/configuration/endpointBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import type { BuildEnv } from '../../boot/init'
import { timeStampNow } from '../../tools/timeUtils'
import { normalizeUrl } from '../../tools/urlPolyfill'
import { generateUUID } from '../../tools/utils'
import type { InitConfiguration } from './configuration'

// replaced at build time
declare const __BUILD_ENV__SDK_VERSION__: string

export const ENDPOINTS = {
logs: 'logs',
rum: 'rum',
Expand All @@ -24,7 +26,6 @@ export type EndpointBuilder = ReturnType<typeof createEndpointBuilder>

export function createEndpointBuilder(
initConfiguration: InitConfiguration,
buildEnv: BuildEnv,
endpointType: EndpointType,
tags: string[],
source?: string
Expand All @@ -41,9 +42,9 @@ export function createEndpointBuilder(
build() {
let parameters =
`ddsource=${source || 'browser'}` +
`&ddtags=${encodeURIComponent([`sdk_version:${buildEnv.sdkVersion}`].concat(tags).join(','))}` +
`&ddtags=${encodeURIComponent([`sdk_version:${__BUILD_ENV__SDK_VERSION__}`].concat(tags).join(','))}` +
`&dd-api-key=${clientToken}` +
`&dd-evp-origin-version=${encodeURIComponent(buildEnv.sdkVersion)}` +
`&dd-evp-origin-version=${encodeURIComponent(__BUILD_ENV__SDK_VERSION__)}` +
`&dd-evp-origin=browser` +
`&dd-request-id=${generateUUID()}`

Expand Down
Loading

0 comments on commit 59100a2

Please sign in to comment.