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-1267] remove circular dependencies part 1 #1559

Merged
merged 7 commits into from
May 24, 2022
Merged
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ module.exports = {
'@typescript-eslint/no-unused-vars': ['error', { args: 'all', argsIgnorePattern: '^_', vars: 'all' }],
'@typescript-eslint/triple-slash-reference': ['error', { path: 'always', types: 'prefer-import', lib: 'always' }],

'import/no-cycle': ['warn'],
'import/no-default-export': 'error',
'import/no-duplicates': 'error',
'import/no-extraneous-dependencies': 'error',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
OLD_SESSION_COOKIE_NAME,
tryOldCookiesMigration,
} from './oldCookiesMigration'
import { SESSION_EXPIRATION_DELAY } from './sessionStore'
import { SESSION_EXPIRATION_DELAY } from './sessionConstants'
import { SESSION_COOKIE_NAME } from './sessionCookieStore'

describe('old cookies migration', () => {
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/domain/session/sessionConstants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { ONE_HOUR, ONE_MINUTE } from '../../tools/utils'

export const SESSION_TIME_OUT_DELAY = 4 * ONE_HOUR
export const SESSION_EXPIRATION_DELAY = 15 * ONE_MINUTE
2 changes: 1 addition & 1 deletion packages/core/src/domain/session/sessionCookieStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { getCookie, setCookie } from '../../browser/cookie'
import { isChromium } from '../../tools/browserDetection'
import * as utils from '../../tools/utils'
import { monitor } from '../internalMonitoring'
import { SESSION_EXPIRATION_DELAY } from './sessionConstants'
import type { SessionState } from './sessionStore'
import { SESSION_EXPIRATION_DELAY } from './sessionStore'

const SESSION_ENTRY_REGEXP = /^([a-z]+)=([a-z0-9-]+)$/
const SESSION_ENTRY_SEPARATOR = '&'
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/domain/session/sessionManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import type { RelativeTime } from '../../tools/timeUtils'
import { isIE } from '../../tools/browserDetection'
import type { SessionManager } from './sessionManager'
import { startSessionManager, stopSessionManager, VISIBILITY_CHECK_DELAY } from './sessionManager'
import { SESSION_TIME_OUT_DELAY, SESSION_EXPIRATION_DELAY } from './sessionStore'
import { SESSION_COOKIE_NAME } from './sessionCookieStore'
import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants'

const enum FakeTrackingType {
NOT_TRACKED = 'not-tracked',
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/domain/session/sessionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import { ContextHistory } from '../../tools/contextHistory'
import type { RelativeTime } from '../../tools/timeUtils'
import { relativeNow, clocksOrigin } from '../../tools/timeUtils'
import { tryOldCookiesMigration } from './oldCookiesMigration'
import { startSessionStore, SESSION_TIME_OUT_DELAY } from './sessionStore'
import { startSessionStore } from './sessionStore'
import { SESSION_TIME_OUT_DELAY } from './sessionConstants'

export interface SessionManager<TrackingType extends string> {
findActiveSession: (startTime?: RelativeTime) => SessionContext<TrackingType> | undefined
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/domain/session/sessionStore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import { mockClock } from '../../../test/specHelper'
import type { CookieOptions } from '../../browser/cookie'
import { getCookie, setCookie, COOKIE_ACCESS_DELAY } from '../../browser/cookie'
import type { SessionStore } from './sessionStore'
import { startSessionStore, SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionStore'
import { startSessionStore } from './sessionStore'
import { SESSION_COOKIE_NAME } from './sessionCookieStore'
import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants'

const enum FakeTrackingType {
TRACKED = 'tracked',
Expand Down
4 changes: 1 addition & 3 deletions packages/core/src/domain/session/sessionStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { COOKIE_ACCESS_DELAY } from '../../browser/cookie'
import { Observable } from '../../tools/observable'
import * as utils from '../../tools/utils'
import { monitor } from '../internalMonitoring'
import { SESSION_TIME_OUT_DELAY } from './sessionConstants'
import { retrieveSession, withCookieLockAccess } from './sessionCookieStore'

export interface SessionStore {
Expand All @@ -23,9 +24,6 @@ export interface SessionState {
[key: string]: string | undefined
}

export const SESSION_EXPIRATION_DELAY = 15 * utils.ONE_MINUTE
export const SESSION_TIME_OUT_DELAY = 4 * utils.ONE_HOUR

/**
* Different session concepts:
* - tracked, the session has an id and is updated along the user navigation
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export {
} from './domain/session/sessionManager'
export {
SESSION_TIME_OUT_DELAY, // Exposed for tests
} from './domain/session/sessionStore'
} from './domain/session/sessionConstants'
export {
HttpRequest,
Batch,
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/tools/createEventRateLimiter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { RawError } from '..'
import { clocksNow, ErrorSource, ONE_MINUTE } from '..'
import type { RawError } from './error'
import { ErrorSource } from './error'
import { clocksNow } from './timeUtils'
import { ONE_MINUTE } from './utils'

export type EventRateLimiter = ReturnType<typeof createEventRateLimiter>

Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/transport/startBatchWithReplica.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Configuration, EndpointBuilder } from '../domain/configuration'
import type { Context } from '../tools/context'
import { Batch, HttpRequest } from './index'
import { Batch } from './batch'
import { HttpRequest } from './httpRequest'

export function startBatchWithReplica<T extends Context>(
configuration: Configuration,
Expand Down
75 changes: 73 additions & 2 deletions packages/rum/src/domain/record/privacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import {

export const MAX_ATTRIBUTE_VALUE_CHAR_LENGTH = 100_000

import { shouldIgnoreElement } from './serialize'

const TEXT_MASKING_CHAR = 'x'

/**
Expand Down Expand Up @@ -215,3 +213,76 @@ export function getTextContent(
}
return textContent
}

/**
* TODO: Preserve CSS element order, and record the presence of the tag, just don't render
* We don't need this logic on the recorder side.
* For security related meta's, customer can mask themmanually given they
* are easy to identify in the HEAD tag.
*/
export function shouldIgnoreElement(element: Element): boolean {
if (element.nodeName === 'SCRIPT') {
return true
}

if (element.nodeName === 'LINK') {
const relAttribute = getLowerCaseAttribute('rel')
return (
// Scripts
(relAttribute === 'preload' && getLowerCaseAttribute('as') === 'script') ||
// Favicons
relAttribute === 'shortcut icon' ||
relAttribute === 'icon'
)
}

if (element.nodeName === 'META') {
const nameAttribute = getLowerCaseAttribute('name')
const relAttribute = getLowerCaseAttribute('rel')
const propertyAttribute = getLowerCaseAttribute('property')
return (
// Favicons
/^msapplication-tile(image|color)$/.test(nameAttribute) ||
nameAttribute === 'application-name' ||
relAttribute === 'icon' ||
relAttribute === 'apple-touch-icon' ||
relAttribute === 'shortcut icon' ||
// Description
nameAttribute === 'keywords' ||
nameAttribute === 'description' ||
// Social
/^(og|twitter|fb):/.test(propertyAttribute) ||
/^(og|twitter):/.test(nameAttribute) ||
nameAttribute === 'pinterest' ||
// Robots
nameAttribute === 'robots' ||
nameAttribute === 'googlebot' ||
nameAttribute === 'bingbot' ||
// Http headers. Ex: X-UA-Compatible, Content-Type, Content-Language, cache-control,
// X-Translated-By
element.hasAttribute('http-equiv') ||
// Authorship
nameAttribute === 'author' ||
nameAttribute === 'generator' ||
nameAttribute === 'framework' ||
nameAttribute === 'publisher' ||
nameAttribute === 'progid' ||
/^article:/.test(propertyAttribute) ||
/^product:/.test(propertyAttribute) ||
// Verification
nameAttribute === 'google-site-verification' ||
nameAttribute === 'yandex-verification' ||
nameAttribute === 'csrf-token' ||
nameAttribute === 'p:domain_verify' ||
nameAttribute === 'verify-v1' ||
nameAttribute === 'verification' ||
nameAttribute === 'shopify-checkout-api-token'
)
}

function getLowerCaseAttribute(name: string) {
return (element.getAttribute(name) || '').toLowerCase()
}

return false
}
73 changes: 0 additions & 73 deletions packages/rum/src/domain/record/serialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,79 +173,6 @@ export function serializeElementNode(element: Element, options: SerializeOptions
}
}

/**
* TODO: Preserve CSS element order, and record the presence of the tag, just don't render
* We don't need this logic on the recorder side.
* For security related meta's, customer can mask themmanually given they
* are easy to identify in the HEAD tag.
*/
export function shouldIgnoreElement(element: Element): boolean {
if (element.nodeName === 'SCRIPT') {
return true
}

if (element.nodeName === 'LINK') {
const relAttribute = getLowerCaseAttribute('rel')
return (
// Scripts
(relAttribute === 'preload' && getLowerCaseAttribute('as') === 'script') ||
// Favicons
relAttribute === 'shortcut icon' ||
relAttribute === 'icon'
)
}

if (element.nodeName === 'META') {
const nameAttribute = getLowerCaseAttribute('name')
const relAttribute = getLowerCaseAttribute('rel')
const propertyAttribute = getLowerCaseAttribute('property')
return (
// Favicons
/^msapplication-tile(image|color)$/.test(nameAttribute) ||
nameAttribute === 'application-name' ||
relAttribute === 'icon' ||
relAttribute === 'apple-touch-icon' ||
relAttribute === 'shortcut icon' ||
// Description
nameAttribute === 'keywords' ||
nameAttribute === 'description' ||
// Social
/^(og|twitter|fb):/.test(propertyAttribute) ||
/^(og|twitter):/.test(nameAttribute) ||
nameAttribute === 'pinterest' ||
// Robots
nameAttribute === 'robots' ||
nameAttribute === 'googlebot' ||
nameAttribute === 'bingbot' ||
// Http headers. Ex: X-UA-Compatible, Content-Type, Content-Language, cache-control,
// X-Translated-By
element.hasAttribute('http-equiv') ||
// Authorship
nameAttribute === 'author' ||
nameAttribute === 'generator' ||
nameAttribute === 'framework' ||
nameAttribute === 'publisher' ||
nameAttribute === 'progid' ||
/^article:/.test(propertyAttribute) ||
/^product:/.test(propertyAttribute) ||
// Verification
nameAttribute === 'google-site-verification' ||
nameAttribute === 'yandex-verification' ||
nameAttribute === 'csrf-token' ||
nameAttribute === 'p:domain_verify' ||
nameAttribute === 'verify-v1' ||
nameAttribute === 'verification' ||
nameAttribute === 'shopify-checkout-api-token'
)
}

function getLowerCaseAttribute(name: string) {
return (element.getAttribute(name) || '').toLowerCase()
}

return false
}

/**
* Text Nodes are dependant on Element nodes
* Privacy levels are set on elements so we check the parentElement of a text node
Expand Down
2 changes: 1 addition & 1 deletion performances/src/profilers/startCpuProfiling.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { CDPSession } from 'puppeteer'
import type { ProfilingOptions } from '../types'
import { isSdkBundleUrl } from './startProfiling'
import { isSdkBundleUrl } from '../utils'

export async function startCPUProfiling(options: ProfilingOptions, client: CDPSession) {
await client.send('Profiler.enable')
Expand Down
2 changes: 1 addition & 1 deletion performances/src/profilers/startMemoryProfiling.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { CDPSession } from 'puppeteer'
import { isSdkBundleUrl } from '../utils'
import type { ProfilingOptions } from '../types'
import { isSdkBundleUrl } from './startProfiling'

export async function startMemoryProfiling(options: ProfilingOptions, client: CDPSession) {
await client.send('HeapProfiler.enable')
Expand Down
2 changes: 1 addition & 1 deletion performances/src/profilers/startNetworkProfiling.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { CDPSession, Protocol } from 'puppeteer'
import type { ProfilingOptions } from '../types'
import { isSdkBundleUrl } from './startProfiling'
import { isSdkBundleUrl } from '../utils'

export async function startNetworkProfiling(options: ProfilingOptions, client: CDPSession) {
await client.send('Network.enable')
Expand Down
4 changes: 0 additions & 4 deletions performances/src/profilers/startProfiling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,3 @@ export async function startProfiling(options: ProfilingOptions, page: Page) {
}),
}
}

export function isSdkBundleUrl(options: ProfilingOptions, url: string) {
return url === options.bundleUrl
}
5 changes: 5 additions & 0 deletions performances/src/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import type { ProfilingOptions } from './types'

export function isSdkBundleUrl(options: ProfilingOptions, url: string) {
return url === options.bundleUrl
}
2 changes: 1 addition & 1 deletion test/e2e/lib/framework/createTest.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { LogsInitConfiguration } from '@datadog/browser-logs'
import type { RumInitConfiguration } from '@datadog/browser-rum-core'
import { deleteAllCookies, getBrowserName, withBrowserLogs } from '../helpers/browser'
import { flushEvents } from '../helpers/flushEvents'
import { validateRumFormat } from '../helpers/validation'
import { EventRegistry } from './eventsRegistry'
import { flushEvents } from './flushEvents'
import type { Servers } from './httpServers'
import { getTestServers, waitForServersIdle } from './httpServers'
import { log } from './logger'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getTestServers, waitForServersIdle } from '../framework'
import { browserExecuteAsync } from './browser'
import { browserExecuteAsync } from '../helpers/browser'
import { getTestServers, waitForServersIdle } from './httpServers'

export async function flushEvents() {
// wait to process actions + event loop before switching page
Expand Down
1 change: 1 addition & 0 deletions test/e2e/lib/framework/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export { createTest } from './createTest'
export { bundleSetup, html } from './pageSetups'
export { EventRegistry } from './eventsRegistry'
export { getTestServers, waitForServersIdle } from './httpServers'
export { flushEvents } from './flushEvents'
3 changes: 1 addition & 2 deletions test/e2e/scenario/eventBridge.scenario.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { browserExecute, flushBrowserLogs } from '../lib/helpers/browser'
import { createTest, html } from '../lib/framework'
import { flushEvents } from '../lib/helpers/flushEvents'
import { createTest, flushEvents, html } from '../lib/framework'

describe('bridge present', () => {
createTest('send action')
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/scenario/internalMonitoring.scenario.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { TelemetryErrorEvent } from '@datadog/browser-core/src/domain/internalMonitoring/telemetryEvent.types'
import { bundleSetup, createTest } from '../lib/framework'
import { bundleSetup, createTest, flushEvents } from '../lib/framework'
import { browserExecute } from '../lib/helpers/browser'
import { flushEvents } from '../lib/helpers/flushEvents'

describe('internal monitoring', () => {
createTest('send errors for logs')
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/scenario/logs.scenario.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { DEFAULT_REQUEST_ERROR_RESPONSE_LENGTH_LIMIT } from '@datadog/browser-logs/cjs/domain/configuration'
import { createTest } from '../lib/framework'
import { createTest, flushEvents } from '../lib/framework'
import { UNREACHABLE_URL } from '../lib/helpers/constants'
import { browserExecute, browserExecuteAsync, flushBrowserLogs, withBrowserLogs } from '../lib/helpers/browser'
import { flushEvents } from '../lib/helpers/flushEvents'

describe('logs', () => {
createTest('send logs')
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/scenario/recorder/recorder.scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ import {
} from '@datadog/browser-rum/test/utils'
import { renewSession } from '../../lib/helpers/session'
import type { EventRegistry } from '../../lib/framework'
import { createTest, bundleSetup, html } from '../../lib/framework'
import { flushEvents, createTest, bundleSetup, html } from '../../lib/framework'
import { browserExecute } from '../../lib/helpers/browser'
import { flushEvents } from '../../lib/helpers/flushEvents'

const INTEGER_RE = /^\d+$/
const TIMESTAMP_RE = /^\d{13}$/
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/scenario/recorder/viewports.scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ import type { RumInitConfiguration } from '@datadog/browser-rum-core'

import { findAllIncrementalSnapshots, findAllVisualViewports } from '@datadog/browser-rum/test/utils'
import type { EventRegistry } from '../../lib/framework'
import { createTest, bundleSetup, html } from '../../lib/framework'
import { flushEvents, createTest, bundleSetup, html } from '../../lib/framework'
import { browserExecute, getBrowserName, getPlatformName } from '../../lib/helpers/browser'
import { flushEvents } from '../../lib/helpers/flushEvents'

const NAVBAR_HEIGHT_CHANGE_UPPER_BOUND = 30
const VIEWPORT_META_TAGS = `
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/scenario/rum/actions.scenario.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { withBrowserLogs } from '../../lib/helpers/browser'
import { createTest, html, waitForServersIdle } from '../../lib/framework'
import { flushEvents } from '../../lib/helpers/flushEvents'
import { createTest, flushEvents, html, waitForServersIdle } from '../../lib/framework'

describe('action collection', () => {
createTest('track a click action')
Expand Down
Loading