Merge branch 'master' of github.com:PostHog/posthog-js

macobo · macobo · commit dbd65003e148 · 2020-12-02T10:08:05.000+02:00
diff --git a/src/__tests__/capture-metrics.js b/src/__tests__/capture-metrics.js
@@ -0,0 +1,92 @@
+import { CaptureMetrics } from '../capture-metrics'
+
+import { _ } from '../utils'
+
+jest.mock('../utils')
+
+describe('CaptureMetrics()', () => {
+    given('captureMetrics', () => new CaptureMetrics(given.enabled, given.capture, given.getTime))
+
+    given('enabled', () => true)
+    given('capture', () => jest.fn())
+
+    given('getTime', () => jest.fn())
+
+    describe('incr() and decr()', () => {
+        it('supports incrementing and decrementing metrics', () => {
+            given.captureMetrics.incr('key')
+            given.captureMetrics.incr('key2')
+            given.captureMetrics.incr('key', 3)
+            given.captureMetrics.decr('key')
+
+            expect(given.captureMetrics.metrics).toEqual({ 'phjs-key': 3, 'phjs-key2': 1 })
+        })
+
+        it('does nothing when not enabled', () => {
+            given('enabled', () => false)
+
+            given.captureMetrics.incr('key')
+
+            expect(given.captureMetrics.metrics).toEqual({})
+        })
+    })
+
+    describe('tracking requests', () => {
+        beforeEach(() => {
+            let i = 0
+            _.UUID.mockImplementation(() => i++)
+        })
+
+        it('handles starting and finishing a request', () => {
+            given.getTime.mockReturnValue(5000)
+            const id = given.captureMetrics.startRequest({ size: 123 })
+
+            given.getTime.mockReturnValue(5100)
+            const payload = given.captureMetrics.finishRequest(id)
+
+            expect(id).toEqual(0)
+            expect(payload).toEqual({ size: 123, duration: 100 })
+        })
+
+        it('handles marking a request as failed', () => {
+            given.captureMetrics.markRequestFailed({ foo: 'bar' })
+
+            expect(given.capture).toHaveBeenCalledWith('$capture_failed_request', { foo: 'bar' })
+        })
+
+        it('handles marking all in-flight requests as failed', () => {
+            given.getTime.mockReturnValue(5000)
+            given.captureMetrics.startRequest({ size: 100 })
+
+            given.getTime.mockReturnValue(5100)
+            given.captureMetrics.startRequest({ size: 200 })
+
+            given.getTime.mockReturnValue(5500)
+
+            given.captureMetrics.captureInProgressRequests()
+
+            expect(given.capture).toHaveBeenCalledTimes(2)
+            expect(given.capture).toHaveBeenCalledWith('$capture_failed_request', {
+                size: 100,
+                duration: 500,
+                type: 'inflight_at_unload',
+            })
+            expect(given.capture).toHaveBeenCalledWith('$capture_failed_request', {
+                size: 200,
+                duration: 400,
+                type: 'inflight_at_unload',
+            })
+        })
+
+        it('does nothing if not enabled', () => {
+            given('enabled', () => false)
+
+            given.captureMetrics.startRequest({ size: 100 })
+            given.captureMetrics.captureInProgressRequests()
+            given.captureMetrics.markRequestFailed({ foo: 'bar' })
+            given.captureMetrics.finishRequest(null)
+
+            expect(given.capture).not.toHaveBeenCalled()
+        })
+    })
+})
diff --git a/src/__tests__/extensions/sessionrecording.js b/src/__tests__/extensions/sessionrecording.js
@@ -99,6 +99,7 @@ describe('SessionRecording', () => {
                     compression: 'lz64',
                     _noTruncate: true,
                     _batchKey: 'sessionRecording',
+                    _metrics: expect.anything(),
                 }
             )
             expect(given.posthog.capture).toHaveBeenCalledWith(
@@ -114,6 +115,7 @@ describe('SessionRecording', () => {
                     compression: 'lz64',
                     _noTruncate: true,
                     _batchKey: 'sessionRecording',
+                    _metrics: expect.anything(),
                 }
             )
         })
diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js
@@ -1,4 +1,5 @@
 import { PostHogLib } from '../posthog-core'
+import { CaptureMetrics } from '../capture-metrics'
 import { _ } from '../utils'
 
 given('lib', () => Object.assign(new PostHogLib(), given.overrides))
@@ -111,9 +112,7 @@ describe('capture()', () => {
             properties: jest.fn(),
         },
         compression: {},
-        _captureMetrics: {
-            incr: jest.fn(),
-        },
+        _captureMetrics: new CaptureMetrics(),
         __captureHooks: [],
     }))
 
diff --git a/src/capture-metrics.js b/src/capture-metrics.js
@@ -1,20 +1,59 @@
+import { _ } from './utils'
+
 export class CaptureMetrics {
-    constructor(capture) {
+    constructor(enabled, capture, getTime = () => new Date().getTime()) {
+        this.enabled = enabled
         this.capture = capture
+        this.getTime = getTime
         this.metrics = {}
+        this.requests = {}
     }
 
     incr(key, by = 1) {
-        if (this.capture) {
+        if (this.enabled) {
             key = `phjs-${key}`
             this.metrics[key] = (this.metrics[key] || 0) + by
         }
     }
 
     decr(key) {
-        if (this.capture) {
+        if (this.enabled) {
             key = `phjs-${key}`
             this.metrics[key] = (this.metrics[key] || 0) - 1
         }
     }
+
+    startRequest(payload) {
+        if (this.enabled) {
+            const requestId = _.UUID()
+
+            this.requests[requestId] = [this.getTime(), payload]
+
+            return requestId
+        }
+    }
+
+    finishRequest(requestId) {
+        if (this.enabled) {
+            const [startTime, payload] = this.requests[requestId]
+            payload['duration'] = this.getTime() - startTime
+            delete this.requests[requestId]
+            return payload
+        }
+    }
+
+    markRequestFailed(payload) {
+        if (this.enabled) {
+            this.capture('$capture_failed_request', payload)
+        }
+    }
+
+    captureInProgressRequests() {
+        if (this.enabled) {
+            Object.keys(this.requests).forEach((requestId) => {
+                const payload = this.finishRequest(requestId)
+                this.markRequestFailed({ ...payload, type: 'inflight_at_unload' })
+            })
+        }
+    }
 }
diff --git a/src/extensions/sessionrecording.js b/src/extensions/sessionrecording.js
@@ -88,6 +88,9 @@ export class SessionRecording {
             compression: 'lz64', // Force lz64 even if /decide endpoint has not yet responded
             _noTruncate: true,
             _batchKey: 'sessionRecording',
+            _metrics: {
+                rrweb_full_snapshot: properties.$snapshot_data.type === 2,
+            },
         })
     }
 }
diff --git a/src/posthog-core.js b/src/posthog-core.js
@@ -215,7 +215,8 @@ PostHogLib.prototype._init = function (token, config, name) {
 
     this['_jsc'] = function () {}
 
-    this._captureMetrics = new CaptureMetrics(this.get_config('_capture_metrics'))
+    this._captureMetrics = new CaptureMetrics(this.get_config('_capture_metrics'), _.bind(this.capture, this))
+
     this._requestQueue = new RequestQueue(this._captureMetrics, _.bind(this._handle_queued_event, this))
     this.__captureHooks = []
     this.__request_queue = []
@@ -326,6 +327,7 @@ PostHogLib.prototype._handle_unload = function () {
     if (this.get_config('_capture_metrics')) {
         this._requestQueue.updateUnloadMetrics()
         this.capture('$capture_metrics', this._captureMetrics.metrics)
+        this._captureMetrics.captureInProgressRequests()
     }
     this._requestQueue.unload()
 }
@@ -368,12 +370,15 @@ PostHogLib.prototype._send_request = function (url, data, options, callback) {
     var useSendBeacon = window.navigator.sendBeacon && options.transport.toLowerCase() === 'sendbeacon'
     var use_post = useSendBeacon || options.method === 'POST'
 
-    const classifier = data.length > 1000 ? 'large' : 'small'
     this._captureMetrics.incr('_send_request')
-    this._captureMetrics.incr(`_send_request_${options.transport}`)
-    this._captureMetrics.incr(`_send_request_${classifier}`)
     this._captureMetrics.incr('_send_request_inflight')
 
+    const requestId = this._captureMetrics.startRequest({
+        size: data.length,
+        endpoint: url.slice(url.length - 2),
+        ...options._metrics,
+    })
+
     // needed to correctly format responses
     var verbose_mode = this.get_config('verbose')
     if (data['verbose']) {
@@ -451,8 +456,10 @@ PostHogLib.prototype._send_request = function (url, data, options, callback) {
                 if (req.readyState === 4) {
                     this._captureMetrics.incr(`xhr-response`)
                     this._captureMetrics.incr(`xhr-response-${req.status}`)
-                    this._captureMetrics.incr(`xhr-done-${classifier}-${req.status}`)
                     this._captureMetrics.decr('_send_request_inflight')
+
+                    const data = this._captureMetrics.finishRequest(requestId)
+
                     // XMLHttpRequest.DONE == 4, except in safari 4
                     if (req.status === 200) {
                         if (callback) {
@@ -468,6 +475,14 @@ PostHogLib.prototype._send_request = function (url, data, options, callback) {
                     } else {
                         var error = 'Bad HTTP status: ' + req.status + ' ' + req.statusText
                         console.error(error)
+
+                        this._captureMetrics.markRequestFailed({
+                            ...data,
+                            type: 'non_200',
+                            status: req.status,
+                            statusText: req.statusText,
+                        })
+
                         if (callback) {
                             if (verbose_mode) {
                                 callback({ status: 0, error: error })
diff --git a/src/request-queue.js b/src/request-queue.js
@@ -98,7 +98,14 @@ export class RequestQueue {
         _.each(this._event_queue, (request) => {
             const { url, data, options } = request
             const key = (options ? options._batchKey : null) || url
-            if (requests[key] === undefined) requests[key] = { data: [], url, options }
+            if (requests[key] === undefined) {
+                requests[key] = { data: [], url, options }
+            }
+
+            // :TRICKY: Metrics-only code
+            if (options && requests[key].options && requests[key].options._metrics) {
+                requests[key].options._metrics['rrweb_full_snapshot'] ||= options._metrics['rrweb_full_snapshot']
+            }
             requests[key].data.push(data)
         })
         return requests

Original file line number	Diff line number	Diff line change
`@@ -99,6 +99,7 @@ describe('SessionRecording', () => {`
`99`	`99`	`compression: 'lz64',`
`100`	`100`	`_noTruncate: true,`
`101`	`101`	`_batchKey: 'sessionRecording',`
	`102`	`+ _metrics: expect.anything(),`
`102`	`103`	`}`
`103`	`104`	`)`
`104`	`105`	`expect(given.posthog.capture).toHaveBeenCalledWith(`
`@@ -114,6 +115,7 @@ describe('SessionRecording', () => {`
`114`	`115`	`compression: 'lz64',`
`115`	`116`	`_noTruncate: true,`
`116`	`117`	`_batchKey: 'sessionRecording',`
	`118`	`+ _metrics: expect.anything(),`
`117`	`119`	`}`
`118`	`120`	`)`
`119`	`121`	`})`
Original file line number	Diff line number	Diff line change
`@@ -88,6 +88,9 @@ export class SessionRecording {`
`88`	`88`	`compression: 'lz64', // Force lz64 even if /decide endpoint has not yet responded`
`89`	`89`	`_noTruncate: true,`
`90`	`90`	`_batchKey: 'sessionRecording',`
	`91`	`+ _metrics: {`
	`92`	`+ rrweb_full_snapshot: properties.$snapshot_data.type === 2,`
	`93`	`+ },`
`91`	`94`	`})`
`92`	`95`	`}`
`93`	`96`	`}`