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

Send full logs as an attachment to our dogfood instance #1715

Merged
merged 7 commits into from
Sep 22, 2022

Conversation

hubertdeng123
Copy link
Member

@hubertdeng123 hubertdeng123 commented Sep 21, 2022

This change allows us to support sending envelopes to our sentry dogfood instance.

  1. An envelope file is created with envelope headers
  2. Log file contents are dumped into the file as the attachment payload
  3. File is deleted

getsentry/team-ospo#46

@hubertdeng123
Copy link
Member Author

install/error-handling.sh Outdated Show resolved Hide resolved
@ethanhs
Copy link
Contributor

ethanhs commented Sep 21, 2022

I'm not sure I agree we should delete the envelope, if someone runs into an error, tries to fix it, runs again and hits the same error, we don't want 2 envelopes I don't think. I originally put the envelope file in /tmp so that they will get cleaned up periodically but while someone is working on an issue we don't send multiple.

@hubertdeng123
Copy link
Member Author

I see your point there, my thought for deleting the file was that it's possible to have the same traceback but a different error and in that case I think we should send both. What we could do is determine the event id using both the traceback hash and error. It actually is probably better that way since we have to scroll through all the events to see what kind of errors people are seeing on a certain line. Then we don't have to delete the file

@ethanhs
Copy link
Contributor

ethanhs commented Sep 21, 2022

it's possible to have the same traceback but a different error
What we could do is determine the event id using both the traceback hash and error.

I agree this makes a lot of sense.

Copy link
Contributor

@ethanhs ethanhs left a comment

Choose a reason for hiding this comment

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

Looks great!

@hubertdeng123 hubertdeng123 merged commit b95e926 into master Sep 22, 2022
@hubertdeng123 hubertdeng123 deleted the ethanhs/attach-logs branch September 22, 2022 22:48
@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants