From 4878707ec5fd56923545c65a1540933ab7fcfb68 Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Fri, 28 May 2021 14:16:59 -0400
Subject: [PATCH 1/9] fix(sessions): Correctly compute session duration

Closes #3615

* Update the @sentry/browser client to not send durations when capturing
  sessions.

* Leverage timestampInSeconds from @sentry/utils instead of Date.now()
  to make sure that durations are correctly reported to Sentry.
---
 packages/browser/src/client.ts | 11 ++++++++++-
 packages/core/src/index.ts     |  2 +-
 packages/hub/src/session.ts    | 22 +++++++++++-----------
 packages/types/src/session.ts  |  6 +++---
 4 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts
index aeb1539e2a7d..ee97fa37687f 100644
--- a/packages/browser/src/client.ts
+++ b/packages/browser/src/client.ts
@@ -1,4 +1,4 @@
-import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
+import { BaseClient, Scope, SDK_VERSION, Session } from '@sentry/core';
 import { Event, EventHint } from '@sentry/types';
 import { getGlobalObject, logger } from '@sentry/utils';
 
@@ -57,6 +57,15 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
     });
   }
 
+  /**
+   * @inheritDoc
+   */
+  public captureSession(session: Session): void {
+    // Make sure session duration is not sent
+    session.duration = undefined;
+    super.captureSession(session);
+  }
+
   /**
    * @inheritDoc
    */
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
index 5fe8450de3ed..e521d9179be8 100644
--- a/packages/core/src/index.ts
+++ b/packages/core/src/index.ts
@@ -13,7 +13,7 @@ export {
   setUser,
   withScope,
 } from '@sentry/minimal';
-export { addGlobalEventProcessor, getCurrentHub, getHubFromCarrier, Hub, makeMain, Scope } from '@sentry/hub';
+export { addGlobalEventProcessor, getCurrentHub, getHubFromCarrier, Hub, makeMain, Scope, Session } from '@sentry/hub';
 export { API } from './api';
 export { BaseClient } from './baseclient';
 export { BackendClass, BaseBackend } from './basebackend';
diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts
index 49f3d168e187..54da1c5fa742 100644
--- a/packages/hub/src/session.ts
+++ b/packages/hub/src/session.ts
@@ -8,7 +8,7 @@ import {
   SessionStatus,
   Transport,
 } from '@sentry/types';
-import { dropUndefinedKeys, logger, uuid4 } from '@sentry/utils';
+import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils';
 
 import { getCurrentHub } from './hub';
 
@@ -21,9 +21,9 @@ export class Session implements SessionInterface {
   public release?: string;
   public sid: string = uuid4();
   public did?: string;
-  public timestamp: number = Date.now();
-  public started: number = Date.now();
-  public duration: number = 0;
+  public timestamp: number = timestampInSeconds;
+  public started: number = timestampInSeconds;
+  public duration?: number = 0;
   public status: SessionStatus = SessionStatus.Ok;
   public environment?: string;
   public ipAddress?: string;
@@ -48,7 +48,7 @@ export class Session implements SessionInterface {
       }
     }
 
-    this.timestamp = context.timestamp || Date.now();
+    this.timestamp = context.timestamp || timestampInSeconds;
 
     if (context.sid) {
       // Good enough uuid validation. — Kamil
@@ -63,7 +63,7 @@ export class Session implements SessionInterface {
     if (typeof context.started === 'number') {
       this.started = context.started;
     }
-    if (typeof context.duration === 'number') {
+    if (typeof context.duration === 'number' || typeof context.duration === undefined) {
       this.duration = context.duration;
     } else {
       this.duration = this.timestamp - this.started;
@@ -104,9 +104,9 @@ export class Session implements SessionInterface {
     init: boolean;
     sid: string;
     did?: string;
-    timestamp: string;
-    started: string;
-    duration: number;
+    timestamp: number;
+    started: number;
+    duration?: number;
     status: SessionStatus;
     errors: number;
     attrs?: {
@@ -119,8 +119,8 @@ export class Session implements SessionInterface {
     return dropUndefinedKeys({
       sid: `${this.sid}`,
       init: this.init,
-      started: new Date(this.started).toISOString(),
-      timestamp: new Date(this.timestamp).toISOString(),
+      started: this.started,
+      timestamp: this.timestamp,
       status: this.status,
       errors: this.errors,
       did: typeof this.did === 'number' || typeof this.did === 'string' ? `${this.did}` : undefined,
diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts
index 2875a30d309c..91b99cad99e3 100644
--- a/packages/types/src/session.ts
+++ b/packages/types/src/session.ts
@@ -15,9 +15,9 @@ export interface Session extends SessionContext {
     init: boolean;
     sid: string;
     did?: string;
-    timestamp: string;
-    started: string;
-    duration: number;
+    timestamp: number;
+    started: number;
+    duration?: number;
     status: SessionStatus;
     errors: number;
     attrs?: {

From 042e20e0b5371752c0a6dd18526c912962439681 Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Mon, 31 May 2021 09:25:23 -0400
Subject: [PATCH 2/9] ref: Differentiate between browser/other sessions

Previously in c9b3d107180c03d2256278b266b9ca46d9fe0818, the duration
was updated to be undefined when the browser SDK sent sessions to
Sentry. This patch removes that logic and instead adds a browser flag
to the Session class, which indicates if a duration should be included
or not.
---
 packages/browser/src/sdk.ts   |  4 ++--
 packages/hub/src/session.ts   | 24 ++++++++++++++++++------
 packages/types/src/session.ts |  2 ++
 packages/utils/src/time.ts    |  4 ++--
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts
index 444e06ecbe90..c1427d536091 100644
--- a/packages/browser/src/sdk.ts
+++ b/packages/browser/src/sdk.ts
@@ -199,7 +199,7 @@ function startSessionTracking(): void {
     return;
   }
 
-  hub.startSession();
+  hub.startSession({ browser: true });
   hub.captureSession();
 
   // We want to create a session for every navigation as well
@@ -209,7 +209,7 @@ function startSessionTracking(): void {
       if (from === undefined || from === to) {
         return;
       }
-      hub.startSession();
+      hub.startSession({ browser: true });
       hub.captureSession();
     },
     type: 'history',
diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts
index 54da1c5fa742..c4676177a69b 100644
--- a/packages/hub/src/session.ts
+++ b/packages/hub/src/session.ts
@@ -21,13 +21,14 @@ export class Session implements SessionInterface {
   public release?: string;
   public sid: string = uuid4();
   public did?: string;
-  public timestamp: number = timestampInSeconds;
-  public started: number = timestampInSeconds;
+  public timestamp: number = timestampInSeconds();
+  public started: number = timestampInSeconds();
   public duration?: number = 0;
   public status: SessionStatus = SessionStatus.Ok;
   public environment?: string;
   public ipAddress?: string;
   public init: boolean = true;
+  public browser: boolean = false;
 
   public constructor(context?: Omit<SessionContext, 'started' | 'status'>) {
     if (context) {
@@ -48,8 +49,11 @@ export class Session implements SessionInterface {
       }
     }
 
-    this.timestamp = context.timestamp || timestampInSeconds;
+    this.timestamp = context.timestamp || timestampInSeconds();
 
+    if (context.browser) {
+      this.browser = context.browser;
+    }
     if (context.sid) {
       // Good enough uuid validation. — Kamil
       this.sid = context.sid.length === 32 ? context.sid : uuid4();
@@ -63,10 +67,18 @@ export class Session implements SessionInterface {
     if (typeof context.started === 'number') {
       this.started = context.started;
     }
-    if (typeof context.duration === 'number' || typeof context.duration === undefined) {
-      this.duration = context.duration;
+    // The session duration for browser sessions does not track a meaningful
+    // concept that can be used as a metric.
+    // Automatically captured sessions are akin to page views, and thus we
+    // discard their duration.
+    if (this.browser) {
+      this.duration = undefined;
     } else {
-      this.duration = this.timestamp - this.started;
+      if (typeof context.duration === 'number') {
+        this.duration = context.duration;
+      } else {
+        this.duration = this.timestamp - this.started;
+      }
     }
     if (context.release) {
       this.release = context.release;
diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts
index 91b99cad99e3..e7cead648672 100644
--- a/packages/types/src/session.ts
+++ b/packages/types/src/session.ts
@@ -50,6 +50,8 @@ export interface SessionContext {
   ipAddress?: string;
   errors?: number;
   user?: User | null;
+  // Sessions are handled differently in browser environments
+  browser?: boolean;
 }
 
 /**
diff --git a/packages/utils/src/time.ts b/packages/utils/src/time.ts
index 2c2215c1c2e1..5ef33920a4cc 100644
--- a/packages/utils/src/time.ts
+++ b/packages/utils/src/time.ts
@@ -103,7 +103,7 @@ const timestampSource: TimestampSource =
 /**
  * Returns a timestamp in seconds since the UNIX epoch using the Date API.
  */
-export const dateTimestampInSeconds = dateTimestampSource.nowSeconds.bind(dateTimestampSource);
+export const dateTimestampInSeconds: () => number = dateTimestampSource.nowSeconds.bind(dateTimestampSource);
 
 /**
  * Returns a timestamp in seconds since the UNIX epoch using either the Performance or Date APIs, depending on the
@@ -116,7 +116,7 @@ export const dateTimestampInSeconds = dateTimestampSource.nowSeconds.bind(dateTi
  * skew can grow to arbitrary amounts like days, weeks or months.
  * See https://github.com/getsentry/sentry-javascript/issues/2590.
  */
-export const timestampInSeconds = timestampSource.nowSeconds.bind(timestampSource);
+export const timestampInSeconds: () => number = timestampSource.nowSeconds.bind(timestampSource);
 
 // Re-exported with an old name for backwards-compatibility.
 export const timestampWithMs = timestampInSeconds;

From a4857c934ee7831ad77e6e81ca6bf30edf69df58 Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Mon, 31 May 2021 10:04:50 -0400
Subject: [PATCH 3/9] fix: Init timestamp and started with same value

---
 packages/browser/src/client.ts |  9 ---------
 packages/hub/src/session.ts    | 15 +++++++++------
 packages/types/src/session.ts  |  8 ++++----
 3 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts
index ee97fa37687f..f5bea8e4ade2 100644
--- a/packages/browser/src/client.ts
+++ b/packages/browser/src/client.ts
@@ -57,15 +57,6 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
     });
   }
 
-  /**
-   * @inheritDoc
-   */
-  public captureSession(session: Session): void {
-    // Make sure session duration is not sent
-    session.duration = undefined;
-    super.captureSession(session);
-  }
-
   /**
    * @inheritDoc
    */
diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts
index c4676177a69b..fbf8c750adf4 100644
--- a/packages/hub/src/session.ts
+++ b/packages/hub/src/session.ts
@@ -21,8 +21,8 @@ export class Session implements SessionInterface {
   public release?: string;
   public sid: string = uuid4();
   public did?: string;
-  public timestamp: number = timestampInSeconds();
-  public started: number = timestampInSeconds();
+  public timestamp: number;
+  public started: number;
   public duration?: number = 0;
   public status: SessionStatus = SessionStatus.Ok;
   public environment?: string;
@@ -31,6 +31,9 @@ export class Session implements SessionInterface {
   public browser: boolean = false;
 
   public constructor(context?: Omit<SessionContext, 'started' | 'status'>) {
+    const startingTime = timestampInSeconds();
+    this.timestamp = startingTime;
+    this.started = startingTime;
     if (context) {
       this.update(context);
     }
@@ -116,8 +119,8 @@ export class Session implements SessionInterface {
     init: boolean;
     sid: string;
     did?: string;
-    timestamp: number;
-    started: number;
+    timestamp: string;
+    started: string;
     duration?: number;
     status: SessionStatus;
     errors: number;
@@ -131,8 +134,8 @@ export class Session implements SessionInterface {
     return dropUndefinedKeys({
       sid: `${this.sid}`,
       init: this.init,
-      started: this.started,
-      timestamp: this.timestamp,
+      started: new Date(this.started).toISOString(),
+      timestamp: new Date(this.timestamp).toISOString(),
       status: this.status,
       errors: this.errors,
       did: typeof this.did === 'number' || typeof this.did === 'string' ? `${this.did}` : undefined,
diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts
index e7cead648672..b1b1837b7663 100644
--- a/packages/types/src/session.ts
+++ b/packages/types/src/session.ts
@@ -15,8 +15,8 @@ export interface Session extends SessionContext {
     init: boolean;
     sid: string;
     did?: string;
-    timestamp: number;
-    started: number;
+    timestamp: string;
+    started: string;
     duration?: number;
     status: SessionStatus;
     errors: number;
@@ -40,8 +40,8 @@ export interface SessionContext {
   sid?: string;
   did?: string;
   init?: boolean;
-  timestamp?: number;
-  started?: number;
+  timestamp?: string;
+  started?: string;
   duration?: number;
   status?: SessionStatus;
   release?: string;

From 6d438fd79c5fea41b62ded57639d8cb713549f58 Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Mon, 31 May 2021 10:32:17 -0400
Subject: [PATCH 4/9] fix some types

---
 packages/types/src/session.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts
index b1b1837b7663..60e917247b3a 100644
--- a/packages/types/src/session.ts
+++ b/packages/types/src/session.ts
@@ -40,8 +40,8 @@ export interface SessionContext {
   sid?: string;
   did?: string;
   init?: boolean;
-  timestamp?: string;
-  started?: string;
+  timestamp?: number;
+  started?: number;
   duration?: number;
   status?: SessionStatus;
   release?: string;

From f1557994979ecd969963f53c27ca946379d721f3 Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Mon, 31 May 2021 11:32:05 -0400
Subject: [PATCH 5/9] remove Session export

---
 packages/browser/src/client.ts | 2 +-
 packages/core/src/index.ts     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts
index f5bea8e4ade2..aeb1539e2a7d 100644
--- a/packages/browser/src/client.ts
+++ b/packages/browser/src/client.ts
@@ -1,4 +1,4 @@
-import { BaseClient, Scope, SDK_VERSION, Session } from '@sentry/core';
+import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
 import { Event, EventHint } from '@sentry/types';
 import { getGlobalObject, logger } from '@sentry/utils';
 
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
index e521d9179be8..5fe8450de3ed 100644
--- a/packages/core/src/index.ts
+++ b/packages/core/src/index.ts
@@ -13,7 +13,7 @@ export {
   setUser,
   withScope,
 } from '@sentry/minimal';
-export { addGlobalEventProcessor, getCurrentHub, getHubFromCarrier, Hub, makeMain, Scope, Session } from '@sentry/hub';
+export { addGlobalEventProcessor, getCurrentHub, getHubFromCarrier, Hub, makeMain, Scope } from '@sentry/hub';
 export { API } from './api';
 export { BaseClient } from './baseclient';
 export { BackendClass, BaseBackend } from './basebackend';

From 2b9c467c444c05137f160861d5a1923b53c53897 Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Mon, 31 May 2021 14:12:35 -0400
Subject: [PATCH 6/9] test: Add table tests for session update

---
 packages/browser/src/sdk.ts       |   4 +-
 packages/hub/src/session.ts       |  19 ++---
 packages/hub/test/session.test.ts | 132 ++++++++++++++++++++++++++++++
 packages/types/src/session.ts     |   4 +-
 4 files changed, 146 insertions(+), 13 deletions(-)
 create mode 100644 packages/hub/test/session.test.ts

diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts
index c1427d536091..acbdefbcbc06 100644
--- a/packages/browser/src/sdk.ts
+++ b/packages/browser/src/sdk.ts
@@ -199,7 +199,7 @@ function startSessionTracking(): void {
     return;
   }
 
-  hub.startSession({ browser: true });
+  hub.startSession({ isBrowser: true });
   hub.captureSession();
 
   // We want to create a session for every navigation as well
@@ -209,7 +209,7 @@ function startSessionTracking(): void {
       if (from === undefined || from === to) {
         return;
       }
-      hub.startSession({ browser: true });
+      hub.startSession({ isBrowser: true });
       hub.captureSession();
     },
     type: 'history',
diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts
index fbf8c750adf4..7dc94ac85002 100644
--- a/packages/hub/src/session.ts
+++ b/packages/hub/src/session.ts
@@ -28,9 +28,10 @@ export class Session implements SessionInterface {
   public environment?: string;
   public ipAddress?: string;
   public init: boolean = true;
-  public browser: boolean = false;
+  public isBrowser: boolean = false;
 
   public constructor(context?: Omit<SessionContext, 'started' | 'status'>) {
+    // Both timestamp and started are in seconds since the UNIX epoch.
     const startingTime = timestampInSeconds();
     this.timestamp = startingTime;
     this.started = startingTime;
@@ -53,9 +54,8 @@ export class Session implements SessionInterface {
     }
 
     this.timestamp = context.timestamp || timestampInSeconds();
-
-    if (context.browser) {
-      this.browser = context.browser;
+    if (context.isBrowser) {
+      this.isBrowser = context.isBrowser;
     }
     if (context.sid) {
       // Good enough uuid validation. — Kamil
@@ -74,14 +74,13 @@ export class Session implements SessionInterface {
     // concept that can be used as a metric.
     // Automatically captured sessions are akin to page views, and thus we
     // discard their duration.
-    if (this.browser) {
+    if (this.isBrowser) {
       this.duration = undefined;
+    } else if (typeof context.duration === 'number') {
+      this.duration = context.duration;
     } else {
-      if (typeof context.duration === 'number') {
-        this.duration = context.duration;
-      } else {
-        this.duration = this.timestamp - this.started;
-      }
+      const duration = this.timestamp - this.started;
+      this.duration = duration >= 0 ? duration : 0;
     }
     if (context.release) {
       this.release = context.release;
diff --git a/packages/hub/test/session.test.ts b/packages/hub/test/session.test.ts
new file mode 100644
index 000000000000..59c296cd1d6c
--- /dev/null
+++ b/packages/hub/test/session.test.ts
@@ -0,0 +1,132 @@
+import { SessionContext, SessionStatus } from '@sentry/types';
+
+import { Session } from '../src/session';
+
+describe('Session', () => {
+  it('initializes with the proper defaults', () => {
+    const session = new Session().toJSON();
+
+    const sessionStartTime = session.timestamp;
+
+    expect(session).toEqual({
+      attrs: {},
+      duration: 0,
+      errors: 0,
+      init: true,
+      sid: expect.any(String),
+      started: expect.any(String),
+      status: SessionStatus.Ok,
+      timestamp: expect.any(String),
+    });
+
+    expect(session.sid).toHaveLength(32);
+
+    // started and timestamp should be the same on creation
+    expect(session.started).toEqual(sessionStartTime);
+    expect(session.timestamp).toEqual(sessionStartTime);
+  });
+
+  describe('update', () => {
+    const DEFAULT_OUT = { duration: expect.any(Number) };
+    // [ name, in, out ]
+    const table: Array<[string, SessionContext, Record<string, any>]> = [
+      ['sets an ip address', { user: { ip_address: '0.0.0.0' } }, { attrs: { ip_address: '0.0.0.0' }, ...DEFAULT_OUT }],
+      ['sets a did', { user: { id: 'specialID123' } }, { did: 'specialID123', ...DEFAULT_OUT }],
+      [
+        'sets a timestamp',
+        { timestamp: 1622481662 },
+        { timestamp: new Date(1622481662).toISOString(), ...DEFAULT_OUT },
+      ],
+      [
+        'sets a sid',
+        { sid: '99705f22a3f1468e95ba7386e84691aa' },
+        { sid: '99705f22a3f1468e95ba7386e84691aa', ...DEFAULT_OUT },
+      ],
+      [
+        'requires custom sid to be of certain length',
+        { sid: '123' },
+        { sid: expect.not.stringMatching('123'), ...DEFAULT_OUT },
+      ],
+      [
+        'requires custom sid to be of certain length',
+        { sid: '123' },
+        { sid: expect.not.stringMatching('123'), ...DEFAULT_OUT },
+      ],
+      ['sets an init', { init: false }, { init: false, ...DEFAULT_OUT }],
+      ['sets an did', { did: 'specialID123' }, { did: 'specialID123', ...DEFAULT_OUT }],
+      [
+        'overwrites user did with custom did',
+        { did: 'custom-did', user: { id: 'user-id' } },
+        { did: 'custom-did', ...DEFAULT_OUT },
+      ],
+      ['sets a started time', { started: 1622481662 }, { started: new Date(1622481662).toISOString(), ...DEFAULT_OUT }],
+      ['does not set a duration for browser env', { isBrowser: true }, { duration: undefined }],
+      ['sets a duration', { duration: 12000 }, { duration: 12000 }],
+      ['does not use custom duration for browser env', { duration: 12000, isBrowser: true }, { duration: undefined }],
+      [
+        'does not set a negative duration',
+        { timestamp: 10, started: 100 },
+        // TODO(abhi): What should the behaviour here be?
+        { duration: 0, timestamp: expect.any(String), started: expect.any(String) },
+      ],
+      [
+        'sets duration based on timestamp and started',
+        { timestamp: 100, started: 10 },
+        { duration: 90, timestamp: expect.any(String), started: expect.any(String) },
+      ],
+      [
+        'sets a release',
+        { release: 'f1557994979ecd969963f53c27ca946379d721f3' },
+        { attrs: { release: 'f1557994979ecd969963f53c27ca946379d721f3' }, ...DEFAULT_OUT },
+      ],
+      ['sets an environment', { environment: 'staging' }, { attrs: { environment: 'staging' }, ...DEFAULT_OUT }],
+      ['sets an ipAddress', { ipAddress: '0.0.0.0' }, { attrs: { ip_address: '0.0.0.0' }, ...DEFAULT_OUT }],
+      [
+        'overwrites user ip_address did with custom ipAddress',
+        { ipAddress: '0.0.0.0', user: { ip_address: '1.1.1.1' } },
+        { attrs: { ip_address: '0.0.0.0' }, ...DEFAULT_OUT },
+      ],
+      ['sets an userAgent', { userAgent: 'Mozilla/5.0' }, { attrs: { user_agent: 'Mozilla/5.0' }, ...DEFAULT_OUT }],
+      ['sets errors', { errors: 3 }, { errors: 3, ...DEFAULT_OUT }],
+      ['sets status', { status: SessionStatus.Crashed }, { status: SessionStatus.Crashed, ...DEFAULT_OUT }],
+    ];
+
+    test.each(table)('%s', (...test) => {
+      const session = new Session();
+      const initSessionProps = session.toJSON();
+
+      session.update(test[1]);
+      expect(session.toJSON()).toEqual({ ...initSessionProps, ...test[2] });
+    });
+  });
+
+  describe('close', () => {
+    it('exits a normal session', () => {
+      const session = new Session();
+      const mockUpdate = jest.spyOn(session, 'update');
+      expect(mockUpdate).toHaveBeenCalledTimes(0);
+      session.close();
+      expect(mockUpdate).toHaveBeenCalledTimes(1);
+      expect(mockUpdate).toHaveBeenLastCalledWith({ status: SessionStatus.Exited });
+    });
+
+    it('updates session status when give status', () => {
+      const session = new Session();
+      const mockUpdate = jest.spyOn(session, 'update');
+
+      session.close(SessionStatus.Crashed);
+      expect(mockUpdate).toHaveBeenCalledTimes(1);
+      expect(mockUpdate).toHaveBeenLastCalledWith({ status: SessionStatus.Crashed });
+    });
+
+    it('only changes status ok', () => {
+      const session = new Session();
+      session.status = SessionStatus.Abnormal;
+      const mockUpdate = jest.spyOn(session, 'update');
+
+      session.close();
+      expect(mockUpdate).toHaveBeenCalledTimes(1);
+      expect(mockUpdate).toHaveBeenLastCalledWith();
+    });
+  });
+});
diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts
index 60e917247b3a..0e7f93dc539b 100644
--- a/packages/types/src/session.ts
+++ b/packages/types/src/session.ts
@@ -40,7 +40,9 @@ export interface SessionContext {
   sid?: string;
   did?: string;
   init?: boolean;
+  // seconds since the UNIX epoch
   timestamp?: number;
+  // seconds since the UNIX epoch
   started?: number;
   duration?: number;
   status?: SessionStatus;
@@ -51,7 +53,7 @@ export interface SessionContext {
   errors?: number;
   user?: User | null;
   // Sessions are handled differently in browser environments
-  browser?: boolean;
+  isBrowser?: boolean;
 }
 
 /**

From b9906aa0781e2bf35c5a64fc0681e00b781d195e Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Mon, 31 May 2021 21:02:12 -0400
Subject: [PATCH 7/9] fix: correctly convert seconds

---
 packages/hub/src/session.ts       |  5 ++-
 packages/hub/test/session.test.ts | 63 ++++++++++++-------------------
 2 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts
index 7dc94ac85002..bd1c7d509410 100644
--- a/packages/hub/src/session.ts
+++ b/packages/hub/src/session.ts
@@ -133,8 +133,9 @@ export class Session implements SessionInterface {
     return dropUndefinedKeys({
       sid: `${this.sid}`,
       init: this.init,
-      started: new Date(this.started).toISOString(),
-      timestamp: new Date(this.timestamp).toISOString(),
+      // Make sure that sec is converted to ms for date constructor
+      started: new Date(this.started * 1000).toISOString(),
+      timestamp: new Date(this.timestamp * 1000).toISOString(),
       status: this.status,
       errors: this.errors,
       did: typeof this.did === 'number' || typeof this.did === 'string' ? `${this.did}` : undefined,
diff --git a/packages/hub/test/session.test.ts b/packages/hub/test/session.test.ts
index 59c296cd1d6c..fcf0764fbcbd 100644
--- a/packages/hub/test/session.test.ts
+++ b/packages/hub/test/session.test.ts
@@ -1,6 +1,7 @@
 import { SessionContext, SessionStatus } from '@sentry/types';
 
 import { Session } from '../src/session';
+import { timestampInSeconds } from '@sentry/utils';
 
 describe('Session', () => {
   it('initializes with the proper defaults', () => {
@@ -27,39 +28,19 @@ describe('Session', () => {
   });
 
   describe('update', () => {
-    const DEFAULT_OUT = { duration: expect.any(Number) };
+    const time = timestampInSeconds();
     // [ name, in, out ]
     const table: Array<[string, SessionContext, Record<string, any>]> = [
-      ['sets an ip address', { user: { ip_address: '0.0.0.0' } }, { attrs: { ip_address: '0.0.0.0' }, ...DEFAULT_OUT }],
-      ['sets a did', { user: { id: 'specialID123' } }, { did: 'specialID123', ...DEFAULT_OUT }],
-      [
-        'sets a timestamp',
-        { timestamp: 1622481662 },
-        { timestamp: new Date(1622481662).toISOString(), ...DEFAULT_OUT },
-      ],
-      [
-        'sets a sid',
-        { sid: '99705f22a3f1468e95ba7386e84691aa' },
-        { sid: '99705f22a3f1468e95ba7386e84691aa', ...DEFAULT_OUT },
-      ],
-      [
-        'requires custom sid to be of certain length',
-        { sid: '123' },
-        { sid: expect.not.stringMatching('123'), ...DEFAULT_OUT },
-      ],
-      [
-        'requires custom sid to be of certain length',
-        { sid: '123' },
-        { sid: expect.not.stringMatching('123'), ...DEFAULT_OUT },
-      ],
-      ['sets an init', { init: false }, { init: false, ...DEFAULT_OUT }],
-      ['sets an did', { did: 'specialID123' }, { did: 'specialID123', ...DEFAULT_OUT }],
-      [
-        'overwrites user did with custom did',
-        { did: 'custom-did', user: { id: 'user-id' } },
-        { did: 'custom-did', ...DEFAULT_OUT },
-      ],
-      ['sets a started time', { started: 1622481662 }, { started: new Date(1622481662).toISOString(), ...DEFAULT_OUT }],
+      ['sets an ip address', { user: { ip_address: '0.0.0.0' } }, { attrs: { ip_address: '0.0.0.0' } }],
+      ['sets a did', { user: { id: 'specialID123' } }, { did: 'specialID123' }],
+      ['sets a timestamp', { timestamp: time }, { timestamp: new Date(time * 1000).toISOString() }],
+      ['sets a sid', { sid: '99705f22a3f1468e95ba7386e84691aa' }, { sid: '99705f22a3f1468e95ba7386e84691aa' }],
+      ['requires custom sid to be of certain length', { sid: '123' }, { sid: expect.not.stringMatching('123') }],
+      ['requires custom sid to be of certain length', { sid: '123' }, { sid: expect.not.stringMatching('123') }],
+      ['sets an init', { init: false }, { init: false }],
+      ['sets an did', { did: 'specialID123' }, { did: 'specialID123' }],
+      ['overwrites user did with custom did', { did: 'custom-did', user: { id: 'user-id' } }, { did: 'custom-did' }],
+      ['sets a started time', { started: time }, { started: new Date(time * 1000).toISOString() }],
       ['does not set a duration for browser env', { isBrowser: true }, { duration: undefined }],
       ['sets a duration', { duration: 12000 }, { duration: 12000 }],
       ['does not use custom duration for browser env', { duration: 12000, isBrowser: true }, { duration: undefined }],
@@ -77,26 +58,30 @@ describe('Session', () => {
       [
         'sets a release',
         { release: 'f1557994979ecd969963f53c27ca946379d721f3' },
-        { attrs: { release: 'f1557994979ecd969963f53c27ca946379d721f3' }, ...DEFAULT_OUT },
+        { attrs: { release: 'f1557994979ecd969963f53c27ca946379d721f3' } },
       ],
-      ['sets an environment', { environment: 'staging' }, { attrs: { environment: 'staging' }, ...DEFAULT_OUT }],
-      ['sets an ipAddress', { ipAddress: '0.0.0.0' }, { attrs: { ip_address: '0.0.0.0' }, ...DEFAULT_OUT }],
+      ['sets an environment', { environment: 'staging' }, { attrs: { environment: 'staging' } }],
+      ['sets an ipAddress', { ipAddress: '0.0.0.0' }, { attrs: { ip_address: '0.0.0.0' } }],
       [
         'overwrites user ip_address did with custom ipAddress',
         { ipAddress: '0.0.0.0', user: { ip_address: '1.1.1.1' } },
-        { attrs: { ip_address: '0.0.0.0' }, ...DEFAULT_OUT },
+        { attrs: { ip_address: '0.0.0.0' } },
       ],
-      ['sets an userAgent', { userAgent: 'Mozilla/5.0' }, { attrs: { user_agent: 'Mozilla/5.0' }, ...DEFAULT_OUT }],
-      ['sets errors', { errors: 3 }, { errors: 3, ...DEFAULT_OUT }],
-      ['sets status', { status: SessionStatus.Crashed }, { status: SessionStatus.Crashed, ...DEFAULT_OUT }],
+      ['sets an userAgent', { userAgent: 'Mozilla/5.0' }, { attrs: { user_agent: 'Mozilla/5.0' } }],
+      ['sets errors', { errors: 3 }, { errors: 3 }],
+      ['sets status', { status: SessionStatus.Crashed }, { status: SessionStatus.Crashed }],
     ];
 
     test.each(table)('%s', (...test) => {
+      // duration and timestamp can vary after session update, so let's expect anything unless
+      // the out variable in a test explicitly refers to it.
+      const DEFAULT_OUT = { duration: expect.any(Number), timestamp: expect.any(String) };
+
       const session = new Session();
       const initSessionProps = session.toJSON();
 
       session.update(test[1]);
-      expect(session.toJSON()).toEqual({ ...initSessionProps, ...test[2] });
+      expect(session.toJSON()).toEqual({ ...initSessionProps, ...DEFAULT_OUT, ...test[2] });
     });
   });
 

From cdf1c8746646e9754ab38dcecc2bd7c73387816c Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Mon, 31 May 2021 22:51:19 -0400
Subject: [PATCH 8/9] fix linter

---
 packages/hub/test/session.test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/hub/test/session.test.ts b/packages/hub/test/session.test.ts
index fcf0764fbcbd..7c4578a5fe3d 100644
--- a/packages/hub/test/session.test.ts
+++ b/packages/hub/test/session.test.ts
@@ -1,7 +1,7 @@
 import { SessionContext, SessionStatus } from '@sentry/types';
+import { timestampInSeconds } from '@sentry/utils';
 
 import { Session } from '../src/session';
-import { timestampInSeconds } from '@sentry/utils';
 
 describe('Session', () => {
   it('initializes with the proper defaults', () => {

From 9ba5968d26b4c779e84ff0875113e974bfb08c43 Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Tue, 1 Jun 2021 07:42:27 -0400
Subject: [PATCH 9/9] fix: Address PR comments

* ref: Switch from isBrowser -> ignoreDuration
* test: Add sanity check test for sec to ms conversion
* ref: Update session.close tests to assert on session
  instead of spying on implementation
---
 packages/browser/src/sdk.ts       |  8 ++++--
 packages/hub/src/session.ts       | 12 +++------
 packages/hub/test/session.test.ts | 42 +++++++++++++++----------------
 packages/types/src/session.ts     |  3 +--
 4 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts
index acbdefbcbc06..63657517907e 100644
--- a/packages/browser/src/sdk.ts
+++ b/packages/browser/src/sdk.ts
@@ -199,7 +199,11 @@ function startSessionTracking(): void {
     return;
   }
 
-  hub.startSession({ isBrowser: true });
+  // The session duration for browser sessions does not track a meaningful
+  // concept that can be used as a metric.
+  // Automatically captured sessions are akin to page views, and thus we
+  // discard their duration.
+  hub.startSession({ ignoreDuration: true });
   hub.captureSession();
 
   // We want to create a session for every navigation as well
@@ -209,7 +213,7 @@ function startSessionTracking(): void {
       if (from === undefined || from === to) {
         return;
       }
-      hub.startSession({ isBrowser: true });
+      hub.startSession({ ignoreDuration: true });
       hub.captureSession();
     },
     type: 'history',
diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts
index bd1c7d509410..0de919e43a65 100644
--- a/packages/hub/src/session.ts
+++ b/packages/hub/src/session.ts
@@ -28,7 +28,7 @@ export class Session implements SessionInterface {
   public environment?: string;
   public ipAddress?: string;
   public init: boolean = true;
-  public isBrowser: boolean = false;
+  public ignoreDuration: boolean = false;
 
   public constructor(context?: Omit<SessionContext, 'started' | 'status'>) {
     // Both timestamp and started are in seconds since the UNIX epoch.
@@ -54,8 +54,8 @@ export class Session implements SessionInterface {
     }
 
     this.timestamp = context.timestamp || timestampInSeconds();
-    if (context.isBrowser) {
-      this.isBrowser = context.isBrowser;
+    if (context.ignoreDuration) {
+      this.ignoreDuration = context.ignoreDuration;
     }
     if (context.sid) {
       // Good enough uuid validation. — Kamil
@@ -70,11 +70,7 @@ export class Session implements SessionInterface {
     if (typeof context.started === 'number') {
       this.started = context.started;
     }
-    // The session duration for browser sessions does not track a meaningful
-    // concept that can be used as a metric.
-    // Automatically captured sessions are akin to page views, and thus we
-    // discard their duration.
-    if (this.isBrowser) {
+    if (this.ignoreDuration) {
       this.duration = undefined;
     } else if (typeof context.duration === 'number') {
       this.duration = context.duration;
diff --git a/packages/hub/test/session.test.ts b/packages/hub/test/session.test.ts
index 7c4578a5fe3d..a25f1c10fdf5 100644
--- a/packages/hub/test/session.test.ts
+++ b/packages/hub/test/session.test.ts
@@ -7,24 +7,23 @@ describe('Session', () => {
   it('initializes with the proper defaults', () => {
     const session = new Session().toJSON();
 
-    const sessionStartTime = session.timestamp;
-
+    // Grab current year to check if we are converting from sec -> ms correctly
+    const currentYear = new Date(timestampInSeconds() * 1000).toISOString().slice(0, 4);
     expect(session).toEqual({
       attrs: {},
       duration: 0,
       errors: 0,
       init: true,
       sid: expect.any(String),
-      started: expect.any(String),
+      started: expect.stringMatching(currentYear),
       status: SessionStatus.Ok,
-      timestamp: expect.any(String),
+      timestamp: expect.stringMatching(currentYear),
     });
 
     expect(session.sid).toHaveLength(32);
 
     // started and timestamp should be the same on creation
-    expect(session.started).toEqual(sessionStartTime);
-    expect(session.timestamp).toEqual(sessionStartTime);
+    expect(session.started).toEqual(session.timestamp);
   });
 
   describe('update', () => {
@@ -41,13 +40,16 @@ describe('Session', () => {
       ['sets an did', { did: 'specialID123' }, { did: 'specialID123' }],
       ['overwrites user did with custom did', { did: 'custom-did', user: { id: 'user-id' } }, { did: 'custom-did' }],
       ['sets a started time', { started: time }, { started: new Date(time * 1000).toISOString() }],
-      ['does not set a duration for browser env', { isBrowser: true }, { duration: undefined }],
+      ['does not set a duration for browser env', { ignoreDuration: true }, { duration: undefined }],
       ['sets a duration', { duration: 12000 }, { duration: 12000 }],
-      ['does not use custom duration for browser env', { duration: 12000, isBrowser: true }, { duration: undefined }],
+      [
+        'does not use custom duration for browser env',
+        { duration: 12000, ignoreDuration: true },
+        { duration: undefined },
+      ],
       [
         'does not set a negative duration',
         { timestamp: 10, started: 100 },
-        // TODO(abhi): What should the behaviour here be?
         { duration: 0, timestamp: expect.any(String), started: expect.any(String) },
       ],
       [
@@ -88,30 +90,26 @@ describe('Session', () => {
   describe('close', () => {
     it('exits a normal session', () => {
       const session = new Session();
-      const mockUpdate = jest.spyOn(session, 'update');
-      expect(mockUpdate).toHaveBeenCalledTimes(0);
+      expect(session.status).toEqual(SessionStatus.Ok);
       session.close();
-      expect(mockUpdate).toHaveBeenCalledTimes(1);
-      expect(mockUpdate).toHaveBeenLastCalledWith({ status: SessionStatus.Exited });
+      expect(session.status).toEqual(SessionStatus.Exited);
     });
 
     it('updates session status when give status', () => {
       const session = new Session();
-      const mockUpdate = jest.spyOn(session, 'update');
+      expect(session.status).toEqual(SessionStatus.Ok);
 
-      session.close(SessionStatus.Crashed);
-      expect(mockUpdate).toHaveBeenCalledTimes(1);
-      expect(mockUpdate).toHaveBeenLastCalledWith({ status: SessionStatus.Crashed });
+      session.close(SessionStatus.Abnormal);
+      expect(session.status).toEqual(SessionStatus.Abnormal);
     });
 
-    it('only changes status ok', () => {
+    it('only changes status ok to exited', () => {
       const session = new Session();
-      session.status = SessionStatus.Abnormal;
-      const mockUpdate = jest.spyOn(session, 'update');
+      session.update({ status: SessionStatus.Crashed });
+      expect(session.status).toEqual(SessionStatus.Crashed);
 
       session.close();
-      expect(mockUpdate).toHaveBeenCalledTimes(1);
-      expect(mockUpdate).toHaveBeenLastCalledWith();
+      expect(session.status).toEqual(SessionStatus.Crashed);
     });
   });
 });
diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts
index 0e7f93dc539b..9b53772b4918 100644
--- a/packages/types/src/session.ts
+++ b/packages/types/src/session.ts
@@ -52,8 +52,7 @@ export interface SessionContext {
   ipAddress?: string;
   errors?: number;
   user?: User | null;
-  // Sessions are handled differently in browser environments
-  isBrowser?: boolean;
+  ignoreDuration?: boolean;
 }
 
 /**