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

getSelectorFromElement scales poorly with a large DOM and ids with numbers are not used #2751

Open
tygary opened this issue May 9, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@tygary
Copy link

tygary commented May 9, 2024

Describe the bug
When using the browser agent with RUM enabled, the agent will attempt to resolve a unique selector for every click event.

That process uses getSelectorFromElement to perform an iterative algorithm to generate the globally unique selector.

However, this process scales in performance along with the size of the dom. In our use-case, for pages with a very large dom the datadog processing on pointerdown will consume 10-25% of the processing time of our entire click event. This is contributing to poor performance on clients with constrained CPU bandwidth.

The ideal solution is to set more ID attributes on our dom to shorten the selectors and improve the queryAll runtime, however the
getIDSelector method calls isGeneratedValue with the id and this decides that any id attribute with a number it should not be used as part of the selector.

Our UI generally follows this pattern (greatly simplified)

Container
     item-1
     item-2
     ....
     item-5000
          table
              tbody
                 tr
                   td
                      something you click on.  

So that excludes all of our ids since everything is numbered or has uuids.

Here is an example selector that gets generated by datadog:

#buffers>DIV.buffer-container>DIV.buffer-contentWrapper>DIV.buffer-contentWrapper-reset>DIV.buffer-content>DIV.buffer>DIV.buffer-tab-wrapper>DIV.buffer-tab>DIV.noteEditor>DIV.noteEditor-container>DIV.noteEditor-scrollContainer>DIV.u-cursor-text>DIV.noteEditor-contentWrapper>DIV.editor>DIV:nth-of-type(98)>DIV>DIV.mediocre-tableEditable>DIV.u-position-relative>DIV.mediocre-tableEditable-scrollArea>DIV.mediocre-tableEditable-tableWrapper>DIV.mediocre-tableEditable-flexBoxWrapper>TABLE.mediocre-tableEditable-table>TBODY>TR:nth-of-type(3)
Screenshot 2024-05-09 at 3 05 04 PM

It took ~11ms to generate that selector out of the 45ms of the entire click event and subsequent render, so 25% of the click time was spent on datadog. (This is on my new macbook pro, whereas our customers the click may have taken >250ms and datadog consumes ~75ms and causes significant visual delay)

If we use an attribute like data-dd-action-name that brings selector generation down to ~5% of our click time, versus if we use an id selector without numbers ("my-item-fiftysix") that brings the selector generation down to around 1-2% of our click time.

In your code inside isGeneratedValue is the comment:

    // To compute the "URL path group", the backend replaces every URL path parts as a question mark
    // if it thinks the part is an identifier. The condition it uses is to checks whether a digit is
    // present.
    //
    // Here, we use the same strategy: if a the value contains a digit, we consider it generated. This
    // strategy might be a bit naive and fail in some cases, but there are many fallbacks to generate
    // CSS selectors so it should be fine most of the time. We might want to allow customers to
    // provide their own `isGeneratedValue` at some point.

Why do ids with numbers need to be rejected? Can we provide our own isGeneratedValue method to override this?

Or must I continue converting numbers into words in our IDs so that the datadog rum agent doesn't cause more latency to our web application. 😞

To Reproduce
Steps to reproduce the behavior:

  1. Set up a web page with a large dom and many id tags with numbers in them
  2. Click around the page
  3. Observe the unique selectors generated and sent along with the rum network request (like first_input_target_selector)

Expected behavior
This selector uses the closest id to the clicked element

Actual behavior
The selector generated is extremely long and goes all the way up to the first non-numeric id (basically the root of the dom tree)

@tygary tygary added the bug Something isn't working label May 9, 2024
@thomas-lebeau
Copy link
Contributor

Hi @tygary,

Thank you for the detailed description. We will look into the performance issue you are facing.

In the meantime, let me try to answer your questions:

Why do ids with numbers need to be rejected

What need to be rejected is randomly generated ids. This is so the selectors are consistent across all the visitors of your website and can be used to generate heatmaps.

Can we provide our own isGeneratedValue method to override this?

Unfortunately, at this moment this is not possible

@tygary
Copy link
Author

tygary commented May 10, 2024

@thomas-lebeau What do you mean by "generated"?

As in IDs that are not stable across page loads for the same URL?

I would assume you're looking for IDs that are consistent across all users where this ID always means this consistent element for this particular URL?

Because in some sense, nearly everything on our page is dynamic and generated. The user can customize and re-arrange items on screen and creating unique IDs that are not numbered is not possible.
But for a particular URL in our app, the id "item-56" will always point to the same item for any user that visits that particular page.

However, "item-56" may point to one thing at URL A and another totally different thing at URL B.

@tygary
Copy link
Author

tygary commented May 10, 2024

At the moment, we either need to disable RUM entirely for customers experiencing slow web pages (which is exactly the opposite of what we'd want from datadog), or we need to get the selector generation under control. Even if that comes at the expense of less usable heatmaps.

On a side note, in the datadog browser init I set trackUserInteractions: false and datadog is still generating these selectors and sending them. I would have assumed that turning off trackUserInteractions would disable this code without us having to turn off RUM entirely.

@BenoitZugmeyer
Copy link
Member

BenoitZugmeyer commented May 13, 2024

Hey @tygary, thanks again to bring this up to our attention.

@thomas-lebeau What do you mean by "generated"?

As in IDs that are not stable across page loads for the same URL?

It's the idea, yes!

However, "item-56" may point to one thing at URL A and another totally different thing at URL B.

That's also something we want to avoid (ideally, selectors should target something similar across pages).

Even if URL A and B are different, they can still share the same "view name". For example, /dashboard/foo and /dashboard/bar can both be named /dashboard/:id). I'm not sure if it applies to your case, but definitly something we came across when designing CSS selectors for the heatmaps.

Because this heuristic is not trivial to grasp if you don't have the full context, letting SDK users to customize it (like you are suggesting with providing your own isGeneratedValue) might not be such a good idea. Ideally, we should tackle it internally and find a way to improve the performance for everyone without requiring users to fine tune the internals of the SDK.

On a side note, in the datadog browser init I set trackUserInteractions: false and datadog is still generating these selectors and sending them. I would have assumed that turning off trackUserInteractions would disable this code without us having to turn off RUM entirely.

Selectors are also used for Core Web Vital attribution (identify which element is causing a layout shift for example). This is why you are seeing some selectors even with trackUserInteractions: false. This can happen even for clicks because of Interaction To Next Paint (INP) CWV.


After some discussion with the team, we have a couple of ideas on how to improve the performance. We still need to implement and test it, but it might solve your issue altogether.

In the meantime, you could leverage what we call "stable attributes" here: those attributes are used in the same way as the id attribute (meaning the selector computation will stop there if it's unique enough), but there is no isGeneratedValue check, so if you set your id to both the id attribute and any of those stable attributes (ex: <div id="item-12" data-qa="item-12">) it should fix your issue in the same way as if isGeneratedValue wasn't there. I understand it's not great, but we will try to provide a fix shortly.

@tygary
Copy link
Author

tygary commented May 14, 2024

Thanks! For the moment, I have put some ids on a few important elements where I converted the numbers to words. That has fixed the immediate performance issues, but I filed a ticket on myself to remove that hack once we have a better solution or just improved performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants