feat: Add serializeAttribute option to breadcrumbs / dom. (#3620)

* Add `serializeAttribute` option to breadcrumbs / dom.

* Add jsdom as a dev-dependency of utils package.

* Enable `jsdom` legacy versions for `utils` package.

Author: onurtemizkan

packages/browser/src/integrations/breadcrumbs.ts

@@ -14,7 +14,7 @@ import {
 /** JSDoc */
 interface BreadcrumbsOptions {
   console: boolean;
-  dom: boolean;
+  dom: boolean | { serializeAttribute: string };
   fetch: boolean;
   history: boolean;
   sentry: boolean;
@@ -162,12 +162,13 @@ export class Breadcrumbs implements Integration {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   private _domBreadcrumb(handlerData: { [key: string]: any }): void {
     let target;
+    const keyAttr = typeof this._options.dom === 'object' ? this._options.dom.serializeAttribute : undefined;
 
     // Accessing event.target can throw (see getsentry/raven-js#838, #768)
     try {
       target = handlerData.event.target
-        ? htmlTreeAsString(handlerData.event.target as Node)
-        : htmlTreeAsString((handlerData.event as unknown) as Node);
+        ? htmlTreeAsString(handlerData.event.target as Node, keyAttr)
+        : htmlTreeAsString((handlerData.event as unknown) as Node, keyAttr);
     } catch (e) {
       target = '<unknown>';
     }

packages/utils/package.json

@@ -23,6 +23,7 @@
     "@sentry-internal/eslint-config-sdk": "6.5.1",
     "chai": "^4.1.2",
     "jest": "^24.7.1",
+    "jsdom": "^16.2.2",
     "npm-run-all": "^4.1.2",
     "prettier": "1.19.0",
     "rimraf": "^2.6.3",

packages/utils/src/browser.ts

@@ -6,7 +6,7 @@ import { isString } from './is';
  * e.g. [HTMLElement] => body > div > input#foo.btn[name=baz]
  * @returns generated DOM path
  */
-export function htmlTreeAsString(elem: unknown): string {
+export function htmlTreeAsString(elem: unknown, keyAttr?: string): string {
   type SimpleNode = {
     parentNode: SimpleNode;
   } | null;
@@ -28,7 +28,7 @@ export function htmlTreeAsString(elem: unknown): string {
 
     // eslint-disable-next-line no-plusplus
     while (currentElem && height++ < MAX_TRAVERSE_HEIGHT) {
-      nextStr = _htmlElementAsString(currentElem);
+      nextStr = _htmlElementAsString(currentElem, keyAttr);
       // bail out if
       // - nextStr is the 'html' element
       // - the length of the string that would be created exceeds MAX_OUTPUT_LEN
@@ -54,7 +54,7 @@ export function htmlTreeAsString(elem: unknown): string {
  * e.g. [HTMLElement] => input#foo.btn[name=baz]
  * @returns generated DOM path
  */
-function _htmlElementAsString(el: unknown): string {
+function _htmlElementAsString(el: unknown, keyAttr?: string): string {
   const elem = el as {
     tagName?: string;
     id?: string;
@@ -74,16 +74,22 @@ function _htmlElementAsString(el: unknown): string {
   }
 
   out.push(elem.tagName.toLowerCase());
-  if (elem.id) {
-    out.push(`#${elem.id}`);
-  }
 
-  // eslint-disable-next-line prefer-const
-  className = elem.className;
-  if (className && isString(className)) {
-    classes = className.split(/\s+/);
-    for (i = 0; i < classes.length; i++) {
-      out.push(`.${classes[i]}`);
+  const keyAttrValue = keyAttr ? elem.getAttribute(keyAttr) : null;
+  if (keyAttrValue) {
+    out.push(`[${keyAttr}="${keyAttrValue}"]`);
+  } else {
+    if (elem.id) {
+      out.push(`#${elem.id}`);
+    }
+
+    // eslint-disable-next-line prefer-const
+    className = elem.className;
+    if (className && isString(className)) {
+      classes = className.split(/\s+/);
+      for (i = 0; i < classes.length; i++) {
+        out.push(`.${classes[i]}`);
+      }
     }
   }
   const allowedAttrs = ['type', 'name', 'title', 'alt'];

packages/utils/test/browser.test.ts

@@ -0,0 +1,47 @@
+import { JSDOM } from 'jsdom';
+
+import { htmlTreeAsString } from '../src/browser';
+
+beforeAll(() => {
+  const dom = new JSDOM();
+  // @ts-ignore need to override global document
+  global.document = dom.window.document;
+});
+
+describe('htmlTreeAsString', () => {
+  it('generates html tree for a simple element', () => {
+    const el = document.createElement('ul');
+    el.innerHTML = `<li class="container">
+      <button id="err-btn" class="button" />
+    </li>`;
+    document.body.appendChild(el);
+
+    expect(htmlTreeAsString(document.getElementById('err-btn'))).toBe(
+      'body > ul > li.container > button#err-btn.button',
+    );
+  });
+
+  it('inserts pre-defined attribute values by default', () => {
+    const el = document.createElement('ul');
+    el.innerHTML = `<li title="container-title" class="container">
+      <img id="cat" test-id="cat-test-id" alt="kitten" />
+    </li>`;
+    document.body.appendChild(el);
+
+    expect(htmlTreeAsString(document.getElementById('cat'))).toBe(
+      'body > ul > li.container[title="container-title"] > img#cat[alt="kitten"]',
+    );
+  });
+
+  it('insert key attribute instead of class names or ids when serializeAttribute is defined and the element has it', () => {
+    const el = document.createElement('ul');
+    el.innerHTML = `<li class="li-class" title="li-title">
+      <img id="cat-2" class="catimg" test-id="cat-2-test-id"/>
+    </li>`;
+    document.body.appendChild(el);
+
+    expect(htmlTreeAsString(document.getElementById('cat-2'), 'test-id')).toBe(
+      'body > ul > li.li-class[title="li-title"] > img[test-id="cat-2-test-id"]',
+    );
+  });
+});

scripts/test.sh

@@ -12,6 +12,9 @@ if [[ "$(cut -d. -f1 <<<"$NODE_VERSION")" -le 6 ]]; then
   cd packages/tracing
   yarn add --dev --ignore-engines --ignore-scripts [email protected]
   cd ../..
+  cd packages/utils
+  yarn add --dev --ignore-engines --ignore-scripts [email protected]
+  cd ../..
 
   # only test against @sentry/node and its dependencies - node 6 is too old for anything else to work
   yarn test --scope="@sentry/core" --scope="@sentry/hub" --scope="@sentry/minimal" --scope="@sentry/node" --scope="@sentry/utils" --scope="@sentry/tracing"
@@ -23,6 +26,9 @@ elif [[ "$(cut -d. -f1 <<<"$NODE_VERSION")" -le 8 ]]; then
   cd packages/tracing
   yarn add --dev --ignore-engines --ignore-scripts [email protected]
   cd ../..
+  cd packages/utils
+  yarn add --dev --ignore-engines --ignore-scripts [email protected]
+  cd ../..
 
   # ember tests happen separately, and the rest fail on node 8 for various syntax or dependency reasons
   yarn test --ignore="@sentry/ember" --ignore="@sentry-internal/eslint-plugin-sdk" --ignore="@sentry/react" --ignore="@sentry/wasm" --ignore="@sentry/gatsby" --ignore="@sentry/serverless" --ignore="@sentry/nextjs"