Skip to content
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
4 changes: 4 additions & 0 deletions packages/eslint-config-sdk/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ module.exports = {
// Enforce type annotations to maintain consistency. This is especially important as
// we have a public API, so we want changes to be very explicit.
'@typescript-eslint/typedef': ['error', { arrowParameter: false }],

// Although for most codebases inferencing the return type is fine, we explicitly ask to annotate
// all functions with a return type. This is so that intent is as clear as possible. We are guarding against
// cases where you accidently refactor a function's return type to be the wrong type.
'@typescript-eslint/explicit-function-return-type': ['error', { allowExpressions: true }],

// Consistent ordering of fields, methods and constructors for classes should be enforced
Expand Down
28 changes: 24 additions & 4 deletions packages/tracing/src/browser/metrics.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable max-lines */
/* eslint-disable @typescript-eslint/no-explicit-any */
import { SpanContext } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';
Expand Down Expand Up @@ -211,10 +212,17 @@ function addMeasureSpans(
return measureStartTimestamp;
}

export interface ResourceEntry extends Record<string, unknown> {
initiatorType?: string;
transferSize?: number;
encodedBodySize?: number;
decodedBodySize?: number;
}

/** Create resource related spans */
function addResourceSpans(
export function addResourceSpans(
transaction: Transaction,
entry: Record<string, any>,
entry: ResourceEntry,
resourceName: string,
startTime: number,
duration: number,
Expand All @@ -226,14 +234,26 @@ function addResourceSpans(
return undefined;
}

const data: Record<string, any> = {};
if ('transferSize' in entry) {
data['Transfer Size'] = entry.transferSize;
}
if ('encodedBodySize' in entry) {
data['Encoded Body Size'] = entry.encodedBodySize;
}
if ('decodedBodySize' in entry) {
data['Decoded Body Size'] = entry.decodedBodySize;
}

Copy link
Member

@k-fish k-fish Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need a timing (such as responseStart) to exclude cross-origin resources that aren't setup to allow timing.

Copy link
Member

@k-fish k-fish Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also could include some sort of comparison (eg. entry.name.startsWith(window.location.origin)) to determine whether the resource is cross-origin explicitly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will leave this for the future, merging this in for now.

Opening a ticket.

const startTimestamp = timeOrigin + startTime;
const endTimestamp = startTimestamp + duration;

_startChild(transaction, {
description: `${entry.initiatorType} ${resourceName}`,
description: resourceName,
endTimestamp,
op: 'resource',
op: entry.initiatorType ? `resource.${entry.initiatorType}` : 'resource',
startTimestamp,
data,
});

return endTimestamp;
Expand Down
131 changes: 130 additions & 1 deletion packages/tracing/test/browser/metrics.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Span, Transaction } from '../../src';
import { _startChild } from '../../src/browser/metrics';
import { _startChild, addResourceSpans, ResourceEntry } from '../../src/browser/metrics';

describe('_startChild()', () => {
it('creates a span with given properties', () => {
Expand Down Expand Up @@ -38,3 +38,132 @@ describe('_startChild()', () => {
expect(transaction.startTimestamp).toEqual(123);
});
});

describe('addResourceSpans', () => {
const transaction = new Transaction({ name: 'hello' });
beforeEach(() => {
transaction.startChild = jest.fn();
});

// We already track xhr, we don't need to use
it('does not create spans for xmlhttprequest', () => {
const entry: ResourceEntry = {
initiatorType: 'xmlhttprequest',
transferSize: 256,
encodedBodySize: 256,
decodedBodySize: 256,
};
addResourceSpans(transaction, entry, '/assets/to/me', 123, 456, 100);

// eslint-disable-next-line @typescript-eslint/unbound-method
expect(transaction.startChild).toHaveBeenCalledTimes(0);
});

it('does not create spans for fetch', () => {
const entry: ResourceEntry = {
initiatorType: 'fetch',
transferSize: 256,
encodedBodySize: 256,
decodedBodySize: 256,
};
addResourceSpans(transaction, entry, '/assets/to/me', 123, 456, 100);

// eslint-disable-next-line @typescript-eslint/unbound-method
expect(transaction.startChild).toHaveBeenCalledTimes(0);
});

it('creates spans for resource spans', () => {
const entry: ResourceEntry = {
initiatorType: 'css',
transferSize: 256,
encodedBodySize: 456,
decodedBodySize: 593,
};

const timeOrigin = 100;
const startTime = 23;
const duration = 356;

const endTimestamp = addResourceSpans(transaction, entry, '/assets/to/css', startTime, duration, timeOrigin);

// eslint-disable-next-line @typescript-eslint/unbound-method
expect(transaction.startChild).toHaveBeenCalledTimes(1);
// eslint-disable-next-line @typescript-eslint/unbound-method
expect(transaction.startChild).toHaveBeenLastCalledWith({
data: {
['Decoded Body Size']: entry.decodedBodySize,
['Encoded Body Size']: entry.encodedBodySize,
['Transfer Size']: entry.transferSize,
},
description: '/assets/to/css',
endTimestamp: timeOrigin + startTime + duration,
op: 'resource.css',
startTimestamp: timeOrigin + startTime,
});

expect(endTimestamp).toBe(timeOrigin + startTime + duration);
});

it('creates a variety of resource spans', () => {
const table = [
{
initiatorType: undefined,
op: 'resource',
},
{
initiatorType: '',
op: 'resource',
},
{
initiatorType: 'css',
op: 'resource.css',
},
{
initiatorType: 'image',
op: 'resource.image',
},
{
initiatorType: 'script',
op: 'resource.script',
},
];

for (const { initiatorType, op } of table) {
const entry: ResourceEntry = {
initiatorType,
};
addResourceSpans(transaction, entry, '/assets/to/me', 123, 234, 465);

// eslint-disable-next-line @typescript-eslint/unbound-method
expect(transaction.startChild).toHaveBeenLastCalledWith(
expect.objectContaining({
op,
}),
);
}
});

it('allows for enter size of 0', () => {
const entry: ResourceEntry = {
initiatorType: 'css',
transferSize: 0,
encodedBodySize: 0,
decodedBodySize: 0,
};

addResourceSpans(transaction, entry, '/assets/to/css', 100, 23, 345);

// eslint-disable-next-line @typescript-eslint/unbound-method
expect(transaction.startChild).toHaveBeenCalledTimes(1);
// eslint-disable-next-line @typescript-eslint/unbound-method
expect(transaction.startChild).toHaveBeenLastCalledWith(
expect.objectContaining({
data: {
['Decoded Body Size']: entry.decodedBodySize,
['Encoded Body Size']: entry.encodedBodySize,
['Transfer Size']: entry.transferSize,
},
}),
);
});
});