From 242981446a6a03ffe3b9bb8c1de34a4b1f9195e9 Mon Sep 17 00:00:00 2001 From: Alberto Delgado Roda Date: Thu, 10 Feb 2022 16:57:39 +0100 Subject: [PATCH] feat: send events when user leaves the page (#1146) * feat: send events when user leaves the page * chore: add tests for fetch * chore: fix flaky tests * chore: observe page visibility when initialising * chore: set limit to 60 KiB * chore: describe reason behind the unobserve * chore: use existing constants for services * chore: change global state restore strategy * chore: do not process calls to agent endpoints * chore: improve keepalive check * chore: avoid executing keepalive if beforeSend defined * chore: move variables inside function * chore: use customEvent util * chore: remove needless spy --- dev-utils/karma.js | 2 +- dev-utils/webdriver.js | 19 +- packages/rum-core/src/bootstrap.js | 17 -- packages/rum-core/src/common/apm-server.js | 99 +++--- .../rum-core/src/common/config-service.js | 8 + packages/rum-core/src/common/constants.js | 14 +- packages/rum-core/src/common/http/fetch.js | 107 +++++++ .../src/common/http/response-status.js | 33 ++ packages/rum-core/src/common/http/xhr.js | 79 +++++ .../rum-core/src/common/page-visibility.js | 95 ++++++ .../src/common/patching/fetch-patch.js | 3 +- packages/rum-core/src/error-logging/index.js | 8 +- packages/rum-core/src/index.js | 6 +- packages/rum-core/src/opentracing/index.js | 5 +- .../src/performance-monitoring/index.js | 7 +- .../performance-monitoring.js | 16 +- packages/rum-core/src/state.js | 1 + .../test/benchmarks/compress.bench.js | 3 +- .../performance-monitoring.bench.js | 10 +- .../benchmarks/transaction-service.bench.js | 4 +- packages/rum-core/test/bootstrap.spec.js | 73 ++++- .../rum-core/test/common/apm-server.spec.js | 172 ++++++++++- .../rum-core/test/common/compress.spec.js | 5 +- .../rum-core/test/common/http/fetch.spec.js | 198 ++++++++++++ .../test/common/http/response-status.spec.js | 40 +++ .../test/common/page-visibility.spec.js | 288 ++++++++++++++++++ .../test/common/service-factory.spec.js | 10 +- .../test/error-logging/error-logging.spec.js | 13 +- packages/rum-core/test/index.js | 3 +- .../test/opentracing/opentracing.spec.js | 11 +- .../performance-monitoring.spec.js | 71 ++++- packages/rum-core/test/polyfills.js | 1 + .../rum-core/test/utils/apm-server-mock.js | 1 + .../rum-react/src/get-with-transaction.js | 4 +- packages/rum-react/test/e2e/index.js | 5 +- .../test/specs/get-apm-route.spec.js | 12 +- .../test/specs/get-with-transaction.spec.js | 23 +- packages/rum-vue/test/e2e/index.js | 5 +- packages/rum/src/apm-base.js | 21 +- packages/rum/src/index.js | 2 +- packages/rum/test/e2e/index.js | 5 +- packages/rum/test/specs/apm-base.spec.js | 37 ++- packages/rum/test/specs/index.spec.js | 4 +- 43 files changed, 1355 insertions(+), 185 deletions(-) create mode 100644 packages/rum-core/src/common/http/fetch.js create mode 100644 packages/rum-core/src/common/http/response-status.js create mode 100644 packages/rum-core/src/common/http/xhr.js create mode 100644 packages/rum-core/src/common/page-visibility.js create mode 100644 packages/rum-core/test/common/http/fetch.spec.js create mode 100644 packages/rum-core/test/common/http/response-status.spec.js create mode 100644 packages/rum-core/test/common/page-visibility.spec.js diff --git a/dev-utils/karma.js b/dev-utils/karma.js index 546f7c710..4910428b7 100644 --- a/dev-utils/karma.js +++ b/dev-utils/karma.js @@ -39,7 +39,7 @@ const { getWebpackConfig, BUNDLE_TYPES } = require('./build') const polyfills = 'test/polyfills.+(js|ts)' const specPattern = - 'test/{*.spec.+(js|ts),!(e2e|integration|node|bundle|types)/*.spec.+(js|ts)}' + 'test/{*.spec.+(js|ts),!(e2e|integration|node|bundle|types)/**/*.spec.+(js|ts)}' const { tunnelIdentifier } = getSauceConnectOptions() // makes all object properties configurable by default diff --git a/dev-utils/webdriver.js b/dev-utils/webdriver.js index bbb34b44d..9ecd07615 100644 --- a/dev-utils/webdriver.js +++ b/dev-utils/webdriver.js @@ -44,16 +44,21 @@ const logLevels = { const debugMode = false const debugLevel = logLevels.INFO.value -function isLogEntryATestFailure(entry, whitelist) { +function isLogEntryATestFailure(entry, whitelist = []) { var result = false if (logLevels[entry.level].value > logLevels.WARNING.value) { result = true - if (whitelist) { - for (var i = 0, l = whitelist.length; i < l; i++) { - if (entry.message.indexOf(whitelist[i]) !== -1) { - result = false - break - } + + // Chrome's versions lower than 81 had a bug where a preflight request with keepalive specified was not supported + // Bug info: https://bugs.chromium.org/p/chromium/issues/detail?id=835821 + whitelist.push( + 'Preflight request for request with keepalive specified is currently not supported' + ) + + for (var i = 0, l = whitelist.length; i < l; i++) { + if (entry.message.indexOf(whitelist[i]) !== -1) { + result = false + break } } } diff --git a/packages/rum-core/src/bootstrap.js b/packages/rum-core/src/bootstrap.js index 573e0ce63..185c5aa9e 100644 --- a/packages/rum-core/src/bootstrap.js +++ b/packages/rum-core/src/bootstrap.js @@ -31,7 +31,6 @@ let enabled = false export function bootstrap() { if (isPlatformSupported()) { patchAll() - bootstrapPerf() state.bootstrapTime = now() enabled = true } else if (isBrowser) { @@ -44,19 +43,3 @@ export function bootstrap() { return enabled } - -export function bootstrapPerf() { - if (document.visibilityState === 'hidden') { - state.lastHiddenStart = 0 - } - - window.addEventListener( - 'visibilitychange', - () => { - if (document.visibilityState === 'hidden') { - state.lastHiddenStart = performance.now() - } - }, - { capture: true } - ) -} diff --git a/packages/rum-core/src/common/apm-server.js b/packages/rum-core/src/common/apm-server.js index 822b5daf1..4b29f0aef 100644 --- a/packages/rum-core/src/common/apm-server.js +++ b/packages/rum-core/src/common/apm-server.js @@ -26,9 +26,13 @@ import Queue from './queue' import throttle from './throttle' import NDJSON from './ndjson' -import { XHR_IGNORE } from './patching/patch-utils' import { truncateModel, METADATA_MODEL } from './truncate' -import { ERRORS, TRANSACTIONS } from './constants' +import { + ERRORS, + HTTP_REQUEST_TIMEOUT, + QUEUE_FLUSH, + TRANSACTIONS +} from './constants' import { noop } from './utils' import { Promise } from './polyfills' import { @@ -38,6 +42,8 @@ import { compressPayload } from './compress' import { __DEV__ } from '../state' +import { sendFetchRequest, shouldUseFetchWithKeepAlive } from './http/fetch' +import { sendXHR } from './http/xhr' /** * Throttling interval defaults to 60 seconds @@ -75,6 +81,10 @@ class ApmServer { () => this._loggingService.warn('Dropped events due to throttling!'), { limit, interval: THROTTLE_INTERVAL } ) + + this._configService.observeEvent(QUEUE_FLUSH, () => { + this.queue.flush() + }) } _postJson(endPoint, payload) { @@ -124,58 +134,33 @@ class ApmServer { _makeHttpRequest( method, url, - { timeout = 10000, payload, headers, beforeSend } = {} + { timeout = HTTP_REQUEST_TIMEOUT, payload, headers, beforeSend } = {} ) { - return new Promise(function (resolve, reject) { - var xhr = new window.XMLHttpRequest() - xhr[XHR_IGNORE] = true - xhr.open(method, url, true) - xhr.timeout = timeout - - if (headers) { - for (var header in headers) { - if (headers.hasOwnProperty(header)) { - xhr.setRequestHeader(header, headers[header]) - } - } - } - - xhr.onreadystatechange = function () { - if (xhr.readyState === 4) { - const { status, responseText } = xhr - // An http 4xx or 5xx error. Signal an error. - if (status === 0 || (status > 399 && status < 600)) { - reject({ url, status, responseText }) - } else { - resolve(xhr) - } + // This bring the possibility of sending requests that outlive the page. + if (!beforeSend && shouldUseFetchWithKeepAlive(method, payload)) { + return sendFetchRequest(method, url, { + keepalive: true, + timeout, + payload, + headers + }).catch(reason => { + // Chrome, before the version 81 had a bug where a preflight request with keepalive specified was not supported + // xhr will be used as a fallback to cover fetch network errors, more info: https://fetch.spec.whatwg.org/#concept-network-error + // Bug info: https://bugs.chromium.org/p/chromium/issues/detail?id=835821 + if (reason instanceof TypeError) { + return sendXHR(method, url, { timeout, payload, headers, beforeSend }) } - } - xhr.onerror = () => { - const { status, responseText } = xhr - reject({ url, status, responseText }) - } + // bubble other kind of reasons to keep handling the failure + throw reason + }) + } - let canSend = true - if (typeof beforeSend === 'function') { - canSend = beforeSend({ url, method, headers, payload, xhr }) - } - if (canSend) { - xhr.send(payload) - } else { - reject({ - url, - status: 0, - responseText: 'Request rejected by user configuration.' - }) - } - }) + return sendXHR(method, url, { timeout, payload, headers, beforeSend }) } fetchConfig(serviceName, environment) { - const serverUrl = this._configService.get('serverUrl') - var configEndpoint = `${serverUrl}/config/v1/rum/agents` + var { configEndpoint } = this.getEndpoints() if (!serviceName) { return Promise.reject( 'serviceName is required for fetching central config.' @@ -336,10 +321,24 @@ class ApmServer { this.ndjsonTransactions(filteredPayload[TRANSACTIONS], compress) ) const ndjsonPayload = ndjson.join('') + const { intakeEndpoint } = this.getEndpoints() + return this._postJson(intakeEndpoint, ndjsonPayload) + } + + getEndpoints() { + const serverUrl = this._configService.get('serverUrl') + const apiVersion = this._configService.get('apiVersion') const serverUrlPrefix = - cfg.get('serverUrlPrefix') || `/intake/v${apiVersion}/rum/events` - const endPoint = cfg.get('serverUrl') + serverUrlPrefix - return this._postJson(endPoint, ndjsonPayload) + this._configService.get('serverUrlPrefix') || + `/intake/v${apiVersion}/rum/events` + + const intakeEndpoint = serverUrl + serverUrlPrefix + const configEndpoint = `${serverUrl}/config/v1/rum/agents` + + return { + intakeEndpoint, + configEndpoint + } } } diff --git a/packages/rum-core/src/common/config-service.js b/packages/rum-core/src/common/config-service.js index d4a0f6b43..71407b083 100644 --- a/packages/rum-core/src/common/config-service.js +++ b/packages/rum-core/src/common/config-service.js @@ -272,6 +272,14 @@ class Config { storage.setItem(LOCAL_CONFIG_KEY, JSON.stringify(config)) } } + + dispatchEvent(name, args) { + this.events.send(name, args) + } + + observeEvent(name, fn) { + return this.events.observe(name, fn) + } } export default Config diff --git a/packages/rum-core/src/common/constants.js b/packages/rum-core/src/common/constants.js index 223fe46d8..6837cfc06 100644 --- a/packages/rum-core/src/common/constants.js +++ b/packages/rum-core/src/common/constants.js @@ -101,6 +101,8 @@ const TRANSACTION_END = 'transaction:end' * Internal Events */ const CONFIG_CHANGE = 'config:change' +const QUEUE_FLUSH = 'queue:flush' +const QUEUE_ADD_TRANSACTION = 'queue:add_transaction' /** * Events types that are used to toggle auto instrumentations @@ -147,6 +149,7 @@ const TRANSACTIONS = 'transactions' */ const CONFIG_SERVICE = 'ConfigService' const LOGGING_SERVICE = 'LoggingService' +const TRANSACTION_SERVICE = 'TransactionService' const APM_SERVER = 'ApmServer' /** @@ -164,6 +167,11 @@ const KEYWORD_LIMIT = 1024 */ const SESSION_TIMEOUT = 30 * 60000 +/** + * Default http request is set to 10 seconds (in milliseconds) + */ +const HTTP_REQUEST_TIMEOUT = 10000 + export { SCHEDULE, INVOKE, @@ -180,6 +188,8 @@ export { TRANSACTION_START, TRANSACTION_END, CONFIG_CHANGE, + QUEUE_FLUSH, + QUEUE_ADD_TRANSACTION, XMLHTTPREQUEST, FETCH, HISTORY, @@ -204,11 +214,13 @@ export { TRANSACTIONS, CONFIG_SERVICE, LOGGING_SERVICE, + TRANSACTION_SERVICE, APM_SERVER, TRUNCATED_TYPE, FIRST_INPUT, LAYOUT_SHIFT, OUTCOME_SUCCESS, OUTCOME_FAILURE, - SESSION_TIMEOUT + SESSION_TIMEOUT, + HTTP_REQUEST_TIMEOUT } diff --git a/packages/rum-core/src/common/http/fetch.js b/packages/rum-core/src/common/http/fetch.js new file mode 100644 index 000000000..32efbf0f6 --- /dev/null +++ b/packages/rum-core/src/common/http/fetch.js @@ -0,0 +1,107 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +import { HTTP_REQUEST_TIMEOUT } from '../constants' +import { isResponseSuccessful } from './response-status' + +// keepalive flag tends to limit the payload size to 64 KB +// although this size if set, will be up to the user agent +// in order to be conservative we set a limit a little lower than that +export const BYTE_LIMIT = 60 * 1000 + +export function shouldUseFetchWithKeepAlive(method, payload) { + if (!isFetchSupported()) { + return false + } + + const isKeepAliveSupported = 'keepalive' in new Request('') + if (!isKeepAliveSupported) { + return false + } + + const size = calculateSize(payload) + return method === 'POST' && size < BYTE_LIMIT +} + +export function sendFetchRequest( + method, + url, + { keepalive = false, timeout = HTTP_REQUEST_TIMEOUT, payload, headers } +) { + let timeoutConfig = {} + if (typeof AbortController === 'function') { + const controller = new AbortController() + timeoutConfig.signal = controller.signal + setTimeout(() => controller.abort(), timeout) + } + + let fetchResponse + return window + .fetch(url, { + body: payload, + headers, + method, + keepalive, // used to allow the request to outlive the page. + credentials: 'omit', + ...timeoutConfig + }) + .then(response => { + fetchResponse = response + return fetchResponse.text() + }) + .then(responseText => { + const bodyResponse = { + url, + status: fetchResponse.status, + responseText + } + + if (!isResponseSuccessful(fetchResponse.status)) { + throw bodyResponse + } + + return bodyResponse + }) +} + +export function isFetchSupported() { + return ( + typeof window.fetch === 'function' && typeof window.Request === 'function' + ) +} + +function calculateSize(payload) { + if (!payload) { + // IE 11 cannot create Blob from undefined + return 0 + } + + // If the payload is compressed it is going to be already a Blob + if (payload instanceof Blob) { + return payload.size + } + + return new Blob([payload]).size +} diff --git a/packages/rum-core/src/common/http/response-status.js b/packages/rum-core/src/common/http/response-status.js new file mode 100644 index 000000000..42bc2e062 --- /dev/null +++ b/packages/rum-core/src/common/http/response-status.js @@ -0,0 +1,33 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +export function isResponseSuccessful(status) { + // An http 4xx or 5xx error. Signal an error. + if (status === 0 || (status > 399 && status < 600)) { + return false + } + + return true +} diff --git a/packages/rum-core/src/common/http/xhr.js b/packages/rum-core/src/common/http/xhr.js new file mode 100644 index 000000000..c8aab6a11 --- /dev/null +++ b/packages/rum-core/src/common/http/xhr.js @@ -0,0 +1,79 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +import { XHR_IGNORE } from '../patching/patch-utils' +import { isResponseSuccessful } from './response-status' +import { Promise } from '../polyfills' + +export function sendXHR( + method, + url, + { timeout = HTTP_REQUEST_TIMEOUT, payload, headers, beforeSend } +) { + return new Promise(function (resolve, reject) { + var xhr = new window.XMLHttpRequest() + xhr[XHR_IGNORE] = true + xhr.open(method, url, true) + xhr.timeout = timeout + + if (headers) { + for (var header in headers) { + if (headers.hasOwnProperty(header)) { + xhr.setRequestHeader(header, headers[header]) + } + } + } + + xhr.onreadystatechange = function () { + if (xhr.readyState === 4) { + const { status, responseText } = xhr + if (isResponseSuccessful(status)) { + resolve(xhr) + } else { + reject({ url, status, responseText }) + } + } + } + + xhr.onerror = () => { + const { status, responseText } = xhr + reject({ url, status, responseText }) + } + + let canSend = true + if (typeof beforeSend === 'function') { + canSend = beforeSend({ url, method, headers, payload, xhr }) + } + if (canSend) { + xhr.send(payload) + } else { + reject({ + url, + status: 0, + responseText: 'Request rejected by user configuration.' + }) + } + }) +} diff --git a/packages/rum-core/src/common/page-visibility.js b/packages/rum-core/src/common/page-visibility.js new file mode 100644 index 000000000..a202b2d77 --- /dev/null +++ b/packages/rum-core/src/common/page-visibility.js @@ -0,0 +1,95 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +import { QUEUE_ADD_TRANSACTION, QUEUE_FLUSH } from './constants' +import { state } from '../state' +import { now } from './utils' + +/** + * @param configService + * @param transactionService + */ +export function observePageVisibility(configService, transactionService) { + if (document.visibilityState === 'hidden') { + state.lastHiddenStart = 0 + } + + const visibilityChangeHandler = () => { + if (document.visibilityState === 'hidden') { + onPageHidden(configService, transactionService) + } + } + const pageHideHandler = () => onPageHidden(configService, transactionService) + + // visibilitychange and pagehide listeners are added to window and in the + // capture phase because it executes before the target or bubble phases, + // so adding listeners there helps ensure they run before other code can cancel them. + // More info: https://developers.google.com/web/updates/2018/07/page-lifecycle-api + + const useCapture = true + window.addEventListener( + 'visibilitychange', + visibilityChangeHandler, + useCapture + ) + + // Safari 13 does not trigger visibilityChange event when reloading page + // due to that fact pagehide listener is also added + window.addEventListener('pagehide', pageHideHandler, useCapture) + + return () => { + window.removeEventListener( + 'visibilitychange', + visibilityChangeHandler, + useCapture + ) + window.removeEventListener('pagehide', pageHideHandler, useCapture) + } +} + +/** + * Ends an ongoing transaction if any and flushes the events queue + * @param configService + * @param transactionService + */ +function onPageHidden(configService, transactionService) { + const tr = transactionService.getCurrentTransaction() + if (tr) { + const unobserve = configService.observeEvent(QUEUE_ADD_TRANSACTION, () => { + configService.dispatchEvent(QUEUE_FLUSH) + state.lastHiddenStart = now() + + // unsubscribe listener to execute it only once. + // otherwise in SPAs we might be finding situations where we would be executing + // this logic as many times as we observed to this event + unobserve() + }) + + tr.end() + } else { + configService.dispatchEvent(QUEUE_FLUSH) + state.lastHiddenStart = now() + } +} diff --git a/packages/rum-core/src/common/patching/fetch-patch.js b/packages/rum-core/src/common/patching/fetch-patch.js index 1e43de0e6..9df3db293 100644 --- a/packages/rum-core/src/common/patching/fetch-patch.js +++ b/packages/rum-core/src/common/patching/fetch-patch.js @@ -27,9 +27,10 @@ import { Promise } from '../polyfills' import { globalState } from './patch-utils' import { SCHEDULE, INVOKE, FETCH } from '../constants' import { scheduleMicroTask } from '../utils' +import { isFetchSupported } from '../http/fetch' export function patchFetch(callback) { - if (!window.fetch || !window.Request) { + if (!isFetchSupported()) { return } diff --git a/packages/rum-core/src/error-logging/index.js b/packages/rum-core/src/error-logging/index.js index 16e797ab4..b57e51a4b 100644 --- a/packages/rum-core/src/error-logging/index.js +++ b/packages/rum-core/src/error-logging/index.js @@ -24,7 +24,11 @@ */ import ErrorLogging from './error-logging' -import { CONFIG_SERVICE, APM_SERVER } from '../common/constants' +import { + CONFIG_SERVICE, + TRANSACTION_SERVICE, + APM_SERVER +} from '../common/constants' import { serviceCreators } from '../common/service-factory' function registerServices() { @@ -36,7 +40,7 @@ function registerServices() { ] = serviceFactory.getService([ APM_SERVER, CONFIG_SERVICE, - 'TransactionService' + TRANSACTION_SERVICE ]) return new ErrorLogging(apmServer, configService, transactionService) } diff --git a/packages/rum-core/src/index.js b/packages/rum-core/src/index.js index a2a1546de..e9a2bbb31 100644 --- a/packages/rum-core/src/index.js +++ b/packages/rum-core/src/index.js @@ -35,11 +35,13 @@ import { isBrowser } from './common/utils' import { patchAll, patchEventHandler } from './common/patching' +import { observePageVisibility } from './common/page-visibility' import { PAGE_LOAD, ERROR, CONFIG_SERVICE, LOGGING_SERVICE, + TRANSACTION_SERVICE, APM_SERVER } from './common/constants' import { getInstrumentationFlags } from './common/instrument' @@ -70,6 +72,8 @@ export { PAGE_LOAD, CONFIG_SERVICE, LOGGING_SERVICE, + TRANSACTION_SERVICE, APM_SERVER, - bootstrap + bootstrap, + observePageVisibility } diff --git a/packages/rum-core/src/opentracing/index.js b/packages/rum-core/src/opentracing/index.js index b6ee49e0f..3180fa539 100644 --- a/packages/rum-core/src/opentracing/index.js +++ b/packages/rum-core/src/opentracing/index.js @@ -25,12 +25,13 @@ import Tracer from './tracer' import Span from './span' +import { TRANSACTION_SERVICE, LOGGING_SERVICE } from '../common/constants' function createTracer(serviceFactory) { var performanceMonitoring = serviceFactory.getService('PerformanceMonitoring') - var transactionService = serviceFactory.getService('TransactionService') + var transactionService = serviceFactory.getService(TRANSACTION_SERVICE) var errorLogging = serviceFactory.getService('ErrorLogging') - var loggingService = serviceFactory.getService('LoggingService') + var loggingService = serviceFactory.getService(LOGGING_SERVICE) return new Tracer( performanceMonitoring, transactionService, diff --git a/packages/rum-core/src/performance-monitoring/index.js b/packages/rum-core/src/performance-monitoring/index.js index c8854252b..5dd47b310 100644 --- a/packages/rum-core/src/performance-monitoring/index.js +++ b/packages/rum-core/src/performance-monitoring/index.js @@ -28,13 +28,14 @@ import TransactionService from './transaction-service' import { APM_SERVER, CONFIG_SERVICE, - LOGGING_SERVICE + LOGGING_SERVICE, + TRANSACTION_SERVICE } from '../common/constants' import { serviceCreators } from '../common/service-factory' function registerServices() { - serviceCreators['TransactionService'] = serviceFactory => { + serviceCreators[TRANSACTION_SERVICE] = serviceFactory => { const [loggingService, configService] = serviceFactory.getService([ LOGGING_SERVICE, CONFIG_SERVICE @@ -52,7 +53,7 @@ function registerServices() { APM_SERVER, CONFIG_SERVICE, LOGGING_SERVICE, - 'TransactionService' + TRANSACTION_SERVICE ]) return new PerformanceMonitoring( apmServer, diff --git a/packages/rum-core/src/performance-monitoring/performance-monitoring.js b/packages/rum-core/src/performance-monitoring/performance-monitoring.js index d24536ba6..737df27ad 100644 --- a/packages/rum-core/src/performance-monitoring/performance-monitoring.js +++ b/packages/rum-core/src/performance-monitoring/performance-monitoring.js @@ -47,7 +47,8 @@ import { HTTP_REQUEST_TYPE, USER_INTERACTION, OUTCOME_FAILURE, - OUTCOME_SUCCESS + OUTCOME_SUCCESS, + QUEUE_ADD_TRANSACTION } from '../common/constants' import { truncateModel, @@ -159,6 +160,7 @@ export default class PerformanceMonitoring { const payload = this.createTransactionPayload(tr) if (payload) { this._apmServer.addTransaction(payload) + this._configService.dispatchEvent(QUEUE_ADD_TRANSACTION) } }) @@ -258,6 +260,18 @@ export default class PerformanceMonitoring { const configService = this._configService const transactionService = this._transactionService + // do not process calls to our own endpoints + if (task.data && task.data.url) { + const endpoints = this._apmServer.getEndpoints() + const isOwnEndpoint = Object.keys(endpoints).some( + endpoint => task.data.url.indexOf(endpoints[endpoint]) !== -1 + ) + + if (isOwnEndpoint) { + return + } + } + if (event === SCHEDULE && task.data) { const data = task.data const requestUrl = new Url(data.url) diff --git a/packages/rum-core/src/state.js b/packages/rum-core/src/state.js index 4fe8b5329..bf7deea70 100644 --- a/packages/rum-core/src/state.js +++ b/packages/rum-core/src/state.js @@ -24,6 +24,7 @@ */ const __DEV__ = process.env.NODE_ENV !== 'production' + const state = { // Time when agent is bootstrapped and patching of modules happens bootstrapTime: null, diff --git a/packages/rum-core/test/benchmarks/compress.bench.js b/packages/rum-core/test/benchmarks/compress.bench.js index 4b92bbb72..a0569eb31 100644 --- a/packages/rum-core/test/benchmarks/compress.bench.js +++ b/packages/rum-core/test/benchmarks/compress.bench.js @@ -25,13 +25,14 @@ import { compressPayload } from '../../src/common/compress' import { createServiceFactory } from '../../src' import { generateTestTransaction } from './' +import { APM_SERVER } from '@elastic/apm-rum-core' suite('Compress', () => { const serviceFactory = createServiceFactory() const performanceMonitoring = serviceFactory.getService( 'PerformanceMonitoring' ) - const apmServer = serviceFactory.getService('ApmServer') + const apmServer = serviceFactory.getService(APM_SERVER) const sampledTransactions = Array(10).fill(generateTestTransaction(10, true)) const transactions = sampledTransactions.map(tr => diff --git a/packages/rum-core/test/benchmarks/performance-monitoring.bench.js b/packages/rum-core/test/benchmarks/performance-monitoring.bench.js index 71e07a9b4..50a4409ef 100644 --- a/packages/rum-core/test/benchmarks/performance-monitoring.bench.js +++ b/packages/rum-core/test/benchmarks/performance-monitoring.bench.js @@ -28,7 +28,11 @@ import { generateTransactionWithGroupedSpans } from './' import { createServiceFactory } from '../../src' -import { TRANSACTIONS } from '../../src/common/constants' +import { + TRANSACTIONS, + CONFIG_SERVICE, + APM_SERVER +} from '../../src/common/constants' import { groupSmallContinuouslySimilarSpans } from '../../src/performance-monitoring/performance-monitoring' import { getGlobalConfig } from '../../../../dev-utils/test-config' @@ -50,9 +54,9 @@ suite('PerformanceMonitoring', () => { const performanceMonitoring = serviceFactory.getService( 'PerformanceMonitoring' ) - const apmServer = serviceFactory.getService('ApmServer') + const apmServer = serviceFactory.getService(APM_SERVER) const _postJson = apmServer._postJson - const configService = serviceFactory.getService('ConfigService') + const configService = serviceFactory.getService(CONFIG_SERVICE) configService.setConfig({ serviceName: 'benchmark-performance-monitoring' }) configService.setConfig(agentConfig) diff --git a/packages/rum-core/test/benchmarks/transaction-service.bench.js b/packages/rum-core/test/benchmarks/transaction-service.bench.js index 48ab147dc..5aa884ba2 100644 --- a/packages/rum-core/test/benchmarks/transaction-service.bench.js +++ b/packages/rum-core/test/benchmarks/transaction-service.bench.js @@ -24,11 +24,11 @@ */ import { createServiceFactory } from '../../src/index' -import { PAGE_LOAD } from '../../src/common/constants' +import { PAGE_LOAD, TRANSACTION_SERVICE } from '../../src/common/constants' suite('TransactionService', () => { const serviceFactory = createServiceFactory() - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) benchmark('managed sampled transaction overhead', async () => { const tr = transactionService.startTransaction('/index', PAGE_LOAD, { diff --git a/packages/rum-core/test/bootstrap.spec.js b/packages/rum-core/test/bootstrap.spec.js index 52b7ed3ef..4016eb995 100644 --- a/packages/rum-core/test/bootstrap.spec.js +++ b/packages/rum-core/test/bootstrap.spec.js @@ -25,28 +25,69 @@ import { bootstrap } from '../src/bootstrap' import * as patcher from '../src/common/patching' +import * as utils from '../src/common/utils' +import { state } from '../src/state' import { spyOnFunction } from '../../../dev-utils/jasmine' describe('bootstrap', function () { - it('should log warning on unsupported environments', () => { - // Pass unsupported check - const nowFn = window.performance.now - window.performance.now = undefined - - spyOn(console, 'log') - bootstrap() - - expect(console.log).toHaveBeenCalledWith( - '[Elastic APM] platform is not supported!' - ) - window.performance.now = nowFn + describe('on unsupported environments', () => { + let nowFn + + beforeEach(() => { + nowFn = window.performance.now + window.performance.now = undefined + }) + + afterEach(() => { + window.performance.now = nowFn + }) + + it('should log warning', () => { + spyOn(console, 'log') + + bootstrap() + + expect(console.log).toHaveBeenCalledWith( + '[Elastic APM] platform is not supported!' + ) + }) + + it('should return false', () => { + expect(bootstrap()).toBe(false) + }) }) - it('should call patchAll', () => { - const patchAllSpy = spyOnFunction(patcher, 'patchAll') + describe('on supported environments', () => { + let originalBootstrapTime + beforeEach(() => { + originalBootstrapTime = state.bootstrapTime + }) + + afterEach(() => { + // Since the flow of bootstrap mutates the global state + // it should be restored + state.bootstrapTime = originalBootstrapTime + }) + + it('should return true', () => { + expect(bootstrap()).toBe(true) + }) + + it('should call patchAll', () => { + const patchAllSpy = spyOnFunction(patcher, 'patchAll') + + bootstrap() + + expect(patchAllSpy).toHaveBeenCalledTimes(1) + }) + + it('should set a bootstrap time', () => { + const anyTime = 123456789 + spyOnFunction(utils, 'now').and.returnValue(anyTime) - bootstrap() + bootstrap() - expect(patchAllSpy).toHaveBeenCalledTimes(1) + expect(state.bootstrapTime).toBe(anyTime) + }) }) }) diff --git a/packages/rum-core/test/common/apm-server.spec.js b/packages/rum-core/test/common/apm-server.spec.js index 127755ce9..790530ca1 100644 --- a/packages/rum-core/test/common/apm-server.spec.js +++ b/packages/rum-core/test/common/apm-server.spec.js @@ -28,11 +28,18 @@ import Transaction from '../../src/performance-monitoring/transaction' import { LOCAL_CONFIG_KEY, ERRORS, - TRANSACTIONS + TRANSACTIONS, + QUEUE_FLUSH, + HTTP_REQUEST_TIMEOUT, + LOGGING_SERVICE, + CONFIG_SERVICE, + APM_SERVER } from '../../src/common/constants' import { getGlobalConfig } from '../../../../dev-utils/test-config' -import { describeIf } from '../../../../dev-utils/jasmine' +import { describeIf, spyOnFunction } from '../../../../dev-utils/jasmine' import { createServiceFactory, generateTransaction, generateErrors } from '../' +import * as fetchSender from '../../src/common/http/fetch' +import * as xhrSender from '../../src/common/http/xhr' const { agentConfig, testConfig } = getGlobalConfig('rum-core') @@ -53,10 +60,10 @@ describe('ApmServer', function () { originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL jasmine.DEFAULT_TIMEOUT_INTERVAL = 50000 serviceFactory = createServiceFactory() - configService = serviceFactory.getService('ConfigService') + configService = serviceFactory.getService(CONFIG_SERVICE) configService.setConfig(agentConfig) - loggingService = serviceFactory.getService('LoggingService') - apmServer = serviceFactory.getService('ApmServer') + loggingService = serviceFactory.getService(LOGGING_SERVICE) + apmServer = serviceFactory.getService(APM_SERVER) performanceMonitoring = serviceFactory.getService('PerformanceMonitoring') errorLogging = serviceFactory.getService('ErrorLogging') }) @@ -109,6 +116,15 @@ describe('ApmServer', function () { expect(apmServer.queue.items.length).toBe(0) }) + it('should call sendEvents queue when QUEUE_FLUSH event is dispatched', function () { + apmServer.init() + spyOn(apmServer, 'sendEvents') + + configService.dispatchEvent(QUEUE_FLUSH) + + expect(apmServer.sendEvents).toHaveBeenCalledTimes(1) + }) + it('should not add any items to queue when not initialized', function () { const clock = jasmine.clock() clock.install() @@ -422,12 +438,12 @@ describe('ApmServer', function () { }) it('should reject the request if beforeSend returns falsy', async () => { - let promise = apmServer._makeHttpRequest('POST', '/test', { + let promise = apmServer._makeHttpRequest('GET', '/test', { payload: 'test', beforeSend: ({ xhr, url, method, headers, payload }) => { expect(xhr).toBeDefined() expect(url).toBe('/test') - expect(method).toBe('POST') + expect(method).toBe('GET') expect(payload).toBe('test') expect(headers).toBeUndefined() return false @@ -440,6 +456,148 @@ describe('ApmServer', function () { }) }) + describe('http strategies', () => { + let originalCompressionStream = window.CompressionStream + + beforeEach(() => { + window.CompressionStream = void 0 + }) + + afterEach(() => { + window.CompressionStream = originalCompressionStream + }) + + it('should use fetch when keep alive is the choice', async () => { + spyOnFunction(fetchSender, 'shouldUseFetchWithKeepAlive').and.returnValue( + true + ) + spyOnFunction(fetchSender, 'sendFetchRequest').and.resolveTo({}) + spyOnFunction(xhrSender, 'sendXHR') + const serverUrl = 'http://localhost' + const serverUrlPrefix = '/prefix' + configService.setConfig({ + serverUrl, + serverUrlPrefix + }) + + await apmServer.sendEvents([{ [TRANSACTIONS]: { test: 'test' } }]) + + expect(fetchSender.sendFetchRequest).toHaveBeenCalledTimes(1) + expect(fetchSender.sendFetchRequest).toHaveBeenCalledWith( + 'POST', + 'http://localhost/prefix', + { + keepalive: true, + timeout: HTTP_REQUEST_TIMEOUT, + payload: + '{"metadata":{"service":{"name":"test","agent":{"name":"rum-js","version":"N/A"},"language":{"name":"javascript"}}}}\n{"transaction":{"test":"test"}}\n', + headers: { + 'Content-Type': 'application/x-ndjson' + } + } + ) + expect(xhrSender.sendXHR).toHaveBeenCalledTimes(0) + }) + + it('should use xhr as a fallback if beforeSend function is defined', async () => { + spyOnFunction(fetchSender, 'shouldUseFetchWithKeepAlive').and.returnValue( + true + ) + spyOnFunction(fetchSender, 'sendFetchRequest') + spyOnFunction(xhrSender, 'sendXHR').and.resolveTo({}) + const beforeSend = () => {} + + const serverUrl = 'http://localhost' + const serverUrlPrefix = '/prefix' + configService.setConfig({ + serverUrl, + serverUrlPrefix, + apmRequest: beforeSend + }) + + await apmServer.sendEvents([{ [TRANSACTIONS]: { test: 'test' } }]) + + expect(fetchSender.sendFetchRequest).toHaveBeenCalledTimes(0) + + // fallback expected + expect(xhrSender.sendXHR).toHaveBeenCalledTimes(1) + expect(xhrSender.sendXHR).toHaveBeenCalledWith( + 'POST', + 'http://localhost/prefix', + { + timeout: HTTP_REQUEST_TIMEOUT, + payload: + '{"metadata":{"service":{"name":"test","agent":{"name":"rum-js","version":"N/A"},"language":{"name":"javascript"}}}}\n{"transaction":{"test":"test"}}\n', + headers: { + 'Content-Type': 'application/x-ndjson' + }, + beforeSend + } + ) + }) + + it('should use xhr as a fallback when fetch fails due to an exception', async () => { + spyOnFunction(fetchSender, 'shouldUseFetchWithKeepAlive').and.returnValue( + true + ) + spyOnFunction(fetchSender, 'sendFetchRequest').and.rejectWith( + new TypeError('network error') + ) + spyOnFunction(xhrSender, 'sendXHR').and.resolveTo({}) + const serverUrl = 'http://localhost' + const serverUrlPrefix = '/prefix' + configService.setConfig({ + serverUrl, + serverUrlPrefix + }) + + await apmServer.sendEvents([{ [TRANSACTIONS]: { test: 'test' } }]) + + expect(fetchSender.sendFetchRequest).toHaveBeenCalledTimes(1) + + // fallback expected + expect(xhrSender.sendXHR).toHaveBeenCalledTimes(1) + expect(xhrSender.sendXHR).toHaveBeenCalledWith( + 'POST', + 'http://localhost/prefix', + { + timeout: HTTP_REQUEST_TIMEOUT, + payload: + '{"metadata":{"service":{"name":"test","agent":{"name":"rum-js","version":"N/A"},"language":{"name":"javascript"}}}}\n{"transaction":{"test":"test"}}\n', + headers: { + 'Content-Type': 'application/x-ndjson' + }, + beforeSend: null + } + ) + }) + + it('should not use xhr as a fallback when fetch failure reason is not an exception', async () => { + spyOnFunction(fetchSender, 'shouldUseFetchWithKeepAlive').and.returnValue( + true + ) + spyOnFunction(fetchSender, 'sendFetchRequest').and.rejectWith({ + url: '/test', + status: 400, + responseText: 'bad input' + }) + spyOnFunction(xhrSender, 'sendXHR').and.resolveTo({}) + + const promise = apmServer.sendEvents([ + { [TRANSACTIONS]: { test: 'test' } } + ]) + + await expectAsync(promise).toBeRejectedWith({ + url: '/test', + status: 400, + responseText: 'bad input' + }) + + expect(fetchSender.sendFetchRequest).toHaveBeenCalledTimes(1) + expect(xhrSender.sendXHR).toHaveBeenCalledTimes(0) + }) + }) + /** * Stream API in blob is only supported on few browsers */ diff --git a/packages/rum-core/test/common/compress.spec.js b/packages/rum-core/test/common/compress.spec.js index 8c8ca8331..4e62fd5c6 100644 --- a/packages/rum-core/test/common/compress.spec.js +++ b/packages/rum-core/test/common/compress.spec.js @@ -30,6 +30,7 @@ import { compressError, compressPayload } from '../../src/common/compress' +import { CONFIG_SERVICE, APM_SERVER } from '../../src/common/constants' import { addTransactionContext } from '../../src/common/context' /** @@ -171,8 +172,8 @@ describe('Compress', function () { beforeEach(function () { serviceFactory = createServiceFactory() - configService = serviceFactory.getService('ConfigService') - apmServer = serviceFactory.getService('ApmServer') + configService = serviceFactory.getService(CONFIG_SERVICE) + apmServer = serviceFactory.getService(APM_SERVER) performanceMonitoring = serviceFactory.getService('PerformanceMonitoring') errorLogging = serviceFactory.getService('ErrorLogging') configService.setConfig({ diff --git a/packages/rum-core/test/common/http/fetch.spec.js b/packages/rum-core/test/common/http/fetch.spec.js new file mode 100644 index 000000000..ba9138c5f --- /dev/null +++ b/packages/rum-core/test/common/http/fetch.spec.js @@ -0,0 +1,198 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +import { + BYTE_LIMIT, + sendFetchRequest, + shouldUseFetchWithKeepAlive +} from '../../../src/common/http/fetch' +import { describeIf } from '../../../../../dev-utils/jasmine' + +describe('shouldUseFetchWithKeepAlive', () => { + function generatePayload(payloadSize) { + return ''.padEnd(payloadSize) + } + + describeIf( + 'browsers not supporting fetch API', + () => { + it('should return false', () => { + const payloadSize = BYTE_LIMIT - 1 + const payload = generatePayload(payloadSize) + + expect(shouldUseFetchWithKeepAlive('POST', payload)).toBe(false) + }) + }, + !window.fetch + ) + + describeIf( + 'browsers supporting fetch API', + () => { + function setRequestWithKeepAliveSupport() { + Object.defineProperty(window, 'Request', { + value: class Request { + constructor() { + this.keepalive = true + } + } + }) + } + + function setRequestWithoutKeepAliveSupport() { + Object.defineProperty(window, 'Request', { + value: class Request { + constructor() { + // browsers that do not support keepalive don't have the property defined in the request object + } + } + }) + } + + let originalRequest + beforeEach(() => { + originalRequest = window.Request + setRequestWithKeepAliveSupport() + }) + + afterEach(() => { + window.Request = originalRequest + }) + + it('should return true for a POST request whose payload size is lower than BYTE_LIMIT', () => { + const payloadSize = BYTE_LIMIT - 1 + const payload = generatePayload(payloadSize) + + expect(shouldUseFetchWithKeepAlive('POST', payload)).toBe(true) + }) + + it('should return false for a POST request whose payload size is equal to BYTE_LIMIT', () => { + const payloadSize = BYTE_LIMIT + const payload = generatePayload(payloadSize) + + expect(shouldUseFetchWithKeepAlive('POST', payload)).toBe(false) + }) + + it('should return false for a POST request whose payload size is greater than BYTE_LIMIT', () => { + const payloadSize = BYTE_LIMIT + 1 + const payload = generatePayload(payloadSize) + + expect(shouldUseFetchWithKeepAlive('POST', payload)).toBe(false) + }) + + it('should return false for a GET request', () => { + expect(shouldUseFetchWithKeepAlive('GET')).toBe(false) + }) + + it('should return false if browser does not support keepalive', () => { + setRequestWithoutKeepAliveSupport() + + const payloadSize = BYTE_LIMIT - 1 + const payload = generatePayload(payloadSize) + + expect(shouldUseFetchWithKeepAlive('POST', payload)).toBe(false) + }) + }, + window.fetch + ) +}) + +describeIf( + 'sendFetchRequest', + () => { + it('should call fetchApi', async () => { + const response = { + status: 200, + text() { + return Promise.resolve({}) + } + } + const fetchSpy = spyOn(window, 'fetch').and.resolveTo(response) + + await sendFetchRequest('POST', 'anyUrl', { + payload: 'anyPayload', + keepalive: true, + headers: {} + }) + + expect(fetchSpy).toHaveBeenCalledTimes(1) + expect(fetchSpy).toHaveBeenCalledWith( + 'anyUrl', + jasmine.objectContaining({ + keepalive: true, + headers: {}, + body: 'anyPayload', + method: 'POST', + credentials: 'omit' + }) + ) + }) + + it('should get response if response is successful', async () => { + const response = { + status: 200, + text() { + return Promise.resolve({}) + } + } + spyOn(window, 'fetch').and.resolveTo(response) + + const result = await sendFetchRequest('POST', 'anyUrl', { + payload: 'anyPayload', + keepalive: true, + headers: {} + }) + + expect(result).toEqual({ + url: 'anyUrl', + status: response.status, + responseText: {} + }) + }) + + it('should throw if response is not successful', async () => { + const response = { + status: 403, + text() { + return Promise.resolve({}) + } + } + spyOn(window, 'fetch').and.resolveTo(response) + + await expectAsync( + sendFetchRequest('POST', 'anyUrl', { + payload: 'anyPayload', + keepalive: true, + headers: {} + }) + ).toBeRejectedWith({ + status: response.status, + url: 'anyUrl', + responseText: {} + }) + }) + }, + window.fetch +) diff --git a/packages/rum-core/test/common/http/response-status.spec.js b/packages/rum-core/test/common/http/response-status.spec.js new file mode 100644 index 000000000..b590b7f1f --- /dev/null +++ b/packages/rum-core/test/common/http/response-status.spec.js @@ -0,0 +1,40 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +import { isResponseSuccessful } from '../../../src/common/http/response-status' + +describe('isResponseSuccessful', () => { + it('should return true if response status is 204', () => { + expect(isResponseSuccessful(204)).toBe(true) + }) + + it('should return false if response status is 500', () => { + expect(isResponseSuccessful(500)).toBe(false) + }) + + it('should return false if response status is 400', () => { + expect(isResponseSuccessful(400)).toBe(false) + }) +}) diff --git a/packages/rum-core/test/common/page-visibility.spec.js b/packages/rum-core/test/common/page-visibility.spec.js new file mode 100644 index 000000000..26c008031 --- /dev/null +++ b/packages/rum-core/test/common/page-visibility.spec.js @@ -0,0 +1,288 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +import { observePageVisibility } from '../../src/common/page-visibility' +import { createServiceFactory } from '../../src' +import { state } from '../../src/state' +import { + QUEUE_FLUSH, + TRANSACTION_SERVICE, + CONFIG_SERVICE +} from '../../src/common/constants' +import * as utils from '../../src/common/utils' +import { createCustomEvent } from '../../test/' +import { spyOnFunction, waitFor } from '../../../../dev-utils/jasmine' + +describe('observePageVisibility', () => { + let serviceFactory + let configService + let transactionService + let unobservePageVisibility + let originalLastHiddenStart + + function hidePageSynthetically(eventName) { + if (eventName === 'visibilitychange') { + setDocumentVisibilityState('hidden') + } + + dispatchBrowserEvent(eventName) + } + + function dispatchBrowserEvent(eventName) { + document.dispatchEvent(createCustomEvent(eventName)) + } + + function setDocumentVisibilityState(visibilityState) { + Object.defineProperty(document, 'visibilityState', { + get() { + return visibilityState + } + }) + } + + beforeEach(() => { + serviceFactory = createServiceFactory() + configService = serviceFactory.getService(CONFIG_SERVICE) + transactionService = serviceFactory.getService(TRANSACTION_SERVICE) + setDocumentVisibilityState('visible') // default value for the tests + }) + + afterEach(() => { + // Since the flow page visibility mutates the global state + // it should be restored + state.lastHiddenStart = originalLastHiddenStart + + // Remove the added listener since window object is shared among all tests + unobservePageVisibility() + }) + + it('should reset lastHiddenStart if document is hidden', () => { + setDocumentVisibilityState('hidden') + const anyTime = 1234567810 + state.lastHiddenStart = anyTime + + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + + expect(state.lastHiddenStart).toBe(0) + }) + + it('should not reset lastHiddenStart if document is visible', () => { + const anyTime = 1234567811 + // Set an arbitrary value + state.lastHiddenStart = anyTime + + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + + expect(state.lastHiddenStart).toBe(anyTime) + }) + + // the observed events by the agent... + ;['visibilitychange', 'pagehide'].forEach(eventName => { + describe(`when page becomes hidden due to ${eventName.toUpperCase()} event`, () => { + describe(`with every transaction already ended`, () => { + it('should dispatch the QUEUE_FLUSH event', () => { + const dispatchEventSpy = spyOn(configService, 'dispatchEvent') + + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + hidePageSynthetically(eventName) + + expect(dispatchEventSpy).toHaveBeenCalledTimes(1) + expect(dispatchEventSpy).toHaveBeenCalledWith(QUEUE_FLUSH) + }) + + it('should set lastHiddenStart', () => { + const anyTime = 1234567834 + spyOnFunction(utils, 'now').and.returnValue(anyTime) + + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + hidePageSynthetically(eventName) + + expect(state.lastHiddenStart).toBe(anyTime) + }) + }) + + describe(`with a transaction that has not ended yet`, () => { + // Starts performanceMonitoring to observe how transaction ends + function startsPerformanceMonitoring() { + const performanceMonitoring = serviceFactory.getService( + 'PerformanceMonitoring' + ) + performanceMonitoring.init() + } + + function createTransaction() { + const transaction = transactionService.startTransaction( + 'test-tr', + 'test-type' + ) + + let span = transaction.startSpan('test span', 'test span type', { + startTime: 0 + }) + span.end(100) + + return transaction + } + + it('should end the transaction', () => { + const transaction = createTransaction() + const endTransactionSpy = spyOn(transaction, 'end') + spyOn(transactionService, 'getCurrentTransaction').and.returnValue( + transaction + ) + + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + hidePageSynthetically(eventName) + + expect(endTransactionSpy).toHaveBeenCalledTimes(1) + }) + + it('should dispatch the QUEUE_FLUSH event once the transaction ends and added to the queue', async () => { + // Arrange + const transaction = createTransaction() + const dispatchEventSpy = spyOn( + configService, + 'dispatchEvent' + ).and.callThrough() + spyOn(transactionService, 'getCurrentTransaction').and.returnValue( + transaction + ) + startsPerformanceMonitoring() + + // Act + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + hidePageSynthetically(eventName) + await waitFor(() => dispatchEventSpy.calls.any()) + + // Assert + expect(dispatchEventSpy).toHaveBeenCalledWith(QUEUE_FLUSH) + }) + + it('should remove the QUEUE_ADD_TRANSACTION event listener once the transaction ends and added to the queue', async () => { + // Arrange + const unobserveSpy = jasmine.createSpy() + const observeEvent = configService.observeEvent.bind(configService) + spyOn(configService, 'observeEvent').and.callFake((name, fn) => { + observeEvent(name, fn) + return unobserveSpy + }) + const transaction = createTransaction() + const dispatchEventSpy = spyOn( + configService, + 'dispatchEvent' + ).and.callThrough() + spyOn(transactionService, 'getCurrentTransaction').and.returnValue( + transaction + ) + startsPerformanceMonitoring() + + // Act + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + hidePageSynthetically(eventName) + await waitFor(() => dispatchEventSpy.calls.any()) + + // Assert + expect(unobserveSpy).toHaveBeenCalledTimes(1) + }) + + it('should set lastHiddenStart once the transaction ends and added to the queue', async () => { + // Arrange + const anyTime = 1234567834 + spyOnFunction(utils, 'now').and.returnValue(anyTime) + const transaction = createTransaction() + const dispatchEventSpy = spyOn( + configService, + 'dispatchEvent' + ).and.callThrough() + spyOn(transactionService, 'getCurrentTransaction').and.returnValue( + transaction + ) + startsPerformanceMonitoring() + + // Act + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + hidePageSynthetically(eventName) + await waitFor(() => dispatchEventSpy.calls.any()) + + // Assert + expect(state.lastHiddenStart).toBe(anyTime) + }) + }) + }) + }) + + describe('when visibility changes into visible', () => { + it('should not dispatch the QUEUE_FLUSH event', () => { + const dispatchEventSpy = spyOn(configService, 'dispatchEvent') + + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + dispatchBrowserEvent('visibilitychange') + + expect(dispatchEventSpy).toHaveBeenCalledTimes(0) + }) + + it('should not set lastHiddenStart', () => { + const anyTime = 1234567834 + spyOnFunction(utils, 'now').and.returnValue(anyTime) + + unobservePageVisibility = observePageVisibility( + configService, + transactionService + ) + + state.lastHiddenStart = 111 + dispatchBrowserEvent('visibilitychange') + + expect(state.lastHiddenStart).toBe(111) + }) + }) +}) diff --git a/packages/rum-core/test/common/service-factory.spec.js b/packages/rum-core/test/common/service-factory.spec.js index f6bee9463..b2b1d3683 100644 --- a/packages/rum-core/test/common/service-factory.spec.js +++ b/packages/rum-core/test/common/service-factory.spec.js @@ -23,6 +23,7 @@ * */ +import { LOGGING_SERVICE, CONFIG_SERVICE } from '../../src/common/constants' import { ServiceFactory } from '../../src/common/service-factory' describe('ServiceFactory', function () { @@ -32,9 +33,9 @@ describe('ServiceFactory', function () { beforeEach(function () { serviceFactory = new ServiceFactory() serviceFactory.init() - configService = serviceFactory.getService('ConfigService') + configService = serviceFactory.getService(CONFIG_SERVICE) - loggingService = serviceFactory.getService('LoggingService') + loggingService = serviceFactory.getService(LOGGING_SERVICE) spyOn(loggingService, 'debug') }) @@ -50,10 +51,7 @@ describe('ServiceFactory', function () { }) it('should get multiple services', function () { - let services = serviceFactory.getService([ - 'ConfigService', - 'LoggingService' - ]) + let services = serviceFactory.getService([CONFIG_SERVICE, LOGGING_SERVICE]) expect(services).toEqual([configService, loggingService]) }) }) 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 9897287f5..94e6e3899 100644 --- a/packages/rum-core/test/error-logging/error-logging.spec.js +++ b/packages/rum-core/test/error-logging/error-logging.spec.js @@ -24,7 +24,12 @@ */ import { createServiceFactory, createCustomEvent } from '../' -import { ERRORS } from '../../src/common/constants' +import { + ERRORS, + TRANSACTION_SERVICE, + CONFIG_SERVICE, + APM_SERVER +} from '../../src/common/constants' import { getGlobalConfig } from '../../../../dev-utils/test-config' const { agentConfig } = getGlobalConfig('rum-core') @@ -38,11 +43,11 @@ describe('ErrorLogging', function () { beforeEach(function () { var serviceFactory = createServiceFactory() - configService = serviceFactory.getService('ConfigService') + configService = serviceFactory.getService(CONFIG_SERVICE) configService.setConfig(agentConfig) - apmServer = serviceFactory.getService('ApmServer') + apmServer = serviceFactory.getService(APM_SERVER) errorLogging = serviceFactory.getService('ErrorLogging') - transactionService = serviceFactory.getService('TransactionService') + transactionService = serviceFactory.getService(TRANSACTION_SERVICE) }) const getEvents = () => apmServer.queue.items diff --git a/packages/rum-core/test/index.js b/packages/rum-core/test/index.js index 1dbace5c0..232acb5b6 100644 --- a/packages/rum-core/test/index.js +++ b/packages/rum-core/test/index.js @@ -26,11 +26,12 @@ import { createServiceFactory as originalFactory } from '../src' import Transaction from '../src/performance-monitoring/transaction' import { captureBreakdown } from '../src/performance-monitoring/breakdown' +import { APM_SERVER } from '@elastic/apm-rum-core' export function createServiceFactory() { var serviceFactory = originalFactory() if (window.globalConfigs && window.globalConfigs.useMocks) { - var apmServer = serviceFactory.getService('ApmServer') + var apmServer = serviceFactory.getService(APM_SERVER) apmServer._makeHttpRequest = function () { return Promise.resolve() } diff --git a/packages/rum-core/test/opentracing/opentracing.spec.js b/packages/rum-core/test/opentracing/opentracing.spec.js index 445c243af..5ee8c53ee 100644 --- a/packages/rum-core/test/opentracing/opentracing.spec.js +++ b/packages/rum-core/test/opentracing/opentracing.spec.js @@ -29,14 +29,19 @@ import ElasticTracer from '../../src/opentracing/tracer' import Transaction from '../../src/performance-monitoring/transaction' import Span from '../../src/performance-monitoring/span' import { Reference, REFERENCE_CHILD_OF } from 'opentracing' +import { + TRANSACTION_SERVICE, + LOGGING_SERVICE, + CONFIG_SERVICE +} from '../../src/common/constants' function createTracer(config) { var serviceFactory = createServiceFactory() var performanceMonitoring = serviceFactory.getService('PerformanceMonitoring') - var transactionService = serviceFactory.getService('TransactionService') + var transactionService = serviceFactory.getService(TRANSACTION_SERVICE) var errorLogging = serviceFactory.getService('ErrorLogging') - var loggingService = serviceFactory.getService('LoggingService') - var configService = serviceFactory.getService('ConfigService') + var loggingService = serviceFactory.getService(LOGGING_SERVICE) + var configService = serviceFactory.getService(CONFIG_SERVICE) configService.setConfig(config) return new ElasticTracer( performanceMonitoring, diff --git a/packages/rum-core/test/performance-monitoring/performance-monitoring.spec.js b/packages/rum-core/test/performance-monitoring/performance-monitoring.spec.js index ab99d35e4..b1c7cb29e 100644 --- a/packages/rum-core/test/performance-monitoring/performance-monitoring.spec.js +++ b/packages/rum-core/test/performance-monitoring/performance-monitoring.spec.js @@ -44,7 +44,12 @@ import { PAGE_LOAD, ROUTE_CHANGE, TRANSACTION_END, - TRANSACTIONS + TRANSACTIONS, + QUEUE_ADD_TRANSACTION, + TRANSACTION_SERVICE, + LOGGING_SERVICE, + CONFIG_SERVICE, + APM_SERVER } from '../../src/common/constants' import { state } from '../../src/state' import patchEventHandler from '../common/patch' @@ -62,11 +67,11 @@ describe('PerformanceMonitoring', function () { beforeEach(function () { serviceFactory = createServiceFactory() - configService = serviceFactory.getService('ConfigService') - logger = serviceFactory.getService('LoggingService') + configService = serviceFactory.getService(CONFIG_SERVICE) + logger = serviceFactory.getService(LOGGING_SERVICE) configService.setConfig(agentConfig) - apmServer = serviceFactory.getService('ApmServer') + apmServer = serviceFactory.getService(APM_SERVER) performanceMonitoring = serviceFactory.getService('PerformanceMonitoring') }) @@ -146,6 +151,28 @@ describe('PerformanceMonitoring', function () { ) }) + it('should initialize and notify the transaction has been added to the queue', async () => { + performanceMonitoring.init() + spyOn(configService, 'dispatchEvent') + + const tr = performanceMonitoring._transactionService.startTransaction( + 'transaction', + 'transaction', + { startTime: 0 } + ) + let span = tr.startSpan('test span', 'test span type', { startTime: 0 }) + span.end(100) + span = tr.startSpan('test span 2', 'test span type', { startTime: 20 }) + span.end(300) + tr.end(400) + + await waitFor(() => configService.dispatchEvent.calls.any()) + + expect(configService.dispatchEvent).toHaveBeenCalledWith( + QUEUE_ADD_TRANSACTION + ) + }) + it('should create correct payload', function () { var tr = new Transaction('transaction1', 'transaction1type', { transactionSampleRate: 1 @@ -170,7 +197,7 @@ describe('PerformanceMonitoring', function () { it('should sendPageLoadMetrics', function (done) { const unMock = mockGetEntriesByType() - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) configService.events.observe(TRANSACTION_END, function (tr) { expect(tr.captureTimings).toBe(true) @@ -582,7 +609,7 @@ describe('PerformanceMonitoring', function () { } it('should create http-request transaction if no current transaction exist', done => { - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) spyOn(transactionService, 'startTransaction').and.callThrough() let task = createXHRTask('GET', '/') @@ -615,8 +642,36 @@ describe('PerformanceMonitoring', function () { }, 100) }) + it('should not create http-request transaction if task url is intake endpoint', () => { + const { intakeEndpoint } = apmServer.getEndpoints() + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) + spyOn(transactionService, 'startTransaction').and.callThrough() + + let task = createXHRTask('GET', intakeEndpoint) + + performanceMonitoring.processAPICalls('schedule', task) + expect(transactionService.startTransaction).not.toHaveBeenCalled() + + let tr = transactionService.getCurrentTransaction() + expect(tr).toBeUndefined() + }) + + it('should not create http-request transaction if task url is config endpoint', () => { + const { intakeEndpoint } = apmServer.getEndpoints() + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) + spyOn(transactionService, 'startTransaction').and.callThrough() + + let task = createXHRTask('GET', intakeEndpoint) + + performanceMonitoring.processAPICalls('schedule', task) + expect(transactionService.startTransaction).not.toHaveBeenCalled() + + let tr = transactionService.getCurrentTransaction() + expect(tr).toBeUndefined() + }) + it('should include multiple XHRs in the same transaction', () => { - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) spyOn(transactionService, 'startTransaction').and.callThrough() let task1 = createXHRTask('GET', '/first') @@ -643,7 +698,7 @@ describe('PerformanceMonitoring', function () { }) it('should send span context destination details to apm-server', done => { - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) const tr = transactionService.startTransaction('with-context', 'custom') const data = { diff --git a/packages/rum-core/test/polyfills.js b/packages/rum-core/test/polyfills.js index 87a3b9f50..f4e0e245c 100644 --- a/packages/rum-core/test/polyfills.js +++ b/packages/rum-core/test/polyfills.js @@ -25,6 +25,7 @@ import 'promise-polyfill/src/polyfill' import 'core-js/features/array/map' +import 'core-js/features/string/pad-end' Object.setPrototypeOf = Object.setPrototypeOf || diff --git a/packages/rum-core/test/utils/apm-server-mock.js b/packages/rum-core/test/utils/apm-server-mock.js index 6e6f6e2a1..5334fa01d 100644 --- a/packages/rum-core/test/utils/apm-server-mock.js +++ b/packages/rum-core/test/utils/apm-server-mock.js @@ -85,6 +85,7 @@ class ApmServerMock { this.addError = _apmServer.addError.bind(_apmServer) this.addTransaction = _apmServer.addTransaction.bind(_apmServer) this.init = _apmServer.init.bind(_apmServer) + this.getEndpoints = _apmServer.getEndpoints.bind(_apmServer) } } diff --git a/packages/rum-react/src/get-with-transaction.js b/packages/rum-react/src/get-with-transaction.js index 1a8ff87ee..ee957a774 100644 --- a/packages/rum-react/src/get-with-transaction.js +++ b/packages/rum-react/src/get-with-transaction.js @@ -25,7 +25,7 @@ import React from 'react' import hoistStatics from 'hoist-non-react-statics' -import { afterFrame } from '@elastic/apm-rum-core' +import { afterFrame, LOGGING_SERVICE } from '@elastic/apm-rum-core' /** * Check if the given component is class based component @@ -53,7 +53,7 @@ function getWithTransaction(apm) { } if (!Component) { - const loggingService = apm.serviceFactory.getService('LoggingService') + const loggingService = apm.serviceFactory.getService(LOGGING_SERVICE) loggingService.warn( `${name} is not instrumented since component property is not provided` ) diff --git a/packages/rum-react/test/e2e/index.js b/packages/rum-react/test/e2e/index.js index 132990cf8..38641233e 100644 --- a/packages/rum-react/test/e2e/index.js +++ b/packages/rum-react/test/e2e/index.js @@ -25,6 +25,7 @@ import 'promise-polyfill/src/polyfill' import { apmBase } from '@elastic/apm-rum' +import { APM_SERVER } from '@elastic/apm-rum-core' import { getGlobalConfig } from '../../../../dev-utils/test-config' import ApmServerMock from '../../../rum-core/test/utils/apm-server-mock' @@ -32,11 +33,11 @@ const globalConfig = getGlobalConfig() export default function createApmBase(config) { console.log('E2E Global Configs', JSON.stringify(globalConfig, null, 2)) - const apmServer = apmBase.serviceFactory.getService('ApmServer') + const apmServer = apmBase.serviceFactory.getService(APM_SERVER) const { serverUrl } = globalConfig.agentConfig config.serverUrl = serverUrl const serverMock = new ApmServerMock(apmServer, globalConfig.useMocks) - apmBase.serviceFactory.instances['ApmServer'] = serverMock + apmBase.serviceFactory.instances[APM_SERVER] = serverMock return apmBase.init(config) } diff --git a/packages/rum-react/test/specs/get-apm-route.spec.js b/packages/rum-react/test/specs/get-apm-route.spec.js index 7425802f0..8bf81c71e 100644 --- a/packages/rum-react/test/specs/get-apm-route.spec.js +++ b/packages/rum-react/test/specs/get-apm-route.spec.js @@ -36,7 +36,11 @@ Enzyme.configure({ adapter: new Adapter() }) import { MemoryRouter as Router, Route } from 'react-router-dom' import { ApmBase } from '@elastic/apm-rum' -import { createServiceFactory } from '@elastic/apm-rum-core' +import { + createServiceFactory, + TRANSACTION_SERVICE, + LOGGING_SERVICE +} from '@elastic/apm-rum-core' import { getApmRoute } from '../../src/get-apm-route' import { getGlobalConfig } from '../../../../dev-utils/test-config' @@ -80,7 +84,7 @@ describe('ApmRoute', function () { }) it('should work with Route render and log warning', function () { - const loggingService = serviceFactory.getService('LoggingService') + const loggingService = serviceFactory.getService(LOGGING_SERVICE) const ApmRoute = getApmRoute(apmBase) spyOn(loggingService, 'warn') @@ -108,7 +112,7 @@ describe('ApmRoute', function () { it('should work correctly with path array in props', function () { const ApmRoute = getApmRoute(apmBase) - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) const dummyTr = { name: 'test', detectFinish: () => {} @@ -144,7 +148,7 @@ describe('ApmRoute', function () { it('should not trigger full rerender on query change', function () { const ApmRoute = getApmRoute(apmBase) - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) spyOn(transactionService, 'startTransaction') const rendered = mount( diff --git a/packages/rum-react/test/specs/get-with-transaction.spec.js b/packages/rum-react/test/specs/get-with-transaction.spec.js index 85351af6c..8ada482bb 100644 --- a/packages/rum-react/test/specs/get-with-transaction.spec.js +++ b/packages/rum-react/test/specs/get-with-transaction.spec.js @@ -31,7 +31,12 @@ Enzyme.configure({ adapter: new Adapter() }) import { getWithTransaction } from '../../src/get-with-transaction' import { ApmBase } from '@elastic/apm-rum' -import { createServiceFactory, afterFrame } from '@elastic/apm-rum-core' +import { + createServiceFactory, + afterFrame, + TRANSACTION_SERVICE, + LOGGING_SERVICE +} from '@elastic/apm-rum-core' import { getGlobalConfig } from '../../../../dev-utils/test-config' function TestComponent(apm, cb) { @@ -74,8 +79,8 @@ describe('withTransaction', function () { it('should not log warning or create transaction if apm is not active', function () { const [loggingService, transactionService] = serviceFactory.getService([ - 'LoggingService', - 'TransactionService' + LOGGING_SERVICE, + TRANSACTION_SERVICE ]) spyOn(loggingService, 'warn') spyOn(transactionService, 'startTransaction') @@ -90,7 +95,7 @@ describe('withTransaction', function () { }) it('should start transaction for components', function () { - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) spyOn(transactionService, 'startTransaction') TestComponent(apmBase) @@ -105,7 +110,7 @@ describe('withTransaction', function () { }) it('should return WrappedComponent on falsy value and log warning', function () { - const loggingService = serviceFactory.getService('LoggingService') + const loggingService = serviceFactory.getService(LOGGING_SERVICE) spyOn(loggingService, 'warn') const withTransaction = getWithTransaction(apmBase) @@ -117,7 +122,7 @@ describe('withTransaction', function () { }) it('should not instrument the route when rum is inactive', () => { - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) spyOn(transactionService, 'startTransaction') apmBase.config({ active: false }) @@ -137,7 +142,7 @@ describe('withTransaction', function () { }) it('should not create new transaction on every render', () => { - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) spyOn(transactionService, 'startTransaction') const wrapper = TestComponent(apmBase) @@ -164,7 +169,7 @@ describe('withTransaction', function () { }) it('should accept callback function for withTransaction', () => { - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) const labels = { foo: 'bar' } @@ -178,7 +183,7 @@ describe('withTransaction', function () { }) it('should end transaction when component unmounts', done => { - const transactionService = serviceFactory.getService('TransactionService') + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) const detectFinishSpy = jasmine.createSpy('detectFinish') spyOn(transactionService, 'startTransaction').and.returnValue({ detectFinish: detectFinishSpy diff --git a/packages/rum-vue/test/e2e/index.js b/packages/rum-vue/test/e2e/index.js index 4902c0eba..7370f58da 100644 --- a/packages/rum-vue/test/e2e/index.js +++ b/packages/rum-vue/test/e2e/index.js @@ -25,6 +25,7 @@ import 'promise-polyfill/src/polyfill' import { apmBase } from '@elastic/apm-rum' +import { APM_SERVER } from '@elastic/apm-rum-core' import { getGlobalConfig } from '../../../../dev-utils/test-config' import ApmServerMock from '@elastic/apm-rum-core/test/utils/apm-server-mock' @@ -32,9 +33,9 @@ const globalConfig = getGlobalConfig() export function getApmBase() { console.log('E2E Global Configs', JSON.stringify(globalConfig, null, 2)) - const apmServer = apmBase.serviceFactory.getService('ApmServer') + const apmServer = apmBase.serviceFactory.getService(APM_SERVER) const serverMock = new ApmServerMock(apmServer, globalConfig.useMocks) - apmBase.serviceFactory.instances['ApmServer'] = serverMock + apmBase.serviceFactory.instances[APM_SERVER] = serverMock return apmBase } diff --git a/packages/rum/src/apm-base.js b/packages/rum/src/apm-base.js index ae4014c92..09fb258ca 100644 --- a/packages/rum/src/apm-base.js +++ b/packages/rum/src/apm-base.js @@ -29,7 +29,9 @@ import { ERROR, CONFIG_SERVICE, LOGGING_SERVICE, - APM_SERVER + TRANSACTION_SERVICE, + APM_SERVER, + observePageVisibility } from '@elastic/apm-rum-core' export default class ApmBase { @@ -51,9 +53,14 @@ export default class ApmBase { init(config) { if (this.isEnabled() && !this._initialized) { this._initialized = true - const [configService, loggingService] = this.serviceFactory.getService([ + const [ + configService, + loggingService, + transactionService + ] = this.serviceFactory.getService([ CONFIG_SERVICE, - LOGGING_SERVICE + LOGGING_SERVICE, + TRANSACTION_SERVICE ]) /** * Set Agent version to be sent as part of metadata to the APM Server @@ -107,6 +114,8 @@ export default class ApmBase { } else { sendPageLoad() } + + observePageVisibility(configService, transactionService) } else { this._disable = true loggingService.warn('RUM agent is inactive') @@ -257,7 +266,7 @@ export default class ApmBase { startTransaction(name, type, options) { if (this.isEnabled()) { var transactionService = this.serviceFactory.getService( - 'TransactionService' + TRANSACTION_SERVICE ) return transactionService.startTransaction(name, type, options) } @@ -266,7 +275,7 @@ export default class ApmBase { startSpan(name, type, options) { if (this.isEnabled()) { var transactionService = this.serviceFactory.getService( - 'TransactionService' + TRANSACTION_SERVICE ) return transactionService.startSpan(name, type, options) } @@ -275,7 +284,7 @@ export default class ApmBase { getCurrentTransaction() { if (this.isEnabled()) { var transactionService = this.serviceFactory.getService( - 'TransactionService' + TRANSACTION_SERVICE ) return transactionService.getCurrentTransaction() } diff --git a/packages/rum/src/index.js b/packages/rum/src/index.js index 2bf79f0c1..db501fb49 100644 --- a/packages/rum/src/index.js +++ b/packages/rum/src/index.js @@ -32,7 +32,7 @@ import ApmBase from './apm-base' /** * Use a single instance of ApmBase across all instance of the agent - * including the instanes used in framework specific integrations + * including the instances used in framework specific integrations */ function getApmBase() { if (isBrowser && window.elasticApm) { diff --git a/packages/rum/test/e2e/index.js b/packages/rum/test/e2e/index.js index a5b1f9071..86391f669 100644 --- a/packages/rum/test/e2e/index.js +++ b/packages/rum/test/e2e/index.js @@ -32,18 +32,19 @@ import 'promise-polyfill/src/polyfill' import { apmBase } from '../../src' import { getGlobalConfig } from '../../../../dev-utils/test-config' import ApmServerMock from '../../../rum-core/test/utils/apm-server-mock' +import { APM_SERVER } from '@elastic/apm-rum-core' const globalConfig = getGlobalConfig() function createApmBase(config) { console.log('E2E Global Configs', JSON.stringify(globalConfig, null, 2)) - const apmServer = apmBase.serviceFactory.getService('ApmServer') + const apmServer = apmBase.serviceFactory.getService(APM_SERVER) const { serverUrl } = globalConfig.agentConfig if (serverUrl) { config.serverUrl = serverUrl } const serverMock = new ApmServerMock(apmServer, globalConfig.useMocks) - apmBase.serviceFactory.instances['ApmServer'] = serverMock + apmBase.serviceFactory.instances[APM_SERVER] = serverMock return apmBase.init(config) } diff --git a/packages/rum/test/specs/apm-base.spec.js b/packages/rum/test/specs/apm-base.spec.js index 7cb0b9e10..eaf0e8c41 100644 --- a/packages/rum/test/specs/apm-base.spec.js +++ b/packages/rum/test/specs/apm-base.spec.js @@ -27,7 +27,11 @@ import ApmBase from '../../src/apm-base' import { createServiceFactory, bootstrap, - PAGE_LOAD + PAGE_LOAD, + TRANSACTION_SERVICE, + LOGGING_SERVICE, + CONFIG_SERVICE, + APM_SERVER } from '@elastic/apm-rum-core' import { TRANSACTION_END } from '@elastic/apm-rum-core/src/common/constants' import { getGlobalConfig } from '../../../../dev-utils/test-config' @@ -39,6 +43,7 @@ const { serviceName, serverUrl } = getGlobalConfig('rum').agentConfig describe('ApmBase', function () { let serviceFactory let apmBase + beforeEach(function () { serviceFactory = createServiceFactory() apmBase = new ApmBase(serviceFactory, !enabled) @@ -79,7 +84,7 @@ describe('ApmBase', function () { }) it('should disable all auto instrumentations when instrument is false', () => { - const trService = serviceFactory.getService('TransactionService') + const trService = serviceFactory.getService(TRANSACTION_SERVICE) const ErrorLogging = serviceFactory.getService('ErrorLogging') const loggingInstane = ErrorLogging['__proto__'] spyOn(loggingInstane, 'registerListeners') @@ -97,7 +102,7 @@ describe('ApmBase', function () { }) it('should selectively enable/disable instrumentations based on config', () => { - const trService = serviceFactory.getService('TransactionService') + const trService = serviceFactory.getService(TRANSACTION_SERVICE) const ErrorLogging = serviceFactory.getService('ErrorLogging') const loggingInstane = ErrorLogging['__proto__'] spyOn(loggingInstane, 'registerListeners') @@ -134,7 +139,7 @@ describe('ApmBase', function () { }) it('should be noop for auto instrumentaion when agent is not active', done => { - const loggingService = serviceFactory.getService('LoggingService') + const loggingService = serviceFactory.getService(LOGGING_SERVICE) spyOn(loggingService, 'warn') apmBase.init({ active: false }) @@ -158,7 +163,7 @@ describe('ApmBase', function () { }) it('should be noop when API methods are used and agent is not active', () => { - const loggingService = serviceFactory.getService('LoggingService') + const loggingService = serviceFactory.getService(LOGGING_SERVICE) spyOn(loggingService, 'warn') apmBase.init({ active: false }) @@ -172,7 +177,7 @@ describe('ApmBase', function () { }) it('should use user provided logLevel when agent is inactive', () => { - const loggingService = serviceFactory.getService('LoggingService') + const loggingService = serviceFactory.getService(LOGGING_SERVICE) apmBase.init({ active: false, logLevel: 'error' @@ -183,7 +188,7 @@ describe('ApmBase', function () { it('should provide the public api', function () { apmBase.init({ serviceName, serverUrl }) apmBase.setInitialPageLoadName('test') - var configService = serviceFactory.getService('ConfigService') + var configService = serviceFactory.getService(CONFIG_SERVICE) expect(configService.get('pageLoadTransactionName')).toBe('test') @@ -237,7 +242,7 @@ describe('ApmBase', function () { serviceName, serverUrl }) - var transactionService = serviceFactory.getService('TransactionService') + var transactionService = serviceFactory.getService(TRANSACTION_SERVICE) transactionService.currentTransaction = undefined var tr var req = new window.XMLHttpRequest() @@ -254,7 +259,7 @@ describe('ApmBase', function () { }) it('should patch xhr when not active', function (done) { - const loggingService = serviceFactory.getService('LoggingService') + const loggingService = serviceFactory.getService(LOGGING_SERVICE) spyOn(loggingService, 'warn') apmBase.init({ active: false }) @@ -278,7 +283,7 @@ describe('ApmBase', function () { }) it('should log errors when config is invalid', () => { - const loggingService = serviceFactory.getService('LoggingService') + const loggingService = serviceFactory.getService(LOGGING_SERVICE) spyOn(loggingService, 'warn') const logErrorSpy = spyOn(loggingService, 'error') apmBase.init({ @@ -288,7 +293,7 @@ describe('ApmBase', function () { expect(loggingService.error).toHaveBeenCalledWith( `RUM agent isn't correctly configured. serverUrl, serviceName is missing` ) - const configService = serviceFactory.getService('ConfigService') + const configService = serviceFactory.getService(CONFIG_SERVICE) expect(configService.get('active')).toEqual(false) logErrorSpy.calls.reset() @@ -337,7 +342,7 @@ describe('ApmBase', function () { serverUrl, disableInstrumentations: [PAGE_LOAD] }) - const configService = serviceFactory.getService('ConfigService') + const configService = serviceFactory.getService(CONFIG_SERVICE) expect(configService.get('context.tags')).toEqual(labels) }) @@ -361,8 +366,8 @@ describe('ApmBase', function () { }) it('should fetch central config', done => { - const apmServer = serviceFactory.getService('ApmServer') - const configService = serviceFactory.getService('ConfigService') + const apmServer = serviceFactory.getService(APM_SERVER) + const configService = serviceFactory.getService(CONFIG_SERVICE) spyOn(configService, 'setLocalConfig') spyOn(configService, 'getLocalConfig') @@ -393,7 +398,7 @@ describe('ApmBase', function () { } } - const loggingService = serviceFactory.getService('LoggingService') + const loggingService = serviceFactory.getService(LOGGING_SERVICE) spyOn(loggingService, 'warn') apmServer._makeHttpRequest = createPayloadCallback('test') apmBase @@ -417,7 +422,7 @@ describe('ApmBase', function () { }) it('should wait for remote config before sending the page load', done => { - const loggingService = serviceFactory.getService('LoggingService') + const loggingService = serviceFactory.getService(LOGGING_SERVICE) spyOn(apmBase, 'fetchCentralConfig').and.callThrough() spyOn(apmBase, '_sendPageLoadMetrics').and.callFake(() => { done() diff --git a/packages/rum/test/specs/index.spec.js b/packages/rum/test/specs/index.spec.js index f743235b7..242e1198c 100644 --- a/packages/rum/test/specs/index.spec.js +++ b/packages/rum/test/specs/index.spec.js @@ -25,7 +25,7 @@ import Promise from 'promise-polyfill' import { apmBase } from '../../src/' -import { isPlatformSupported } from '@elastic/apm-rum-core' +import { isPlatformSupported, APM_SERVER } from '@elastic/apm-rum-core' import { getGlobalConfig } from '../../../../dev-utils/test-config' describe('index', function () { @@ -47,7 +47,7 @@ describe('index', function () { }) it('should init ApmBase', function (done) { - var apmServer = apmBase.serviceFactory.getService('ApmServer') + var apmServer = apmBase.serviceFactory.getService(APM_SERVER) if (globalConfig.useMocks) { apmServer._makeHttpRequest = function () { return Promise.resolve()