Skip to content

Commit

Permalink
ref: Remove static methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@AbhiPrasad
AbhiPrasad committed Jul 7, 2020
1 parent 9418bf0 commit f0a2d4c
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 58 deletions.
116 changes: 58 additions & 58 deletions packages/tracing/src/browser/browsertracing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,6 @@ export interface BrowserTracingOptions {
// TODO: Should this be an option, or a static class variable and passed
// in and we use something like `BrowserTracing.addRoutingProcessor()`
routingInstrumentationProcessors: routingInstrumentationProcessor[];

/**
* Flag to enable default routing instrumentation.
*
* Default: true
*/
defaultRoutingInstrumentation: boolean;
}

/**
Expand All @@ -108,33 +101,31 @@ export class BrowserTracing implements Integration {
public static id: string = 'BrowserTracing';

/** Browser Tracing integration options */
public static options: BrowserTracingOptions;
public options: BrowserTracingOptions = {
beforeNavigate(location: LocationType): string | undefined {
return location.pathname;
},
debug: {
writeAsBreadcrumbs: false,
},
idleTimeout: DEFAULT_IDLE_TIMEOUT,
routingInstrumentationProcessors: [],
startTransactionOnLocationChange: true,
startTransactionOnPageLoad: true,
};

/**
* @inheritDoc
*/
public name: string = BrowserTracing.id;

private static _activeTransaction?: IdleTransaction;
private _activeTransaction?: IdleTransaction;

private static _getCurrentHub?: () => Hub;
private _getCurrentHub?: () => Hub;

public constructor(_options?: Partial<BrowserTracingOptions>) {
const defaults: BrowserTracingOptions = {
beforeNavigate(location: LocationType): string | undefined {
return location.pathname;
},
debug: {
writeAsBreadcrumbs: false,
},
defaultRoutingInstrumentation: true,
idleTimeout: DEFAULT_IDLE_TIMEOUT,
routingInstrumentationProcessors: [],
startTransactionOnLocationChange: true,
startTransactionOnPageLoad: true,
};
BrowserTracing.options = {
...defaults,
this.options = {
...this.options,
..._options,
};
}
Expand All @@ -143,25 +134,24 @@ export class BrowserTracing implements Integration {
* @inheritDoc
*/
public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
BrowserTracing._getCurrentHub = getCurrentHub;
this._getCurrentHub = getCurrentHub;

if (!global || !global.location) {
return;
}

// TODO: is it fine that this is mutable operation? Could also do = [...routingInstr, setHeaderContext]?
BrowserTracing.options.routingInstrumentationProcessors.push(setHeaderContext);
if (BrowserTracing.options.defaultRoutingInstrumentation) {
BrowserTracing._initRoutingInstrumentation();
}
this._initRoutingInstrumentation();
}

/** Start routing instrumentation */
private static _initRoutingInstrumentation(): void {
const { startTransactionOnPageLoad, startTransactionOnLocationChange } = BrowserTracing.options;
private _initRoutingInstrumentation(): void {
const { startTransactionOnPageLoad, startTransactionOnLocationChange } = this.options;

// TODO: is it fine that this is mutable operation? Could also do = [...routingInstr, setHeaderContext]?
this.options.routingInstrumentationProcessors.push(setHeaderContext);

if (startTransactionOnPageLoad) {
BrowserTracing._activeTransaction = BrowserTracing.createRouteTransaction('pageload');
this._activeTransaction = this._createRouteTransaction('pageload');
}

let startingUrl: string | undefined = global.location.href;
Expand All @@ -170,8 +160,8 @@ export class BrowserTracing implements Integration {
callback: ({ to, from }: { to: string; from?: string }) => {
/**
* This early return is there to account for some cases where navigation transaction
* starts right after long running pageload. We make sure that if from is undefined
* and that a valid startingURL exists, we don't uncessarily create a navigation transaction.
* starts right after long running pageload. We make sure that if `from` is undefined
* and that a valid `startingURL` exists, we don't uncessarily create a navigation transaction.
*
* This was hard to duplicate, but this behaviour stopped as soon as this fix
* was applied. This issue might also only be caused in certain development environments
Expand All @@ -183,55 +173,51 @@ export class BrowserTracing implements Integration {
}
if (startTransactionOnLocationChange && from !== to) {
startingUrl = undefined;
if (BrowserTracing._activeTransaction) {
if (this._activeTransaction) {
// We want to finish all current ongoing idle transactions as we
// are navigating to a new page.
BrowserTracing._activeTransaction.finishIdleTransaction();
this._activeTransaction.finishIdleTransaction();
}
BrowserTracing._activeTransaction = BrowserTracing.createRouteTransaction('navigation');
this._activeTransaction = this._createRouteTransaction('navigation');
}
},
type: 'history',
});
}

/** Create pageload/navigation idle transaction. */
public static createRouteTransaction(
private _createRouteTransaction(
op: 'pageload' | 'navigation',
ctx?: TransactionContext,
context?: TransactionContext,
): IdleTransaction | undefined {
if (!BrowserTracing._getCurrentHub) {
if (!this._getCurrentHub) {
return undefined;
}

const { beforeNavigate, idleTimeout, routingInstrumentationProcessors } = BrowserTracing.options;
const { beforeNavigate, idleTimeout, routingInstrumentationProcessors } = this.options;

// if beforeNavigate returns undefined, we should not start a transaction.
const name = beforeNavigate(global.location);
if (name === undefined) {
this._log(`[Tracing] Cancelling ${op} idleTransaction due to beforeNavigate:`);
return undefined;
}

let context: TransactionContext = { name, op, ...ctx };
if (routingInstrumentationProcessors) {
for (const processor of routingInstrumentationProcessors) {
context = processor(context);
}
}
const ctx = createContextFromProcessors({ name, op, ...context }, routingInstrumentationProcessors);

const hub = BrowserTracing._getCurrentHub();
BrowserTracing._log(`[Tracing] starting ${op} idleTransaction on scope with context:`, context);
const activeTransaction = startIdleTransaction(hub, context, idleTimeout, true);
const hub = this._getCurrentHub();
this._log(`[Tracing] starting ${op} idleTransaction on scope with context:`, ctx);
const activeTransaction = startIdleTransaction(hub, ctx, idleTimeout, true);

return activeTransaction;
}

/**
* Uses logger.log to log things in the SDK or as breadcrumbs if defined in options
*/
private static _log(...args: any[]): void {
if (BrowserTracing.options && BrowserTracing.options.debug && BrowserTracing.options.debug.writeAsBreadcrumbs) {
const _getCurrentHub = BrowserTracing._getCurrentHub;
private _log(...args: any[]): void {
if (this.options && this.options.debug && this.options.debug.writeAsBreadcrumbs) {
const _getCurrentHub = this._getCurrentHub;
if (_getCurrentHub) {
_getCurrentHub().addBreadcrumb({
category: 'tracing',
Expand All @@ -245,9 +231,23 @@ export class BrowserTracing implements Integration {
}
}

/**
* Returns the value of a meta tag
*/
/** Creates transaction context from a set of processors */
export function createContextFromProcessors(
context: TransactionContext,
processors: routingInstrumentationProcessor[],
): TransactionContext {
let ctx = context;
for (const processor of processors) {
const newContext = processor(context);
if (newContext && newContext.name && newContext.op) {
ctx = newContext;
}
}

return ctx;
}

/** Returns the value of a meta tag */
export function getMetaContent(metaName: string): string | null {
const el = document.querySelector(`meta[name=${metaName}]`);
return el ? el.getAttribute('content') : null;
Expand Down
9 changes: 9 additions & 0 deletions packages/tracing/test/browser/browsertracing.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,15 @@ describe('BrowserTracing', () => {
expect(transaction.sampled).toBe(true);
});

it('uses a custom routing instrumentation processor', () => {
createBrowserTracing(true, { routingInstrumentationProcessors: [] });
const transaction = getActiveTransaction(hub) as IdleTransaction;

expect(transaction.traceId).toBe('126de09502ae4e0fb26c6967190756a4');
expect(transaction.parentSpanId).toBe('b6e54397b12a2a0f');
expect(transaction.sampled).toBe(true);
});

it('is created with a default idleTimeout', () => {
createBrowserTracing(true);
const mockFinish = jest.fn();
Expand Down

0 comments on commit f0a2d4c

Please sign in to comment.