Skip to content

Commit

Permalink
feat: touch breadcrumbs info improvements (#3899)
Browse files Browse the repository at this point in the history
* cleanup & fix touchevent ignore behaviour

* feat: support annotated components in touch evetns

* feat: deduplicate subsequent events in the touch path

* chore: add changelog

* Update CHANGELOG.md

Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>

* Update CHANGELOG.md

* review comments

---------

Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com>
  • Loading branch information
3 people authored Jun 25, 2024
1 parent 11321ee commit 4a6664f
Show file tree
Hide file tree
Showing 9 changed files with 207 additions and 69 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Features

- Improve touch event component info if annotated with [`@sentry/babel-plugin-component-annotate`](https://www.npmjs.com/package/@sentry/babel-plugin-component-annotate) ([#3899](https://github.com/getsentry/sentry-react-native/pull/3899))

## 5.24.1

### Fixes
Expand Down Expand Up @@ -1033,6 +1039,7 @@ This has been fixed in [version `5.9.1`](https://github.com/getsentry/sentry-rea
## 5.4.0
### Features
- Add TS 4.1 typings ([#2995](https://github.com/getsentry/sentry-react-native/pull/2995))
- TS 3.8 are present and work automatically with older projects
- Add CPU Info to Device Context ([#2984](https://github.com/getsentry/sentry-react-native/pull/2984))
Expand Down
5 changes: 4 additions & 1 deletion samples/expo/babel.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
module.exports = function(api) {
const componentAnnotatePlugin = require('@sentry/babel-plugin-component-annotate');

module.exports = function (api) {
api.cache(false);
return {
presets: ['babel-preset-expo'],
Expand All @@ -11,6 +13,7 @@ module.exports = function(api) {
},
},
],
componentAnnotatePlugin,
],
};
};
1 change: 1 addition & 0 deletions samples/expo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"devDependencies": {
"@babel/core": "^7.20.0",
"@babel/preset-env": "7.1.6",
"@sentry/babel-plugin-component-annotate": "^2.18.0",
"@types/node": "20.10.4"
},
"overrides": {
Expand Down
5 changes: 5 additions & 0 deletions samples/expo/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2413,6 +2413,11 @@
component-type "^1.2.1"
join-component "^1.1.0"

"@sentry/babel-plugin-component-annotate@^2.18.0":
version "2.18.0"
resolved "https://registry.yarnpkg.com/@sentry/babel-plugin-component-annotate/-/babel-plugin-component-annotate-2.18.0.tgz#3bee98f94945643b0762ceed1f6cca60db52bdbd"
integrity sha512-9L4RbhS3WNtc/SokIhc0dwgcvs78YSQPakZejsrIgnzLzCi8mS6PeT+BY0+QCtsXxjd1egM8hqcJeB0lukBkXA==

"@sideway/address@^4.1.3":
version "4.1.4"
resolved "https://registry.yarnpkg.com/@sideway/address/-/address-4.1.4.tgz#03dccebc6ea47fdc226f7d3d1ad512955d4783f0"
Expand Down
3 changes: 3 additions & 0 deletions samples/react-native/babel.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const componentAnnotatePlugin = require('@sentry/babel-plugin-component-annotate');

module.exports = {
presets: ['module:@react-native/babel-preset'],
plugins: [
Expand All @@ -9,5 +11,6 @@ module.exports = {
},
},
],
componentAnnotatePlugin,
],
};
1 change: 1 addition & 0 deletions samples/react-native/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"@react-native/eslint-config": "^0.73.1",
"@react-native/metro-config": "^0.73.1",
"@react-native/typescript-config": "^0.73.1",
"@sentry/babel-plugin-component-annotate": "^2.18.0",
"@types/react": "^18.2.65",
"@types/react-native-vector-icons": "^6.4.18",
"@types/react-test-renderer": "^18.0.0",
Expand Down
5 changes: 5 additions & 0 deletions samples/react-native/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3127,6 +3127,11 @@
color "^4.2.3"
warn-once "^0.1.0"

"@sentry/babel-plugin-component-annotate@^2.18.0":
version "2.18.0"
resolved "https://registry.yarnpkg.com/@sentry/babel-plugin-component-annotate/-/babel-plugin-component-annotate-2.18.0.tgz#3bee98f94945643b0762ceed1f6cca60db52bdbd"
integrity sha512-9L4RbhS3WNtc/SokIhc0dwgcvs78YSQPakZejsrIgnzLzCi8mS6PeT+BY0+QCtsXxjd1egM8hqcJeB0lukBkXA==

"@sideway/address@^4.1.3":
version "4.1.4"
resolved "https://registry.yarnpkg.com/@sideway/address/-/address-4.1.4.tgz#03dccebc6ea47fdc226f7d3d1ad512955d4783f0"
Expand Down
143 changes: 81 additions & 62 deletions src/js/touchevents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { addBreadcrumb, getCurrentHub } from '@sentry/core';
import type { SeverityLevel } from '@sentry/types';
import { logger } from '@sentry/utils';
import * as React from 'react';
import type { GestureResponderEvent} from 'react-native';
import type { GestureResponderEvent } from 'react-native';
import { StyleSheet, View } from 'react-native';

import { createIntegration } from './integrations/factory';
Expand Down Expand Up @@ -53,6 +53,9 @@ const DEFAULT_BREADCRUMB_TYPE = 'user';
const DEFAULT_MAX_COMPONENT_TREE_SIZE = 20;

const SENTRY_LABEL_PROP_KEY = 'sentry-label';
const SENTRY_COMPONENT_PROP_KEY = 'data-sentry-component';
const SENTRY_ELEMENT_PROP_KEY = 'data-sentry-element';
const SENTRY_FILE_PROP_KEY = 'data-sentry-source-file';

interface ElementInstance {
elementType?: {
Expand All @@ -63,6 +66,13 @@ interface ElementInstance {
return?: ElementInstance;
}

interface TouchedComponentInfo {
name?: string;
label?: string;
element?: string;
file?: string;
}

interface PrivateGestureResponderEvent extends GestureResponderEvent {
_targetInst?: ElementInstance;
}
Expand All @@ -71,7 +81,6 @@ interface PrivateGestureResponderEvent extends GestureResponderEvent {
* Boundary to log breadcrumbs for interaction events.
*/
class TouchEventBoundary extends React.Component<TouchEventBoundaryProps> {

public static displayName: string = '__Sentry.TouchEventBoundary';
public static defaultProps: Partial<TouchEventBoundaryProps> = {
breadcrumbCategory: DEFAULT_BREADCRUMB_CATEGORY,
Expand Down Expand Up @@ -113,18 +122,17 @@ class TouchEventBoundary extends React.Component<TouchEventBoundaryProps> {
/**
* Logs the touch event given the component tree names and a label.
*/
private _logTouchEvent(
componentTreeNames: string[],
activeLabel?: string
): void {
private _logTouchEvent(touchPath: TouchedComponentInfo[], label?: string): void {
const level = 'info' as SeverityLevel;

const root = touchPath[0];
const detail = label ? label : `${root.name}${root.file ? ` (${root.file})` : ''}`;

const crumb = {
category: this.props.breadcrumbCategory,
data: { componentTree: componentTreeNames },
data: { path: touchPath },
level: level,
message: activeLabel
? `Touch event within element: ${activeLabel}`
: 'Touch event within component tree',
message: `Touch event within element: ${detail}`,
type: this.props.breadcrumbType,
};
addBreadcrumb(crumb);
Expand All @@ -147,7 +155,7 @@ class TouchEventBoundary extends React.Component<TouchEventBoundaryProps> {
return ignoreNames.some(
(ignoreName: string | RegExp) =>
(typeof ignoreName === 'string' && name === ignoreName) ||
(ignoreName instanceof RegExp && name.match(ignoreName))
(ignoreName instanceof RegExp && name.match(ignoreName)),
);
}

Expand All @@ -166,80 +174,91 @@ class TouchEventBoundary extends React.Component<TouchEventBoundaryProps> {
}

let currentInst: ElementInstance | undefined = e._targetInst;

let activeLabel: string | undefined;
let activeDisplayName: string | undefined;
const componentTreeNames: string[] = [];
const touchPath: TouchedComponentInfo[] = [];

while (
currentInst &&
// maxComponentTreeSize will always be defined as we have a defaultProps. But ts needs a check so this is here.
this.props.maxComponentTreeSize &&
componentTreeNames.length < this.props.maxComponentTreeSize
touchPath.length < this.props.maxComponentTreeSize
) {
if (
// If the loop gets to the boundary itself, break.
currentInst.elementType?.displayName ===
TouchEventBoundary.displayName
currentInst.elementType?.displayName === TouchEventBoundary.displayName
) {
break;
}

const props = currentInst.memoizedProps;
const sentryLabel =
typeof props?.[SENTRY_LABEL_PROP_KEY] !== 'undefined'
? `${props[SENTRY_LABEL_PROP_KEY]}`
const props = currentInst.memoizedProps ?? {};
const info: TouchedComponentInfo = {};

// provided by @sentry/babel-plugin-component-annotate
if (typeof props[SENTRY_COMPONENT_PROP_KEY] === 'string' && props[SENTRY_COMPONENT_PROP_KEY].length > 0 && props[SENTRY_COMPONENT_PROP_KEY] !== 'unknown') {
info.name = props[SENTRY_COMPONENT_PROP_KEY];
}
if (typeof props[SENTRY_ELEMENT_PROP_KEY] === 'string' && props[SENTRY_ELEMENT_PROP_KEY].length > 0 && props[SENTRY_ELEMENT_PROP_KEY] !== 'unknown') {
info.element = props[SENTRY_ELEMENT_PROP_KEY];
}
if (typeof props[SENTRY_FILE_PROP_KEY] === 'string' && props[SENTRY_FILE_PROP_KEY].length > 0 && props[SENTRY_FILE_PROP_KEY] !== 'unknown') {
info.file = props[SENTRY_FILE_PROP_KEY];
}

// use custom label if provided by the user, or displayName if available
const labelValue =
typeof props[SENTRY_LABEL_PROP_KEY] === 'string'
? props[SENTRY_LABEL_PROP_KEY]
: // For some reason type narrowing doesn't work as expected with indexing when checking it all in one go in
// the "check-label" if sentence, so we have to assign it to a variable here first
typeof this.props.labelName === 'string'
? props[this.props.labelName]
: undefined;

// For some reason type narrowing doesn't work as expected with indexing when checking it all in one go in
// the "check-label" if sentence, so we have to assign it to a variable here first
let labelValue;
if (typeof this.props.labelName === 'string')
labelValue = props?.[this.props.labelName];

// Check the label first
if (sentryLabel && !this._isNameIgnored(sentryLabel)) {
if (!activeLabel) {
activeLabel = sentryLabel;
}
componentTreeNames.push(sentryLabel);
} else if (
typeof labelValue === 'string' &&
!this._isNameIgnored(labelValue)
) {
if (!activeLabel) {
activeLabel = labelValue;
}
componentTreeNames.push(labelValue);
} else if (currentInst.elementType) {
const { elementType } = currentInst;

if (
elementType.displayName &&
!this._isNameIgnored(elementType.displayName)
) {
// Check display name
if (!activeDisplayName) {
activeDisplayName = elementType.displayName;
}
componentTreeNames.push(elementType.displayName);
}
if (typeof labelValue === 'string' && labelValue.length > 0) {
info.label = labelValue;
}

if (!info.name && currentInst.elementType?.displayName) {
info.name = currentInst.elementType?.displayName;
}

this._pushIfNotIgnored(touchPath, info);

currentInst = currentInst.return;
}

const finalLabel = activeLabel ?? activeDisplayName;

if (componentTreeNames.length > 0 || finalLabel) {
this._logTouchEvent(componentTreeNames, finalLabel);
const label = touchPath.find(info => info.label)?.label;
if (touchPath.length > 0) {
this._logTouchEvent(touchPath, label);
}

this._tracingIntegration?.startUserInteractionTransaction({
elementId: activeLabel,
elementId: label,
op: UI_ACTION_TOUCH,
});
}

/**
* Pushes the name to the componentTreeNames array if it is not ignored.
*/
private _pushIfNotIgnored(touchPath: TouchedComponentInfo[], value: TouchedComponentInfo): boolean {
if (!value.name && !value.label) {
return false;
}
if (value.name && this._isNameIgnored(value.name)) {
return false;
}
if (value.label && this._isNameIgnored(value.label)) {
return false;
}

// Deduplicate same subsequent items.
if (touchPath.length > 0 && JSON.stringify(touchPath[touchPath.length - 1]) === JSON.stringify(value)) {
return false;
}

touchPath.push(value);
return true;
}
}

/**
Expand All @@ -250,9 +269,9 @@ class TouchEventBoundary extends React.Component<TouchEventBoundaryProps> {
const withTouchEventBoundary = (
// eslint-disable-next-line @typescript-eslint/no-explicit-any
InnerComponent: React.ComponentType<any>,
boundaryProps?: TouchEventBoundaryProps
boundaryProps?: TouchEventBoundaryProps,
): React.FunctionComponent => {
const WrappedComponent: React.FunctionComponent = (props) => (
const WrappedComponent: React.FunctionComponent = props => (
<TouchEventBoundary {...(boundaryProps ?? {})}>
<InnerComponent {...props} />
</TouchEventBoundary>
Expand Down
Loading

0 comments on commit 4a6664f

Please sign in to comment.