From 2879166426208f073263d29008c1a5223df36c77 Mon Sep 17 00:00:00 2001 From: Kostas Pelelis <13784424+kpelelis@users.noreply.github.com> Date: Mon, 16 Dec 2024 17:27:29 +0200 Subject: [PATCH] Faro: Improve performance of TRACKING_URLS regex (#98022) There have been reports of faro performing poorly when the URLs generated are long (there was a 39KB one). The TRACKING_URLS we are using leading wildcard characters, leading to excessive backtracking. This commit uses a simpler regular expression, that ensures we are blocking the appropriate URLs without the performance hit. To prevent that from happening, a timed test is introduced. The timeout threshold is long enough to be hardware independent. --- .../GrafanaJavascriptAgentBackend.test.ts | 20 ++++++++++++++++--- .../GrafanaJavascriptAgentBackend.ts | 5 ++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.test.ts b/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.test.ts index ca7d940b2fc..5ee5b184736 100644 --- a/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.test.ts +++ b/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.test.ts @@ -5,7 +5,11 @@ import * as faroWebSdkModule from '@grafana/faro-web-sdk'; import { BrowserConfig, FetchTransport } from '@grafana/faro-web-sdk'; import { EchoSrvTransport } from './EchoSrvTransport'; -import { GrafanaJavascriptAgentBackend, GrafanaJavascriptAgentBackendOptions } from './GrafanaJavascriptAgentBackend'; +import { + GrafanaJavascriptAgentBackend, + GrafanaJavascriptAgentBackendOptions, + TRACKING_URLS, +} from './GrafanaJavascriptAgentBackend'; describe('GrafanaJavascriptAgentEchoBackend', () => { let mockedSetUser: jest.Mock; @@ -93,8 +97,7 @@ describe('GrafanaJavascriptAgentEchoBackend', () => { expect(initializeFaroMock.mock.calls[0][0].transports?.[0]).toBeInstanceOf(EchoSrvTransport); expect(initializeFaroMock.mock.calls[0][0].transports?.[0].getIgnoreUrls()).toEqual([ /.*\/log-grafana-javascript-agent.*/, - /.*.google-analytics.com*.*/, - /.*.googletagmanager.com*.*/, + /\.(google-analytics|googletagmanager)\.com/, /frontend-metrics/, /\/collect(?:\/[\w]*)?$/, ]); @@ -116,6 +119,17 @@ describe('GrafanaJavascriptAgentEchoBackend', () => { }); }); + test('will ensure the performance of TRACKING_URLS', async () => { + // 10e6 is based on true events + const longString = Array.from({ length: 10e6 }, () => Math.random().toString(36)[2]).join(''); + const maxExecutionTime = 500; + + const start = performance.now(); + TRACKING_URLS.some((u) => u && longString.match(u) !== null); + const end = performance.now(); + expect(end - start).toBeLessThanOrEqual(maxExecutionTime); + }); + //@FIXME - make integration test work // it('integration test with EchoSrv and GrafanaJavascriptAgent', async () => { diff --git a/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.ts b/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.ts index 4ea54e38053..2aa81fb11fc 100644 --- a/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.ts +++ b/public/app/core/services/echo/backends/grafana-javascript-agent/GrafanaJavascriptAgentBackend.ts @@ -37,9 +37,8 @@ export interface GrafanaJavascriptAgentBackendOptions extends BrowserConfig { ignoreUrls: RegExp[]; } -const TRACKING_URLS = [ - /.*.google-analytics.com*.*/, - /.*.googletagmanager.com*.*/, +export const TRACKING_URLS = [ + /\.(google-analytics|googletagmanager)\.com/, /frontend-metrics/, /\/collect(?:\/[\w]*)?$/, ];