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

Fix sentry release information #5582

Merged
merged 5 commits into from
Jun 5, 2024
Merged

Fix sentry release information #5582

merged 5 commits into from
Jun 5, 2024

Conversation

jorg-vr
Copy link
Contributor

@jorg-vr jorg-vr commented Jun 4, 2024

This pull request adds release information to sentry. It also takes the environment information from rails instead of recalculating it based on hostname.

It also removes the sending of release info to sentry, as the release name is now contained in the tag. And sourcemaps are already pushed upon build.

I have also loaded sentry within frames (where it wasn't present yet).

@jorg-vr jorg-vr added the chore Repository/build/dependency maintenance label Jun 4, 2024
@jorg-vr jorg-vr self-assigned this Jun 4, 2024
@jorg-vr jorg-vr added the deploy naos Request a deployment on naos label Jun 4, 2024
@github-actions github-actions bot removed the deploy naos Request a deployment on naos label Jun 4, 2024
@jorg-vr jorg-vr marked this pull request as ready for review June 4, 2024 14:15
@jorg-vr jorg-vr requested a review from a team as a code owner June 4, 2024 14:15
@jorg-vr jorg-vr requested review from bmesuere and niknetniko and removed request for a team June 4, 2024 14:15
Copy link
Member

@niknetniko niknetniko left a comment

Choose a reason for hiding this comment

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

What is the rational for guarding against missing environment/release tags? I would assume that if they are missing, this should be an error, but I can also see not wanting to break all JavaScript just because these tags are missing

app/assets/javascripts/sentry.ts Outdated Show resolved Hide resolved
app/assets/javascripts/sentry.ts Outdated Show resolved Hide resolved
@jorg-vr
Copy link
Contributor Author

jorg-vr commented Jun 4, 2024

If there is an issue with the tags, 'there should be an error' indeed
But that error would result in sentry not being initiated, thus we wouldn't know the error happened.

Now those errors will be logged under 'unknown'. Seeing 'unknown' in sentry should trigger us to wonder why we didn't get the information

@jorg-vr jorg-vr closed this Jun 4, 2024
@jorg-vr jorg-vr reopened this Jun 4, 2024
@jorg-vr jorg-vr requested a review from niknetniko June 4, 2024 14:37
@jorg-vr jorg-vr merged commit 454ba8e into main Jun 5, 2024
25 checks passed
@jorg-vr jorg-vr deleted the chore/sentry-releases branch June 5, 2024 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Repository/build/dependency maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants