feat(utils): Prioritize Component name attributes over HTML Tree String #9496
Conversation
size-limit report 📦
|
| ? getElementIdentifier(event.target, { keyAttrs, maxStringLength }) | ||
| : getElementIdentifier(event, { keyAttrs, maxStringLength }); |
There was a problem hiding this comment.
Right now, getElementIdentifier isn't doing anything with the options passed to it, except passing it to htmlTreeAsString if there is no data-component annotation available. Would we want to make use of any of those options, like maxStringLength on the annotated component names as well?
|
Added some integration tests |
There was a problem hiding this comment.
From doing a search on https://github.com/search?q=data-component&type=code, it seems that data-component is set and used by a lot of frontends/librarys.
Should we use a different attribute? perhaps namescape it with sentry so it's a little more obvious what happens? data-sentry-component?
|
Good point @AbhiPrasad, for the time being do you think we could just put this code behind an experimental flag? Changing this would require forking the annotation plugin and adjusting the attribute names, but I just want to quickly get this in to validate the data internally. When we make our own plugin for this later on, I'll make sure we use a distinct name to avoid these issues |
Updates the frontend SDK version to 7.80.2-alpha.1, this version of the SDK will pick off the values in the DOM labelled as `data-component` so we can start seeing them internally and validate the product experience. However, this change is avoided for the Replay SDKs, as it would cause searching by CSS selector to no longer work. SDK PR: getsentry/sentry-javascript#9496
| @@ -4,6 +4,14 @@ | |||
|
|
|||
| - "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. | |||
There was a problem hiding this comment.
What is the long term plan for replays then? Don't we need this change for them also?
There was a problem hiding this comment.
@AbhiPrasad Had to put replays aside for the sake of the experiment, but there are a few things we will need to include in the backend and SDK to support it. I'll have a document ready with more details later
|
Closing this PR, I've experimented with this in an alpha release of the SDK and iterated on it by making a new PR with some improvements: |
As part of an experimental initiative, add a wrapper function that will check for the
data-componentattribute on DOM nodes. If the attribute does not exist, it will fallback to thehtmlTreeAsStringfunction. This PR replaces all usages ofhtmlTreeAsStringwith the newgetElementIdentifierfunction.In the future, we will have a system that will, during build time, automatically annotate elements in the output DOM of an application with the original name of the component that it belongs to. This PR sets up the SDK to access this property when it is available, so that element identifiers throughout Sentry are comprehensible. This will affect the names of elements seen in multiple places within Sentry; breadcrumbs, replays, and tracing.