getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.92k stars 1.56k forks source link

Allow customizing or omitting `class` in browser breadcrumbs? #4403

Closed joshkel closed 1 year ago

joshkel commented 2 years ago

I'm working on making our application's Sentry's ui.click breadcrumbs more useful, but I'm running into some challenges, because the framework we're using (Material-UI) creates long lists of class names that quickly exceed MAX_OUTPUT_LEN. Here's an example from what I was working on today:

<div class="MuiButtonBase-root MuiListItem-root MuiListItem-gutters MuiListItem-button" tabindex="0" role="button" aria-disabled="false" id="MyInterestingListItem">
    <div class="MuiListItemText-root MuiListItemText-multiline">
        <span class="MuiTypography-root MuiListItemText-primary MuiTypography-body1 MuiTypography-displayBlock">
            Some text for the user
        </span>
        <p class="MuiTypography-root MuiListItemText-secondary MuiTypography-body2 MuiTypography-colorTextSecondary MuiTypography-displayBlock">
            Secondary text
        </p>
    </div>
</div>

I was hoping that div#MyInterestingListItem would show up in the breadcrumbs if the user clicks the inner span or p, but the class lists are so long that I instead see little more than the span or p.

I saw that I can customize the Breadcrumbs integration by passing my own serializeAttribute list, but that doesn't seem to suffice: if none of the attributes I list are found, then Sentry falls back to including the class list after all.

Would it be worthwhile to add an option to never include classes? Or would it be worthwhile to change the behavior of serializeAttribute so that, if it's specified and results in an empty list, Sentry accepts that instead of falling back to ['id', 'class']? (In an ideal world, perhaps, Sentry could summarize the class list and only include "interesting" DOM nodes. However, figuring out reliable logic for summarizing arbitrary class names and filtering interesting DOM notes does not at all seem practical.)

AbhiPrasad commented 2 years ago

Hey, thanks for writing in! I think this is a worthwhile change. Not sure if we want to change serializeAttribute (to avoid breaking changes as much as possible), perhaps we introduce another option? I'll backlog for now, but PRs are welcome!

HazAT commented 1 year ago

Closing - PRs welcome :)