feat(node): Release Health Session Aggregates #3319

ahmedetefy · 2021-03-10T09:21:27Z

This PR adds:

Capturing request mode sessions/ Session Aggregates in Node.js/Express etc... through using the requestHandler middleware
Using a SessionFlusher on Client that bypasses the Backend for Request mode sessions/ SessionAggregates that flushes the aggregated payloads every 60s
When requestHandler middleware is used, it opts user into Session Aggregates mode and deletes Single Session on scope.
In the case of an error (uncaught error) bubbles up to the errorHandler middleware, the crashed count is incremented
In the case of handled exceptions, the errored count is incremented
In case of healthy request mode session, exited count is incremented
Calling captureEvent with an event that contains exceptions, will increment the errored count

Note: This PR re-enables autoSessionTracking by default #3511
Docs PR: getsentry/sentry-docs#3541

github-actions · 2021-03-10T09:38:37Z

size-limit report

Path	Size
@sentry/browser - CDN Bundle (gzipped)	20.73 KB (+0.41% 🔺)
@sentry/browser - Webpack	21.95 KB (+1.96% 🔺)
@sentry/react - Webpack	21.98 KB (+1.96% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)	28.12 KB (+0.33% 🔺)

rhcarvalho

Did a partial initial review. Sharing some comments.

packages/browser/src/transports/base.ts

packages/browser/test/unit/transports/fetch.test.ts

packages/core/src/baseclient.ts

packages/core/src/request.ts

packages/hub/src/scope.ts

rhcarvalho

another pass

packages/core/src/request.ts

packages/hub/src/scope.ts

packages/node/src/handlers.ts

packages/node/src/sdk.ts

packages/node/src/transports/http.ts

packages/core/src/baseclient.ts

rhcarvalho · 2021-04-26T12:42:54Z

When we ship this we should change the dev docs https://develop.sentry.dev/sdk/sessions/#session-aggregates-payload

Right now the API is marked experimental and limited to sentry.io usage only.

HazAT

Small comments, but tests are still failing.
otherwise LGTM

packages/hub/src/scope.ts

packages/node/src/handlers.ts

Captures request mode sessions associated with a specific release, and sends them to the Sentry as part of the Release Health functionality.

stale

packages/hub/src/session.ts

packages/core/src/request.ts

packages/hub/src/scope.ts

packages/hub/src/session.ts

packages/node/src/handlers.ts

packages/types/src/scope.ts

packages/types/src/session.ts

Co-authored-by: Kamil Ogórek <[email protected]>

packages/node/src/transports/base.ts

ahmedetefy · 2021-05-14T09:09:00Z

packages/node/src/transports/base.ts

-  protected async _send(sentryReq: SentryRequest, originalPayload?: Event | Session): Promise<Response> {
+  protected async _send(
+    sentryReq: SentryRequest,
+    originalPayload?: Event | Session | SessionAggregates,


We do not need to send originalPayload of type SessionAggregates because that is only required for rate limiting

Typescript needs this or it will complain :/

ahmedetefy · 2021-05-14T09:09:45Z

...ages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js


 class DummyTransport extends BaseDummyTransport {
  sendSession(session) {
-    console.error('FAIL: Received Single Session which should have been deleted in favor of Session Aggregates!');


Removing this section means we no longer check if the application mode session was dropped in favor of SessionAggregates mode

There is only sendSession anymore and we do an assert to check for content, so if a single session reaches this it would fail.

packages/browser/src/transports/base.ts

AbhiPrasad · 2021-05-14T10:51:42Z

packages/hub/src/session.ts

+  /** Massages the entries in `pendingAggregates` and returns aggregated sessions */
+  public getSessionAggregates(): SessionAggregates {
+    const aggregates: AggregationCounts[] = Object.keys(this._pendingAggregates).map((key: string) => {
+      return this._pendingAggregates[parseInt(key)];


Are the keys not typed as numbers? Why is it a string here?

Keys are always coerced to strings, despite being stored as numbers, so parseInt can be skipped here.

lol, typescript u dumb :3

packages/core/src/request.ts

packages/hub/src/session.ts

Co-authored-by: Kamil Ogórek <[email protected]>

ahmedetefy force-pushed the release-health branch from a12e6a8 to d67f2fb Compare March 10, 2021 09:38

ahmedetefy force-pushed the release-health branch 3 times, most recently from 0b578a1 to 6c1c72e Compare March 10, 2021 09:45

ahmedetefy changed the title ~~Release health~~ feat: Release health for Node Apr 8, 2021

ahmedetefy force-pushed the release-health branch from dc9dd33 to dc88889 Compare April 9, 2021 13:07

ahmedetefy changed the title ~~feat: Release health for Node~~ feat(node.js): Release health for Node Apr 9, 2021

rhcarvalho reviewed Apr 9, 2021

View reviewed changes

ahmedetefy force-pushed the release-health branch from 220ea59 to 3326a3e Compare April 13, 2021 09:51

ahmedetefy marked this pull request as ready for review April 13, 2021 09:51

ahmedetefy requested a review from kamilogorek as a code owner April 13, 2021 09:51

ahmedetefy requested a review from rhcarvalho April 13, 2021 09:52

ahmedetefy marked this pull request as draft April 14, 2021 09:15

ahmedetefy marked this pull request as ready for review April 14, 2021 14:10

ahmedetefy force-pushed the release-health branch from 56565c6 to b80cf1b Compare April 16, 2021 08:34

rhcarvalho reviewed Apr 16, 2021

View reviewed changes

rhcarvalho previously requested changes Apr 23, 2021

View reviewed changes

packages/core/src/baseclient.ts Outdated Show resolved Hide resolved

ahmedetefy marked this pull request as draft April 23, 2021 09:56

ahmedetefy force-pushed the release-health branch 10 times, most recently from 9641f64 to 5ad2112 Compare May 10, 2021 12:57

ahmedetefy requested review from HazAT and rhcarvalho May 11, 2021 09:25

ahmedetefy force-pushed the release-health branch 2 times, most recently from 2aef14c to 062ae6b Compare May 11, 2021 09:50

HazAT requested changes May 11, 2021

View reviewed changes

packages/hub/src/scope.ts Outdated Show resolved Hide resolved

packages/node/src/handlers.ts Outdated Show resolved Hide resolved

ahmedetefy force-pushed the release-health branch from 062ae6b to 83c5d6c Compare May 11, 2021 10:13

feat(node): Request mode sessions

d952992

Captures request mode sessions associated with a specific release, and sends them to the Sentry as part of the Release Health functionality.

ahmedetefy force-pushed the release-health branch from 83c5d6c to d952992 Compare May 11, 2021 10:18

ahmedetefy requested a review from HazAT May 11, 2021 10:30

HazAT approved these changes May 11, 2021

View reviewed changes

AbhiPrasad reviewed May 12, 2021

View reviewed changes

packages/hub/src/session.ts Show resolved Hide resolved

kamilogorek suggested changes May 12, 2021

View reviewed changes

HazAT and others added 6 commits May 14, 2021 09:54

Apply suggestions from code review

8558c12

Co-authored-by: Kamil Ogórek <[email protected]>

ref: Only have sendSession

158d93d

ref: RequestSession getter/setter on scope

5a9058e

fix: Tests getRequestSession

42c6f9f

ref: initSessionFlusher

f1f11de

fix: Linter

0e19077

ahmedetefy commented May 14, 2021

View reviewed changes

fix: CR

f230bd8

AbhiPrasad reviewed May 14, 2021

View reviewed changes

kamilogorek reviewed May 14, 2021

View reviewed changes

packages/core/src/request.ts Outdated Show resolved Hide resolved

kamilogorek self-requested a review May 14, 2021 12:34

kamilogorek reviewed May 14, 2021

View reviewed changes

packages/hub/src/session.ts Show resolved Hide resolved

Apply suggestions from code review

022c427

Co-authored-by: Kamil Ogórek <[email protected]>

kamilogorek approved these changes May 14, 2021

View reviewed changes

fix: Lints

16fd132

HazAT merged commit e59bb03 into master May 17, 2021

HazAT deleted the release-health branch May 17, 2021 06:20

Uh oh!

feat(node): Release Health Session Aggregates #3319

feat(node): Release Health Session Aggregates #3319

Uh oh!

Conversation

ahmedetefy commented Mar 10, 2021 • edited by HazAT Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report

Uh oh!

rhcarvalho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhcarvalho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhcarvalho commented Apr 26, 2021

Uh oh!

HazAT left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahmedetefy May 14, 2021

Choose a reason for hiding this comment

Uh oh!

HazAT May 14, 2021

Choose a reason for hiding this comment

Uh oh!

ahmedetefy May 14, 2021

Choose a reason for hiding this comment

Uh oh!

HazAT May 14, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AbhiPrasad May 14, 2021

Choose a reason for hiding this comment

Uh oh!

kamilogorek May 14, 2021

Choose a reason for hiding this comment

Uh oh!

kamilogorek May 14, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

ahmedetefy commented Mar 10, 2021 •

edited by HazAT

Loading

github-actions bot commented Mar 10, 2021 •

edited

Loading