Skip to content

Commit

Permalink
Merge branch 'develop' into jb/profiling/expose-profiler
Browse files Browse the repository at this point in the history
  • Loading branch information
JonasBa authored Aug 29, 2024
2 parents ecb8037 + 03d67c9 commit 280aa42
Show file tree
Hide file tree
Showing 22 changed files with 150 additions and 152 deletions.
3 changes: 3 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ updates:
allow:
- dependency-name: "@sentry/cli"
- dependency-name: "@sentry/vite-plugin"
- dependency-name: "@sentry/webpack-plugin"
- dependency-name: "@sentry/rollup-plugin"
- dependency-name: "@sentry/esbuild-plugin"
- dependency-name: "@opentelemetry/*"
- dependency-name: "@prisma/instrumentation"
- dependency-name: "opentelemetry-instrumentation-fetch-node"
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

Work in this release was contributed by @leopoldkristjansson. Thank you for your contribution!
Work in this release was contributed by @leopoldkristjansson and @filips123. Thank you for your contributions!

## 8.27.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ export const expectedCLSPerformanceSpan = {
data: {
value: expect.any(Number),
nodeIds: expect.any(Array),
attributions: expect.any(Array),
rating: expect.any(String),
size: expect.any(Number),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"devDependencies": {
"rollup": "^4.0.2",
"vitest": "^0.34.6",
"@sentry/rollup-plugin": "2.14.2"
"@sentry/rollup-plugin": "2.22.3"
},
"volta": {
"extends": "../../package.json"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export const ReplayRecordingData = [
data: {
value: expect.any(Number),
size: expect.any(Number),
nodeId: 16,
nodeIds: [16],
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ export const ReplayRecordingData = [
size: expect.any(Number),
rating: expect.any(String),
nodeIds: expect.any(Array),
attributions: expect.any(Array),
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"@sentry/node": "8.27.0",
"@sentry/types": "8.27.0",
"@sentry/utils": "8.27.0",
"@sentry/vite-plugin": "^2.20.1"
"@sentry/vite-plugin": "^2.22.3"
},
"devDependencies": {
"astro": "^3.5.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/lib/transports/offline.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ describe('makeOfflineTransport', () => {
START_DELAY + 2_000,
);

// eslint-disable-next-line jest/no-disabled-tests
// eslint-disable-next-line @sentry-internal/sdk/no-skipped-tests
it.skip(
'Follows the Retry-After header',
async () => {
Expand Down
20 changes: 2 additions & 18 deletions packages/eslint-config-sdk/src/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,24 +184,8 @@ module.exports = {
'@sentry-internal/sdk/no-optional-chaining': 'off',
'@sentry-internal/sdk/no-nullish-coalescing': 'off',
'@typescript-eslint/no-floating-promises': 'off',
},
},
{
// Configuration only for test files (this won't apply to utils or other files in test directories)
plugins: ['jest'],
env: {
jest: true,
},
files: ['test.ts', '*.test.ts', '*.test.tsx', '*.test.js', '*.test.jsx'],
rules: {
// Prevent permanent usage of `it.only`, `fit`, `test.only` etc
// We want to avoid debugging leftovers making their way into the codebase
'jest/no-focused-tests': 'error',

// Prevent permanent usage of `it.skip`, `xit`, `test.skip` etc
// We want to avoid debugging leftovers making their way into the codebase
// If there's a good reason to skip a test (e.g. bad flakiness), just add an ignore comment
'jest/no-disabled-tests': 'error',
'@sentry-internal/sdk/no-focused-tests': 'error',
'@sentry-internal/sdk/no-skipped-tests': 'error',
},
},
{
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin-sdk/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@ module.exports = {
'no-eq-empty': require('./rules/no-eq-empty'),
'no-class-field-initializers': require('./rules/no-class-field-initializers'),
'no-regexp-constructor': require('./rules/no-regexp-constructor'),
'no-focused-tests': require('./rules/no-focused-tests'),
'no-skipped-tests': require('./rules/no-skipped-tests'),
},
};
32 changes: 32 additions & 0 deletions packages/eslint-plugin-sdk/src/rules/no-focused-tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use strict';

/**
* This rule was created to flag usages of the `.only` function in vitest and jest tests.
* Usually, we don't want to commit focused tests as this causes other tests to be skipped.
*/
module.exports = {
meta: {
docs: {
description: "Do not focus tests via `.only` to ensure we don't commit accidentally skip the other tests.",
},
schema: [],
},
create: function (context) {
return {
CallExpression(node) {
if (
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'Identifier' &&
['test', 'it', 'describe'].includes(node.callee.object.name) &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === 'only'
) {
context.report({
node,
message: "Do not focus tests via `.only` to ensure we don't commit accidentally skip the other tests.",
});
}
},
};
},
};
33 changes: 33 additions & 0 deletions packages/eslint-plugin-sdk/src/rules/no-skipped-tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict';

/**
* This rule was created to flag usages of the `.skip` function in vitest and jest tests.
* Usually, we don't want to commit skipped tests as this causes other tests to be skipped.
* Sometimes, skipping is valid (e.g. flaky tests), in which case, we can simply eslint-disable the rule.
*/
module.exports = {
meta: {
docs: {
description: "Do not skip tests via `.skip` to ensure we don't commit accidentally skipped tests.",
},
schema: [],
},
create: function (context) {
return {
CallExpression(node) {
if (
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'Identifier' &&
['test', 'it', 'describe'].includes(node.callee.object.name) &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === 'skip'
) {
context.report({
node,
message: "Do not skip tests via `.skip` to ensure we don't commit accidentally skipped tests.",
});
}
},
};
},
};
2 changes: 1 addition & 1 deletion packages/gatsby/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"@sentry/react": "8.27.0",
"@sentry/types": "8.27.0",
"@sentry/utils": "8.27.0",
"@sentry/webpack-plugin": "2.16.0"
"@sentry/webpack-plugin": "2.22.3"
},
"peerDependencies": {
"gatsby": "^2.0.0 || ^3.0.0 || ^4.0.0 || ^5.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"@sentry/types": "8.27.0",
"@sentry/utils": "8.27.0",
"@sentry/vercel-edge": "8.27.0",
"@sentry/webpack-plugin": "2.20.1",
"@sentry/webpack-plugin": "2.22.3",
"chalk": "3.0.0",
"resolve": "1.22.8",
"rollup": "3.29.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/nuxt/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"@sentry/opentelemetry": "8.27.0",
"@sentry/types": "8.27.0",
"@sentry/utils": "8.27.0",
"@sentry/vite-plugin": "2.20.1",
"@sentry/vite-plugin": "2.22.3",
"@sentry/vue": "8.27.0"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/profiling-node/test/cpu_profiler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ describe('Profiler bindings', () => {
expect(profile?.measurements?.['memory_footprint']?.values.length).toBeLessThanOrEqual(300);
});

// eslint-disable-next-line jest/no-disabled-tests
// eslint-disable-next-line @sentry-internal/sdk/no-skipped-tests
it.skip('includes deopt reason', async () => {
// https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#52-the-object-being-iterated-is-not-a-simple-enumerable
function iterateOverLargeHashTable() {
Expand Down
4 changes: 4 additions & 0 deletions packages/replay-internal/src/types/performance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ export interface WebVitalData {
* The recording id of the web vital nodes. -1 if not found
*/
nodeIds?: number[];
/**
* The layout shifts of a CLS metric
*/
attributions?: { value: number; sources?: number[] }[];
}

/**
Expand Down
47 changes: 37 additions & 10 deletions packages/replay-internal/src/util/createPerformanceEntries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ export interface Metric {
* The array may also be empty if the metric value was not based on any
* entries (e.g. a CLS value of 0 given no layout shifts).
*/
entries: PerformanceEntry[] | PerformanceEventTiming[];
entries: PerformanceEntry[] | LayoutShift[];
}

interface LayoutShift extends PerformanceEntry {
value: number;
sources: LayoutShiftAttribution[];
hadRecentInput: boolean;
}

interface LayoutShiftAttribution {
Expand All @@ -52,6 +58,11 @@ interface LayoutShiftAttribution {
currentRect: DOMRectReadOnly;
}

interface Attribution {
value: number;
nodeIds?: number[];
}

/**
* Handler creater for web vitals
*/
Expand Down Expand Up @@ -187,22 +198,32 @@ export function getLargestContentfulPaint(metric: Metric): ReplayPerformanceEntr
return getWebVital(metric, 'largest-contentful-paint', node);
}

function isLayoutShift(entry: PerformanceEntry | LayoutShift): entry is LayoutShift {
return (entry as LayoutShift).sources !== undefined;
}

/**
* Add a CLS event to the replay based on a CLS metric.
*/
export function getCumulativeLayoutShift(metric: Metric): ReplayPerformanceEntry<WebVitalData> {
const lastEntry = metric.entries[metric.entries.length - 1] as
| (PerformanceEntry & { sources?: LayoutShiftAttribution[] })
| undefined;
const layoutShifts: Attribution[] = [];
const nodes: Node[] = [];
if (lastEntry && lastEntry.sources) {
for (const source of lastEntry.sources) {
if (source.node) {
nodes.push(source.node);
for (const entry of metric.entries) {
if (isLayoutShift(entry)) {
const nodeIds = [];
for (const source of entry.sources) {
if (source.node) {
nodes.push(source.node);
const nodeId = record.mirror.getId(source.node);
if (nodeId) {
nodeIds.push(nodeId);
}
}
}
layoutShifts.push({ value: entry.value, nodeIds });
}
}
return getWebVital(metric, 'cumulative-layout-shift', nodes);
return getWebVital(metric, 'cumulative-layout-shift', nodes, layoutShifts);
}

/**
Expand All @@ -226,7 +247,12 @@ export function getInteractionToNextPaint(metric: Metric): ReplayPerformanceEntr
/**
* Add an web vital event to the replay based on the web vital metric.
*/
function getWebVital(metric: Metric, name: string, nodes: Node[] | undefined): ReplayPerformanceEntry<WebVitalData> {
function getWebVital(
metric: Metric,
name: string,
nodes: Node[] | undefined,
attributions?: Attribution[],
): ReplayPerformanceEntry<WebVitalData> {
const value = metric.value;
const rating = metric.rating;

Expand All @@ -242,6 +268,7 @@ function getWebVital(metric: Metric, name: string, nodes: Node[] | undefined): R
size: value,
rating,
nodeIds: nodes ? nodes.map(node => record.mirror.getId(node)) : undefined,
attributions,
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('Unit | util | createPerformanceEntries', () => {
name: 'largest-contentful-paint',
start: 1672531205.108299,
end: 1672531205.108299,
data: { value: 5108.299, rating: 'good', size: 5108.299, nodeIds: undefined },
data: { value: 5108.299, rating: 'good', size: 5108.299, nodeIds: undefined, attributions: undefined },
});
});
});
Expand All @@ -103,7 +103,7 @@ describe('Unit | util | createPerformanceEntries', () => {
name: 'cumulative-layout-shift',
start: 1672531205.108299,
end: 1672531205.108299,
data: { value: 5108.299, size: 5108.299, rating: 'good', nodeIds: [] },
data: { value: 5108.299, size: 5108.299, rating: 'good', nodeIds: [], attributions: [] },
});
});
});
Expand All @@ -123,7 +123,7 @@ describe('Unit | util | createPerformanceEntries', () => {
name: 'first-input-delay',
start: 1672531205.108299,
end: 1672531205.108299,
data: { value: 5108.299, size: 5108.299, rating: 'good', nodeIds: undefined },
data: { value: 5108.299, size: 5108.299, rating: 'good', nodeIds: undefined, attributions: undefined },
});
});
});
Expand All @@ -143,7 +143,7 @@ describe('Unit | util | createPerformanceEntries', () => {
name: 'interaction-to-next-paint',
start: 1672531205.108299,
end: 1672531205.108299,
data: { value: 5108.299, size: 5108.299, rating: 'good', nodeIds: undefined },
data: { value: 5108.299, size: 5108.299, rating: 'good', nodeIds: undefined, attributions: undefined },
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/solidstart/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"@sentry/solid": "8.27.0",
"@sentry/types": "8.27.0",
"@sentry/utils": "8.27.0",
"@sentry/vite-plugin": "2.19.0"
"@sentry/vite-plugin": "2.22.3"
},
"devDependencies": {
"@solidjs/router": "^0.13.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/sveltekit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"@sentry/svelte": "8.27.0",
"@sentry/types": "8.27.0",
"@sentry/utils": "8.27.0",
"@sentry/vite-plugin": "2.22.0",
"@sentry/vite-plugin": "2.22.3",
"magic-string": "0.30.7",
"magicast": "0.2.8",
"sorcery": "0.11.0"
Expand Down
Loading

0 comments on commit 280aa42

Please sign in to comment.