Skip to content

feat(tracing): Record transaction name source when name set directly #5396

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
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: 10 additions & 1 deletion packages/hub/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ export function withScope(callback: (scope: Scope) => void): ReturnType<Hub['wit
* The transaction must be finished with a call to its `.finish()` method, at which point the transaction with all its
* finished child spans will be sent to Sentry.
*
* NOTE: This function should only be used for *manual* instrumentation. Auto-instrumentation should call
* `startTransaction` directly on the hub.
*
* @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}.
Expand All @@ -178,5 +181,11 @@ export function startTransaction(
context: TransactionContext,
customSamplingContext?: CustomSamplingContext,
): ReturnType<Hub['startTransaction']> {
return getCurrentHub().startTransaction({ ...context }, customSamplingContext);
return getCurrentHub().startTransaction(
{
metadata: { source: 'custom' },
...context,
},
customSamplingContext,
);
}
30 changes: 30 additions & 0 deletions packages/hub/test/exports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
setTag,
setTags,
setUser,
startTransaction,
withScope,
} from '../src/exports';

Expand Down Expand Up @@ -184,6 +185,35 @@ describe('Top Level API', () => {
});
});

describe('startTransaction', () => {
beforeEach(() => {
global.__SENTRY__ = {
hub: undefined,
extensions: {
startTransaction: (context: any) => ({
name: context.name,
// Spread rather than assign in case it's undefined
metadata: { ...context.metadata },
}),
},
};
});

it("sets source to `'custom'` if no source provided", () => {
const transaction = startTransaction({ name: 'dogpark' });

expect(transaction.name).toEqual('dogpark');
expect(transaction.metadata.source).toEqual('custom');
});

it('uses given `source` value', () => {
const transaction = startTransaction({ name: 'dogpark', metadata: { source: 'route' } });

expect(transaction.name).toEqual('dogpark');
expect(transaction.metadata.source).toEqual('route');
});
});

