Skip to content

feat(tracing): Deprecate transaction-related options for BrowserTracing #10303

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 24, 2024
Merged

feat(tracing): Deprecate transaction-related options for BrowserTracing #10303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions MIGRATION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ npx @sentry/migr8@latest
This will let you select which updates to run, and automatically update your code. Make sure to still review all code
changes!

## Deprecate transaction-related options to `BrowserTracing`

When configuring the `BrowserTracing` integration, some options have been renamed:

- `startTransactionOnPageLoad` --> `spanOnPageLoad`
- `startTransactionOnLocationChange` --> `spanOnLocationChange`
- `markBackgroundTransactions` --> `markBackgroundSpan`

Also, `beforeNavigate` is replaced with a `beforeStartSpan` callback, which receives `StartSpanOptions`.

## Deprecate using `getClient()` to check if the SDK was initialized

In v8, `getClient()` will stop returning `undefined` if `Sentry.init()` was not called. For cases where this may be used
Expand Down
113 changes: 2 additions & 111 deletions packages/core/src/tracing/trace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,5 @@
import type {
Instrumenter,
Primitive,
Scope,
Span,
SpanTimeInput,
TransactionContext,
TransactionMetadata,
} from '@sentry/types';
import type { SpanAttributes } from '@sentry/types';
import type { SpanOrigin } from '@sentry/types';
import type { TransactionSource } from '@sentry/types';
import type { Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types';

import { dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
Expand All @@ -20,105 +10,6 @@ import { handleCallbackErrors } from '../utils/handleCallbackErrors';
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';

interface StartSpanOptions extends TransactionContext {
/** A manually specified start time for the created `Span` object. */
startTime?: SpanTimeInput;

/** If defined, start this span off this scope instead off the current scope. */
scope?: Scope;

/** The name of the span. */
name: string;

/** An op for the span. This is a categorization for spans. */
op?: string;

/** The origin of the span - if it comes from auto instrumenation or manual instrumentation. */
origin?: SpanOrigin;

/** Attributes for the span. */
attributes?: SpanAttributes;

// All remaining fields are deprecated

/**
* @deprecated Manually set the end timestamp instead.
*/
trimEnd?: boolean;

/**
* @deprecated This cannot be set manually anymore.
*/
parentSampled?: boolean;

/**
* @deprecated Use attributes or set data on scopes instead.
*/
metadata?: Partial<TransactionMetadata>;

/**
* The name thingy.
* @deprecated Use `name` instead.
*/
description?: string;

/**
* @deprecated Use `span.setStatus()` instead.
*/
status?: string;

/**
* @deprecated Use `scope` instead.
*/
parentSpanId?: string;

/**
* @deprecated You cannot manually set the span to sampled anymore.
*/
sampled?: boolean;

/**
* @deprecated You cannot manually set the spanId anymore.
*/
spanId?: string;

/**
* @deprecated You cannot manually set the traceId anymore.
*/
traceId?: string;

/**
* @deprecated Use an attribute instead.
*/
source?: TransactionSource;

/**
* @deprecated Use attributes or set tags on the scope instead.
*/
tags?: { [key: string]: Primitive };

/**
* @deprecated Use attributes instead.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
data?: { [key: string]: any };

/**
* @deprecated Use `startTime` instead.
*/
startTimestamp?: number;

/**
* @deprecated Use `span.end()` instead.
*/
endTimestamp?: number;

/**
* @deprecated You cannot set the instrumenter manually anymore.
*/
instrumenter?: Instrumenter;
}

/**
* Wraps a function with a transaction/span and finishes the span after the function is done.
*
Expand Down
120 changes: 90 additions & 30 deletions packages/tracing-internal/src/browser/browsertracing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable max-lines */
/* eslint-disable max-lines, complexity */
import type { Hub, IdleTransaction } from '@sentry/core';
import {
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
Expand All @@ -9,7 +9,14 @@ import {
spanToJSON,
startIdleTransaction,
} from '@sentry/core';
import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types';
import type {
EventProcessor,
Integration,
StartSpanOptions,
Transaction,
TransactionContext,
TransactionSource,
} from '@sentry/types';
import { getDomElement, logger, tracingContextFromHeaders } from '@sentry/utils';

import { DEBUG_BUILD } from '../common/debug-build';
Expand Down Expand Up @@ -60,27 +67,48 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
heartbeatInterval: number;

/**
* Flag to enable/disable creation of `navigation` transaction on history changes.
* If a span should be created on location (history) changes.
* Default: true
*/
spanOnLocationChange: boolean;

/**
* If a span should be created on pageload.
* Default: true
*/
spanOnPageLoad: boolean;

/**
* Flag spans where tabs moved to background with "cancelled". Browser background tab timing is
* not suited towards doing precise measurements of operations. By default, we recommend that this option
* be enabled as background transactions can mess up your statistics in nondeterministic ways.
*
* Default: true
*/
startTransactionOnLocationChange: boolean;
markBackgroundSpan: boolean;

/**
* Flag to enable/disable creation of `navigation` transaction on history changes.
* Default: true
* @deprecated Configure `spanOnLocationChange` instead.
*/
startTransactionOnLocationChange?: boolean;

/**
* Flag to enable/disable creation of `pageload` transaction on first pageload.
*
* Default: true
* @deprecated Configure `spanOnPageLoad` instead.
*/
startTransactionOnPageLoad: boolean;
startTransactionOnPageLoad?: boolean;

/**
* Flag Transactions where tabs moved to background with "cancelled". Browser background tab timing is
* not suited towards doing precise measurements of operations. By default, we recommend that this option
* be enabled as background transactions can mess up your statistics in nondeterministic ways.
*
* Default: true
* @deprecated Configure `markBackgroundSpan` instead.
*/
markBackgroundTransactions: boolean;
markBackgroundTransactions?: boolean;

/**
* If true, Sentry will capture long tasks and add them to the corresponding transaction.
Expand Down Expand Up @@ -117,6 +145,12 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
onStartRouteTransaction: (t: Transaction | undefined, ctx: TransactionContext, getCurrentHub: () => Hub) => void;
}>;

/**
* A callback which is called before a span for a pageload or navigation is started.
* It receives the options passed to `startSpan`, and expects to return an updated options object.
*/
beforeStartSpan?: (options: StartSpanOptions) => StartSpanOptions;

/**
* beforeNavigate is called before a pageload/navigation transaction is created and allows users to modify transaction
* context data, or drop the transaction entirely (by setting `sampled = false` in the context).
Expand All @@ -126,6 +160,8 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
* @param context: The context data which will be passed to `startTransaction` by default
*
* @returns A (potentially) modified context object, with `sampled = false` if the transaction should be dropped.
*
* @deprecated Use `beforeStartSpan` instead.
*/
beforeNavigate?(this: void, context: TransactionContext): TransactionContext | undefined;

Expand All @@ -143,10 +179,10 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {

const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
...TRACING_DEFAULTS,
markBackgroundTransactions: true,
routingInstrumentation: instrumentRoutingWithDefaults,
startTransactionOnLocationChange: true,
startTransactionOnPageLoad: true,
spanOnLocationChange: true,
spanOnPageLoad: true,
markBackgroundSpan: true,
enableLongTask: true,
_experiments: {},
...defaultRequestInstrumentationOptions,
Expand Down Expand Up @@ -182,20 +218,42 @@ export class BrowserTracing implements Integration {

private _hasSetTracePropagationTargets: boolean;

public constructor(_options?: Partial<BrowserTracingOptions>) {
public constructor(_options: Partial<BrowserTracingOptions> = {}) {
this.name = BROWSER_TRACING_INTEGRATION_ID;
this._hasSetTracePropagationTargets = false;

addTracingExtensions();

if (DEBUG_BUILD) {
this._hasSetTracePropagationTargets = !!(
_options &&
// eslint-disable-next-line deprecation/deprecation
(_options.tracePropagationTargets || _options.tracingOrigins)
);
}

// Migrate legacy options
// TODO v8: Remove this
/* eslint-disable deprecation/deprecation */
if (typeof _options.startTransactionOnPageLoad === 'boolean') {
_options.spanOnPageLoad = _options.startTransactionOnPageLoad;
}
if (typeof _options.startTransactionOnLocationChange === 'boolean') {
_options.spanOnLocationChange = _options.startTransactionOnLocationChange;
}
if (typeof _options.markBackgroundTransactions === 'boolean') {
_options.markBackgroundSpan = _options.markBackgroundTransactions;
}
/* eslint-enable deprecation/deprecation */

// TODO (v8): remove this block after tracingOrigins is removed
// Set tracePropagationTargets to tracingOrigins if specified by the user
// In case both are specified, tracePropagationTargets takes precedence
// eslint-disable-next-line deprecation/deprecation
if (!_options.tracePropagationTargets && _options.tracingOrigins) {
// eslint-disable-next-line deprecation/deprecation
_options.tracePropagationTargets = _options.tracingOrigins;
}

this.options = {
...DEFAULT_BROWSER_TRACING_OPTIONS,
..._options,
Expand All @@ -207,15 +265,6 @@ export class BrowserTracing implements Integration {
this.options.enableLongTask = this.options._experiments.enableLongTask;
}

// TODO (v8): remove this block after tracingOrigins is removed
// Set tracePropagationTargets to tracingOrigins if specified by the user
// In case both are specified, tracePropagationTargets takes precedence
// eslint-disable-next-line deprecation/deprecation
if (_options && !_options.tracePropagationTargets && _options.tracingOrigins) {
// eslint-disable-next-line deprecation/deprecation
this.options.tracePropagationTargets = _options.tracingOrigins;
}

this._collectWebVitals = startTrackingWebVitals();
if (this.options.enableLongTask) {
startTrackingLongTasks();
Expand All @@ -237,9 +286,9 @@ export class BrowserTracing implements Integration {

const {
routingInstrumentation: instrumentRouting,
startTransactionOnLocationChange,
startTransactionOnPageLoad,
markBackgroundTransactions,
spanOnPageLoad,
spanOnLocationChange,
markBackgroundSpan,
traceFetch,
traceXHR,
shouldCreateSpanForRequest,
Expand Down Expand Up @@ -275,11 +324,11 @@ export class BrowserTracing implements Integration {

return transaction;
},
startTransactionOnPageLoad,
startTransactionOnLocationChange,
spanOnPageLoad,
spanOnLocationChange,
);

if (markBackgroundTransactions) {
if (markBackgroundSpan) {
registerBackgroundTabDetection();
}

Expand All @@ -306,7 +355,14 @@ export class BrowserTracing implements Integration {

const hub = this._getCurrentHub();

const { beforeNavigate, idleTimeout, finalTimeout, heartbeatInterval } = this.options;
const {
// eslint-disable-next-line deprecation/deprecation
beforeNavigate,
beforeStartSpan,
idleTimeout,
finalTimeout,
heartbeatInterval,
} = this.options;

const isPageloadTransaction = context.op === 'pageload';

Expand All @@ -328,7 +384,11 @@ export class BrowserTracing implements Integration {
trimEnd: true,
};

const modifiedContext = typeof beforeNavigate === 'function' ? beforeNavigate(expandedContext) : expandedContext;
const contextAfterProcessing = beforeStartSpan ? beforeStartSpan(expandedContext) : expandedContext;

const modifiedContext =
// eslint-disable-next-line deprecation/deprecation
typeof beforeNavigate === 'function' ? beforeNavigate(contextAfterProcessing) : contextAfterProcessing;

// For backwards compatibility reasons, beforeNavigate can return undefined to "drop" the transaction (prevent it
// from being sent to Sentry).
Expand Down
Loading