Skip to content

feat(core): Deprecate startTransaction() #10073

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 2 commits into from
Jan 5, 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
11 changes: 11 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ 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 `startTransaction()`

In v8, the old performance API `startTransaction()` (as well as `hub.startTransaction()`) will be removed.
Instead, use the new performance APIs:

* `startSpan()`
* `startSpanManual()`
* `startInactiveSpan()`

You can [read more about the new performance APIs here](./docs/v8-new-performance-apis.md).

## Deprecate `Sentry.lastEventId()` and `hub.lastEventId()`

`Sentry.lastEventId()` sometimes causes race conditons, so we are deprecating it in favour of the `beforeSend` callback.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as Sentry from '@sentry/nextjs';
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' });
Sentry.getCurrentHub().getScope().setSpan(transaction);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ app.get('/test-param/:param', function (req, res) {
});

app.get('/test-transaction', async function (req, res) {
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test-transaction', op: 'e2e-test' });
Sentry.getCurrentScope().setSpan(transaction);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Sentry.init({
tracesSampleRate: 1.0,
});

// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test_transaction_1' });

transaction.end();
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Sentry.init({
tracesSampleRate: 1.0,
});

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

// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test_transaction', op: 'transaction' });

Sentry.getCurrentScope().setSpan(transaction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const client = new MongoClient(process.env.MONGO_URL || '', {
});

async function run(): Promise<void> {
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({
name: 'Test Transaction',
op: 'transaction',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ connection.connect(function (err: unknown) {
}
});

// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({
op: 'transaction',
name: 'Test Transaction',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ connection.connect(function (err: unknown) {
}
});

// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({
op: 'transaction',
name: 'Test Transaction',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const connection = mysql.createConnection({
password: 'docker',
});

// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({
op: 'transaction',
name: 'Test Transaction',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Sentry.init({
integrations: [...Sentry.autoDiscoverNodePerformanceMonitoringIntegrations()],
});

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

async function run(): Promise<void> {
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({
name: 'Test Transaction',
op: 'transaction',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Sentry.init({
integrations: [new Sentry.Integrations.Http({ tracing: true })],
});

// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test_transaction' });

Sentry.getCurrentScope().setSpan(transaction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const server = new ApolloServer({
resolvers,
});

// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test_transaction', op: 'transaction' });

Sentry.getCurrentScope().setSpan(transaction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const client = new MongoClient(process.env.MONGO_URL || '', {
});

async function run(): Promise<void> {
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({
name: 'Test Transaction',
op: 'transaction',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ connection.connect(function (err: unknown) {
}
});

// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({
op: 'transaction',
name: 'Test Transaction',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Sentry.init({
tracesSampleRate: 1.0,
});

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

async function run(): Promise<void> {
// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({
name: 'Test Transaction',
op: 'transaction',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Sentry.init({
integrations: [new Sentry.Integrations.Http({ tracing: true })],
});

// eslint-disable-next-line deprecation/deprecation
const transaction = Sentry.startTransaction({ name: 'test_transaction' });

Sentry.getCurrentScope().setSpan(transaction);
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export {
Hub,
makeMain,
Scope,
// eslint-disable-next-line deprecation/deprecation
startTransaction,
SDK_VERSION,
setContext,
Expand Down
1 change: 1 addition & 0 deletions packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export {
lastEventId,
makeMain,
Scope,
// eslint-disable-next-line deprecation/deprecation
startTransaction,
getActiveSpan,
startSpan,
Expand Down
1 change: 1 addition & 0 deletions packages/bun/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export {
makeMain,
runWithAsyncContext,
Scope,
// eslint-disable-next-line deprecation/deprecation
startTransaction,
SDK_VERSION,
setContext,
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,14 @@ export function withScope<T>(callback: (scope: Scope) => T): T {
* default values). See {@link Options.tracesSampler}.
*
* @returns The transaction which was just started
*
* @deprecated Use `startSpan()`, `startSpanManual()` or `startInactiveSpan()` instead.
*/
export function startTransaction(
context: TransactionContext,
customSamplingContext?: CustomSamplingContext,
): ReturnType<Hub['startTransaction']> {
// eslint-disable-next-line deprecation/deprecation
return getCurrentHub().startTransaction({ ...context }, customSamplingContext);
}

Expand Down
18 changes: 17 additions & 1 deletion packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,23 @@ export class Hub implements HubInterface {
}

/**
* @inheritDoc
* Starts a new `Transaction` and returns it. This is the entry point to manual tracing instrumentation.
*
* A tree structure can be built by adding child spans to the transaction, and child spans to other spans. To start a
* new child span within the transaction or any span, call the respective `.startChild()` method.
*
* Every child span must be finished before the transaction is finished, otherwise the unfinished spans are discarded.
*
* The transaction must be finished with a call to its `.end()` method, at which point the transaction with all its
* finished child spans will be sent to Sentry.
*
* @param context Properties of the new `Transaction`.
* @param customSamplingContext Information given to the transaction sampling function (along with context-dependent
* default values). See {@link Options.tracesSampler}.
*
* @returns The transaction which was just started
*
* @deprecated Use `startSpan()`, `startSpanManual()` or `startInactiveSpan()` instead.
*/
public startTransaction(context: TransactionContext, customSamplingContext?: CustomSamplingContext): Transaction {
const result = this._callExtensionMethod<Transaction>('startTransaction', context, customSamplingContext);
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export {
flush,
// eslint-disable-next-line deprecation/deprecation
lastEventId,
// eslint-disable-next-line deprecation/deprecation
startTransaction,
setContext,
setExtra,
Expand Down
10 changes: 8 additions & 2 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,10 @@ export function startInactiveSpan(context: TransactionContext): Span | undefined

const hub = getCurrentHub();
const parentSpan = getActiveSpan();
return parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx);
return parentSpan
? parentSpan.startChild(ctx)
: // eslint-disable-next-line deprecation/deprecation
hub.startTransaction(ctx);
Comment on lines +159 to +162
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not relevant for this PR but this makes me realize we'll probably need to create an internal startTxn utility for us as a replacement of hub.startTransaction when its removed as long as we're still sending spans in transacitons.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jup, also had that thought - for now we'll still need some internal thing, but we can refactor this on the v8 branch then I'd say :)

}

/**
Expand Down Expand Up @@ -235,7 +238,10 @@ function createChildSpanOrTransaction(
if (!hasTracingEnabled()) {
return undefined;
}
return parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx);
return parentSpan
? parentSpan.startChild(ctx)
: // eslint-disable-next-line deprecation/deprecation
hub.startTransaction(ctx);
}

function normalizeContext(context: TransactionContext): TransactionContext {
Expand Down
33 changes: 13 additions & 20 deletions packages/core/test/lib/tracing/errors.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BrowserClient } from '@sentry/browser';
import { Hub, addTracingExtensions, makeMain } from '@sentry/core';
import { Hub, addTracingExtensions, makeMain, startInactiveSpan, startSpan } from '@sentry/core';
import type { HandlerDataError, HandlerDataUnhandledRejection } from '@sentry/types';

import { getDefaultBrowserClientOptions } from '../../../../tracing/test/testutils';
Expand Down Expand Up @@ -30,19 +30,14 @@ beforeAll(() => {
});

describe('registerErrorHandlers()', () => {
let hub: Hub;
beforeEach(() => {
mockAddGlobalErrorInstrumentationHandler.mockClear();
mockAddGlobalUnhandledRejectionInstrumentationHandler.mockClear();
const options = getDefaultBrowserClientOptions();
hub = new Hub(new BrowserClient(options));
const options = getDefaultBrowserClientOptions({ enableTracing: true });
const hub = new Hub(new BrowserClient(options));
makeMain(hub);
});

afterEach(() => {
hub.getScope().setSpan(undefined);
});

it('registers error instrumentation', () => {
registerErrorInstrumentation();
expect(mockAddGlobalErrorInstrumentationHandler).toHaveBeenCalledTimes(1);
Expand All @@ -53,7 +48,8 @@ describe('registerErrorHandlers()', () => {

it('does not set status if transaction is not on scope', () => {
registerErrorInstrumentation();
const transaction = hub.startTransaction({ name: 'test' });

const transaction = startInactiveSpan({ name: 'test' })!;
expect(transaction.status).toBe(undefined);

mockErrorCallback({} as HandlerDataError);
Expand All @@ -66,22 +62,19 @@ describe('registerErrorHandlers()', () => {

it('sets status for transaction on scope on error', () => {
registerErrorInstrumentation();
const transaction = hub.startTransaction({ name: 'test' });
hub.getScope().setSpan(transaction);

mockErrorCallback({} as HandlerDataError);
expect(transaction.status).toBe('internal_error');

transaction.end();
startSpan({ name: 'test' }, span => {
mockErrorCallback({} as HandlerDataError);
expect(span?.status).toBe('internal_error');
});
});

it('sets status for transaction on scope on unhandledrejection', () => {
registerErrorInstrumentation();
const transaction = hub.startTransaction({ name: 'test' });
hub.getScope().setSpan(transaction);

mockUnhandledRejectionCallback({});
expect(transaction.status).toBe('internal_error');
transaction.end();
startSpan({ name: 'test' }, span => {
mockUnhandledRejectionCallback({});
expect(span?.status).toBe('internal_error');
});
});
});
1 change: 1 addition & 0 deletions packages/deno/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export {
makeMain,
runWithAsyncContext,
Scope,
// eslint-disable-next-line deprecation/deprecation
startTransaction,
SDK_VERSION,
setContext,
Expand Down
1 change: 1 addition & 0 deletions packages/hub/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export const configureScope = configureScopeCore;
/**
* @deprecated This export has moved to @sentry/core. The @sentry/hub package will be removed in v8.
*/
// eslint-disable-next-line deprecation/deprecation
export const startTransaction = startTransactionCore;

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/nextjs/src/common/utils/wrapperUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
if (platformSupportsStreaming()) {
let spanToContinue: Span;
if (previousSpan === undefined) {
// TODO: Refactor this to use `startSpan()`
// eslint-disable-next-line deprecation/deprecation
const newTransaction = startTransaction(
{
op: 'http.server',
Expand Down Expand Up @@ -136,6 +138,8 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
status: 'ok',
});
} else {
// TODO: Refactor this to use `startSpan()`
// eslint-disable-next-line deprecation/deprecation
dataFetcherSpan = startTransaction({
op: 'function.nextjs',
name: `${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`,
Expand Down
10 changes: 7 additions & 3 deletions packages/nextjs/test/clientSdk.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { BaseClient, getCurrentHub } from '@sentry/core';
import * as SentryReact from '@sentry/react';
import { BrowserTracing, WINDOW } from '@sentry/react';
import { BrowserTracing, WINDOW, getCurrentScope } from '@sentry/react';
import type { Integration } from '@sentry/types';
import type { UserIntegrationsFunction } from '@sentry/utils';
import { logger } from '@sentry/utils';
Expand Down Expand Up @@ -89,8 +89,12 @@ describe('Client init()', () => {
const hub = getCurrentHub();
const transportSend = jest.spyOn(hub.getClient()!.getTransport()!, 'send');

const transaction = hub.startTransaction({ name: '/404' });
transaction.end();
// Ensure we have no current span, so our next span is a transaction
getCurrentScope().setSpan(undefined);

SentryReact.startSpan({ name: '/404' }, () => {
// noop
});

expect(transportSend).not.toHaveBeenCalled();
expect(captureEvent.mock.results[0].value).toBeUndefined();
Expand Down
5 changes: 3 additions & 2 deletions packages/nextjs/test/serverSdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ describe('Server init()', () => {
const hub = getCurrentHub();
const transportSend = jest.spyOn(hub.getClient()!.getTransport()!, 'send');

const transaction = hub.startTransaction({ name: '/404' });
transaction.end();
SentryNode.startSpan({ name: '/404' }, () => {
// noop
});

// We need to flush because the event processor pipeline is async whereas transaction.end() is sync.
await SentryNode.flush();
Expand Down
Loading