From 8be49558d7c006241d3e2664ab2fe42c0c8cb881 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Wed, 8 Nov 2023 15:46:22 -0500 Subject: [PATCH 01/17] Create util function to access component names if available --- .../browser/src/integrations/breadcrumbs.ts | 6 +++--- packages/replay/src/coreHandlers/handleDom.ts | 4 ++-- .../src/coreHandlers/handleKeyboardEvent.ts | 4 ++-- .../src/browser/metrics/index.ts | 8 ++++---- packages/utils/src/browser.ts | 19 +++++++++++++++++++ packages/utils/src/object.ts | 4 ++-- 6 files changed, 32 insertions(+), 13 deletions(-) diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts index f71361b7d96e..59e7c8327f5a 100644 --- a/packages/browser/src/integrations/breadcrumbs.ts +++ b/packages/browser/src/integrations/breadcrumbs.ts @@ -10,8 +10,8 @@ import type { } from '@sentry/types/build/types/breadcrumb'; import { addInstrumentationHandler, + getElementIdentifier, getEventDescription, - htmlTreeAsString, logger, parseUrl, safeJoin, @@ -153,8 +153,8 @@ function _domBreadcrumb(dom: BreadcrumbsOptions['dom']): (handlerData: HandlerDa try { const event = handlerData.event as Event | Node; target = _isEvent(event) - ? htmlTreeAsString(event.target, { keyAttrs, maxStringLength }) - : htmlTreeAsString(event, { keyAttrs, maxStringLength }); + ? getElementIdentifier(event.target, { keyAttrs, maxStringLength }) + : getElementIdentifier(event, { keyAttrs, maxStringLength }); } catch (e) { target = ''; } diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 60220f0a5a66..85c8b25e94c7 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -2,7 +2,7 @@ import { record } from '@sentry-internal/rrweb'; import type { serializedElementNodeWithId, serializedNodeWithId } from '@sentry-internal/rrweb-snapshot'; import { NodeType } from '@sentry-internal/rrweb-snapshot'; import type { Breadcrumb } from '@sentry/types'; -import { htmlTreeAsString } from '@sentry/utils'; +import { getElementIdentifier } from '@sentry/utils'; import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; @@ -98,7 +98,7 @@ function getDomTarget(handlerData: DomHandlerData): { target: Node | null; messa // Accessing event.target can throw (see getsentry/raven-js#838, #768) try { target = isClick ? getClickTargetNode(handlerData.event) : getTargetNode(handlerData.event); - message = htmlTreeAsString(target, { maxStringLength: 200 }) || ''; + message = getElementIdentifier(target, { maxStringLength: 200 }) || ''; } catch (e) { message = ''; } diff --git a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts index 0f7560f39584..db450d0b14ca 100644 --- a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts +++ b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts @@ -1,5 +1,5 @@ import type { Breadcrumb } from '@sentry/types'; -import { htmlTreeAsString } from '@sentry/utils'; +import { getElementIdentifier } from '@sentry/utils'; import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; @@ -45,7 +45,7 @@ export function getKeyboardBreadcrumb(event: KeyboardEvent): Breadcrumb | null { return null; } - const message = htmlTreeAsString(target, { maxStringLength: 200 }) || ''; + const message = getElementIdentifier(target, { maxStringLength: 200 }) || ''; const baseBreadcrumb = getBaseDomBreadcrumb(target as Node, message); return createBreadcrumb({ diff --git a/packages/tracing-internal/src/browser/metrics/index.ts b/packages/tracing-internal/src/browser/metrics/index.ts index edeedcee679f..05b12c4b2ff4 100644 --- a/packages/tracing-internal/src/browser/metrics/index.ts +++ b/packages/tracing-internal/src/browser/metrics/index.ts @@ -2,7 +2,7 @@ import type { IdleTransaction, Transaction } from '@sentry/core'; import { getActiveTransaction } from '@sentry/core'; import type { Measurements } from '@sentry/types'; -import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/utils'; +import { browserPerformanceTimeOrigin, getElementIdentifier, logger } from '@sentry/utils'; import { addClsInstrumentationHandler, @@ -100,7 +100,7 @@ export function startTrackingInteractions(): void { const duration = msToSec(entry.duration); transaction.startChild({ - description: htmlTreeAsString(entry.target), + description: getElementIdentifier(entry.target), op: `ui.interaction.${entry.name}`, origin: 'auto.ui.browser.metrics', startTimestamp: startTime, @@ -470,7 +470,7 @@ function _tagMetricInfo(transaction: Transaction): void { // Capture Properties of the LCP element that contributes to the LCP. if (_lcpEntry.element) { - transaction.setTag('lcp.element', htmlTreeAsString(_lcpEntry.element)); + transaction.setTag('lcp.element', getElementIdentifier(_lcpEntry.element)); } if (_lcpEntry.id) { @@ -489,7 +489,7 @@ function _tagMetricInfo(transaction: Transaction): void { if (_clsEntry && _clsEntry.sources) { __DEBUG_BUILD__ && logger.log('[Measurements] Adding CLS Data'); _clsEntry.sources.forEach((source, index) => - transaction.setTag(`cls.source.${index + 1}`, htmlTreeAsString(source.node)), + transaction.setTag(`cls.source.${index + 1}`, getElementIdentifier(source.node)), ); } } diff --git a/packages/utils/src/browser.ts b/packages/utils/src/browser.ts index d2d8f7af9a72..be610b2a55cb 100644 --- a/packages/utils/src/browser.ts +++ b/packages/utils/src/browser.ts @@ -157,3 +157,22 @@ export function getDomElement(selector: string): E | null { } return null; } + +/** + * Given + * + * Given a child DOM element, returns a query-selector statement describing that + * and its ancestors + * e.g. [HTMLElement] => body > div > input#foo.btn[name=baz] + * @returns generated DOM path + */ +export function getElementIdentifier( + elem: unknown, + options: string[] | { keyAttrs?: string[]; maxStringLength?: number } = {}, +): string { + if (elem instanceof HTMLElement && elem.dataset) { + return elem.dataset['component'] || htmlTreeAsString(elem, options) + } + + return htmlTreeAsString(elem, options) + } diff --git a/packages/utils/src/object.ts b/packages/utils/src/object.ts index e4e866bf2763..f6356bccf3b7 100644 --- a/packages/utils/src/object.ts +++ b/packages/utils/src/object.ts @@ -2,7 +2,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import type { WrappedFunction } from '@sentry/types'; -import { htmlTreeAsString } from './browser'; +import { getElementIdentifier } from './browser'; import { isElement, isError, isEvent, isInstanceOf, isPlainObject, isPrimitive } from './is'; import { logger } from './logger'; import { truncate } from './string'; @@ -150,7 +150,7 @@ export function convertToPlainObject(value: V): /** Creates a string representation of the target of an `Event` object */ function serializeEventTarget(target: unknown): string { try { - return isElement(target) ? htmlTreeAsString(target) : Object.prototype.toString.call(target); + return isElement(target) ? getElementIdentifier(target) : Object.prototype.toString.call(target); } catch (_oO) { return ''; } From 63bce61ce66c6499d41673452d20a9363cea505b Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Wed, 8 Nov 2023 16:08:29 -0500 Subject: [PATCH 02/17] Adjust documentation for function --- packages/utils/src/browser.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/utils/src/browser.ts b/packages/utils/src/browser.ts index be610b2a55cb..20a3e9cbd77d 100644 --- a/packages/utils/src/browser.ts +++ b/packages/utils/src/browser.ts @@ -159,12 +159,9 @@ export function getDomElement(selector: string): E | null { } /** - * Given - * - * Given a child DOM element, returns a query-selector statement describing that - * and its ancestors - * e.g. [HTMLElement] => body > div > input#foo.btn[name=baz] - * @returns generated DOM path + * Given a child DOM element, returns the component name of the element. + * If the component name does not exist, this function will fallback to `htmlTreeAsString` + * e.g. [HTMLElement] => MyComponentName */ export function getElementIdentifier( elem: unknown, From c935be16a728a1f3c3671fb0695297c7fa2b9a24 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Wed, 8 Nov 2023 16:31:24 -0500 Subject: [PATCH 03/17] run prettier --- packages/utils/src/browser.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/utils/src/browser.ts b/packages/utils/src/browser.ts index 20a3e9cbd77d..4cfc454d3ce2 100644 --- a/packages/utils/src/browser.ts +++ b/packages/utils/src/browser.ts @@ -168,8 +168,8 @@ export function getElementIdentifier( options: string[] | { keyAttrs?: string[]; maxStringLength?: number } = {}, ): string { if (elem instanceof HTMLElement && elem.dataset) { - return elem.dataset['component'] || htmlTreeAsString(elem, options) + return elem.dataset['component'] || htmlTreeAsString(elem, options); } - return htmlTreeAsString(elem, options) - } + return htmlTreeAsString(elem, options); +} From 2b5f2de33f3fe31403d4932f670a03a1d0c30742 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Thu, 9 Nov 2023 12:44:06 -0500 Subject: [PATCH 04/17] Add test --- packages/utils/src/browser.ts | 2 +- packages/utils/test/browser.test.ts | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/utils/src/browser.ts b/packages/utils/src/browser.ts index 4cfc454d3ce2..4b98736550cc 100644 --- a/packages/utils/src/browser.ts +++ b/packages/utils/src/browser.ts @@ -168,7 +168,7 @@ export function getElementIdentifier( options: string[] | { keyAttrs?: string[]; maxStringLength?: number } = {}, ): string { if (elem instanceof HTMLElement && elem.dataset) { - return elem.dataset['component'] || htmlTreeAsString(elem, options); + return elem.dataset['component'] || elem.dataset['element'] || htmlTreeAsString(elem, options); } return htmlTreeAsString(elem, options); diff --git a/packages/utils/test/browser.test.ts b/packages/utils/test/browser.test.ts index 040789fe8426..a54b93458b04 100644 --- a/packages/utils/test/browser.test.ts +++ b/packages/utils/test/browser.test.ts @@ -1,11 +1,14 @@ import { JSDOM } from 'jsdom'; -import { getDomElement, htmlTreeAsString } from '../src/browser'; +import { getDomElement, getElementIdentifier, htmlTreeAsString } from '../src/browser'; beforeAll(() => { const dom = new JSDOM(); + // @ts-expect-error need to override global document global.document = dom.window.document; + // @ts-expect-error + global.HTMLElement = new JSDOM().window.HTMLElement; }); describe('htmlTreeAsString', () => { @@ -84,3 +87,14 @@ describe('getDomElement', () => { expect(el?.id).toEqual('mydiv'); }); }); + +describe('getElementIdentifier', () => { + it('returns the component name if it exists', () => { + const el = document.createElement('div'); + el.innerHTML = '
😎
'; + el.setAttribute('data-component', 'VeryCoolComponent'); + expect(el).toBeDefined(); + + expect(getElementIdentifier(el)).toBe('VeryCoolComponent'); + }); +}); From cbda225b5270f99d2f588288c33d3647c6abc60d Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Thu, 9 Nov 2023 13:00:09 -0500 Subject: [PATCH 05/17] Nvm lets not make this test with JSDOM --- packages/utils/test/browser.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/utils/test/browser.test.ts b/packages/utils/test/browser.test.ts index a54b93458b04..646123826f85 100644 --- a/packages/utils/test/browser.test.ts +++ b/packages/utils/test/browser.test.ts @@ -87,14 +87,3 @@ describe('getDomElement', () => { expect(el?.id).toEqual('mydiv'); }); }); - -describe('getElementIdentifier', () => { - it('returns the component name if it exists', () => { - const el = document.createElement('div'); - el.innerHTML = '
😎
'; - el.setAttribute('data-component', 'VeryCoolComponent'); - expect(el).toBeDefined(); - - expect(getElementIdentifier(el)).toBe('VeryCoolComponent'); - }); -}); From f48b3c7f01750574e96af6106f62f5226a824bbb Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Thu, 9 Nov 2023 14:04:12 -0500 Subject: [PATCH 06/17] Add integration test for checking a click interaction uses the annotated component name --- .../interactions/assets/script.js | 1 + .../browsertracing/interactions/template.html | 1 + .../browsertracing/interactions/test.ts | 32 +++++++++++++++++++ packages/utils/test/browser.test.ts | 2 +- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/assets/script.js b/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/assets/script.js index 89d814bd397d..a37a2c70ad27 100644 --- a/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/assets/script.js +++ b/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/assets/script.js @@ -14,3 +14,4 @@ const delay = e => { }; document.querySelector('[data-test-id=interaction-button]').addEventListener('click', delay); +document.querySelector('[data-test-id=annotated-button]').addEventListener('click', delay); diff --git a/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/template.html b/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/template.html index e16deb9ee519..1a93874744b7 100644 --- a/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/template.html +++ b/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/template.html @@ -6,6 +6,7 @@
Rendered Before Long Task
+ diff --git a/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/test.ts b/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/test.ts index e79b724ec91a..fd4910faf2cc 100644 --- a/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/test.ts +++ b/packages/browser-integration-tests/suites/tracing/browsertracing/interactions/test.ts @@ -80,3 +80,35 @@ sentryTest( } }, ); + +sentryTest( + 'should use the component name for a clicked element when it is available', + async ({ browserName, getLocalTestPath, page }) => { + const supportedBrowsers = ['chromium', 'firefox']; + + if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) { + sentryTest.skip(); + } + + await page.route('**/path/to/script.js', (route: Route) => + route.fulfill({ path: `${__dirname}/assets/script.js` }), + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + await getFirstSentryEnvelopeRequest(page); + + await page.locator('[data-test-id=annotated-button]').click(); + + const envelopes = await getMultipleSentryEnvelopeRequests(page, 1); + expect(envelopes).toHaveLength(1); + const eventData = envelopes[0]; + + expect(eventData.spans).toHaveLength(1); + + const interactionSpan = eventData.spans![0]; + expect(interactionSpan.op).toBe('ui.interaction.click'); + expect(interactionSpan.description).toBe('AnnotatedButton'); + }, +); diff --git a/packages/utils/test/browser.test.ts b/packages/utils/test/browser.test.ts index 646123826f85..52f2821cf31e 100644 --- a/packages/utils/test/browser.test.ts +++ b/packages/utils/test/browser.test.ts @@ -1,6 +1,6 @@ import { JSDOM } from 'jsdom'; -import { getDomElement, getElementIdentifier, htmlTreeAsString } from '../src/browser'; +import { getDomElement, htmlTreeAsString } from '../src/browser'; beforeAll(() => { const dom = new JSDOM(); From cac7d8219ae258352c9fa56a203f54d10e8bbfe6 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Thu, 9 Nov 2023 14:39:23 -0500 Subject: [PATCH 07/17] Add integration test for breadcrumbs --- .../Breadcrumbs/dom/click/template.html | 1 + .../Breadcrumbs/dom/click/test.ts | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/template.html b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/template.html index 5048dfd754f2..e4bf6204f578 100644 --- a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/template.html +++ b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/template.html @@ -7,5 +7,6 @@ + diff --git a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/test.ts b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/test.ts index bd8f0e9270c6..fc69fd891d5e 100644 --- a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/test.ts +++ b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/test.ts @@ -57,3 +57,35 @@ sentryTest('captures Breadcrumb for clicks & debounces them for a second', async }, ]); }); + +sentryTest('prioritizes the annotated component nam', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('**/foo', route => { + return route.fulfill({ + status: 200, + body: JSON.stringify({ + userNames: ['John', 'Jane'], + }), + headers: { + 'Content-Type': 'application/json', + }, + }); + }); + + const promise = getFirstSentryEnvelopeRequest(page); + + await page.goto(url); + await page.click('#annotated-button'); + await page.evaluate('Sentry.captureException("test exception")'); + + const eventData = await promise; + + expect(eventData.breadcrumbs).toEqual([ + { + timestamp: expect.any(Number), + category: 'ui.click', + message: 'AnnotatedButton', + }, + ]); +}); From 431603f4e2c68a12ee4f8aa503f0ac54746bfd64 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Thu, 9 Nov 2023 14:46:36 -0500 Subject: [PATCH 08/17] Add another integration test for input breadcrumbs --- .../Breadcrumbs/dom/click/test.ts | 55 ++++++++++--------- .../Breadcrumbs/dom/textInput/template.html | 1 + .../Breadcrumbs/dom/textInput/test.ts | 43 +++++++++++++++ 3 files changed, 73 insertions(+), 26 deletions(-) diff --git a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/test.ts b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/test.ts index fc69fd891d5e..d5d5caa684e9 100644 --- a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/test.ts +++ b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/click/test.ts @@ -58,34 +58,37 @@ sentryTest('captures Breadcrumb for clicks & debounces them for a second', async ]); }); -sentryTest('prioritizes the annotated component nam', async ({ getLocalTestUrl, page }) => { - const url = await getLocalTestUrl({ testDir: __dirname }); - - await page.route('**/foo', route => { - return route.fulfill({ - status: 200, - body: JSON.stringify({ - userNames: ['John', 'Jane'], - }), - headers: { - 'Content-Type': 'application/json', - }, +sentryTest( + 'prioritizes the annotated component name within the breadcrumb message', + async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('**/foo', route => { + return route.fulfill({ + status: 200, + body: JSON.stringify({ + userNames: ['John', 'Jane'], + }), + headers: { + 'Content-Type': 'application/json', + }, + }); }); - }); - const promise = getFirstSentryEnvelopeRequest(page); + const promise = getFirstSentryEnvelopeRequest(page); - await page.goto(url); - await page.click('#annotated-button'); - await page.evaluate('Sentry.captureException("test exception")'); + await page.goto(url); + await page.click('#annotated-button'); + await page.evaluate('Sentry.captureException("test exception")'); - const eventData = await promise; + const eventData = await promise; - expect(eventData.breadcrumbs).toEqual([ - { - timestamp: expect.any(Number), - category: 'ui.click', - message: 'AnnotatedButton', - }, - ]); -}); + expect(eventData.breadcrumbs).toEqual([ + { + timestamp: expect.any(Number), + category: 'ui.click', + message: 'AnnotatedButton', + }, + ]); + }, +); diff --git a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/textInput/template.html b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/textInput/template.html index b3d53fbf9a3e..a09f5ccff8ec 100644 --- a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/textInput/template.html +++ b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/textInput/template.html @@ -7,5 +7,6 @@ + diff --git a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/textInput/test.ts b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/textInput/test.ts index b3393561f331..b6cd4241e78a 100644 --- a/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/textInput/test.ts +++ b/packages/browser-integration-tests/suites/integrations/Breadcrumbs/dom/textInput/test.ts @@ -64,3 +64,46 @@ sentryTest('captures Breadcrumb for events on inputs & debounced them', async ({ }, ]); }); + +sentryTest( + 'prioritizes the annotated component name within the breadcrumb message', + async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.route('**/foo', route => { + return route.fulfill({ + status: 200, + body: JSON.stringify({ + userNames: ['John', 'Jane'], + }), + headers: { + 'Content-Type': 'application/json', + }, + }); + }); + + const promise = getFirstSentryEnvelopeRequest(page); + + await page.goto(url); + + await page.click('#annotated-input'); + await page.type('#annotated-input', 'John', { delay: 1 }); + + await page.evaluate('Sentry.captureException("test exception")'); + const eventData = await promise; + expect(eventData.exception?.values).toHaveLength(1); + + expect(eventData.breadcrumbs).toEqual([ + { + timestamp: expect.any(Number), + category: 'ui.click', + message: 'AnnotatedInput', + }, + { + timestamp: expect.any(Number), + category: 'ui.input', + message: 'AnnotatedInput', + }, + ]); + }, +); From 2259daef51ca7d23681463a73ea71a75d48a10f2 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Thu, 9 Nov 2023 15:12:28 -0500 Subject: [PATCH 09/17] Add description to ts-expect-error --- packages/utils/test/browser.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/test/browser.test.ts b/packages/utils/test/browser.test.ts index 52f2821cf31e..3099da0680e2 100644 --- a/packages/utils/test/browser.test.ts +++ b/packages/utils/test/browser.test.ts @@ -7,7 +7,7 @@ beforeAll(() => { // @ts-expect-error need to override global document global.document = dom.window.document; - // @ts-expect-error + // @ts-expect-error need to add HTMLElement type or it will not be found global.HTMLElement = new JSDOM().window.HTMLElement; }); From a37035c6c7f25ec7432de9d99f6fd5858652afce Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Fri, 10 Nov 2023 15:35:37 -0500 Subject: [PATCH 10/17] Add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07790b7ddfc1..51bfa89bc4bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.80.0-alpha.0 + +- feat(utils): Prioritize Component name attributes over HTML Tree String (#9496) + ## 7.80.0 - feat(astro): Add distributed tracing via `` tags (#9483) From cc74458ae348142904f821496db206488608e0e4 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Tue, 14 Nov 2023 11:21:35 -0500 Subject: [PATCH 11/17] Add the version changelog back in --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6315e5ba18af..e3743a6e1fec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.80.2-alpha.0 + +- feat(utils): Prioritize Component name attributes over HTML Tree String (#9496) + ## 7.80.1 - fix(astro): Adjust Vite plugin config to upload server source maps (#9541) From c35331c6043b9ac1a96a1c6d9e5f7d3dc700f928 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Tue, 14 Nov 2023 17:01:07 -0500 Subject: [PATCH 12/17] Keep html string for replays, add componentName as additional field --- packages/replay/src/coreHandlers/handleDom.ts | 5 +++-- packages/replay/src/coreHandlers/handleKeyboardEvent.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 85c8b25e94c7..acdff32767d0 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -2,7 +2,7 @@ import { record } from '@sentry-internal/rrweb'; import type { serializedElementNodeWithId, serializedNodeWithId } from '@sentry-internal/rrweb-snapshot'; import { NodeType } from '@sentry-internal/rrweb-snapshot'; import type { Breadcrumb } from '@sentry/types'; -import { getElementIdentifier } from '@sentry/utils'; +import { getElementIdentifier, htmlTreeAsString } from '@sentry/utils'; import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; @@ -70,6 +70,7 @@ export function getBaseDomBreadcrumb(target: Node | null, message: string): Brea .map(text => (text as string).trim()) .join(''), attributes: getAttributesToRecord(element.attributes), + componentName: getElementIdentifier(target), }, } : {}, @@ -98,7 +99,7 @@ function getDomTarget(handlerData: DomHandlerData): { target: Node | null; messa // Accessing event.target can throw (see getsentry/raven-js#838, #768) try { target = isClick ? getClickTargetNode(handlerData.event) : getTargetNode(handlerData.event); - message = getElementIdentifier(target, { maxStringLength: 200 }) || ''; + message = htmlTreeAsString(target, { maxStringLength: 200 }) || ''; } catch (e) { message = ''; } diff --git a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts index db450d0b14ca..15540a3a79a8 100644 --- a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts +++ b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts @@ -1,5 +1,5 @@ import type { Breadcrumb } from '@sentry/types'; -import { getElementIdentifier } from '@sentry/utils'; +import { getElementIdentifier, htmlTreeAsString } from '@sentry/utils'; import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; @@ -45,7 +45,7 @@ export function getKeyboardBreadcrumb(event: KeyboardEvent): Breadcrumb | null { return null; } - const message = getElementIdentifier(target, { maxStringLength: 200 }) || ''; + const message = htmlTreeAsString(target, { maxStringLength: 200 }) || ''; const baseBreadcrumb = getBaseDomBreadcrumb(target as Node, message); return createBreadcrumb({ @@ -58,6 +58,7 @@ export function getKeyboardBreadcrumb(event: KeyboardEvent): Breadcrumb | null { ctrlKey, altKey, key, + componentName: getElementIdentifier(target), }, }); } From e8e37a9f9b1933efb91154a7187a883e9e7ea639 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Tue, 14 Nov 2023 17:11:11 -0500 Subject: [PATCH 13/17] Add new entry in changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3743a6e1fec..b9deca4c9dde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.80.2-alpha.1 + +No longer will prioritize the component names for replays, as this will break searching by CSS selector. +Instead, adds `componentName` as an additional field in the payload + ## 7.80.2-alpha.0 - feat(utils): Prioritize Component name attributes over HTML Tree String (#9496) From 6db53ea169aed22d8cdc2cf7955c5f41e5c26cb5 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Tue, 14 Nov 2023 17:45:48 -0500 Subject: [PATCH 14/17] Attempt to fix tests --- .../suites/replay/keyboardEvents/test.ts | 3 +++ .../browser-integration-tests/utils/replayEventTemplates.ts | 1 + .../replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts | 2 ++ 3 files changed, 6 insertions(+) diff --git a/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts index edeacb7f2db0..7b911ac4c62a 100644 --- a/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts +++ b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts @@ -75,6 +75,7 @@ sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPa ctrlKey: true, altKey: false, key: 'Control', + componentName: 'body', }, }, { @@ -90,6 +91,7 @@ sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPa ctrlKey: true, altKey: false, key: 'A', + componentName: 'body', }, }, { @@ -105,6 +107,7 @@ sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPa tagName: 'input', textContent: '', }, + componentName: 'body > input#input', }, }, ]); diff --git a/packages/browser-integration-tests/utils/replayEventTemplates.ts b/packages/browser-integration-tests/utils/replayEventTemplates.ts index 8824d8ab1812..ff16cc9d41e7 100644 --- a/packages/browser-integration-tests/utils/replayEventTemplates.ts +++ b/packages/browser-integration-tests/utils/replayEventTemplates.ts @@ -212,6 +212,7 @@ export const expectedClickBreadcrumb = { tagName: expect.any(String), textContent: expect.any(String), }, + componentName: expect.any(String), }, }; diff --git a/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts b/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts index d08f1ef1a800..9ea909c73eab 100644 --- a/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts +++ b/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts @@ -39,6 +39,7 @@ describe('Unit | coreHandlers | handleKeyboardEvent', () => { key: 'Escape', metaKey: false, shiftKey: false, + componentName: 'body', }, message: 'body', timestamp: expect.any(Number), @@ -69,6 +70,7 @@ describe('Unit | coreHandlers | handleKeyboardEvent', () => { key, metaKey: false, shiftKey: false, + componentName: 'body', }, message: 'body', timestamp: expect.any(Number), From 96263f2a746d8c4848c7453d29728b2d5e7ef3a1 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Tue, 14 Nov 2023 18:08:12 -0500 Subject: [PATCH 15/17] Do not add componentName as data in replay payload --- .../suites/replay/keyboardEvents/test.ts | 3 --- .../browser-integration-tests/utils/replayEventTemplates.ts | 1 - packages/replay/src/coreHandlers/handleDom.ts | 1 - packages/replay/src/coreHandlers/handleKeyboardEvent.ts | 1 - .../replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts | 2 -- 5 files changed, 8 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts index 7b911ac4c62a..edeacb7f2db0 100644 --- a/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts +++ b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts @@ -75,7 +75,6 @@ sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPa ctrlKey: true, altKey: false, key: 'Control', - componentName: 'body', }, }, { @@ -91,7 +90,6 @@ sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPa ctrlKey: true, altKey: false, key: 'A', - componentName: 'body', }, }, { @@ -107,7 +105,6 @@ sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPa tagName: 'input', textContent: '', }, - componentName: 'body > input#input', }, }, ]); diff --git a/packages/browser-integration-tests/utils/replayEventTemplates.ts b/packages/browser-integration-tests/utils/replayEventTemplates.ts index ff16cc9d41e7..8824d8ab1812 100644 --- a/packages/browser-integration-tests/utils/replayEventTemplates.ts +++ b/packages/browser-integration-tests/utils/replayEventTemplates.ts @@ -212,7 +212,6 @@ export const expectedClickBreadcrumb = { tagName: expect.any(String), textContent: expect.any(String), }, - componentName: expect.any(String), }, }; diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index acdff32767d0..2a9b6a9f9d62 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -70,7 +70,6 @@ export function getBaseDomBreadcrumb(target: Node | null, message: string): Brea .map(text => (text as string).trim()) .join(''), attributes: getAttributesToRecord(element.attributes), - componentName: getElementIdentifier(target), }, } : {}, diff --git a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts index 15540a3a79a8..f4baa73a6585 100644 --- a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts +++ b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts @@ -58,7 +58,6 @@ export function getKeyboardBreadcrumb(event: KeyboardEvent): Breadcrumb | null { ctrlKey, altKey, key, - componentName: getElementIdentifier(target), }, }); } diff --git a/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts b/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts index 9ea909c73eab..d08f1ef1a800 100644 --- a/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts +++ b/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts @@ -39,7 +39,6 @@ describe('Unit | coreHandlers | handleKeyboardEvent', () => { key: 'Escape', metaKey: false, shiftKey: false, - componentName: 'body', }, message: 'body', timestamp: expect.any(Number), @@ -70,7 +69,6 @@ describe('Unit | coreHandlers | handleKeyboardEvent', () => { key, metaKey: false, shiftKey: false, - componentName: 'body', }, message: 'body', timestamp: expect.any(Number), From a8c8096830ddd3d23bcb3f407fa40e873f399ca2 Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Tue, 14 Nov 2023 18:08:26 -0500 Subject: [PATCH 16/17] Update changelog --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9deca4c9dde..60ff525786f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,6 @@ ## 7.80.2-alpha.1 No longer will prioritize the component names for replays, as this will break searching by CSS selector. -Instead, adds `componentName` as an additional field in the payload ## 7.80.2-alpha.0 From fc3a7ce7472dcb1e9a506b221efc98c55bd0389e Mon Sep 17 00:00:00 2001 From: Ash Anand Date: Tue, 14 Nov 2023 18:18:04 -0500 Subject: [PATCH 17/17] Remove unused function --- packages/replay/src/coreHandlers/handleDom.ts | 2 +- packages/replay/src/coreHandlers/handleKeyboardEvent.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 2a9b6a9f9d62..60220f0a5a66 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -2,7 +2,7 @@ import { record } from '@sentry-internal/rrweb'; import type { serializedElementNodeWithId, serializedNodeWithId } from '@sentry-internal/rrweb-snapshot'; import { NodeType } from '@sentry-internal/rrweb-snapshot'; import type { Breadcrumb } from '@sentry/types'; -import { getElementIdentifier, htmlTreeAsString } from '@sentry/utils'; +import { htmlTreeAsString } from '@sentry/utils'; import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; diff --git a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts index f4baa73a6585..0f7560f39584 100644 --- a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts +++ b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts @@ -1,5 +1,5 @@ import type { Breadcrumb } from '@sentry/types'; -import { getElementIdentifier, htmlTreeAsString } from '@sentry/utils'; +import { htmlTreeAsString } from '@sentry/utils'; import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb';