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

update Sentry #4914

Closed
wants to merge 5 commits into from
Closed

update Sentry #4914

wants to merge 5 commits into from

Conversation

ruslandoga
Copy link
Contributor

@ruslandoga ruslandoga commented Dec 17, 2024

Changes

This PR updates Sentry client to the latest version to take advantage of the recent Sentry.LoggerHandler improvements:

This change would need to be tried out on staging somehow.

iex> require Logger
iex> Logger.error("check if this gets into sentry")
iex> Sentry.capture_message("and this")
iex> spawn(fn -> 1 / 0 end)
#    etc.: plug errors, phoenix_live_view errors, ...

This PR also removes some of the workarounds that are no longer necessary like inspect(error)

@ruslandoga
Copy link
Contributor Author

ruslandoga commented Dec 17, 2024

Alternatively, we can just revert #4862 :)

@ruslandoga ruslandoga marked this pull request as draft December 17, 2024 13:09
@@ -16,7 +16,7 @@ defmodule PlausibleWeb.EmailView do
Calendar.strftime(date, "%-d %b %Y")
end

def sentry_link(trace_id, dsn \\ Sentry.Config.dsn()) do
def sentry_link(trace_id, dsn \\ Sentry.get_dsn()) do
Copy link
Contributor Author

@ruslandoga ruslandoga Dec 17, 2024

Choose a reason for hiding this comment

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

Was Sentry.Config.dsn() ever a binary? I remember it being an opaque tuple and now it's a struct ...

Really confusing API.

@@ -10,17 +10,6 @@ defmodule Plausible.SentryFilter do
def before_send(%{original_exception: %Plug.CSRFProtection.InvalidCSRFTokenError{}}), do: false
def before_send(%{original_exception: %Plug.Static.InvalidPathError{}}), do: false

def before_send(
%{exception: [%{type: "Clickhousex.Error"}], original_exception: %{code: code}} = event
Copy link
Contributor Author

@ruslandoga ruslandoga Dec 17, 2024

Choose a reason for hiding this comment

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

Clickhousex is gone.

@ruslandoga ruslandoga closed this Dec 17, 2024
@ukutaht
Copy link
Contributor

ukutaht commented Dec 17, 2024

Looks good, why did you close it?

@ruslandoga
Copy link
Contributor Author

ruslandoga commented Dec 17, 2024

There are too many changes that I'm not sure about. I think it would be safer to revert to Sentry.LoggerBackend and handle the upgrade later.

https://3.basecamp.com/5308029/buckets/29267832/card_tables/cards/8135181616

I'll open a PR with the "revert" tomorrow.

@ruslandoga ruslandoga deleted the update-sentry branch December 18, 2024 05:20
@ruslandoga
Copy link
Contributor Author

#4919

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nested Logger metadata is being dropped
2 participants