Skip to content

Commit

Permalink
feat(core): Update span performance API names (#8971)
Browse files Browse the repository at this point in the history
As per the new changes in RFC 101 in
getsentry/rfcs#113, update the span performance
API names.

- `startActiveSpan` -> `startSpan`
- `startSpan` -> `startInactiveSpan`

https://github.com/getsentry/rfcs/blob/main/text/0101-revamping-the-sdk-performance-api.md

`startActiveSpan` is deprecated, while `startInactiveSpan` is being
introduced. The breaking change is that `startSpan` is being changed,
but considering that basically no-one is using the `startSpan` API, we
should be fine to break here for correctness reasons. Better break now
than to have everyone refactor their code in v8.
  • Loading branch information
AbhiPrasad committed Sep 7, 2023
1 parent eafe791 commit cf75ffe
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 40 deletions.
3 changes: 3 additions & 0 deletions packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ export {
makeMain,
Scope,
startTransaction,
getActiveSpan,
startSpan,
startInactiveSpan,
SDK_VERSION,
setContext,
setExtra,
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/tracing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export { extractTraceparentData, getActiveTransaction } from './utils';
// eslint-disable-next-line deprecation/deprecation
export { SpanStatus } from './spanstatus';
export type { SpanStatusType } from './span';
export { trace, getActiveSpan, startActiveSpan, startSpan } from './trace';
// eslint-disable-next-line deprecation/deprecation
export { trace, getActiveSpan, startSpan, startInactiveSpan, startActiveSpan } from './trace';
export { getDynamicSamplingContextFromClient } from './dynamicSamplingContext';
export { setMeasurement } from './measurement';
21 changes: 13 additions & 8 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ export function trace<T>(

const parentSpan = scope.getSpan();

function startActiveSpan(): Span | undefined {
function createChildSpanOrTransaction(): Span | undefined {
if (!hasTracingEnabled()) {
return undefined;
}
return parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx);
}

const activeSpan = startActiveSpan();
const activeSpan = createChildSpanOrTransaction();
scope.setSpan(activeSpan);

function finishAndSetSpan(): void {
Expand Down Expand Up @@ -82,13 +82,13 @@ export function trace<T>(
* The created span is the active span and will be used as parent by other spans created inside the function
* and can be accessed via `Sentry.getSpan()`, as long as the function is executed while the scope is active.
*
* If you want to create a span that is not set as active, use {@link startSpan}.
* If you want to create a span that is not set as active, use {@link startInactiveSpan}.
*
* Note that if you have not enabled tracing extensions via `addTracingExtensions`
* or you didn't set `tracesSampleRate`, this function will not generate spans
* and the `span` returned from the callback will be undefined.
*/
export function startActiveSpan<T>(context: TransactionContext, callback: (span: Span | undefined) => T): T {
export function startSpan<T>(context: TransactionContext, callback: (span: Span | undefined) => T): T {
const ctx = { ...context };
// If a name is set and a description is not, set the description to the name.
if (ctx.name !== undefined && ctx.description === undefined) {
Expand All @@ -100,14 +100,14 @@ export function startActiveSpan<T>(context: TransactionContext, callback: (span:

const parentSpan = scope.getSpan();

function startActiveSpan(): Span | undefined {
function createChildSpanOrTransaction(): Span | undefined {
if (!hasTracingEnabled()) {
return undefined;
}
return parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx);
}

const activeSpan = startActiveSpan();
const activeSpan = createChildSpanOrTransaction();
scope.setSpan(activeSpan);

function finishAndSetSpan(): void {
Expand Down Expand Up @@ -141,17 +141,22 @@ export function startActiveSpan<T>(context: TransactionContext, callback: (span:
return maybePromiseResult;
}

/**
* @deprecated Use {@link startSpan} instead.
*/
export const startActiveSpan = startSpan;

/**
* Creates a span. This span is not set as active, so will not get automatic instrumentation spans
* as children or be able to be accessed via `Sentry.getSpan()`.
*
* If you want to create a span that is set as active, use {@link startActiveSpan}.
* If you want to create a span that is set as active, use {@link startSpan}.
*
* Note that if you have not enabled tracing extensions via `addTracingExtensions`
* or you didn't set `tracesSampleRate` or `tracesSampler`, this function will not generate spans
* and the `span` returned from the callback will be undefined.
*/
export function startSpan(context: TransactionContext): Span | undefined {
export function startInactiveSpan(context: TransactionContext): Span | undefined {
if (!hasTracingEnabled()) {
return undefined;
}
Expand Down
22 changes: 11 additions & 11 deletions packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { addTracingExtensions, Hub, makeMain } from '../../../src';
import { startActiveSpan } from '../../../src/tracing';
import { startSpan } from '../../../src/tracing';
import { getDefaultTestClientOptions, TestClient } from '../../mocks/client';

beforeAll(() => {
Expand All @@ -14,7 +14,7 @@ const enum Type {
let hub: Hub;
let client: TestClient;

describe('startActiveSpan', () => {
describe('startSpan', () => {
beforeEach(() => {
const options = getDefaultTestClientOptions({ tracesSampleRate: 0.0 });
client = new TestClient(options);
Expand All @@ -38,7 +38,7 @@ describe('startActiveSpan', () => {
])('with %s callback and error %s', (_type, isError, callback, expected) => {
it('should return the same value as the callback', async () => {
try {
const result = await startActiveSpan({ name: 'GET users/[id]' }, () => {
const result = await startSpan({ name: 'GET users/[id]' }, () => {
return callback();
});
expect(result).toEqual(expected);
Expand All @@ -53,7 +53,7 @@ describe('startActiveSpan', () => {
// if tracingExtensions are not enabled
jest.spyOn(hub, 'startTransaction').mockReturnValue(undefined);
try {
const result = await startActiveSpan({ name: 'GET users/[id]' }, () => {
const result = await startSpan({ name: 'GET users/[id]' }, () => {
return callback();
});
expect(result).toEqual(expected);
Expand All @@ -68,7 +68,7 @@ describe('startActiveSpan', () => {
ref = transaction;
});
try {
await startActiveSpan({ name: 'GET users/[id]' }, () => {
await startSpan({ name: 'GET users/[id]' }, () => {
return callback();
});
} catch (e) {
Expand All @@ -86,7 +86,7 @@ describe('startActiveSpan', () => {
ref = transaction;
});
try {
await startActiveSpan(
await startSpan(
{
name: 'GET users/[id]',
parentSampled: true,
Expand All @@ -113,7 +113,7 @@ describe('startActiveSpan', () => {
ref = transaction;
});
try {
await startActiveSpan({ name: 'GET users/[id]' }, span => {
await startSpan({ name: 'GET users/[id]' }, span => {
if (span) {
span.op = 'http.server';
}
Expand All @@ -132,8 +132,8 @@ describe('startActiveSpan', () => {
ref = transaction;
});
try {
await startActiveSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
return startActiveSpan({ name: 'SELECT * from users' }, () => {
await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
return startSpan({ name: 'SELECT * from users' }, () => {
return callback();
});
});
Expand All @@ -153,8 +153,8 @@ describe('startActiveSpan', () => {
ref = transaction;
});
try {
await startActiveSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
return startActiveSpan({ name: 'SELECT * from users' }, childSpan => {
await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
return startSpan({ name: 'SELECT * from users' }, childSpan => {
if (childSpan) {
childSpan.op = 'db.query';
}
Expand Down
13 changes: 9 additions & 4 deletions packages/node-experimental/src/sdk/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import type { NodeExperimentalClient } from './client';
* The created span is the active span and will be used as parent by other spans created inside the function
* and can be accessed via `Sentry.getSpan()`, as long as the function is executed while the scope is active.
*
* If you want to create a span that is not set as active, use {@link startSpan}.
* If you want to create a span that is not set as active, use {@link startInactiveSpan}.
*
* Note that if you have not enabled tracing extensions via `addTracingExtensions`
* or you didn't set `tracesSampleRate`, this function will not generate spans
* and the `span` returned from the callback will be undefined.
*/
export function startActiveSpan<T>(context: TransactionContext, callback: (span: Span | undefined) => T): T {
export function startSpan<T>(context: TransactionContext, callback: (span: Span | undefined) => T): T {
const tracer = getTracer();
if (!tracer) {
return callback(undefined);
Expand Down Expand Up @@ -66,17 +66,22 @@ export function startActiveSpan<T>(context: TransactionContext, callback: (span:
});
}

/**
* @deprecated Use {@link startSpan} instead.
*/
export const startActiveSpan = startSpan;

/**
* Creates a span. This span is not set as active, so will not get automatic instrumentation spans
* as children or be able to be accessed via `Sentry.getSpan()`.
*
* If you want to create a span that is set as active, use {@link startActiveSpan}.
* If you want to create a span that is set as active, use {@link startSpan}.
*
* Note that if you have not enabled tracing extensions via `addTracingExtensions`
* or you didn't set `tracesSampleRate` or `tracesSampler`, this function will not generate spans
* and the `span` returned from the callback will be undefined.
*/
export function startSpan(context: TransactionContext): Span | undefined {
export function startInactiveSpan(context: TransactionContext): Span | undefined {
const tracer = getTracer();
if (!tracer) {
return undefined;
Expand Down
26 changes: 13 additions & 13 deletions packages/node-experimental/test/sdk/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,21 @@ describe('trace', () => {
mockSdkInit({ enableTracing: true });
});

describe('startActiveSpan', () => {
describe('startSpan', () => {
it('works with a sync callback', () => {
const spans: Span[] = [];

expect(Sentry.getActiveSpan()).toEqual(undefined);

Sentry.startActiveSpan({ name: 'outer' }, outerSpan => {
Sentry.startSpan({ name: 'outer' }, outerSpan => {
expect(outerSpan).toBeDefined();
spans.push(outerSpan!);

expect(outerSpan?.name).toEqual('outer');
expect(outerSpan).toBeInstanceOf(Transaction);
expect(Sentry.getActiveSpan()).toEqual(outerSpan);

Sentry.startActiveSpan({ name: 'inner' }, innerSpan => {
Sentry.startSpan({ name: 'inner' }, innerSpan => {
expect(innerSpan).toBeDefined();
spans.push(innerSpan!);

Expand All @@ -49,7 +49,7 @@ describe('trace', () => {

expect(Sentry.getActiveSpan()).toEqual(undefined);

await Sentry.startActiveSpan({ name: 'outer' }, async outerSpan => {
await Sentry.startSpan({ name: 'outer' }, async outerSpan => {
expect(outerSpan).toBeDefined();
spans.push(outerSpan!);

Expand All @@ -59,7 +59,7 @@ describe('trace', () => {
expect(outerSpan).toBeInstanceOf(Transaction);
expect(Sentry.getActiveSpan()).toEqual(outerSpan);

await Sentry.startActiveSpan({ name: 'inner' }, async innerSpan => {
await Sentry.startSpan({ name: 'inner' }, async innerSpan => {
expect(innerSpan).toBeDefined();
spans.push(innerSpan!);

Expand Down Expand Up @@ -89,15 +89,15 @@ describe('trace', () => {

expect(Sentry.getActiveSpan()).toEqual(undefined);

Sentry.startActiveSpan({ name: 'outer' }, outerSpan => {
Sentry.startSpan({ name: 'outer' }, outerSpan => {
expect(outerSpan).toBeDefined();
spans1.push(outerSpan!);

expect(outerSpan?.name).toEqual('outer');
expect(outerSpan).toBeInstanceOf(Transaction);
expect(Sentry.getActiveSpan()).toEqual(outerSpan);

Sentry.startActiveSpan({ name: 'inner' }, innerSpan => {
Sentry.startSpan({ name: 'inner' }, innerSpan => {
expect(innerSpan).toBeDefined();
spans1.push(innerSpan!);

Expand All @@ -108,15 +108,15 @@ describe('trace', () => {
});
});

Sentry.startActiveSpan({ name: 'outer2' }, outerSpan => {
Sentry.startSpan({ name: 'outer2' }, outerSpan => {
expect(outerSpan).toBeDefined();
spans2.push(outerSpan!);

expect(outerSpan?.name).toEqual('outer2');
expect(outerSpan).toBeInstanceOf(Transaction);
expect(Sentry.getActiveSpan()).toEqual(outerSpan);

Sentry.startActiveSpan({ name: 'inner2' }, innerSpan => {
Sentry.startSpan({ name: 'inner2' }, innerSpan => {
expect(innerSpan).toBeDefined();
spans2.push(innerSpan!);

Expand All @@ -133,9 +133,9 @@ describe('trace', () => {
});
});

describe('startSpan', () => {
describe('startInactiveSpan', () => {
it('works at the root', () => {
const span = Sentry.startSpan({ name: 'test' });
const span = Sentry.startInactiveSpan({ name: 'test' });

expect(span).toBeDefined();
expect(span).toBeInstanceOf(Transaction);
Expand All @@ -150,11 +150,11 @@ describe('trace', () => {
});

it('works as a child span', () => {
Sentry.startActiveSpan({ name: 'outer' }, outerSpan => {
Sentry.startSpan({ name: 'outer' }, outerSpan => {
expect(outerSpan).toBeDefined();
expect(Sentry.getActiveSpan()).toEqual(outerSpan);

const innerSpan = Sentry.startSpan({ name: 'test' });
const innerSpan = Sentry.startInactiveSpan({ name: 'test' });

expect(innerSpan).toBeDefined();
expect(innerSpan).toBeInstanceOf(Span);
Expand Down
4 changes: 3 additions & 1 deletion packages/node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ export {
captureCheckIn,
setMeasurement,
getActiveSpan,
startActiveSpan,
startSpan,
// eslint-disable-next-line deprecation/deprecation
startActiveSpan,
startInactiveSpan,
} from '@sentry/core';
export type { SpanStatusType } from '@sentry/core';
export { autoDiscoverNodePerformanceMonitoringIntegrations } from './tracing';
Expand Down
4 changes: 3 additions & 1 deletion packages/serverless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ export {
Integrations,
setMeasurement,
getActiveSpan,
startActiveSpan,
startSpan,
// eslint-disable-next-line deprecation/deprecation
startActiveSpan,
startInactiveSpan,
} from '@sentry/node';
4 changes: 3 additions & 1 deletion packages/sveltekit/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ export {
Handlers,
setMeasurement,
getActiveSpan,
startActiveSpan,
startSpan,
// eslint-disable-next-line deprecation/deprecation
startActiveSpan,
startInactiveSpan,
} from '@sentry/node';

// We can still leave this for the carrier init and type exports
Expand Down

0 comments on commit cf75ffe

Please sign in to comment.