Skip to content
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

feat(am): Introduce startTransaction #2600

Merged
merged 30 commits into from
May 27, 2020
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2685577
feat: Introduce startTransaction
HazAT May 19, 2020
59586bb
fix: Correctly export interfaces
HazAT May 19, 2020
1409700
fix: Additional undefined check
HazAT May 19, 2020
95680f9
Merge branch 'master' into hazat/am-changes
HazAT May 20, 2020
5192e55
Apply suggestions from code review
HazAT May 20, 2020
0d5781a
fix: Log wording
HazAT May 20, 2020
5dcc194
fix: Don't log trace context with non transactions
HazAT May 20, 2020
e416400
fix: Doc Strings
HazAT May 20, 2020
b8dc363
feat: Added new option `startTraceForUserSession`
HazAT May 20, 2020
d44c123
fix: Typing docs
HazAT May 20, 2020
c6338cf
ref: Remove trace option
HazAT May 20, 2020
6556ae7
Update packages/types/src/span.ts
HazAT May 25, 2020
0631c5a
fix: Node, Add startChild
HazAT May 25, 2020
20ec671
fix: Tests
HazAT May 25, 2020
da67297
fix: Remove trace from apply to event
HazAT May 25, 2020
b5f16d9
fix: Lint errors + Vue Integration
HazAT May 25, 2020
0fd1fab
meta: Add changelog
HazAT May 25, 2020
78ecad8
fix: Vue integration
HazAT May 25, 2020
f088aad
fix: Transaction interface
HazAT May 25, 2020
46d2526
ref: CodeReview
HazAT May 26, 2020
de78c96
feat: Safeguard for name prop in transaction
HazAT May 26, 2020
6269bfd
fix: SampleRate and BeforeSend for Transaction
HazAT May 26, 2020
19840c3
ref: StartSpan creates a child of the span on scope
HazAT May 26, 2020
171e2a1
ref: Set unlabled transaction name
HazAT May 26, 2020
39312e8
ref: Slight refactor of startSpan
HazAT May 26, 2020
c288ad4
ref: Also fix traceHeaders
HazAT May 26, 2020
b077373
feat: Add additional tests
HazAT May 26, 2020
43940d7
fix: Small typo in tests
HazAT May 26, 2020
5446d82
fix: One off tests
HazAT May 26, 2020
0f0f051
Merge branch 'master' into hazat/am-changes
HazAT May 27, 2020
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
- [core] fix: sent_at for envelope headers to use same clock #2597
- [apm] fix: Improve bundle size by moving span status to @sentry/apm #2589
- [apm] feat: No longer discard transactions instead mark them deadline exceeded #2588
- [apm] feat: Introduce `Sentry.startTransaction` and `Transaction.startChild` #2600
- [apm] feat: Transactions no longer go through `beforeSend` #2600
HazAT marked this conversation as resolved.
Show resolved Hide resolved

## 5.15.5

Expand Down
76 changes: 42 additions & 34 deletions packages/apm/src/hubextensions.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { getMainCarrier, Hub } from '@sentry/hub';
import { SpanContext } from '@sentry/types';
import { SpanContext, TransactionContext } from '@sentry/types';
import { logger } from '@sentry/utils';

import { Span } from './span';
import { Transaction } from './transaction';

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

