Skip to content

Commit

Permalink
fix: Fix for if timestamp of last span is taken for end of transaction (
Browse files Browse the repository at this point in the history
#2410)

* fix: Fix for if timestamp of last span is taken for end of transaction

* feat: Add 0 to bail out of maxTransactionTimeout

* meta: Changelog
  • Loading branch information
HazAT committed Feb 4, 2020
1 parent fe99c14 commit a51d701
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

## 5.12.1

- [apm] ref: If `maxTransactionTimeout` = `0` there is no timeout
- [apm] fix: Make sure that the `maxTransactionTimeout` is always enforced on transaction events

## 5.12.0

- [core] feat: Provide `normalizeDepth` option and sensible default for scope methods (#2404)
Expand Down
33 changes: 28 additions & 5 deletions packages/apm/src/integrations/tracing.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { EventProcessor, Hub, Integration, Span, SpanContext, SpanStatus } from '@sentry/types';
import { Event, EventProcessor, Hub, Integration, Span, SpanContext, SpanStatus } from '@sentry/types';
import {
addInstrumentationHandler,
getGlobalObject,
Expand Down Expand Up @@ -67,9 +67,10 @@ interface TracingOptions {
/**
* The maximum time a transaction can be before it will be dropped. This is for some edge cases where a browser
* completely freezes the JS state and picks it up later. So after this timeout, the SDK will not send the event.
* If you want to have an unlimited timeout set it to 0.
* Time is in ms.
*
* Default: 120000
* Default: 600000 = 10min
*/
maxTransactionTimeout: number;
}
Expand Down Expand Up @@ -126,7 +127,7 @@ export class Tracing implements Integration {
const defaultTracingOrigins = ['localhost', /^\//];
const defaults = {
idleTimeout: 500,
maxTransactionTimeout: 120000,
maxTransactionTimeout: 600000,
shouldCreateSpanForRequest(url: string): boolean {
const origins = (_options && _options.tracingOrigins) || defaultTracingOrigins;
return (
Expand Down Expand Up @@ -155,7 +156,7 @@ export class Tracing implements Integration {
/**
* @inheritDoc
*/
public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
Tracing._getCurrentHub = getCurrentHub;

if (!Tracing._isEnabled()) {
Expand Down Expand Up @@ -192,6 +193,25 @@ export class Tracing implements Integration {
sampled: true,
});
}

// This EventProcessor makes sure that we never send an transaction that is older than maxTransactionTimeout
addGlobalEventProcessor((event: Event) => {
const self = getCurrentHub().getIntegration(Tracing);
if (!self) {
return event;
}

if (
event.type === 'transaction' &&
event.timestamp &&
Tracing.options.maxTransactionTimeout !== 0 &&
timestampWithMs() > event.timestamp + Tracing.options.maxTransactionTimeout
) {
return null;
}

return event;
});
}

/**
Expand Down Expand Up @@ -282,7 +302,10 @@ export class Tracing implements Integration {
public static finishIdleTransaction(): void {
const active = Tracing._activeTransaction as SpanClass;
if (active) {
if (timestampWithMs() > active.startTimestamp + Tracing.options.maxTransactionTimeout) {
if (
Tracing.options.maxTransactionTimeout !== 0 &&
timestampWithMs() > active.startTimestamp + Tracing.options.maxTransactionTimeout
) {
// If we reached the max timeout of the transaction, we will just not finish it and therefore discard it.
Tracing._activeTransaction = undefined;
} else {
Expand Down

0 comments on commit a51d701

Please sign in to comment.