Skip to content

Commit

Permalink
feat(core): Deprecate scope.getSpan() & scope.setSpan() (#10114)
Browse files Browse the repository at this point in the history
These APIs are not compatible with OTEL tracing, and thus will not be
public API anymore in v8.

In v8, we'll add new (?) methods to the scope that are "internal", as
for non-node based SDKs we'll still need to keep the active span on the
scope. But these should not be reflected publicly, and not in types'
Scope.

Note that we'll also need to make sure to use types' `Scope` for all
callbacks etc., which we currently don't do.
  • Loading branch information
mydea authored Jan 9, 2024
1 parent d41e39e commit f70128d
Show file tree
Hide file tree
Showing 47 changed files with 150 additions and 44 deletions.
5 changes: 5 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ 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 `scope.getSpan()` and `scope.setSpan()`

Instead, you can get the currently active span via `Sentry.getActiveSpan()`.
Setting a span on the scope happens automatically when you use the new performance APIs `startSpan()` and `startSpanManual()`.

## Deprecate `scope.setTransactionName()`

Instead, either set this as attributes or tags, or use an event processor to set `event.transaction`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { NextApiRequest, NextApiResponse } from 'next';
export default function handler(req: NextApiRequest, res: NextApiResponse) {
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test-transaction', op: 'e2e-test' });
// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentHub().getScope().setSpan(transaction);

// eslint-disable-next-line deprecation/deprecation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const server = new ApolloServer({
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test_transaction', op: 'transaction' });

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

// eslint-disable-next-line @typescript-eslint/no-floating-promises
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ async function run(): Promise<void> {
op: 'transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const transaction = Sentry.startTransaction({
name: 'Test Transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

connection.query('SELECT 1 + 1 AS solution', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const transaction = Sentry.startTransaction({
name: 'Test Transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

const query = connection.query('SELECT 1 + 1 AS solution');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const transaction = Sentry.startTransaction({
name: 'Test Transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

connection.query('SELECT 1 + 1 AS solution', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const transaction = Sentry.startTransaction({
name: 'Test Transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

const client = new pg.Client();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ async function run(): Promise<void> {
op: 'transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Sentry.init({
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test_transaction' });

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

http.get('http://match-this-url.com/api/v0');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const server = new ApolloServer({
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test_transaction', op: 'transaction' });

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

// eslint-disable-next-line @typescript-eslint/no-floating-promises
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ async function run(): Promise<void> {
op: 'transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const transaction = Sentry.startTransaction({
name: 'Test Transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

connection.query('SELECT 1 + 1 AS solution', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const transaction = Sentry.startTransaction({
name: 'Test Transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

const client = new pg.Client();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ async function run(): Promise<void> {
op: 'transaction',
});

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Sentry.init({
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test_transaction' });

// eslint-disable-next-line deprecation/deprecation
Sentry.getCurrentScope().setSpan(transaction);

http.get('http://match-this-url.com/api/v0');
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/server/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
import {
captureException,
continueTrace,
getActiveSpan,
getClient,
getCurrentScope,
runWithAsyncContext,
Expand Down Expand Up @@ -70,7 +71,7 @@ export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseH
// if there is an active span, we know that this handle call is nested and hence
// we don't create a new domain for it. If we created one, nested server calls would
// create new transactions instead of adding a child span to the currently active span.
if (getCurrentScope().getSpan()) {
if (getActiveSpan()) {
return instrumentRequest(ctx, next, handlerOptions);
}
return runWithAsyncContext(() => {
Expand Down
7 changes: 5 additions & 2 deletions packages/astro/test/server/middleware.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
import * as SentryNode from '@sentry/node';
import type { Client } from '@sentry/types';
import type { Client, Span } from '@sentry/types';
import { vi } from 'vitest';

import { handleRequest, interpolateRouteFromUrlAndParams } from '../../src/server/middleware';
Expand All @@ -15,7 +15,9 @@ vi.mock('../../src/server/meta', () => ({
describe('sentryMiddleware', () => {
const startSpanSpy = vi.spyOn(SentryNode, 'startSpan');

const getSpanMock = vi.fn(() => {});
const getSpanMock = vi.fn(() => {
return {} as Span | undefined;
});
const setUserMock = vi.fn();

beforeEach(() => {
Expand All @@ -26,6 +28,7 @@ describe('sentryMiddleware', () => {
getSpan: getSpanMock,
} as any;
});
vi.spyOn(SentryNode, 'getActiveSpan').mockImplementation(getSpanMock);
vi.spyOn(SentryNode, 'getClient').mockImplementation(() => ({}) as Client);
});

Expand Down
9 changes: 6 additions & 3 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,9 @@ export class Scope implements ScopeInterface {
}

/**
* @inheritDoc
* 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;
Expand All @@ -317,7 +319,8 @@ export class Scope implements ScopeInterface {
}

/**
* @inheritDoc
* Returns the `Span` if there is one.
* @deprecated Use `getActiveSpan()` instead.
*/
public getSpan(): Span | undefined {
return this._span;
Expand All @@ -330,7 +333,7 @@ export class Scope implements ScopeInterface {
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.getSpan();
const span = this._span;
return span && span.transaction;
}

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/server-runtime-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ export class ServerRuntimeClient<
return [undefined, undefined];
}

// eslint-disable-next-line deprecation/deprecation
const span = scope.getSpan();
if (span) {
const samplingContext = span.transaction ? span.transaction.getDynamicSamplingContext() : undefined;
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/tracing/hubextensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { Transaction } from './transaction';
/** Returns all trace headers that are currently on the top scope. */
function traceHeaders(this: Hub): { [key: string]: string } {
const scope = this.getScope();
// eslint-disable-next-line deprecation/deprecation
const span = scope.getSpan();

return span
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/tracing/idletransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export class IdleTransaction extends Transaction {
// We set the transaction here on the scope so error events pick up the trace
// context and attach it to the error.
DEBUG_BUILD && logger.log(`Setting idle transaction on scope. Span ID: ${this.spanContext().spanId}`);
// eslint-disable-next-line deprecation/deprecation
_idleHub.getScope().setSpan(this);
}

Expand Down Expand Up @@ -198,6 +199,7 @@ export class IdleTransaction extends Transaction {
const scope = this._idleHub.getScope();
// eslint-disable-next-line deprecation/deprecation
if (scope.getTransaction() === this) {
// eslint-disable-next-line deprecation/deprecation
scope.setSpan(undefined);
}
}
Expand Down
16 changes: 13 additions & 3 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,13 @@ export function trace<T>(
): T {
const hub = getCurrentHub();
const scope = getCurrentScope();
// eslint-disable-next-line deprecation/deprecation
const parentSpan = scope.getSpan();

const ctx = normalizeContext(context);
const activeSpan = createChildSpanOrTransaction(hub, parentSpan, ctx);

// eslint-disable-next-line deprecation/deprecation
scope.setSpan(activeSpan);

return handleCallbackErrors(
Expand All @@ -158,6 +160,7 @@ export function trace<T>(
},
() => {
activeSpan && activeSpan.end();
// eslint-disable-next-line deprecation/deprecation
scope.setSpan(parentSpan);
afterFinish();
},
Expand All @@ -180,10 +183,11 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span |

return withScope(context.scope, scope => {
const hub = getCurrentHub();
const scopeForSpan = context.scope || scope;
const parentSpan = scopeForSpan.getSpan();
// eslint-disable-next-line deprecation/deprecation
const parentSpan = scope.getSpan();

const activeSpan = createChildSpanOrTransaction(hub, parentSpan, ctx);
// eslint-disable-next-line deprecation/deprecation
scope.setSpan(activeSpan);

return handleCallbackErrors(
Expand Down Expand Up @@ -223,9 +227,11 @@ export function startSpanManual<T>(

return withScope(context.scope, scope => {
const hub = getCurrentHub();
// eslint-disable-next-line deprecation/deprecation
const parentSpan = scope.getSpan();

const activeSpan = createChildSpanOrTransaction(hub, parentSpan, ctx);
// eslint-disable-next-line deprecation/deprecation
scope.setSpan(activeSpan);

function finishAndSetSpan(): void {
Expand Down Expand Up @@ -261,7 +267,10 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined {

const ctx = normalizeContext(context);
const hub = getCurrentHub();
const parentSpan = context.scope ? context.scope.getSpan() : getActiveSpan();
const parentSpan = context.scope
? // eslint-disable-next-line deprecation/deprecation
context.scope.getSpan()
: getActiveSpan();
return parentSpan
? // eslint-disable-next-line deprecation/deprecation
parentSpan.startChild(ctx)
Expand All @@ -273,6 +282,7 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
* Returns the currently active span.
*/
export function getActiveSpan(): Span | undefined {
// eslint-disable-next-line deprecation/deprecation
return getCurrentScope().getSpan();
}

Expand Down
Loading

0 comments on commit f70128d

Please sign in to comment.