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

Expose Sentry related env var in dashboard pod #576

Merged
merged 1 commit into from
May 29, 2024

Conversation

lbarcziova
Copy link
Member

@lbarcziova lbarcziova commented May 23, 2024

@lbarcziova lbarcziova requested a review from mfocko May 23, 2024 13:23

This comment was marked as outdated.

@lbarcziova lbarcziova changed the title Expose SENTRY_AUTH_TOKEN env var in dashboard pod Expose Sentry related env var in dashboard pod May 23, 2024

This comment was marked as outdated.

@lbarcziova
Copy link
Member Author

I see the PR actually requires 2 env vars, I will have a look into the SENTRY_AUTH_TOKEN.

@lbarcziova lbarcziova force-pushed the sentry-dashboard branch 2 times, most recently from 6c0e2a4 to d36b1f5 Compare May 23, 2024 14:25
Copy link
Contributor

Copy link
Member

@mfocko mfocko left a comment

Choose a reason for hiding this comment

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

Why isn't DSN enough? 👀

@Venefilyn
Copy link

Why isn't DSN enough? 👀

@mfocko SENTRY_AUTH_TOKEN is to provide sourcemaps to Sentry so that it can decipher production builds, as they are minimized

@mfocko
Copy link
Member

mfocko commented May 27, 2024

Correct me if I'm wrong, but based on the docs:

I understand that this token needs to be present during the build, not while deployed, so this is something that should be exposed in the dashboard repo and for the GH Action (that actually builds the images).

Based on the docs from the first link, I have a feeling that it should work correctly by matching the commit SHA to the image.

I'm not very happy about having the org-wide Sentry token present in the deployment, if (hopefully) there's no need for it.

(Based on the discussion with @lbarcziova)

@Venefilyn
Copy link

I understand that this token needs to be present during the build, not while deployed

Correct, should only be needed during build

@lbarcziova
Copy link
Member Author

thanks @Venefilyn for confirmation! Then I updated this PR only adding the VITE_SENTRY_DSN and the SENTRY_AUTH_TOKEN will be added in the dashboard repo.

Copy link
Contributor

@lbarcziova lbarcziova added the mergeit When set, zuul wil gate and merge the PR. label May 29, 2024
@lbarcziova lbarcziova merged commit d96138a into packit:main May 29, 2024
1 of 2 checks passed
@lbarcziova lbarcziova deleted the sentry-dashboard branch May 29, 2024 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants