Skip to content

feat(core): Introduce Sentry.startActiveSpan and Sentry.startSpan #8803

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
Aug 15, 2023
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
2 changes: 1 addition & 1 deletion packages/core/src/tracing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ export { extractTraceparentData, getActiveTransaction } from './utils';
// eslint-disable-next-line deprecation/deprecation
export { SpanStatus } from './spanstatus';
export type { SpanStatusType } from './span';
export { trace } from './trace';
export { trace, getActiveSpan, startActiveSpan, startSpan } from './trace';
export { getDynamicSamplingContextFromClient } from './dynamicSamplingContext';
export { setMeasurement } from './measurement';
101 changes: 99 additions & 2 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 getActiveSpan(): Span | undefined {
function startActiveSpan(): Span | undefined {
if (!hasTracingEnabled()) {
return undefined;
}
return parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx);
}

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

function finishAndSetSpan(): void {
Expand Down Expand Up @@ -76,3 +76,100 @@ export function trace<T>(

return maybePromiseResult;
}

/**
* Wraps a function with a transaction/span and finishes the span after the function is done.
* 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}.
*
* 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 {
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) {
ctx.description = ctx.name;
}

const hub = getCurrentHub();
const scope = hub.getScope();

const parentSpan = scope.getSpan();

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

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

function finishAndSetSpan(): void {
activeSpan && activeSpan.finish();
hub.getScope().setSpan(parentSpan);
}

let maybePromiseResult: T;
try {
maybePromiseResult = callback(activeSpan);
} catch (e) {
activeSpan && activeSpan.setStatus('internal_error');
finishAndSetSpan();
throw e;
}

if (isThenable(maybePromiseResult)) {
Promise.resolve(maybePromiseResult).then(
() => {
finishAndSetSpan();
},
() => {
activeSpan && activeSpan.setStatus('internal_error');
finishAndSetSpan();
},
);
} else {
finishAndSetSpan();
}

return maybePromiseResult;
}

/**
* 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}.
*
* 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 {
if (!hasTracingEnabled()) {
return undefined;
}

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) {
ctx.description = ctx.name;
}

const hub = getCurrentHub();
const parentSpan = getActiveSpan();
return parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx);
}

/**
* Returns the currently active span.
*/
export function getActiveSpan(): Span | undefined {
return getCurrentHub().getScope().getSpan();
}
67 changes: 11 additions & 56 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 { trace } from '../../../src/tracing';
import { startActiveSpan } 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('trace', () => {
describe('startActiveSpan', () => {
Copy link
Member

Choose a reason for hiding this comment

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

l: Should we keep the trace tests around? 🤔 not sure, maybe it's fine to remove them as well 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

I replaced the trace tests with startActiveSpan. I'll be removing trace afterwards considering it was an internal API.

beforeEach(() => {
const options = getDefaultTestClientOptions({ tracesSampleRate: 0.0 });
client = new TestClient(options);
Expand All @@ -38,7 +38,7 @@ describe('trace', () => {
])('with %s callback and error %s', (_type, isError, callback, expected) => {
it('should return the same value as the callback', async () => {
try {
const result = await trace({ name: 'GET users/[id]' }, () => {
const result = await startActiveSpan({ name: 'GET users/[id]' }, () => {
return callback();
});
expect(result).toEqual(expected);
Expand All @@ -53,7 +53,7 @@ describe('trace', () => {
// if tracingExtensions are not enabled
jest.spyOn(hub, 'startTransaction').mockReturnValue(undefined);
try {
const result = await trace({ name: 'GET users/[id]' }, () => {
const result = await startActiveSpan({ name: 'GET users/[id]' }, () => {
return callback();
});
expect(result).toEqual(expected);
Expand All @@ -68,7 +68,7 @@ describe('trace', () => {
ref = transaction;
});
try {
await trace({ name: 'GET users/[id]' }, () => {
await startActiveSpan({ name: 'GET users/[id]' }, () => {
return callback();
});
} catch (e) {
Expand All @@ -86,7 +86,7 @@ describe('trace', () => {
ref = transaction;
});
try {
await trace(
await startActiveSpan(
{
name: 'GET users/[id]',
parentSampled: true,
Expand All @@ -113,7 +113,7 @@ describe('trace', () => {
ref = transaction;
});
try {
await trace({ name: 'GET users/[id]' }, span => {
await startActiveSpan({ name: 'GET users/[id]' }, span => {
if (span) {
span.op = 'http.server';
}
Expand All @@ -132,8 +132,8 @@ describe('trace', () => {
ref = transaction;
});
try {
await trace({ name: 'GET users/[id]', parentSampled: true }, () => {
return trace({ name: 'SELECT * from users' }, () => {
await startActiveSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
return startActiveSpan({ name: 'SELECT * from users' }, () => {
return callback();
});
});
Expand All @@ -153,8 +153,8 @@ describe('trace', () => {
ref = transaction;
});
try {
await trace({ name: 'GET users/[id]', parentSampled: true }, () => {
return trace({ name: 'SELECT * from users' }, childSpan => {
await startActiveSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
return startActiveSpan({ name: 'SELECT * from users' }, childSpan => {
if (childSpan) {
childSpan.op = 'db.query';
}
Expand All @@ -168,50 +168,5 @@ describe('trace', () => {
expect(ref.spanRecorder.spans).toHaveLength(2);
expect(ref.spanRecorder.spans[1].op).toEqual('db.query');
});

it('calls `onError` hook', async () => {
const onError = jest.fn();
try {
await trace(
{ name: 'GET users/[id]' },
() => {
return callback();
},
onError,
);
} catch (e) {
expect(onError).toHaveBeenCalledTimes(1);
expect(onError).toHaveBeenCalledWith(e);
}
expect(onError).toHaveBeenCalledTimes(isError ? 1 : 0);
});

it("doesn't create spans but calls onError if tracing is disabled", async () => {
const options = getDefaultTestClientOptions({
/* we don't set tracesSampleRate or tracesSampler */
});
client = new TestClient(options);
hub = new Hub(client);
makeMain(hub);

const startTxnSpy = jest.spyOn(hub, 'startTransaction');

const onError = jest.fn();
try {
await trace(
{ name: 'GET users/[id]' },
() => {
return callback();
},
onError,
);
} catch (e) {
expect(onError).toHaveBeenCalledTimes(1);
expect(onError).toHaveBeenCalledWith(e);
}
expect(onError).toHaveBeenCalledTimes(isError ? 1 : 0);

expect(startTxnSpy).not.toHaveBeenCalled();
});
});
});
3 changes: 3 additions & 0 deletions packages/node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ export {
withScope,
captureCheckIn,
setMeasurement,
getActiveSpan,
startActiveSpan,
startSpan,
} from '@sentry/core';
export type { SpanStatusType } from '@sentry/core';
export { autoDiscoverNodePerformanceMonitoringIntegrations } from './tracing';
Expand Down
3 changes: 3 additions & 0 deletions packages/serverless/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,7 @@ export {
Handlers,
Integrations,
setMeasurement,
getActiveSpan,
startActiveSpan,
startSpan,
} from '@sentry/node';
3 changes: 3 additions & 0 deletions packages/sveltekit/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export {
Integrations,
Handlers,
setMeasurement,
getActiveSpan,
startActiveSpan,
startSpan,
} from '@sentry/node';

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