// This flag determines if we already added the span as a child to the span that currently lives on the scope
// If we do not have this, we will add it later on twice to the span recorder and therefore have too many spans
let addedAsChild = false;
if ((context as TransactionContext).name) {
HazAT marked this conversation as resolved.
Show resolved Hide resolved
// We are dealing with a Transaction
const transaction = new Transaction(context as TransactionContext, this);

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

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

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

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

return span;
// Otherwise we return a new Span
return new Span(context);
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/apm/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as ApmIntegrations from './integrations';

export { ApmIntegrations as Integrations };
export { Span, TRACEPARENT_REGEXP } from './span';
export { Transaction } from './transaction';

export { SpanStatus } from './spanstatus';

Expand Down
88 changes: 61 additions & 27 deletions packages/apm/src/integrations/express.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import { EventProcessor, Hub, Integration } from '@sentry/types';
import { Integration, Transaction } from '@sentry/types';
import { logger } from '@sentry/utils';
// tslint:disable-next-line:no-implicit-dependencies
import { Application, ErrorRequestHandler, NextFunction, Request, RequestHandler, Response } from 'express';

/**
* Internal helper for `__sentry_transaction`
* @hidden
*/
interface SentryTracingResponse {
__sentry_transaction?: Transaction;
}

/**
* Express integration
*
Expand Down Expand Up @@ -35,12 +43,12 @@ export class Express implements Integration {
/**
* @inheritDoc
*/
public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
public setupOnce(): void {
if (!this._app) {
logger.error('ExpressIntegration is missing an Express instance');
return;
}
instrumentMiddlewares(this._app, getCurrentHub);
instrumentMiddlewares(this._app);
}
}

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

switch (arrity) {
case 2: {
return function(this: NodeJS.Global, _req: Request, res: Response): any {
const span = getCurrentHub().startSpan({
description: fn.name,
op: 'middleware',
});
res.once('finish', () => span.finish());
return function(this: NodeJS.Global, _req: Request, res: Response & SentryTracingResponse): any {
const transaction = res.__sentry_transaction;
if (transaction) {
const span = transaction.startChild({
description: fn.name,
op: 'middleware',
});
res.once('finish', () => {
span.finish();
});
}
return fn.apply(this, arguments);
};
}
case 3: {
return function(this: NodeJS.Global, req: Request, res: Response, next: NextFunction): any {
const span = getCurrentHub().startSpan({
description: fn.name,
op: 'middleware',
});
return function(
this: NodeJS.Global,
req: Request,
res: Response & SentryTracingResponse,
next: NextFunction,
): any {
const transaction = res.__sentry_transaction;
const span =
transaction &&
transaction.startChild({
description: fn.name,
op: 'middleware',
});
fn.call(this, req, res, function(this: NodeJS.Global): any {
span.finish();
if (span) {
span.finish();
}
return next.apply(this, arguments);
});
};
}
case 4: {
return function(this: NodeJS.Global, err: any, req: Request, res: Response, next: NextFunction): any {
const span = getCurrentHub().startSpan({
description: fn.name,
op: 'middleware',
});
return function(
this: NodeJS.Global,
err: any,
req: Request,
res: Response & SentryTracingResponse,
next: NextFunction,
): any {
const transaction = res.__sentry_transaction;
const span =
transaction &&
transaction.startChild({
description: fn.name,
op: 'middleware',
});
fn.call(this, err, req, res, function(this: NodeJS.Global): any {
span.finish();
if (span) {
span.finish();
}
return next.apply(this, arguments);
});
};
Expand All @@ -110,16 +144,16 @@ function wrap(fn: Function, getCurrentHub: () => Hub): RequestHandler | ErrorReq
* app.use([<path>], <fn>, ...<fn>)
* app.use([<path>], ...<fn>[])
*/
function wrapUseArgs(args: IArguments, getCurrentHub: () => Hub): unknown[] {
function wrapUseArgs(args: IArguments): unknown[] {
return Array.from(args).map((arg: unknown) => {
if (typeof arg === 'function') {
return wrap(arg, getCurrentHub);
return wrap(arg);
}

if (Array.isArray(arg)) {
return arg.map((a: unknown) => {
if (typeof a === 'function') {
return wrap(a, getCurrentHub);
return wrap(a);
}
return a;
});
Expand All @@ -132,10 +166,10 @@ function wrapUseArgs(args: IArguments, getCurrentHub: () => Hub): unknown[] {
/**
* Patches original app.use to utilize our tracing functionality
*/
function instrumentMiddlewares(app: Application, getCurrentHub: () => Hub): Application {
function instrumentMiddlewares(app: Application): Application {
const originalAppUse = app.use;
app.use = function(): any {
return originalAppUse.apply(this, wrapUseArgs(arguments, getCurrentHub));
return originalAppUse.apply(this, wrapUseArgs(arguments));
};
return app;
}
HazAT marked this conversation as resolved.
Show resolved Hide resolved
Loading