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

DNM: test what adding sentry does to bundle size #11773

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
01e055b
feat(telemetry): add Sentry to client
caugner Nov 10, 2023
f96464a
fix(csp): add *.sentry.io as connect-src
caugner Nov 10, 2023
2d36d02
chore(telemetry): reduce Sentry bundle size
caugner Nov 10, 2023
21fe8ba
chore(telemetry): add minimal integrations
caugner Nov 10, 2023
cf3f5c7
chore(telemetry): load sentry async
caugner Nov 10, 2023
7d32685
chore(telemetry): load Sentry on demand
caugner Nov 10, 2023
b1baa07
fixup! chore(telemetry): load Sentry on demand
caugner Nov 10, 2023
6af261e
fix(telemetry): deduplicate errors before sentry was loaded
caugner Nov 10, 2023
ce6cd4a
fix(telemetry): remove unused initSentry() param
caugner Nov 10, 2023
21e827e
fixup! fix(telemetry): remove unused initSentry() param
caugner Nov 10, 2023
b400d23
fix(telemetry): avoid invalid release value
caugner Nov 10, 2023
54df66c
chore(webpack): quantify gain of sentry treeshaking
caugner Nov 10, 2023
bcaf327
chore(deps): bump @sentry/* from 7.80.0 to 8.29.0
caugner Sep 9, 2024
2abfc92
chore(deps-dev): remove unnecessary @sentry/browser
caugner Sep 9, 2024
8e27d77
chore(build): avoid importing everything from @sentry/node
caugner Sep 9, 2024
2488abd
chore(sentry): disable treeshake
caugner Sep 9, 2024
8ab5269
enhance(build): show bundle size diff on PRs
LeoMcA Sep 6, 2024
6312cbc
fix(bundle-size): conditionally remove hashes to keep filenames static
LeoMcA Sep 6, 2024
b114fbe
Merge branch 'main' into add-sentry-to-yari
caugner Sep 11, 2024
0f7e753
Revert "chore(sentry): disable treeshake"
caugner Sep 11, 2024
bfff095
chore(deps): bump @sentry/browser from 8.29.0 to 8.30.0
caugner Sep 11, 2024
a3b9c8c
Merge remote-tracking branch 'origin/main' into bundlesize-compare
LeoMcA Sep 17, 2024
23ec4a1
fix(bundle-size): partially revert 4a5cf81b8958bb3b5f80b8d20ae0bc1ae1…
LeoMcA Sep 9, 2024
9352601
Merge remote-tracking branch 'origin/add-sentry-to-yari' into bundles…
LeoMcA Sep 17, 2024
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
99 changes: 99 additions & 0 deletions .github/workflows/pr-bundlesize-compare.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
on:
# this action will error unless run in a pr context
pull_request:

jobs:
# Build current and upload stats.json
build-head:
name: "Build head"
permissions:
contents: read
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{github.event.pull_request.head.ref}}

- name: Setup Node.js environment
uses: actions/setup-node@v4
with:
node-version-file: ".nvmrc"
cache: yarn

- name: Cache @vscode/ripgrep bin
uses: actions/cache@v4
with:
key: vscode-ripgrep-bin-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('yarn.lock') }}
path: node_modules/@vscode/ripgrep/bin/

- name: Install all yarn packages
run: yarn --frozen-lockfile
env:
# https://github.com/microsoft/vscode-ripgrep#github-api-limit-note
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Build
run: yarn build:client
env:
ANALYZE_BUNDLE_PR: true

- name: Upload stats.json
uses: actions/upload-artifact@v4
with:
name: head-stats
path: ./client/build/stats.json

# Build base for comparison and upload stats.json
build-base:
name: "Build base"
permissions:
contents: read
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: ${{ github.base_ref }}

- name: Setup Node.js environment
uses: actions/setup-node@v4
with:
node-version-file: ".nvmrc"
cache: yarn

- name: Cache @vscode/ripgrep bin
uses: actions/cache@v4
with:
key: vscode-ripgrep-bin-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('yarn.lock') }}
path: node_modules/@vscode/ripgrep/bin/

- name: Install all yarn packages
run: yarn --frozen-lockfile
env:
# https://github.com/microsoft/vscode-ripgrep#github-api-limit-note
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Build
run: yarn build:client
env:
ANALYZE_BUNDLE_PR: true

- name: Upload stats.json
uses: actions/upload-artifact@v4
with:
name: base-stats
path: ./client/build/stats.json

# run the action against the stats.json files
compare:
name: "Compare base & head bundle sizes"
runs-on: ubuntu-latest
needs: [build-base, build-head]
permissions:
pull-requests: write
steps:
- uses: actions/download-artifact@v4
- uses: github/[email protected]
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
current-stats-json-path: ./head-stats/stats.json
base-stats-json-path: ./base-stats/stats.json
3 changes: 3 additions & 0 deletions .github/workflows/prod-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,9 @@ jobs:
REACT_APP_OBSERVATORY_API_URL: https://observatory-api.mdn.mozilla.net

# Sentry.
REACT_APP_SENTRY_DSN: ${{ secrets.SENTRY_DSN_CLIENT }}
REACT_APP_SENTRY_ENVIRONMENT: prod
REACT_APP_SENTRY_RELEASE: ${{ github.sha }}
SENTRY_DSN_BUILD: ${{ secrets.SENTRY_DSN_BUILD }}
SENTRY_ENVIRONMENT: prod
SENTRY_RELEASE: ${{ github.sha }}
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/stage-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,9 @@ jobs:
REACT_APP_OBSERVATORY_API_URL: https://observatory-api.mdn.allizom.net

# Sentry.
REACT_APP_SENTRY_DSN: ${{ secrets.SENTRY_DSN_CLIENT }}
REACT_APP_SENTRY_ENVIRONMENT: stage
REACT_APP_SENTRY_RELEASE: ${{ github.sha }}
SENTRY_DSN_BUILD: ${{ secrets.SENTRY_DSN_BUILD }}
SENTRY_ENVIRONMENT: stage
SENTRY_RELEASE: ${{ github.sha }}
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/test-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ jobs:

# Observatory
REACT_APP_OBSERVATORY_API_URL: https://observatory-api.mdn.allizom.net

# Sentry.
REACT_APP_SENTRY_DSN: ${{ secrets.SENTRY_DSN_CLIENT }}
REACT_APP_SENTRY_ENVIRONMENT: test
REACT_APP_SENTRY_RELEASE: ${{ github.sha }}

run: |
set -eo pipefail

Expand Down
9 changes: 6 additions & 3 deletions build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import path from "node:path";

import chalk from "chalk";
import * as cheerio from "cheerio";
import * as Sentry from "@sentry/node";
import {
setContext as setSentryContext,
setTags as setSentryTags,
} from "@sentry/node";

import {
MacroInvocationError,
Expand Down Expand Up @@ -182,12 +185,12 @@ export async function buildDocument(
document,
documentOptions: DocumentOptions = {}
): Promise<BuiltDocument> {
Sentry.setContext("doc", {
setSentryContext("doc", {
path: document?.fileInfo?.path,
title: document?.metadata?.title,
url: document?.url,
});
Sentry.setTags({
setSentryTags({
doc_slug: document?.metadata?.slug,
doc_locale: document?.metadata?.locale,
});
Expand Down
56 changes: 43 additions & 13 deletions client/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import CssMinimizerPlugin from "css-minimizer-webpack-plugin";
import { WebpackManifestPlugin } from "webpack-manifest-plugin";
import ESLintPlugin from "eslint-webpack-plugin";
import ReactRefreshWebpackPlugin from "@pmmmwh/react-refresh-webpack-plugin";
import { BundleAnalyzerPlugin } from "webpack-bundle-analyzer";

import paths from "./paths.js";
import getClientEnvironment from "./env.js";
Expand All @@ -21,6 +22,10 @@ import ForkTsCheckerWebpackPlugin from "fork-ts-checker-webpack-plugin";
// Source maps are resource heavy and can cause out of memory issue for large source files.
const shouldUseSourceMap = process.env.GENERATE_SOURCEMAP !== "false";

// to compare file sizes in PRs we need them to not include hashes
const analyzeBundlePR = process.env.ANALYZE_BUNDLE_PR;
const analyzeBundle = analyzeBundlePR || process.env.ANALYZE_BUNDLE;

// This is the production and development configuration.
// It is focused on developer experience, fast rebuilds, and a minimal bundle.
function config(webpackEnv) {
Expand Down Expand Up @@ -118,15 +123,20 @@ function config(webpackEnv) {
// Add /* filename */ comments to generated require()s in the output.
pathinfo: isEnvDevelopment,
// There will be one main bundle, and one file per asynchronous chunk.
// In development, it does not produce real files.
filename: isEnvProduction
? "static/js/[name].[contenthash:8].js"
: isEnvDevelopment && "static/js/bundle.js",
filename: analyzeBundlePR
? "static/js/[name].js"
: isEnvProduction
? "static/js/[name].[contenthash:8].js"
: isEnvDevelopment && "static/js/bundle.js",
// There are also additional JS chunk files if you use code splitting.
chunkFilename: isEnvProduction
? "static/js/[name].[contenthash:8].chunk.js"
: isEnvDevelopment && "static/js/[name].chunk.js",
assetModuleFilename: "static/media/[name].[hash][ext]",
chunkFilename: analyzeBundlePR
? "static/js/[name].chunk.js"
: isEnvProduction
? "static/js/[name].[contenthash:8].chunk.js"
: isEnvDevelopment && "static/js/[name].chunk.js",
assetModuleFilename: analyzeBundlePR
? "static/media/[file]"
: "static/media/[name].[hash][ext]",
publicPath: "/",
// Point sourcemap entries to original disk location (format as URL on Windows)
devtoolModuleFilenameTemplate: isEnvProduction
Expand All @@ -152,8 +162,8 @@ function config(webpackEnv) {
level: "none",
},
optimization: {
chunkIds: isEnvProduction ? "natural" : "named",
moduleIds: isEnvProduction ? "natural" : "named",
chunkIds: isEnvProduction ? "deterministic" : "named",
moduleIds: isEnvProduction ? "deterministic" : "named",
minimize: isEnvProduction,
minimizer: [
// This is only used in production mode
Expand Down Expand Up @@ -251,7 +261,9 @@ function config(webpackEnv) {
{
loader: resolve.sync("file-loader"),
options: {
name: "static/media/[name].[hash].[ext]",
name: analyzeBundlePR
? "static/media/[path][name].[ext]"
: "static/media/[name].[hash].[ext]",
},
},
],
Expand Down Expand Up @@ -399,6 +411,14 @@ function config(webpackEnv) {
// during a production build.
// Otherwise React will be compiled in the very slow development mode.
new webpack.DefinePlugin(env.stringified),
// Treeshake Sentry (saves about 12 kB on the chunk).
new webpack.DefinePlugin({
__SENTRY_DEBUG__: false,
__SENTRY_TRACING__: false,
__RRWEB_EXCLUDE_IFRAME__: true,
__RRWEB_EXCLUDE_SHADOW_DOM__: true,
__SENTRY_EXCLUDE_REPLAY_WORKER__: true,
}),
// Experimental hot reloading for React .
// https://github.com/facebook/react/tree/main/packages/react-refresh
isEnvDevelopment &&
Expand All @@ -413,8 +433,12 @@ function config(webpackEnv) {
new MiniCssExtractPlugin({
// Options similar to the same options in webpackOptions.output
// both options are optional
filename: "static/css/[name].[contenthash:8].css",
chunkFilename: "static/css/[name].[contenthash:8].chunk.css",
filename: analyzeBundlePR
? "static/css/[name].css"
: "static/css/[name].[contenthash:8].css",
chunkFilename: analyzeBundlePR
? "static/css/[name].chunk.css"
: "static/css/[name].[contenthash:8].chunk.css",
}),
// Generate an asset manifest file with the following content:
// - "files" key: Mapping of all asset filenames to their corresponding
Expand Down Expand Up @@ -482,6 +506,12 @@ function config(webpackEnv) {
new ESLintPlugin({
extensions: ["js", "mjs", "jsx", "ts", "tsx"],
}),
isEnvProduction &&
analyzeBundle &&
new BundleAnalyzerPlugin({
analyzerMode: "disabled",
generateStatsFile: true,
}),
].filter(Boolean),
// Turn off performance processing because we utilize
// our own hints via the FileSizeReporter
Expand Down
5 changes: 5 additions & 0 deletions client/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ export const GLEAN_LOG_CLICK = Boolean(
JSON.parse(process.env.REACT_APP_GLEAN_LOG_CLICK || "false")
);

export const SENTRY_DSN = process.env.REACT_APP_SENTRY_DSN || "";
export const SENTRY_ENVIRONMENT =
process.env.REACT_APP_SENTRY_ENVIRONMENT || "";
export const SENTRY_RELEASE = process.env.REACT_APP_SENTRY_RELEASE || "";

export const AI_FEEDBACK_GITHUB_REPO =
process.env.REACT_APP_AI_FEEDBACK_GITHUB_REPO || "mdn/private-ai-feedback";

Expand Down
2 changes: 2 additions & 0 deletions client/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import { UserDataProvider } from "./user-context";
import { UIProvider } from "./ui-context";
import { GleanProvider } from "./telemetry/glean-context";
import { PlacementProvider } from "./placement-context";
import { initSentry } from "./telemetry/sentry";

// import * as serviceWorker from './serviceWorker';
initSentry();

const container = document.getElementById("root");
if (!container) {
Expand Down
41 changes: 41 additions & 0 deletions client/src/telemetry/sentry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { SENTRY_DSN, SENTRY_ENVIRONMENT, SENTRY_RELEASE } from "../env";

let sentryPromise: Promise<any> | null = null;

function loadSentry(): Promise<any> {
if (!sentryPromise) {
sentryPromise = import(
/* webpackChunkName: "sentry" */ "@sentry/react"
).then((Sentry) => {
Sentry.init({
dsn: SENTRY_DSN,
release: SENTRY_RELEASE || "dev",
environment: SENTRY_ENVIRONMENT || "dev",
});
return Sentry;
});
}
return sentryPromise;
}

export function initSentry() {
if (!SENTRY_DSN) {
return;
}
let removeEventListener: (() => void) | null = null;
const capturedMessages = new Set<string>();
const errorHandler = (event: ErrorEvent) => {
loadSentry().then((Sentry) => {
if (removeEventListener) {
removeEventListener();
removeEventListener = null;
}
if (!capturedMessages.has(event.message)) {
Sentry.captureException(event);
capturedMessages.add(event.message);
}
});
};
window.addEventListener("error", errorHandler);
removeEventListener = () => window.removeEventListener("error", errorHandler);
}
1 change: 1 addition & 0 deletions libs/constants/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export const CSP_DIRECTIVES = {
"https://observatory-api.mdn.mozilla.net",

"stats.g.doubleclick.net",
"*.sentry.io",
"https://api.stripe.com",
],
"font-src": ["'self'"],
Expand Down
8 changes: 5 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
},
"scripts": {
"ai-help-macros": "cross-env NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node scripts/ai-help-macros.ts",
"analyze": "source-map-explorer 'client/build/static/js/*.js'",
"analyze:css": "source-map-explorer 'client/build/static/css/*.css'",
"analyze": "(test -f client/build/stats.json || cross-env ANALYZE_BUNDLE=true yarn build:client) && webpack-bundle-analyzer client/build/stats.json",
"build": "cross-env NODE_ENV=production NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node build/cli.ts",
"build:blog": "cross-env NODE_ENV=production NODE_OPTIONS='--no-warnings=ExperimentalWarning --loader ts-node/esm' node build/build-blog.ts",
"build:client": "cd client && cross-env NODE_ENV=production BABEL_ENV=production node scripts/build.js",
Expand Down Expand Up @@ -74,7 +73,9 @@
"@mdn/bcd-utils-api": "^0.0.7",
"@mdn/browser-compat-data": "^5.6.0",
"@mozilla/glean": "5.0.3",
"@sentry/browser": "^8.30.0",
"@sentry/node": "^8.29.0",
"@sentry/react": "^8.29.0",
"@stripe/stripe-js": "^4.5.0",
"@use-it/interval": "^1.0.0",
"@vscode/ripgrep": "^1.15.9",
Expand Down Expand Up @@ -172,6 +173,7 @@
"@types/react": "^18.3.7",
"@types/react-dom": "^18.3.0",
"@types/react-modal": "^3.16.3",
"@types/webpack-bundle-analyzer": "^4.7.0",
"babel-jest": "^29.7.0",
"babel-loader": "^9.2.1",
"babel-plugin-named-asset-import": "^0.3.8",
Expand Down Expand Up @@ -241,7 +243,6 @@
"rough-notation": "^0.5.1",
"sass": "^1.78.0",
"sass-loader": "^16.0.1",
"source-map-explorer": "^2.5.3",
"source-map-loader": "^5.0.0",
"style-loader": "^3.3.4",
"stylelint": "^15.11.0",
Expand All @@ -261,6 +262,7 @@
"typescript": "^5.6.2",
"typescript-eslint": "^8.6.0",
"webpack": "^5.94.0",
"webpack-bundle-analyzer": "^4.10.2",
"webpack-cli": "^5.1.4",
"webpack-dev-server": "^5.1.0",
"webpack-manifest-plugin": "^5.0.0",
Expand Down
Loading
Loading