-
-
Notifications
You must be signed in to change notification settings - Fork 51
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: Track Sessions for each request #289
Conversation
8fa1242
to
4a84515
Compare
4a84515
to
1363af4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure what changed since last review? It still would seem more normal to me if the session was explicitly terminated in the same place for all cases taking care of ensuring the session is errored when needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In symbolicator, we have to tweak the meaning of our session statuses:
- "crashed" can be seen as fatal errors, where we're not able to deliver a symbolication result due to a severe issue. This is if we fail I/O locally, if the task times out, or if the task is killed.
- "errored" should be around problems with the payload and potentially also when required symbols fail to compute.
- Everything else can be OK.
1363af4
to
8fca31b
Compare
So does this PR implement this currently? |
It should, yes. Things are marked as crashed on those conditions, otherwise the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, I need a bigger redder flag on my screen saying i have unfinished reviews. LGTM!
This will create a new sentry session for each symbolication task, and flag that as
abnormal
in case it hits a timeout. Otherwise sessions will be closed when the hub/future they were created on is dropped.