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.

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

Jump to bottom

feat(sentry): migration sur la nouvelle version de Sentry #6454

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

feat(sentry): migration sur la nouvelle version de Sentry #6454

wants to merge 1 commit into from

Conversation

m-maillot
Copy link
Contributor

No description provided.

@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 12, 2025 14:06 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 12, 2025 14:18 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 12, 2025 14:51 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 12, 2025 15:01 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 13, 2025 08:02 Destroyed
@socket-security Socket Security
Copy link

socket-security bot commented Feb 13, 2025 •
edited
Loading

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 19, 2025 09:48 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 19, 2025 09:54 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 19, 2025 10:14 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 19, 2025 10:31 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 19, 2025 13:27 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 19, 2025 15:31 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 19, 2025 15:34 Destroyed
@m-maillot m-maillot requested a review from maxgfr February 24, 2025 09:28
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 24, 2025 10:49 Destroyed
@tokenbureau tokenbureau bot temporarily deployed to review-feat-migrate-sentry-3zi99h February 24, 2025 13:17 Destroyed
@m-maillot
feat(sentry): migration sur la nouvelle version de Sentry
75dbee8 
# Conflicts:
#	.kontinuous/env/dev/values.yaml
@tokenbureau
Copy link

tokenbureau bot commented Feb 24, 2025

🎉 Deployment for commit 75dbee8 :

Ingresses
Docker images
  • 📦 docker pull harbor.fabrique.social.gouv.fr/cdtn/code-du-travail-numerique/app:sha-75dbee833dc9358be618e9959b9853add22a8e6a
Debug

@m-maillot m-maillot marked this pull request as ready for review February 24, 2025 14:48
@m-maillot m-maillot requested a review from a team as a code owner February 24, 2025 14:48
maxgfr
maxgfr approved these changes Feb 25, 2025
View reviewed changes
Dockerfile
Comment on lines +100 to +109
ARG SENTRY_URL
ARG SENTRY_ORG
ARG SENTRY_PROJECT
ARG SENTRY_RELEASE
ARG NEXT_PUBLIC_SENTRY_DSN
ENV SENTRY_URL=$SENTRY_URL
ENV SENTRY_ORG=$SENTRY_ORG
ENV SENTRY_PROJECT=$SENTRY_PROJECT
ENV NEXT_PUBLIC_SENTRY_DSN=$NEXT_PUBLIC_SENTRY_DSN
ENV SENTRY_RELEASE=$SENTRY_RELEASE
Copy link
Member

Choose a reason for hiding this comment

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

On a besoin de ces variables également dans le container qui est lancé et pas qu'aubuild ?

packages/code-du-travail-frontend/app/api/test-sentry-error/route.ts
Copy link
Member

Choose a reason for hiding this comment

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

ça on l'enlèvera avant de merger, c'est bien ça ?

packages/code-du-travail-frontend/app/api/monitoring/envelope/route.ts
Copy link
Member

Choose a reason for hiding this comment

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

Le code est pas simple ici, et faudra voir quand ça bougera. C'est un sample de code commun ? ou alors c'est nous qui avons qui l'avons crafter ?

Je vois qu'il y a beaucoup de code commenté. Est-ce normal ?

packages/code-du-travail-frontend/next.config.mjs
Comment on lines +21 to +25
assets: ".next/**/*.{js,map}",
ignore: ["node_modules/**/*"],
rewrite: true,
stripPrefix: ["webpack://_N_E/", "webpack://", "app://"],
urlPrefix: "app:///_next",
Copy link
Member

Choose a reason for hiding this comment

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

Je pense que l'urlPrefix, est ce qu'il nous manquait à notre conifugration de base... Bien joué encore !!

Je pense qu'avec ces quelques lignes, ça aurait dû marcher de base

packages/code-du-travail-frontend/next.config.mjs
Comment on lines +135 to +158
// Source maps configuration
sourcemaps: {
assets: ".next/**/*.{js,map}",
ignore: ["node_modules/**/*"],
rewrite: true,
stripPrefix: ["webpack://_N_E/", "webpack://", "app://"],
urlPrefix: "app:///_next",
},

// Debug and release configuration
silent: false,
debug: true,
release:
process.env.SENTRY_RELEASE || process.env.NEXT_PUBLIC_GITHUB_SHA || "dev",
dist: process.env.NEXT_PUBLIC_GITHUB_SHA || "dev",
setCommits: {
auto: true,
ignoreMissing: true,
},
deploy: {
env: process.env.NEXT_PUBLIC_SENTRY_ENV || "development",
dist: process.env.NEXT_PUBLIC_GITHUB_SHA || "dev",
},
injectBuildInformation: true,
Copy link
Member

Choose a reason for hiding this comment

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

on ré'-écrit la config là aussi ? est-ce qu'on pourra plutôt utiliser la varaible sentryConfig au dessus ?

Dans le cas où on fait ça autant supprimer la variable sentryConfig

packages/code-du-travail-frontend/pages/api/test-sentry-error-legacy.ts
Copy link
Member

Choose a reason for hiding this comment

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

là aussi, on deletera ce file ?

packages/code-du-travail-frontend/sentry.server.config.ts
Comment on lines +37 to +46
const ignoreErrors = [
"ResizeObserver loop limit exceeded",
"Network request failed",
/^Loading chunk .* failed/,
/^Loading CSS chunk .* failed/,
/^ECONNREFUSED/,
/^ECONNRESET/,
/^ETIMEDOUT/,
"Database connection timeout",
];
Copy link
Member

Choose a reason for hiding this comment

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

nice

packages/code-du-travail-frontend/sentry.server.config.ts
enableTracing: true,
attachStacktrace: true, // Attach stack traces to all messages
normalizeDepth: 10, // Increase stack trace depth for better context
tracesSampleRate: IS_PRODUCTION ? 0.1 : 1.0, // Sample 10% of traces in prod, all in dev
Copy link
Member

Choose a reason for hiding this comment

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

top

packages/code-du-travail-frontend/sentry.client.config.ts
Copy link
Member

Choose a reason for hiding this comment

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

top la config

packages/code-du-travail-frontend/src/modules/sentry/SentryTest.tsx
Copy link
Member

Choose a reason for hiding this comment

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

là aussi, il faudra qu'on supprime ce fichier

Copy link
Member

Choose a reason for hiding this comment

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

ou alors on le garde quand même ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxgfr maxgfr maxgfr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@m-maillot @maxgfr