Skip to content

Commit eede598

Browse files
HazATlobsterkatie
andauthored
feat(am): Introduce startTransaction (getsentry#2600)
* feat: Introduce startTransaction * fix: Correctly export interfaces * fix: Additional undefined check * Apply suggestions from code review Co-authored-by: Katie Byers <katie.byers@sentry.io> * fix: Log wording * fix: Don't log trace context with non transactions * fix: Doc Strings * feat: Added new option `startTraceForUserSession` * fix: Typing docs * ref: Remove trace option * Update packages/types/src/span.ts Co-authored-by: Katie Byers <katie.byers@sentry.io> * fix: Node, Add startChild * fix: Tests * fix: Remove trace from apply to event * fix: Lint errors + Vue Integration * meta: Add changelog * fix: Vue integration * fix: Transaction interface * ref: CodeReview * feat: Safeguard for name prop in transaction * fix: SampleRate and BeforeSend for Transaction * ref: StartSpan creates a child of the span on scope * ref: Set unlabled transaction name * ref: Slight refactor of startSpan * ref: Also fix traceHeaders * feat: Add additional tests * fix: Small typo in tests * fix: One off tests Co-authored-by: Katie Byers <katie.byers@sentry.io>
1 parent c79bab6 commit eede598

File tree

23 files changed

+705
-521
lines changed

23 files changed

+705
-521
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
- [core] fix: sent_at for envelope headers to use same clock #2597
1717
- [apm] fix: Improve bundle size by moving span status to @sentry/apm #2589
1818
- [apm] feat: No longer discard transactions instead mark them deadline exceeded #2588
19+
- [apm] feat: Introduce `Sentry.startTransaction` and `Transaction.startChild` #2600
20+
- [apm] feat: Transactions no longer go through `beforeSend` #2600
1921
- [browser] fix: Emit Sentry Request breadcrumbs from inside the client (#2615)
2022

2123
## 5.15.5

packages/apm/src/hubextensions.ts

+42-34
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { getMainCarrier, Hub } from '@sentry/hub';
2-
import { SpanContext } from '@sentry/types';
2+
import { SpanContext, TransactionContext } from '@sentry/types';
3+
import { logger } from '@sentry/utils';
34

45
import { Span } from './span';
6+
import { Transaction } from './transaction';
57

68
/** Returns all trace headers that are currently on the top scope. */
7-
function traceHeaders(): { [key: string]: string } {
8-
// @ts-ignore
9-
const that = this as Hub;
10-
const scope = that.getScope();
9+
function traceHeaders(this: Hub): { [key: string]: string } {
10+
const scope = this.getScope();
1111
if (scope) {
1212
const span = scope.getSpan();
1313
if (span) {
@@ -24,45 +24,53 @@ function traceHeaders(): { [key: string]: string } {
2424
* the created Span with the SpanContext will have a reference to it and become it's child.
2525
* Otherwise it'll crete a new `Span`.
2626
*
27-
* @param spanContext Properties with which the span should be created
27+
* @param context Properties with which the span should be created
2828
*/
29-
function startSpan(spanContext?: SpanContext): Span {
30-
// @ts-ignore
31-
const hub = this as Hub;
32-
const scope = hub.getScope();
33-
const client = hub.getClient();
34-
let span;
29+
function startSpan(this: Hub, context: SpanContext | TransactionContext): Transaction | Span {
30+
// This is our safeguard so people always get a Transaction in return.
31+
// We set `_isTransaction: true` in {@link Sentry.startTransaction} to have a runtime check
32+
// if the user really wanted to create a Transaction.
33+
if ((context as TransactionContext)._isTransaction && !(context as TransactionContext).name) {
34+
logger.warn('You are trying to start a Transaction but forgot to provide a `name` property.');
35+
logger.warn('Will fall back to <unlabeled transaction>, use `transaction.setName()` to change it.');
36+
(context as TransactionContext).name = '<unlabeled transaction>';
37+
}
3538

36-
// This flag determines if we already added the span as a child to the span that currently lives on the scope
37-
// If we do not have this, we will add it later on twice to the span recorder and therefore have too many spans
38-
let addedAsChild = false;
39+
if ((context as TransactionContext).name) {
40+
// We are dealing with a Transaction
41+
const transaction = new Transaction(context as TransactionContext, this);
3942

40-
if (scope) {
41-
const parentSpan = scope.getSpan() as Span;
42-
if (parentSpan) {
43-
span = parentSpan.child(spanContext);
44-
addedAsChild = true;
43+
const client = this.getClient();
44+
// We only roll the dice on sampling for root spans of transactions because all child spans inherit this state
45+
if (transaction.sampled === undefined) {
46+
const sampleRate = (client && client.getOptions().tracesSampleRate) || 0;
47+
// if true = we want to have the transaction
48+
// if false = we don't want to have it
49+
// Math.random (inclusive of 0, but not 1)
50+
transaction.sampled = Math.random() < sampleRate;
4551
}
46-
}
4752

48-
if (!span) {
49-
span = new Span(spanContext, hub);
50-
}
53+
// We only want to create a span list if we sampled the transaction
54+
// If sampled == false, we will discard the span anyway, so we can save memory by not storing child spans
55+
if (transaction.sampled) {
56+
const experimentsOptions = (client && client.getOptions()._experiments) || {};
57+
transaction.initSpanRecorder(experimentsOptions.maxSpans as number);
58+
}
5159

52-
// We only roll the dice on sampling for "root" spans (transactions) because the childs inherit this state
53-
if (span.sampled === undefined && span.isRootSpan()) {
54-
const sampleRate = (client && client.getOptions().tracesSampleRate) || 0;
55-
span.sampled = Math.random() < sampleRate;
60+
return transaction;
5661
}
5762

58-
// We only want to create a span list if we sampled the transaction
59-
// in case we will discard the span anyway because sampled == false, we safe memory and do not store child spans
60-
if (span.sampled && !addedAsChild) {
61-
const experimentsOptions = (client && client.getOptions()._experiments) || {};
62-
span.initSpanRecorder(experimentsOptions.maxSpans as number);
63+
const scope = this.getScope();
64+
if (scope) {
65+
// If there is a Span on the Scope we start a child and return that instead
66+
const parentSpan = scope.getSpan();
67+
if (parentSpan) {
68+
return parentSpan.startChild(context);
69+
}
6370
}
6471

65-
return span;
72+
// Otherwise we return a new Span
73+
return new Span(context);
6674
}
6775

6876
/**

packages/apm/src/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as ApmIntegrations from './integrations';
33

44
export { ApmIntegrations as Integrations };
55
export { Span, TRACEPARENT_REGEXP } from './span';
6+
export { Transaction } from './transaction';
67

78
export { SpanStatus } from './spanstatus';
89

packages/apm/src/integrations/express.ts

+61-27
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
1-
import { EventProcessor, Hub, Integration } from '@sentry/types';
1+
import { Integration, Transaction } from '@sentry/types';
22
import { logger } from '@sentry/utils';
33
// tslint:disable-next-line:no-implicit-dependencies
44
import { Application, ErrorRequestHandler, NextFunction, Request, RequestHandler, Response } from 'express';
55

6+
/**
7+
* Internal helper for `__sentry_transaction`
8+
* @hidden
9+
*/
10+
interface SentryTracingResponse {
11+
__sentry_transaction?: Transaction;
12+
}
13+
614
/**
715
* Express integration
816
*
@@ -35,12 +43,12 @@ export class Express implements Integration {
3543
/**
3644
* @inheritDoc
3745
*/
38-
public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
46+
public setupOnce(): void {
3947
if (!this._app) {
4048
logger.error('ExpressIntegration is missing an Express instance');
4149
return;
4250
}
43-
instrumentMiddlewares(this._app, getCurrentHub);
51+
instrumentMiddlewares(this._app);
4452
}
4553
}
4654

@@ -56,40 +64,66 @@ export class Express implements Integration {
5664
* // error handler
5765
* app.use(function (err, req, res, next) { ... })
5866
*/
59-
function wrap(fn: Function, getCurrentHub: () => Hub): RequestHandler | ErrorRequestHandler {
67+
function wrap(fn: Function): RequestHandler | ErrorRequestHandler {
6068
const arrity = fn.length;
6169

6270
switch (arrity) {
6371
case 2: {
64-
return function(this: NodeJS.Global, _req: Request, res: Response): any {
65-
const span = getCurrentHub().startSpan({
66-
description: fn.name,
67-
op: 'middleware',
68-
});
69-
res.once('finish', () => span.finish());
72+
return function(this: NodeJS.Global, _req: Request, res: Response & SentryTracingResponse): any {
73+
const transaction = res.__sentry_transaction;
74+
if (transaction) {
75+
const span = transaction.startChild({
76+
description: fn.name,
77+
op: 'middleware',
78+
});
79+
res.once('finish', () => {
80+
span.finish();
81+
});
82+
}
7083
return fn.apply(this, arguments);
7184
};
7285
}
7386
case 3: {
74-
return function(this: NodeJS.Global, req: Request, res: Response, next: NextFunction): any {
75-
const span = getCurrentHub().startSpan({
76-
description: fn.name,
77-
op: 'middleware',
78-
});
87+
return function(
88+
this: NodeJS.Global,
89+
req: Request,
90+
res: Response & SentryTracingResponse,
91+
next: NextFunction,
92+
): any {
93+
const transaction = res.__sentry_transaction;
94+
const span =
95+
transaction &&
96+
transaction.startChild({
97+
description: fn.name,
98+
op: 'middleware',
99+
});
79100
fn.call(this, req, res, function(this: NodeJS.Global): any {
80-
span.finish();
101+
if (span) {
102+
span.finish();
103+
}
81104
return next.apply(this, arguments);
82105
});
83106
};
84107
}
85108
case 4: {
86-
return function(this: NodeJS.Global, err: any, req: Request, res: Response, next: NextFunction): any {
87-
const span = getCurrentHub().startSpan({
88-
description: fn.name,
89-
op: 'middleware',
90-
});
109+
return function(
110+
this: NodeJS.Global,
111+
err: any,
112+
req: Request,
113+
res: Response & SentryTracingResponse,
114+
next: NextFunction,
115+
): any {
116+
const transaction = res.__sentry_transaction;
117+
const span =
118+
transaction &&
119+
transaction.startChild({
120+
description: fn.name,
121+
op: 'middleware',
122+
});
91123
fn.call(this, err, req, res, function(this: NodeJS.Global): any {
92-
span.finish();
124+
if (span) {
125+
span.finish();
126+
}
93127
return next.apply(this, arguments);
94128
});
95129
};
@@ -110,16 +144,16 @@ function wrap(fn: Function, getCurrentHub: () => Hub): RequestHandler | ErrorReq
110144
* app.use([<path>], <fn>, ...<fn>)
111145
* app.use([<path>], ...<fn>[])
112146
*/
113-
function wrapUseArgs(args: IArguments, getCurrentHub: () => Hub): unknown[] {
147+
function wrapUseArgs(args: IArguments): unknown[] {
114148
return Array.from(args).map((arg: unknown) => {
115149
if (typeof arg === 'function') {
116-
return wrap(arg, getCurrentHub);
150+
return wrap(arg);
117151
}
118152

119153
if (Array.isArray(arg)) {
120154
return arg.map((a: unknown) => {
121155
if (typeof a === 'function') {
122-
return wrap(a, getCurrentHub);
156+
return wrap(a);
123157
}
124158
return a;
125159
});
@@ -132,10 +166,10 @@ function wrapUseArgs(args: IArguments, getCurrentHub: () => Hub): unknown[] {
132166
/**
133167
* Patches original app.use to utilize our tracing functionality
134168
*/
135-
function instrumentMiddlewares(app: Application, getCurrentHub: () => Hub): Application {
169+
function instrumentMiddlewares(app: Application): Application {
136170
const originalAppUse = app.use;
137171
app.use = function(): any {
138-
return originalAppUse.apply(this, wrapUseArgs(arguments, getCurrentHub));
172+
return originalAppUse.apply(this, wrapUseArgs(arguments));
139173
};
140174
return app;
141175
}

0 commit comments

Comments
 (0)