test('Clear Scope', () => {
const client: any = new TestClient({});
getCurrentHub().withScope(() => {
Expand Down
7 changes: 4 additions & 3 deletions packages/remix/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { captureException, getCurrentHub, startTransaction } from '@sentry/node';
import { captureException, getCurrentHub } from '@sentry/node';
import { getActiveTransaction } from '@sentry/tracing';
import { addExceptionMechanism, fill, loadModule, logger, stripUrlQueryAndFragment } from '@sentry/utils';

Expand Down Expand Up @@ -175,8 +175,9 @@ function makeWrappedLoader(origAction: DataFunction): DataFunction {

function wrapRequestHandler(origRequestHandler: RequestHandler): RequestHandler {
return async function (this: unknown, request: Request, loadContext?: unknown): Promise<Response> {
const currentScope = getCurrentHub().getScope();
const transaction = startTransaction({
const hub = getCurrentHub();
const currentScope = hub.getScope();
const transaction = hub.startTransaction({
name: stripUrlQueryAndFragment(request.url),
op: 'http.server',
tags: {
Expand Down
15 changes: 4 additions & 11 deletions packages/serverless/src/awslambda.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
/* eslint-disable max-lines */
import * as Sentry from '@sentry/node';
import {
captureException,
captureMessage,
flush,
getCurrentHub,
Scope,
startTransaction,
withScope,
} from '@sentry/node';
import { captureException, captureMessage, flush, getCurrentHub, Scope, withScope } from '@sentry/node';
import { extractTraceparentData } from '@sentry/tracing';
import { Integration } from '@sentry/types';
import { dsnFromString, dsnToString, isString, logger, parseBaggageSetMutability } from '@sentry/utils';
Expand Down Expand Up @@ -320,14 +312,15 @@ export function wrapHandler<TEvent, TResult>(
eventWithHeaders.headers && isString(eventWithHeaders.headers.baggage) && eventWithHeaders.headers.baggage;
const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);

const transaction = startTransaction({
const hub = getCurrentHub();

const transaction = hub.startTransaction({
name: context.functionName,
op: 'awslambda.handler',
...traceparentData,
metadata: { baggage, source: 'component' },
});

const hub = getCurrentHub();
const scope = hub.pushScope();
let rv: TResult;
try {
Expand Down
8 changes: 5 additions & 3 deletions packages/serverless/src/gcpfunction/cloud_events.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
import { captureException, flush, getCurrentHub } from '@sentry/node';
import { logger } from '@sentry/utils';

import { domainify, getActiveDomain, proxyFunction } from '../utils';
Expand Down Expand Up @@ -30,7 +30,9 @@ function _wrapCloudEventFunction(
...wrapOptions,
};
return (context, callback) => {
const transaction = startTransaction({
const hub = getCurrentHub();

const transaction = hub.startTransaction({
name: context.type || '<unknown>',
op: 'gcp.function.cloud_event',
metadata: { source: 'component' },
Expand All @@ -39,7 +41,7 @@ function _wrapCloudEventFunction(
// getCurrentHub() is expected to use current active domain as a carrier
// since functions-framework creates a domain for each incoming request.
// So adding of event processors every time should not lead to memory bloat.
getCurrentHub().configureScope(scope => {
hub.configureScope(scope => {
scope.setContext('gcp.function.context', { ...context });
// We put the transaction on the scope so users can attach children to it
scope.setSpan(transaction);
Expand Down
8 changes: 5 additions & 3 deletions packages/serverless/src/gcpfunction/events.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
import { captureException, flush, getCurrentHub } from '@sentry/node';
import { logger } from '@sentry/utils';

import { domainify, getActiveDomain, proxyFunction } from '../utils';
Expand Down Expand Up @@ -30,7 +30,9 @@ function _wrapEventFunction(
...wrapOptions,
};
return (data, context, callback) => {
const transaction = startTransaction({
const hub = getCurrentHub();

const transaction = hub.startTransaction({
name: context.eventType,
op: 'gcp.function.event',
metadata: { source: 'component' },
Expand All @@ -39,7 +41,7 @@ function _wrapEventFunction(
// getCurrentHub() is expected to use current active domain as a carrier
// since functions-framework creates a domain for each incoming request.
// So adding of event processors every time should not lead to memory bloat.
getCurrentHub().configureScope(scope => {
hub.configureScope(scope => {
scope.setContext('gcp.function.context', { ...context });
// We put the transaction on the scope so users can attach children to it
scope.setSpan(transaction);
Expand Down
7 changes: 4 additions & 3 deletions packages/serverless/src/gcpfunction/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
captureException,
flush,
getCurrentHub,
startTransaction,
} from '@sentry/node';
import { extractTraceparentData } from '@sentry/tracing';
import { isString, logger, parseBaggageSetMutability, stripUrlQueryAndFragment } from '@sentry/utils';
Expand Down Expand Up @@ -83,7 +82,9 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr

const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);

const transaction = startTransaction({
const hub = getCurrentHub();

const transaction = hub.startTransaction({
name: `${reqMethod} ${reqUrl}`,
op: 'gcp.function.http',
...traceparentData,
Expand All @@ -93,7 +94,7 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
// getCurrentHub() is expected to use current active domain as a carrier
// since functions-framework creates a domain for each incoming request.
// So adding of event processors every time should not lead to memory bloat.
getCurrentHub().configureScope(scope => {
hub.configureScope(scope => {
scope.addEventProcessor(event => addRequestDataToEvent(event, req, options.addRequestDataToEventOptions));
// We put the transaction on the scope so users can attach children to it
scope.setSpan(transaction);
Expand Down
1 change: 1 addition & 0 deletions packages/serverless/test/__mocks__/@sentry/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const fakeHub = {
pushScope: jest.fn(() => fakeScope),
popScope: jest.fn(),
getScope: jest.fn(() => fakeScope),
startTransaction: jest.fn(context => ({ ...fakeTransaction, ...context })),
};
export const fakeScope = {
addEventProcessor: jest.fn(),
Expand Down
81 changes: 54 additions & 27 deletions packages/serverless/test/awslambda.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ const fakeCallback: Callback = (err, result) => {
return err;
};

function expectScopeSettings() {
function expectScopeSettings(fakeTransactionContext: any) {
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeScope.setSpan).toBeCalledWith(Sentry.fakeTransaction);
const fakeTransaction = { ...Sentry.fakeTransaction, ...fakeTransactionContext };
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeScope.setTag).toBeCalledWith('server_name', expect.anything());
// @ts-ignore see "Why @ts-ignore" note
Expand Down Expand Up @@ -186,13 +188,17 @@ describe('AWSLambda', () => {
};
const wrappedHandler = wrapHandler(handler);
const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
expect(rv).toStrictEqual(42);
expect(Sentry.startTransaction).toBeCalledWith({

const fakeTransactionContext = {
name: 'functionName',
op: 'awslambda.handler',
metadata: { baggage: [{}, '', true], source: 'component' },
});
expectScopeSettings();
};

expect(rv).toStrictEqual(42);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeTransaction.finish).toBeCalled();
expect(Sentry.flush).toBeCalledWith(2000);
Expand All @@ -210,12 +216,15 @@ describe('AWSLambda', () => {
try {
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
} catch (e) {
expect(Sentry.startTransaction).toBeCalledWith({
const fakeTransactionContext = {
name: 'functionName',
op: 'awslambda.handler',
metadata: { baggage: [{}, '', true], source: 'component' },
});
expectScopeSettings();
};

// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
expect(Sentry.captureException).toBeCalledWith(error);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeTransaction.finish).toBeCalled();
Expand Down Expand Up @@ -244,7 +253,8 @@ describe('AWSLambda', () => {
};

const handler: Handler = (_event, _context, callback) => {
expect(Sentry.startTransaction).toBeCalledWith(
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeHub.startTransaction).toBeCalledWith(
expect.objectContaining({
parentSpanId: '1121201211212012',
parentSampled: false,
Expand Down Expand Up @@ -284,15 +294,18 @@ describe('AWSLambda', () => {
fakeEvent.headers = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0' };
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
} catch (e) {
expect(Sentry.startTransaction).toBeCalledWith({
const fakeTransactionContext = {
name: 'functionName',
op: 'awslambda.handler',
traceId: '12312012123120121231201212312012',
parentSpanId: '1121201211212012',
parentSampled: false,
metadata: { baggage: [{}, '', false], source: 'component' },
});
expectScopeSettings();
};

// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
expect(Sentry.captureException).toBeCalledWith(e);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeTransaction.finish).toBeCalled();
Expand All @@ -310,13 +323,17 @@ describe('AWSLambda', () => {
};
const wrappedHandler = wrapHandler(handler);
const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
expect(rv).toStrictEqual(42);
expect(Sentry.startTransaction).toBeCalledWith({

const fakeTransactionContext = {
name: 'functionName',
op: 'awslambda.handler',
metadata: { baggage: [{}, '', true], source: 'component' },
});
expectScopeSettings();
};

expect(rv).toStrictEqual(42);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeTransaction.finish).toBeCalled();
expect(Sentry.flush).toBeCalled();
Expand Down Expand Up @@ -345,12 +362,15 @@ describe('AWSLambda', () => {
try {
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
} catch (e) {
expect(Sentry.startTransaction).toBeCalledWith({
const fakeTransactionContext = {
name: 'functionName',
op: 'awslambda.handler',
metadata: { baggage: [{}, '', true], source: 'component' },
});
expectScopeSettings();
};

// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
expect(Sentry.captureException).toBeCalledWith(error);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeTransaction.finish).toBeCalled();
Expand Down Expand Up @@ -383,13 +403,17 @@ describe('AWSLambda', () => {
};
const wrappedHandler = wrapHandler(handler);
const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
expect(rv).toStrictEqual(42);
expect(Sentry.startTransaction).toBeCalledWith({

const fakeTransactionContext = {
name: 'functionName',
op: 'awslambda.handler',
metadata: { baggage: [{}, '', true], source: 'component' },
});
expectScopeSettings();
};

expect(rv).toStrictEqual(42);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeTransaction.finish).toBeCalled();
expect(Sentry.flush).toBeCalled();
Expand Down Expand Up @@ -418,12 +442,15 @@ describe('AWSLambda', () => {
try {
await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
} catch (e) {
expect(Sentry.startTransaction).toBeCalledWith({
const fakeTransactionContext = {
name: 'functionName',
op: 'awslambda.handler',
metadata: { baggage: [{}, '', true], source: 'component' },
});
expectScopeSettings();
};

// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
expectScopeSettings(fakeTransactionContext);
expect(Sentry.captureException).toBeCalledWith(error);
// @ts-ignore see "Why @ts-ignore" note
expect(Sentry.fakeTransaction.finish).toBeCalled();
Expand Down
Loading