Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(browser): Extract and send frontend component name when available #9855

Closed
wants to merge 14 commits into from

Conversation

0Calories
Copy link
Contributor

@0Calories 0Calories commented Dec 14, 2023

Iterates on #9496

This is similar to the previous linked PR, but addresses some issues / concerns that were encountered when experimenting with it. Here's what's new this time around:

  • Extract component names from the DOM via the data-sentry-component attribute instead of data-component
  • Changes _htmlElementAsString to check if the data-sentry-component attribute is present on the DOM and use it. This way, htmlTreeAsString will include the React component when building the string. Example: body > div > MyTestComponent
  • Created a new helper function getComponentName to find a representative component for a DOM element. It will traverse up the tree up to a max of 5 ancestors to find a component name, as it is possible that the event is captured on a nested element instead of the component itself
  • Adds componentName to the data attribute for DOM breadcrumbs, instead of replacing the string in the message
  • Adds ui.component_name to the data attribute in interaction and React spans
  • Adds data-sentry-component to data.node.attributes on Replay UI breadcrumbs

@0Calories 0Calories self-assigned this Dec 14, 2023
Copy link
Contributor

github-actions bot commented Dec 14, 2023

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 75.51 KB (+0.23% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 66.91 KB (+0.22% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 60.51 KB (+0.27% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 31.57 KB (+0.47% 🔺)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 30.11 KB (+0.31% 🔺)
@sentry/browser - Webpack (gzipped) 21.83 KB (+0.51% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 72.91 KB (+0.23% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 64.62 KB (+0.25% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 30.88 KB (+0.46% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped) 22.89 KB (+0.48% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 203 KB (+0.22% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 93.08 KB (+0.44% 🔺)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 68.08 KB (+0.53% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 33.72 KB (+0.42% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 67.3 KB (+0.27% 🔺)
@sentry/react - Webpack (gzipped) 21.87 KB (+0.52% 🔺)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 84.05 KB (+0.16% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 48.71 KB (+0.26% 🔺)
@sentry-internal/feedback - Webpack (gzipped) 16.38 KB (+0.17% 🔺)

*/
export function getComponentName(elem: unknown): string | null {
let currentElem = elem as SimpleNode;
const MAX_TRAVERSE_HEIGHT = 5;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 might be a bit too high, but I'm unsure. We could lower this threshold to start out and gradually increase it if necessary, or keep it at 5

@0Calories 0Calories marked this pull request as ready for review December 15, 2023 21:57
@0Calories 0Calories requested review from a team, anonrig, mydea and AbhiPrasad and removed request for a team December 15, 2023 21:57
@AbhiPrasad
Copy link
Member

Could we split this PR up?

  1. Changes to htmlTreeAsString and resulting tests
  2. Changes to replay breadcrumbs
  3. Changes to React Profiler + interaction spans

} catch (e) {
target = '<unknown>';
componentName = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is unnecessary

@0Calories
Copy link
Contributor Author

0Calories commented Dec 20, 2023

As requested, I've decomposed this PR into more digestible chunks.

Utils: #9921
Interaction Spans: #9948
React Profiler Spans: #9949
Breadcrumbs: #9946
Replays: #9947

@0Calories 0Calories closed this Dec 20, 2023
0Calories added a commit that referenced this pull request Dec 21, 2023
One of the PRs scoped from
#9855

Sends component names on the databag of interaction spans
0Calories added a commit that referenced this pull request Dec 21, 2023
One of the PRs scoped from
#9855

Sends component names on Replay UI breadcrumbs so they can be ingested
and indexed. This will allow for searching for Replays by component name
in the future.

---------

Co-authored-by: Yagiz Nizipli <yagiz@nizipli.com>
0Calories added a commit that referenced this pull request Dec 21, 2023
One of the PRs scoped from
#9855

Sends component names on UI event breadcrumbs

---------

Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants