From 330e471a176da6d1aa3d8bbd000315470e35c85b Mon Sep 17 00:00:00 2001 From: vigneshshanmugam Date: Thu, 16 Apr 2020 17:46:23 +0200 Subject: [PATCH 1/3] feat(rum-core): add network info for all transactions --- packages/rum-core/src/common/utils.js | 22 ++++++++++++++-- .../src/error-logging/error-logging.js | 2 +- packages/rum-core/test/common/context.spec.js | 26 +++++++++++++------ .../test/error-logging/error-logging.spec.js | 3 ++- 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/packages/rum-core/src/common/utils.js b/packages/rum-core/src/common/utils.js index bf2631f73..62e863f9f 100644 --- a/packages/rum-core/src/common/utils.js +++ b/packages/rum-core/src/common/utils.js @@ -200,11 +200,29 @@ function getTimeOrigin() { return PERF.timing.fetchStart } -function getPageMetadata() { +function getNetworkInformation(captureNetInfo) { + const connection = window.navigator && window.navigator.connection + if (!captureNetInfo || typeof connection !== 'object') { + return undefined + } + /** + * Ignoring `type` and `downlinkMax` as they are only + * supported on Chrome OS + */ + return { + downlink: connection.downlink, + effective_type: connection.effectiveType, + rtt: connection.rtt, + save_data: !!connection.saveData + } +} + +function getPageMetadata(captureNetInfo = true) { return { page: { referer: document.referrer, - url: window.location.href + url: window.location.href, + netinfo: getNetworkInformation(captureNetInfo) } } } diff --git a/packages/rum-core/src/error-logging/error-logging.js b/packages/rum-core/src/error-logging/error-logging.js index 7eaf728d3..f3cbd4722 100644 --- a/packages/rum-core/src/error-logging/error-logging.js +++ b/packages/rum-core/src/error-logging/error-logging.js @@ -118,7 +118,7 @@ class ErrorLogging { : {} // eslint-disable-next-line no-unused-vars const { tags, ...configContext } = this._configService.get('context') - const pageMetadata = getPageMetadata() + const pageMetadata = getPageMetadata(false) const context = merge( {}, diff --git a/packages/rum-core/test/common/context.spec.js b/packages/rum-core/test/common/context.spec.js index cc93d18da..1b6dc0af9 100644 --- a/packages/rum-core/test/common/context.spec.js +++ b/packages/rum-core/test/common/context.spec.js @@ -182,12 +182,25 @@ describe('Context', () => { message: 'test' } } - addTransactionContext(transaction, configContext) - expect(transaction.context).toEqual({ + + const pageContext = { page: { referer: jasmine.any(String), - url: jasmine.any(String) - }, + url: jasmine.any(String), + netinfo: + { + downlink: jasmine.any(Number), + effective_type: jasmine.any(String), + rtt: jasmine.any(Number), + save_data: jasmine.any(Boolean) + } || undefined + } + } + + addTransactionContext(transaction, configContext) + + expect(transaction.context).toEqual({ + ...pageContext, ...userContext, ...trContext }) @@ -197,10 +210,7 @@ describe('Context', () => { pageloadTr.end() addTransactionContext(pageloadTr, configContext) expect(pageloadTr.context).toEqual({ - page: { - referer: jasmine.any(String), - url: jasmine.any(String) - }, + ...pageContext, response: { transfer_size: 26941, encoded_body_size: 105297, diff --git a/packages/rum-core/test/error-logging/error-logging.spec.js b/packages/rum-core/test/error-logging/error-logging.spec.js index 0529b97f6..681096f20 100644 --- a/packages/rum-core/test/error-logging/error-logging.spec.js +++ b/packages/rum-core/test/error-logging/error-logging.spec.js @@ -171,7 +171,8 @@ describe('ErrorLogging', function() { jasmine.objectContaining({ page: { referer: jasmine.any(String), - url: jasmine.any(String) + url: jasmine.any(String), + netinfo: undefined }, managed: true, dummy: { From 5f8b2603fec5387fc80d63ea45d48dc4f641a37b Mon Sep 17 00:00:00 2001 From: vigneshshanmugam Date: Thu, 16 Apr 2020 23:05:27 +0200 Subject: [PATCH 2/3] chore: add custom matcher --- packages/rum-core/src/common/utils.js | 16 ++++---- packages/rum-core/test/common/context.spec.js | 40 +++++++++++++------ .../test/error-logging/error-logging.spec.js | 3 +- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/packages/rum-core/src/common/utils.js b/packages/rum-core/src/common/utils.js index 62e863f9f..d32332c18 100644 --- a/packages/rum-core/src/common/utils.js +++ b/packages/rum-core/src/common/utils.js @@ -203,7 +203,7 @@ function getTimeOrigin() { function getNetworkInformation(captureNetInfo) { const connection = window.navigator && window.navigator.connection if (!captureNetInfo || typeof connection !== 'object') { - return undefined + return null } /** * Ignoring `type` and `downlinkMax` as they are only @@ -218,13 +218,15 @@ function getNetworkInformation(captureNetInfo) { } function getPageMetadata(captureNetInfo = true) { - return { - page: { - referer: document.referrer, - url: window.location.href, - netinfo: getNetworkInformation(captureNetInfo) - } + const context = { + referer: document.referrer, + url: window.location.href + } + const networkInfo = getNetworkInformation(captureNetInfo) + if (networkInfo != null) { + context.netinfo = networkInfo } + return { page: context } } function stripQueryStringFromUrl(url) { diff --git a/packages/rum-core/test/common/context.spec.js b/packages/rum-core/test/common/context.spec.js index 1b6dc0af9..b0a1d956f 100644 --- a/packages/rum-core/test/common/context.spec.js +++ b/packages/rum-core/test/common/context.spec.js @@ -166,6 +166,24 @@ describe('Context', () => { }) it('should enrich transaction with context info based on type', () => { + /** + * Custom matcher to check if netinfo can either be undefined or + * match object with required fields set on browsers that supports + * connection information + */ + jasmine.addMatchers({ + toEqualOrUndefined: () => ({ + compare: (actual, expected) => { + const result = {} + if (typeof actual === 'undefined') { + result.pass = true + } else { + result.pass = jasmine.matchersUtil.equals(actual, expected) + } + return result + } + }) + }) const transaction = new Transaction('test', 'custom') const trContext = { tags: { tag1: 'tag1' } } transaction.addContext(trContext) @@ -182,23 +200,14 @@ describe('Context', () => { message: 'test' } } - const pageContext = { - page: { + page: jasmine.objectContaining({ referer: jasmine.any(String), - url: jasmine.any(String), - netinfo: - { - downlink: jasmine.any(Number), - effective_type: jasmine.any(String), - rtt: jasmine.any(Number), - save_data: jasmine.any(Boolean) - } || undefined - } + url: jasmine.any(String) + }) } addTransactionContext(transaction, configContext) - expect(transaction.context).toEqual({ ...pageContext, ...userContext, @@ -221,7 +230,12 @@ describe('Context', () => { }, ...userContext }) - + expect(pageloadTr.context.page.netinfo).toEqualOrUndefined({ + downlink: jasmine.any(Number), + effective_type: jasmine.any(String), + rtt: jasmine.any(Number), + save_data: jasmine.any(Boolean) + }) unmock() }) }) diff --git a/packages/rum-core/test/error-logging/error-logging.spec.js b/packages/rum-core/test/error-logging/error-logging.spec.js index 681096f20..0529b97f6 100644 --- a/packages/rum-core/test/error-logging/error-logging.spec.js +++ b/packages/rum-core/test/error-logging/error-logging.spec.js @@ -171,8 +171,7 @@ describe('ErrorLogging', function() { jasmine.objectContaining({ page: { referer: jasmine.any(String), - url: jasmine.any(String), - netinfo: undefined + url: jasmine.any(String) }, managed: true, dummy: { From 8d2902b182166174698681b597bc85cd77903dba Mon Sep 17 00:00:00 2001 From: vigneshshanmugam Date: Fri, 17 Apr 2020 10:13:03 +0200 Subject: [PATCH 3/3] chore: add netinfo to errors --- dev-utils/jasmine.js | 18 ++++++++++ packages/rum-core/src/common/utils.js | 13 +++---- .../src/error-logging/error-logging.js | 2 +- packages/rum-core/test/common/context.spec.js | 20 ++--------- .../test/error-logging/error-logging.spec.js | 35 +++++++++++-------- 5 files changed, 48 insertions(+), 40 deletions(-) diff --git a/dev-utils/jasmine.js b/dev-utils/jasmine.js index 21f473fbd..80eb36b1f 100644 --- a/dev-utils/jasmine.js +++ b/dev-utils/jasmine.js @@ -66,3 +66,21 @@ export function describeIf(description, specDefinitions, condition) { return describeFn.apply(this, [description, specDefinitions]) } + +/** + * Custom Jasmine matcher to check if given field in a object can either be + * undefined or match object deeply + */ +export function toEqualOrUndefined() { + return { + compare: (actual, expected) => { + const result = {} + if (typeof actual === 'undefined') { + result.pass = true + } else { + result.pass = jasmine.matchersUtil.equals(actual, expected) + } + return result + } + } +} diff --git a/packages/rum-core/src/common/utils.js b/packages/rum-core/src/common/utils.js index d32332c18..290269c2d 100644 --- a/packages/rum-core/src/common/utils.js +++ b/packages/rum-core/src/common/utils.js @@ -200,10 +200,11 @@ function getTimeOrigin() { return PERF.timing.fetchStart } -function getNetworkInformation(captureNetInfo) { - const connection = window.navigator && window.navigator.connection - if (!captureNetInfo || typeof connection !== 'object') { - return null +function getNetworkInformation() { + const connection = navigator && navigator.connection + + if (!(connection && typeof connection === 'object')) { + return } /** * Ignoring `type` and `downlinkMax` as they are only @@ -217,12 +218,12 @@ function getNetworkInformation(captureNetInfo) { } } -function getPageMetadata(captureNetInfo = true) { +function getPageMetadata() { const context = { referer: document.referrer, url: window.location.href } - const networkInfo = getNetworkInformation(captureNetInfo) + const networkInfo = getNetworkInformation() if (networkInfo != null) { context.netinfo = networkInfo } diff --git a/packages/rum-core/src/error-logging/error-logging.js b/packages/rum-core/src/error-logging/error-logging.js index f3cbd4722..7eaf728d3 100644 --- a/packages/rum-core/src/error-logging/error-logging.js +++ b/packages/rum-core/src/error-logging/error-logging.js @@ -118,7 +118,7 @@ class ErrorLogging { : {} // eslint-disable-next-line no-unused-vars const { tags, ...configContext } = this._configService.get('context') - const pageMetadata = getPageMetadata(false) + const pageMetadata = getPageMetadata() const context = merge( {}, diff --git a/packages/rum-core/test/common/context.spec.js b/packages/rum-core/test/common/context.spec.js index b0a1d956f..6d88d92bb 100644 --- a/packages/rum-core/test/common/context.spec.js +++ b/packages/rum-core/test/common/context.spec.js @@ -28,6 +28,7 @@ import resourceEntries from '../fixtures/resource-entries' import Span from '../../src/performance-monitoring/span' import Transaction from '../../src/performance-monitoring/transaction' import { PAGE_LOAD } from '../../src/common/constants' +import { toEqualOrUndefined } from '../../../../dev-utils/jasmine' import { mockGetEntriesByType } from '../utils/globals-mock' describe('Context', () => { @@ -166,24 +167,7 @@ describe('Context', () => { }) it('should enrich transaction with context info based on type', () => { - /** - * Custom matcher to check if netinfo can either be undefined or - * match object with required fields set on browsers that supports - * connection information - */ - jasmine.addMatchers({ - toEqualOrUndefined: () => ({ - compare: (actual, expected) => { - const result = {} - if (typeof actual === 'undefined') { - result.pass = true - } else { - result.pass = jasmine.matchersUtil.equals(actual, expected) - } - return result - } - }) - }) + jasmine.addMatchers({ toEqualOrUndefined }) const transaction = new Transaction('test', 'custom') const trContext = { tags: { tag1: 'tag1' } } transaction.addContext(trContext) diff --git a/packages/rum-core/test/error-logging/error-logging.spec.js b/packages/rum-core/test/error-logging/error-logging.spec.js index 0529b97f6..80da1089d 100644 --- a/packages/rum-core/test/error-logging/error-logging.spec.js +++ b/packages/rum-core/test/error-logging/error-logging.spec.js @@ -26,6 +26,7 @@ import { createServiceFactory, createCustomEvent } from '../' import { ERRORS } from '../../src/common/constants' import { getGlobalConfig } from '../../../../dev-utils/test-config' +import { toEqualOrUndefined } from '../../../../dev-utils/jasmine' const { agentConfig } = getGlobalConfig('rum-core') @@ -149,6 +150,7 @@ describe('ErrorLogging', function() { } it('should include context info on error', () => { + jasmine.addMatchers({ toEqualOrUndefined }) const transaction = transactionService.startTransaction('test', 'dummy', { managed: true }) @@ -167,21 +169,24 @@ describe('ErrorLogging', function() { error: new Error(testErrorMessage) } const errorData = errorLogging.createErrorDataModel(errorEvent) - expect(errorData.context).toEqual( - jasmine.objectContaining({ - page: { - referer: jasmine.any(String), - url: jasmine.any(String) - }, - managed: true, - dummy: { - foo: 'bar', - bar: 20 - }, - user: { id: 12, username: 'test' } - }) - ) - transaction.end() + expect(errorData.context).toEqual({ + page: jasmine.objectContaining({ + referer: jasmine.any(String), + url: jasmine.any(String) + }), + managed: true, + dummy: { + foo: 'bar', + bar: 20 + }, + user: { id: 12, username: 'test' } + }) + expect(errorData.context.page.netinfo).toEqualOrUndefined({ + downlink: jasmine.any(Number), + effective_type: jasmine.any(String), + rtt: jasmine.any(Number), + save_data: jasmine.any(Boolean) + }) }) it('should support ErrorEvent', function(done) {