-
-
Notifications
You must be signed in to change notification settings - Fork 220
Guidelines for sessions in SDKs #323
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/sentry/develop/4e1Rx1Ehamnj6cPUb3jkYMb1Xa1y |
| The Hub.endSession() method is explicitly called; or | ||
| The program terminates without errors; or | ||
| The program terminates with an unhandled exception; or | ||
| The program terminates with an unhandled promise rejection. |
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 think this is Node specific.... unhandled promise rejections do not exist in Python for example...
|
|
||
| #### Lifetime of a Session | ||
|
|
||
| Sessions are never tracked nor sent individually, instead they are aggregated and the aggregates are sent every 30s and a final time when the web server is terminating. |
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.
every 60 seconds*
| #### Lifetime of a Session | ||
|
|
||
| Sessions are never tracked nor sent individually, instead they are aggregated and the aggregates are sent every 30s and a final time when the web server is terminating. | ||
| As an implementation hint to the point above, when a "Client" is closed or flushed the associated "Session Flusher" shall also be flushed and submit the current aggregates the the transport, before the transport is flushed/closed. |
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 am not sure if SessionFlusher exists in sdks other than Python and JS, do they?
| ## SDK Implementation Guideline | ||
|
|
||
| We track the health of each release of projects in Sentry by sending session payloads from SDKs. | ||
| The session payload provide data such as session duration and the presence or absence of errors/crashes. |
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.
Not sure this is entirely a correct statement because we do not send duration in SessionAggregates
| - user interacting with a mobile app | ||
| - user loading a web site with their favorite browser | ||
|
|
||
| Session aggregates are used when sending individual sessions would be undesirable or unpractical. To constrain resource usage (namely memory and network), SDKs keep track of summary information about a batch of sessions that occured in the recent past, never actually having to deal with session objects representing the individual sessions that make up the aggregate. This mode is the choice for applications that run for an arbitrarily long time and handle larger throughputs for potentially multiple users, such as web servers, background job workers, etc. Note that for those types of application, a better definition of session matches the execution of a single HTTP request or task, instead of a single execution of the whole application process. |
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.
never actually having to deal with session objects representing the individual sessions that make up the aggregate
This is only valid for Node afaik
Python does use the Session object up until before it is added to the Transport
Not sure that is what your intention was with this comment but I guess it could be true if you are referring to how its finally sent to the server
|
|
||
| In either case, SDKs should create and report sessions by default, choosing to report them individually or as aggregates depending on the type of application. | ||
|
|
||
| If an SDK can detect that an application is better served by session aggregates, then it must not report an application-wide session. The application-wide session may still be created during SDK initialization but must be aborted and never sent to Sentry. As an example, in the Node.js SDK, we can detect an application is probably a web server if it uses the `requestHandler` integration that is provided. |
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.
application-wide session
Would rename this to Single session to have consistent naming across the doc
|
|
||
| In either case, SDKs should create and report sessions by default, choosing to report them individually or as aggregates depending on the type of application. | ||
|
|
||
| If an SDK can detect that an application is better served by session aggregates, then it must not report an application-wide session. The application-wide session may still be created during SDK initialization but must be aborted and never sent to Sentry. As an example, in the Node.js SDK, we can detect an application is probably a web server if it uses the `requestHandler` integration that is provided. |
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 the Node.js SDK, we can detect an application is probably a web server if it uses the
requestHandlerintegration that is provided
I would also add
And in python we detect that an application is probably a web server through the WSGI integration
Just for better clarification
ahmedetefy
left a comment
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.
Another pass at this
|
|
||
| Sessions should be enabled by default only for the global hub/client that is initialized by Sentry.init, and disabled by default for any other manually created client. | ||
| A session is started when the SDK is initialized (ideally when the default client is bound to the global hub) and ended when one of these conditions happen: | ||
| The Hub.endSession() method is explicitly called; or |
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.
Also it would be ended if a user starts a session
which is probably why we need to document that if a user wants to explicitly start and end their own sessions they should disable autoSessionTracking?
| The program terminates with an unhandled exception; or | ||
| The program terminates with an unhandled promise rejection. | ||
|
|
||
| Care must be taken to never attempt to send new session payloads to Sentry for a session that is already ended. For example, if the user manually ends the session with Hub.endSession(), there should not be any new updates to the session when the program terminates. |
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 would clarify this a bit better by saying that we should not send sessions for sessions that are in terminal states
Ref: https://develop.sentry.dev/sdk/sessions/
String, optional, default is ok. The current status of the session. A session can only be in two states effectively: ok which means the session is alive or one of the terminal states. When a session is moved away from ok it must not be updated anymore.
|
|
||
| #### Sending Session to Sentry | ||
|
|
||
| Session is sent initially after a certain (initially hard-coded, less config is better) delay (something between 1s to 30s TBD), and then updated with the duration and final status and error count when the program terminates. Note that, as an optimization, short lived programs will not send 2 session requests to Relay, but only the final one with status and duration. |
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.
It is not so much a delay as an interval every 1 min
|
|
||
| #### Sending Session to Sentry | ||
|
|
||
| Session is sent initially after a certain (initially hard-coded, less config is better) delay (something between 1s to 30s TBD), and then updated with the duration and final status and error count when the program terminates. Note that, as an optimization, short lived programs will not send 2 session requests to Relay, but only the final one with status and duration. |
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.
Note that, as an optimization, short lived programs will not send 2 session requests to Relay, but only the final one with status and duration.
I am not sure what that means?
|
|
||
| Sessions are never tracked nor sent individually, instead they are aggregated and the aggregates are sent every 30s and a final time when the web server is terminating. | ||
| As an implementation hint to the point above, when a "Client" is closed or flushed the associated "Session Flusher" shall also be flushed and submit the current aggregates the the transport, before the transport is flushed/closed. | ||
| Make sure this works reasonably for Serverless — there we shall not use "request mode" and SessionFlusher because we cannot have any work that happens outside of the request-response flow. |
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.
SessionFlusher exists for both Single Sessions and SessionAggregates in Python
SessionFlusher that only serves SessionAggregates is just an implementation detail in Node
| Sessions are never tracked nor sent individually, instead they are aggregated and the aggregates are sent every 30s and a final time when the web server is terminating. | ||
| As an implementation hint to the point above, when a "Client" is closed or flushed the associated "Session Flusher" shall also be flushed and submit the current aggregates the the transport, before the transport is flushed/closed. | ||
| Make sure this works reasonably for Serverless — there we shall not use "request mode" and SessionFlusher because we cannot have any work that happens outside of the request-response flow. | ||
| Provide an easy way to integrate with existing Node frameworks (Express, Next.js, Koa). |
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 would also add explanation on how the requestHandler is used for that in Node frameworks and WSGI is used for Python frameworks
|
Note: We should also add instructions on how you could manually start your process too in terms of
|
|
This is good enough to be merged and we can update this over time. Given this is only a guideline and not defacto the truth. Not merging this will do more harm than merging it, let's keep iterating if necessary. |
#323 (comment)