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

feat: workaround for missing Logger messages (attempt 2) #46

Closed
wants to merge 1 commit into from

Conversation

grzuy
Copy link
Collaborator

@grzuy grzuy commented Feb 10, 2025

closes #43

alternative to #44 to fix #44 (comment)


image

@grzuy grzuy marked this pull request as ready for review February 10, 2025 20:16
@KristerV
Copy link

so i'm running this branch and with {:ecto, github: "elixir-ecto/ecto", override: true} everything is finally working. and the two errors that i currently have in my app got logged just how i expected. and it is incredibly useful!

if you're still thinking if you want to incorporate this into master or not - please do. i don't care if you turn it off by default (but i don't see why you would), definitely include it.

@grzuy
Copy link
Collaborator Author

grzuy commented Feb 11, 2025

3rd attempt: #47

@@ -19,6 +19,13 @@ defmodule TowerErrorTracker.Reporter do
:ok
end

def report_event(%Tower.Event{kind: :message, reason: reason, level: level} = event) do
message = "[#{level}] #{reason}"
ErrorTracker.report({"message #{:erlang.phash2(message)}", message}, [], context(event))
Copy link
Collaborator Author

@grzuy grzuy Feb 11, 2025

Choose a reason for hiding this comment

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

3rd attempt (#47), trying to disambiguate messages with source location and function instead of message phash

Choose a reason for hiding this comment

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

yeah i was just coming back to report on this. you're way ahead of me.

@grzuy
Copy link
Collaborator Author

grzuy commented Feb 11, 2025

Going with #47 for now.

Closing this one.

@grzuy grzuy closed this Feb 11, 2025
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.

Logger.error() doesn't reach ErrorTracker
2 participants