Skip to content

ref(core): Remove scope.setSpan() and scope.getSpan() methods #11051

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

Merged
merged 6 commits into from
Mar 27, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ window.Sentry = Sentry;

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
integrations: [Sentry.browserTracingIntegration({ idleTimeout: 9000, instrumentPageLoad: false })],
integrations: [
Sentry.browserTracingIntegration({
idleTimeout: 9000,
instrumentPageLoad: false,
instrumentNavigation: false,
}),
],
tracesSampleRate: 1,
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ document.getElementById('go-background').addEventListener('click', () => {
});

document.getElementById('start-span').addEventListener('click', () => {
const span = Sentry.startInactiveSpan({ name: 'test-span' });
const span = Sentry.startBrowserTracingNavigationSpan(Sentry.getClient(), { name: 'test-span' });
window.span = span;
Sentry.getCurrentScope().setSpan(span);
});

window.getSpanJson = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ sentryTest('should finish a custom transaction when the page goes background', a
}

const url = await getLocalTestPath({ testDir: __dirname });
page.goto(url);
await page.goto(url);

await page.locator('#start-span').click();
const spanJsonBefore: SpanJSON = await page.evaluate('window.getSpanJson()');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use client';

import { getCurrentScope, startInactiveSpan } from '@sentry/nextjs';
import { startInactiveSpan } from '@sentry/nextjs';
import { Span } from '@sentry/types';
import { PropsWithChildren, createContext, useState } from 'react';

Expand Down Expand Up @@ -29,7 +29,6 @@ export function SpanContextProvider({ children }: PropsWithChildren) {
spanActive: false,
start: (spanName: string) => {
const span = startInactiveSpan({ name: spanName });
getCurrentScope().setSpan(span);
setSpan(span);
},
}
Expand Down
5 changes: 4 additions & 1 deletion dev-packages/rollup-utils/plugins/bundlePlugins.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,12 @@ export function makeTerserPlugin() {
// For v7 backwards-compatibility we need to access txn._frozenDynamicSamplingContext
// TODO (v8): Remove this reserved word
'_frozenDynamicSamplingContext',
// These are used to keep span relationships
// These are used to keep span & scope relationships
'_sentryRootSpan',
'_sentryChildSpans',
'_sentrySpan',
'_sentryScope',
'_sentryIsolationScope',
],
},
},
Expand Down
15 changes: 13 additions & 2 deletions packages/browser/src/index.bundle.tracing.replay.feedback.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { feedbackIntegration } from '@sentry-internal/feedback';
import { replayIntegration } from '@sentry-internal/replay';
import { browserTracingIntegration } from '@sentry-internal/tracing';
import {
browserTracingIntegration,
startBrowserTracingNavigationSpan,
startBrowserTracingPageLoadSpan,
} from '@sentry-internal/tracing';
import { addTracingExtensions } from '@sentry/core';

// We are patching the global object with our hub extension methods
Expand All @@ -16,6 +20,13 @@ export {
getSpanDescendants,
} from '@sentry/core';

export { feedbackIntegration, replayIntegration, browserTracingIntegration, addTracingExtensions };
export {
feedbackIntegration,
replayIntegration,
browserTracingIntegration,
addTracingExtensions,
startBrowserTracingNavigationSpan,
startBrowserTracingPageLoadSpan,
};

export * from './index.bundle.base';
8 changes: 7 additions & 1 deletion packages/browser/src/index.bundle.tracing.replay.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { feedbackIntegrationShim } from '@sentry-internal/integration-shims';
import { replayIntegration } from '@sentry-internal/replay';
import { browserTracingIntegration } from '@sentry-internal/tracing';
import {
browserTracingIntegration,
startBrowserTracingNavigationSpan,
startBrowserTracingPageLoadSpan,
} from '@sentry-internal/tracing';
import { addTracingExtensions } from '@sentry/core';

// We are patching the global object with our hub extension methods
Expand All @@ -21,6 +25,8 @@ export {
feedbackIntegrationShim as feedbackIntegration,
browserTracingIntegration,
addTracingExtensions,
startBrowserTracingNavigationSpan,
startBrowserTracingPageLoadSpan,
};

export * from './index.bundle.base';
8 changes: 7 additions & 1 deletion packages/browser/src/index.bundle.tracing.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// This is exported so the loader does not fail when switching off Replay
import { feedbackIntegrationShim, replayIntegrationShim } from '@sentry-internal/integration-shims';
import { browserTracingIntegration } from '@sentry-internal/tracing';
import {
browserTracingIntegration,
startBrowserTracingNavigationSpan,
startBrowserTracingPageLoadSpan,
} from '@sentry-internal/tracing';
import { addTracingExtensions } from '@sentry/core';

// We are patching the global object with our hub extension methods
Expand All @@ -21,6 +25,8 @@ export {
replayIntegrationShim as replayIntegration,
browserTracingIntegration,
addTracingExtensions,
startBrowserTracingPageLoadSpan,
startBrowserTracingNavigationSpan,
};

export * from './index.bundle.base';
5 changes: 1 addition & 4 deletions packages/core/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,8 @@ export function addTracingHeadersToFetchRequest(
}
| PolymorphicRequestHeaders;
},
requestSpan?: Span,
span?: Span,
): PolymorphicRequestHeaders | undefined {
// eslint-disable-next-line deprecation/deprecation
const span = requestSpan || scope.getSpan();

const isolationScope = getIsolationScope();

const { traceId, spanId, sampled, dsc } = {
Expand Down
37 changes: 8 additions & 29 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ import type {
ScopeData,
Session,
SeverityLevel,
Span,
Transaction,
User,
} from '@sentry/types';
import { dateTimestampInSeconds, isPlainObject, logger, uuid4 } from '@sentry/utils';

import { updateSession } from './session';
import type { SentrySpan } from './tracing/sentrySpan';
import { _getSpanForScope, _setSpanForScope } from './utils/spanOnScope';

/**
* Default value for maximum number of breadcrumbs added to an event.
Expand Down Expand Up @@ -87,9 +87,6 @@ export class Scope implements ScopeInterface {
*/
protected _transactionName?: string;

/** Span */
protected _span?: Span;

/** Session */
protected _session?: Session;

Expand Down Expand Up @@ -134,7 +131,6 @@ export class Scope implements ScopeInterface {
newScope._contexts = { ...this._contexts };
newScope._user = this._user;
newScope._level = this._level;
newScope._span = this._span;
newScope._session = this._session;
newScope._transactionName = this._transactionName;
newScope._fingerprint = this._fingerprint;
Expand All @@ -145,6 +141,8 @@ export class Scope implements ScopeInterface {
newScope._propagationContext = { ...this._propagationContext };
newScope._client = this._client;

_setSpanForScope(newScope, _getSpanForScope(this));

return newScope;
}

Expand Down Expand Up @@ -304,33 +302,14 @@ export class Scope implements ScopeInterface {
return this;
}

/**
* Sets the Span on the scope.
* @param span Span
* @deprecated Instead of setting a span on a scope, use `startSpan()`/`startSpanManual()` instead.
*/
public setSpan(span?: Span): this {
this._span = span;
this._notifyScopeListeners();
return this;
}

/**
* Returns the `Span` if there is one.
* @deprecated Use `getActiveSpan()` instead.
*/
public getSpan(): Span | undefined {
return this._span;
}

/**
* Returns the `Transaction` attached to the scope (if there is one).
* @deprecated You should not rely on the transaction, but just use `startSpan()` APIs instead.
*/
public getTransaction(): Transaction | undefined {
// Often, this span (if it exists at all) will be a transaction, but it's not guaranteed to be. Regardless, it will
// have a pointer to the currently-active transaction.
const span = this._span;
const span = _getSpanForScope(this);

// Cannot replace with getRootSpan because getRootSpan returns a span, not a transaction
// Also, this method will be removed anyway.
Expand Down Expand Up @@ -432,11 +411,12 @@ export class Scope implements ScopeInterface {
this._transactionName = undefined;
this._fingerprint = undefined;
this._requestSession = undefined;
this._span = undefined;
this._session = undefined;
this._notifyScopeListeners();
_setSpanForScope(this, undefined);
this._attachments = [];
this._propagationContext = generatePropagationContext();

this._notifyScopeListeners();
return this;
}

Expand Down Expand Up @@ -522,7 +502,6 @@ export class Scope implements ScopeInterface {
_propagationContext,
_sdkProcessingMetadata,
_transactionName,
_span,
} = this;

return {
Expand All @@ -538,7 +517,7 @@ export class Scope implements ScopeInterface {
propagationContext: _propagationContext,
sdkProcessingMetadata: _sdkProcessingMetadata,
transactionName: _transactionName,
span: _span,
span: _getSpanForScope(this),
};
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/server-runtime-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
getDynamicSamplingContextFromClient,
getDynamicSamplingContextFromSpan,
} from './tracing';
import { _getSpanForScope } from './utils/spanOnScope';
import { getRootSpan, spanToTraceContext } from './utils/spanUtils';

export interface ServerRuntimeClientOptions extends ClientOptions<BaseTransportOptions> {
Expand Down Expand Up @@ -252,8 +253,7 @@ export class ServerRuntimeClient<
return [undefined, undefined];
}

// eslint-disable-next-line deprecation/deprecation
const span = scope.getSpan();
const span = _getSpanForScope(scope);
if (span) {
const rootSpan = getRootSpan(span);
const samplingContext = getDynamicSamplingContextFromSpan(rootSpan);
Expand Down
7 changes: 3 additions & 4 deletions packages/core/src/tracing/idleSpan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { getClient, getCurrentScope } from '../currentScopes';
import { DEBUG_BUILD } from '../debug-build';
import { SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON } from '../semanticAttributes';
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
import { _setSpanForScope } from '../utils/spanOnScope';
import {
getActiveSpan,
getSpanDescendants,
Expand Down Expand Up @@ -232,8 +233,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
beforeSpanEnd(span);
}

// eslint-disable-next-line deprecation/deprecation
scope.setSpan(previousActiveSpan);
_setSpanForScope(scope, previousActiveSpan);

const spanJSON = spanToJSON(span);

Expand Down Expand Up @@ -347,8 +347,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
function _startIdleSpan(options: StartSpanOptions): Span {
const span = startInactiveSpan(options);

// eslint-disable-next-line deprecation/deprecation
getCurrentScope().setSpan(span);
_setSpanForScope(getCurrentScope(), span);

DEBUG_BUILD && logger.log(`Setting idle span on scope. Span ID: ${span.spanContext().spanId}`);

Expand Down
Loading