Skip to content

Fix frontend sentry configuration #1362

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Aug 6, 2024
Merged

Fix frontend sentry configuration #1362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4f5457b
replace frontend ENVIRONMENT var with SENTRY_ENV
ChristopherChudzicki Aug 2, 2024
3e6c1bc
update github action workflows
ChristopherChudzicki Aug 2, 2024
e314a8a
also set version for sentry
ChristopherChudzicki Aug 3, 2024
72b752e
allow undefined LOAD_ENV_FILES
ChristopherChudzicki Aug 3, 2024
de5f449
restore watch:docker
ChristopherChudzicki Aug 3, 2024
bb03662
remove erroneous node_env check
ChristopherChudzicki Aug 5, 2024
5522610
fix webpack_analyze boolean
ChristopherChudzicki Aug 6, 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
5 changes: 5 additions & 0 deletions .github/workflows/production.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ jobs:
- name: Install frontend dependencies
run: yarn install --immutable

- name: Set VERSION
run: echo "VERSION=$(./scripts/get_version.sh)" >> $GITHUB_ENV

- name: Build frontend
run: NODE_ENV=production yarn build
env:
Expand All @@ -40,6 +43,8 @@ jobs:
POSTHOG_TIMEOUT_MS: 1000
POSTHOG_PROJECT_ID: ${{ secrets.POSTHOG_PROJECT_ID_PROD }}
POSTHOG_PROJECT_API_KEY: ${{ secrets.POSTHOG_PROJECT_API_KEY_PROD }}
SENTRY_DSN: ${{ secrets.SENTRY_DSN_PROD }}
SENTRY_ENV: ${{ secrets.MITOPEN_ENV_PROD }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Actions environments would be an improvement here so we are not having env vars that include the environment name - the job would run in the environment context. Outside of the scope of this PR though, given there are others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For us, it is quite canonical. All of our repos have it, and Doof updates it automatically during release via https://github.com/mitodl/release-script/blob/b44bb831e78a5450e3b09e502fd8733b454157f0/version.py#L120

MITOPEN_AXIOS_WITH_CREDENTIALS: true
MITOPEN_API_BASE_URL: https://api.mitopen.odl.mit.edu
MITOPEN_SUPPORT_EMAIL: mitopen-support@mit.edu
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/release-candidate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ jobs:
- name: Install frontend dependencies
run: yarn install --immutable

- name: Set VERSION
run: echo "VERSION=$(./scripts/get_version.sh)" >> $GITHUB_ENV

- name: Build frontend
run: NODE_ENV=production yarn build
env:
Expand All @@ -40,6 +43,8 @@ jobs:
POSTHOG_TIMEOUT_MS: 1000
POSTHOG_PROJECT_ID: ${{ secrets.POSTHOG_PROJECT_ID_RC }}
POSTHOG_PROJECT_API_KEY: ${{ secrets.POSTHOG_PROJECT_API_KEY_RC }}
SENTRY_DSN: ${{ secrets.SENTRY_DSN_RC }}
SENTRY_ENV: ${{ secrets.MITOPEN_ENV_RC }}
MITOPEN_AXIOS_WITH_CREDENTIALS: true
MITOPEN_API_BASE_URL: https://api.mitopen-rc.odl.mit.edu
MITOPEN_SUPPORT_EMAIL: odl-mitopen-rc-support@mit.edu
Expand Down
4 changes: 2 additions & 2 deletions frontends/mit-open/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
"url": "https://github.com/mitodl/mit-open.git"
},
"scripts": {
"watch": "NODE_ENV=development ENVIRONMENT=local webpack serve",
"watch:docker": "NODE_ENV=development ENVIRONMENT=docker webpack serve",
"watch": "NODE_ENV=development LOAD_ENV_FILES=true webpack serve",
"watch:docker": "NODE_ENV=development webpack serve",
"build": "webpack --config webpack.config.js --bail",
"build-exports": "webpack --config webpack.exports.js --bail",
"storybook": "storybook dev -p 6006",
Expand Down
2 changes: 1 addition & 1 deletion frontends/mit-open/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import AppProviders from "./AppProviders"
Sentry.init({
dsn: APP_SETTINGS.SENTRY_DSN,
release: APP_SETTINGS.VERSION,
environment: APP_SETTINGS.ENVIRONMENT,
environment: APP_SETTINGS.SENTRY_ENV,
})

const container = document.getElementById("app-container")
Expand Down
9 changes: 0 additions & 9 deletions frontends/mit-open/src/types/settings.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,3 @@ export type PostHogSettings = {
timeout?: int
bootstrap_flags?: Record<string, string | boolean>
}

export declare global {
const APP_SETTINGS: {
SENTRY_DSN?: string
VERSION?: string
ENVIRONMENT?: string
posthog?: PostHogSettings
}
}
14 changes: 6 additions & 8 deletions frontends/mit-open/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable @typescript-eslint/no-var-requires */
const path = require("path")

if (process.env.ENVIRONMENT === "local") {
if (process.env.LOAD_ENV_FILES?.toLowerCase() === "true") {
console.info("Loading environment from .env files")
require("dotenv").config({
path: [
Expand All @@ -26,7 +26,6 @@ const ReactRefreshWebpackPlugin = require("@pmmmwh/react-refresh-webpack-plugin"
const { cleanEnv, str, bool, port } = require("envalid")

const {
ENVIRONMENT,
NODE_ENV,
PORT,
VERSION,
Expand All @@ -41,10 +40,6 @@ const {
CKEDITOR_UPLOAD_URL,
SENTRY_DSN,
} = cleanEnv(process.env, {
ENVIRONMENT: str({
choices: ["local", "docker", "production"],
default: "production",
}),
Comment on lines -44 to -47
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this variable was confusing. The "docker" option used to be necessary for something, but at this point in our timeline, it isn't used for much.

The only thing this variable was used for is "Should webpack load env files itself"—that's usually not necessary because docker handles it. It is useful if you're running webpack locally outside of docker, so LOAD_ENV_FILES handles that now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes - better to provide flags for specific behaviors where there is little distinction for environments / sets of values.

Is API_DEV_PROXY_BASE_URL still a thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is, though as you've mentioned there are issues running against RC with an authenticated user.

NODE_ENV: str({
choices: ["development", "production", "test"],
default: "production",
Expand Down Expand Up @@ -94,6 +89,10 @@ const {
desc: "Location of the CKEditor uploads handler",
default: "",
}),
SENTRY_ENV: str({
desc: "A label for the environment used for grouping errors in Sentry",
default: "",
}),
SENTRY_DSN: str({
desc: "Sentry Data Source Name",
default: "",
Expand Down Expand Up @@ -216,7 +215,6 @@ module.exports = (env, argv) => {
MITOPEN_API_BASE_URL: JSON.stringify(MITOPEN_API_BASE_URL),
EMBEDLY_KEY: JSON.stringify(EMBEDLY_KEY),
CKEDITOR_UPLOAD_URL: JSON.stringify(CKEDITOR_UPLOAD_URL),
ENVIRONMENT: JSON.stringify(ENVIRONMENT),
VERSION: JSON.stringify(VERSION),
SENTRY_DSN: JSON.stringify(SENTRY_DSN),
POSTHOG: getPostHogSettings(),
Expand All @@ -238,7 +236,7 @@ module.exports = (env, argv) => {
: [new ReactRefreshWebpackPlugin()],
)
.concat(
ENVIRONMENT !== "local" && WEBPACK_ANALYZE === "True"
WEBPACK_ANALYZE
? [
new BundleAnalyzerPlugin({
analyzerMode: "static",
Expand Down
2 changes: 1 addition & 1 deletion frontends/ol-utilities/src/types/settings.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export declare global {
CKEDITOR_UPLOAD_URL?: string
SENTRY_DSN?: string
VERSION?: string
ENVIRONMENT?: string
SENTRY_ENV?: string
POSTHOG?: PostHogSettings
SITE_NAME: string
MITOPEN_SUPPORT_EMAIL: string
Expand Down
6 changes: 6 additions & 0 deletions scripts/get_version.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/usr/bin/env bash

# Get VERSION from settings.py
grep -Eo -m 1 'VERSION\s+= .*(\d+)\.\d+\.\d+.*' main/settings.py |
head -1 |
grep -Eo "\d+\.\d+\.\d+"
Copy link
Contributor Author

@ChristopherChudzicki ChristopherChudzicki Aug 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is not ideal. With VERSION defined in settings.py, it's hard to access outside of Python. (e.g., in our JS
build github action).

For using VERSION during the github action, the options I see are:

  1. Add a new job to the currently JS-only action. It would be a python job, install our deps, run ./manage.py shell and print settings.VERSION, then export that for re-use in the next job that actually publishes frontend. This seems like a lot of set up to just get the version.
  2. this bash script
  3. Update Doof so that it also writes the version to something like version.txt

(3) would be my preference, though I know we are reluctant to update doof much.

Copy link
Contributor

Choose a reason for hiding this comment

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

If settings.py is canonical, it's ok to extract it from there (by any means!). I agree that 3 is nicer. It would be useful to also write this to a version.txt file for the front end build as we do with:

- name: Write commit SHA to file
  run: echo $GITHUB_SHA > frontends/mit-open/build/static/hash.txt