browser-sdk icon indicating copy to clipboard operation
browser-sdk copied to clipboard

beforeSend Editing View Events' Context

Open indeediansbrett opened this issue 2 years ago • 19 comments

Am looking at the docs below regarding editing view context. Am curious what was the reason context for views was made read-only? We were looking to change context, but for views only and want to make sure there's no potential consistency issues. https://docs.datadoghq.com/real_user_monitoring/browser/modifying_data_and_context/?tab=npm

indeediansbrett avatar Jul 27 '22 16:07 indeediansbrett

Hi @indeediansbrett,

In our model, we ensure to have "parent" event context transmitted to the context of their "children" events. So a value of a view context should be present on the context of an action happening on this view.

In order to follow this model, we ensure that the view context can't be modified in the beforeSend, otherwise this modification would not be transmitted to subsequent events happening on this view.

What do you want to achieve by only changing the context of the view?

bcaudan avatar Jul 28 '22 11:07 bcaudan

Essentially we have a context field as a list. For actions, resources, errors, and long task events we want it to be a snapshot. For views we want it to have all items throughout the course of the view.

What seems to be functioning as a workaround was to add an extra context field with the extra information for views. In beforeSend, we remove it from non-view events. We're running on a proxy, so in there we add the extra information into the original field for views and delete the extra context field.

Related to this is that we're manually tracking views. It seems like there's no function like a stopView similar to startView. Is there a way to stop sending data to RUM, change the context, then start the next view? We're deleting the extra context information for route change views, but have to do that after startView or else it changes the old view. Then there's a brief period of time where the first few document versions of the route change will have that context, but have to wait several document versions for it to disappear.

indeediansbrett avatar Jul 28 '22 17:07 indeediansbrett

What seems to be functioning as a workaround was to add an extra context field with the extra information for views. In beforeSend, we remove it from non-view events.

Yes, that seems to be the best way to do it with current capabilities.

Related to this is that we're manually tracking views. It seems like there's no function like a stopView similar to startView. Is there a way to stop sending data to RUM, change the context, then start the next view? We're deleting the extra context information for route change views, but have to do that after startView or else it changes the old view. Then there's a brief period of time where the first few document versions of the route change will have that context, but have to wait several document versions for it to disappear.

Indeed, that seems the better way to achieve that for now.

We may want to introduce a specific view context to ease this use case but nothing planned yet. We'll keep you posted here when we will have more info to share about that.

bcaudan avatar Jul 29 '22 08:07 bcaudan

Hi, any update on this? I'm also wanting the ability to modify a view's context. I've tried implementing a workaround using global context as discussed above, but this doesn't work sometimes, I'm guessing due to conflicts when multiple events are being sent at the same.

In case helpful, my use case is that we massage the view URL and view referrer URL to handle some of our app's architecture-specific routing ~weirdness~ implementation, but we also want to be able to see the original, un-massaged URL. Specifically, we make some changes like:

  • Part of our app uses AngularJS which routes after the hash (#) which means all paths are just /. To handle this, we replace the # with H so that paths look like /H/angular/path
  • The default url_path_grouping was not working well in all cases for us, so we just do our own by replacing model IDs with :modelId

mariaines avatar May 17 '23 23:05 mariaines

We have the similar issue here as well. What we want to achieve here is to attach the system owner of that event (error or view) according to the view URL. This seems very to be a very innocent requirement. We tried to use setGlobalContextProperty to get around this, but it's always a hit or miss. If this is a hard-no, then is there anyway we can attach more attributes in datadog after the event were collected? Thanks!

29decibel avatar Oct 20 '23 21:10 29decibel

@29decibel you could use trackViewsManually (see doc) for that. Because you are know in charge of creating views, you can attach global context reliably after the view is started:

DD_RUM.startView()
DD_RUM.setGlobalContextProperty({ system_owner: 'foo' })

Would that work for you?

BenoitZugmeyer avatar Oct 23 '23 09:10 BenoitZugmeyer

Hi @BenoitZugmeyer thanks for following up. This is what we were trying to do. But unfortunately it doesn't work 100%, as the collection from global context is a black box to us, we don't know when it's being collected. And we only have the startView API to call. What ended up happening to us is that if there are long xhr requests happening in the view1, then we saw it's being collected as view2.

DD_RUM.startView('view1')
DD_RUM.setGlobalContextProperty({ system_owner: 'foo' })

// some xhr is still not finished, or resource is still loading

// user start view2
DD_RUM.startView('view2')
DD_RUM.setGlobalContextProperty({ system_owner: 'bar' }) 

In this case some of the xhr requests will be classified as view2, because the setGlobalContextProperty globally changed the context.

That's why we are seeing in the dashboard that most of the requests are correctly tagged, but there are always quite some are not. Do you have a solution to this?

Also since we are on this context, maybe worth another ticket, but we also seeing that xhr requests are not collected at all in the view level in Datadog when using startView. But it works fine locally (localhost), do you happen to know any cause to that as well?

Thanks!

29decibel avatar Oct 23 '23 18:10 29decibel

Hi @BenoitZugmeyer, I can provide one additional example as to why DD_RUM.setGlobalContextProperty() is not enough to add context to views:

This is where views are manually started:

/* Called on change of location.pathname or router params */
function onRouteCange(pathname: string, params: Record<string, string>) {
    const normalizedViewName = normalizeViewName(pathname, params);
    // The above function generalizes the view depending on the params: /a/123/b/c => /a/:id/b/:tab
    datadogRum.startView(normalizedViewName);
    datadogRum.setGlobalContextProperty("params", params);
}

Now looking at how the views are recorded in DataDog there's a problem:

DATE VIEW NAME @CONTEXT.PARAMS
00:00:17 /zzz/:id {"id": "234"}
00:00:16 /zzz/:id {"id": "123"}
00:00:16 /yyy/d
00:00:13 /yyy/c {"id": "234"}
00:00:12 /yyy/b {"id": "123"}
00:00:08 /yyy/a
00:00:07 /yyy {"name": "cba"}
00:00:04 /xyz/:name {"name": "abc"}
00:00:03 /xyz/:name {"name": "abc"}
00:00:03 /xyz {"name": "abc"}
00:00:00 /

The above is a simplified version of a real session that I just recorded using the above onRouteCange method.

There's multiple issues:

  • Context being set before the view is initialized (for example the /xyz view). Seems to happen if startView is called multiple times in close succession.
  • Context not updating correctly. The second /xyz/:name view should have {"name: "cba"}, but it's not updated in time, and instead shows the previous value. And instead this updated context is shown in the next view, that should not have any params. This can also lead to views that should have params to have the context empty.
  • Context being duplicated out of time for seemingly no reason. In /yyy/b and /yyy/c for no reason the context of theo /zzz/* views.

I've validated that the startView and setGlobalContextProperty are called with the right values at the right time, so it's definitely an issue with how the sdk handles views/context. I've also tried changing the order of this functions, to set the context before starting the view, but that seems to be even worse.

Note that it works mostly fine in most of the views, but around 30-40% of the views recorded have the wrong context.

Seeing as how this system cannot be relied on, the only workaround we've found is to not use this context at all. Just create custom actions with the name of the view and add the params as context, that's guaranteed to be in sync.

Ideally when we call startView we can specify a context for it (either at startView or beforeSend). If all child events are required to have the same context as the view, they can inherit it without needing a global context. Something like:

datadogRum.startView("/zzz/:id/:tab", {id: "123", tab: "home"});
// ...
datadogRum.addAction("xyz", {z: "f"}); // the resulting context will be {id: "123", tab: "home", z: "f"}
// ...
datadogRum.startView("/xyz/:name", {name: "abc"});
// ...
datadogRum.addAction("xyz"); // the resulting context will be {name: "abc"}

I do please ask that the team considers implementing this functionality, it's a major blocker and supported by most of the other RUM tools in the market.

Thank you!

rorik avatar May 27 '24 23:05 rorik

Hi @BenoitZugmeyer,

I'm also experiencing a similar issue. In my case, I can't determine the view name when the view starts because it's defined by our custom Route component and only known when the route is rendered. Therefore, I need to set it asynchronously using beforeEvent. However, because the global context becomes intermixed (as described by rorik above), I can't use it to associate the view with the correct view name.

To work around this I give the view a name corresponding to a unique ID that is the key in an object that will receive the correct view name once its known. in beforeEvent I map the event.view.id to this uniqueID the first time I see it and then for all events of that view I use that mapping to retrieve the real view name.

salazarm avatar May 31 '24 07:05 salazarm

This is what my work around looks like:

const statesById = {};

let viewIdx = 0;
function onPushState() {
  const id = `view:${viewIdx++}`;
  statesById[id] = {viewName: '/' }; // this will be updated by dynamically rendered <Route> components
  datadogRum.startView({viewName: id})
}
const datadogUUIDToViewID: Record<string, string> = {};
...
beforeEvent: (event) => {
  if (datadogUUIDToViewID[event.view.id] === undefined) {
    datadogUUIDToViewID[event.view.id] = event.view.name!;
  }
  const id = datadogUUIDToViewID[event.view.id];
  if (id !== undefined) {
    const state = statesById[id];
    if (state) {
      state.datadogViewId = event.view.id;
      event.view.name = state.viewName;
    } else {
      event.view.name = event.view.name + ' :UNKNOWN:' + id;
    }
  }
}

salazarm avatar Jun 06 '24 09:06 salazarm

Thank you for sharing your use-case. This is a frequently asked feature, I'll bring it again to the team.

BenoitZugmeyer avatar Jun 07 '24 12:06 BenoitZugmeyer

Exact same use case as what's listed here, trying to match the URL/View Name for the event to an ownership list and add the team name as context. +1 to this as it's wrong a not insignificant amount of time.

Additionally, I spent a bit of time trying to just do event.context.team = 'foo', and could not figure out why it didn't work. I'd go further and say that if this is going to be read-only, there should be a warning or error thrown when I try to mutate it...or some other mechanism for letting developers know it doesn't work. (Although who knows if I was doing it right)

comp615 avatar Jul 09 '24 19:07 comp615

Hello, I've been trying out the React integration plugin (@datadog/browser-rum-react).

And it's a major improvement that's highly appreciated. If you just care about transforming the name of the views to their parametrized versions (i.e. "/foo/bar/baz" => "/:param/bar/:id") it will save you some implementation time.
But unfortunately it's still limited by the context issue, as we want to add the value of those params in the context. So for our team we'll have to keep depending in the less reliable global context/manual tracking without this plugin.

rorik avatar Aug 08 '24 04:08 rorik

We are actively working on this, and will integrate it in the react integration plugin once it's ready!

BenoitZugmeyer avatar Aug 09 '24 08:08 BenoitZugmeyer

I spent a bit of time trying to just do event.context.team = 'foo', and could not figure out why it didn't work. I'd go further and say that if this is going to be read-only, there should be a warning or error thrown when I try to mutate it...or some other mechanism for letting developers know it doesn't work. (Although who knows if I was doing it right)

+1 to this too. I also spent way too much time trying to figure out why this didn't work. In fact datadog support was the one who told me to do it, so even among the team it isn't completely obvious.

salazarm avatar Aug 14 '24 14:08 salazarm

I spent a bit of time trying to just do event.context.team = 'foo', and could not figure out why it didn't work. I'd go further and say that if this is going to be read-only, there should be a warning or error thrown when I try to mutate it...or some other mechanism for letting developers know it doesn't work. (Although who knows if I was doing it right)

+1 for us too, we are using context to attach execution namespace by which a particular route is owned by, but around 25% of events are getting tagged wrong, resulting in inaccurate data and resulting metrics and dashboards. Is there any known workaround for this ?

harshVardhan4743 avatar Aug 26 '24 07:08 harshVardhan4743

We are working on it: https://github.com/DataDog/browser-sdk/pull/2939

BenoitZugmeyer avatar Aug 27 '24 13:08 BenoitZugmeyer

@BenoitZugmeyer I might be down a rabbit hole here, but I'm trying out BOTH this one and the #2808 to update view name on redirect navigations. My hunch is that when we update the view name, we also may need to be able to update the context for the same use cases illustrated above. So the two experiments might need an interaction there.

For now, I just moved the tagging back into beforeSend from startView.

comp615 avatar Sep 11 '24 22:09 comp615

@comp615 you should be aware that beforeSend will intermix events just like context does above so if you want to reliably update the view name you'll need to map the UUID of the view to the view name directly rather than relying on beforeSend corresponding to the "current" view.

salazarm avatar Sep 11 '24 22:09 salazarm

Hello,

The view context API has been released in the browser SDK v5.26.0. More information in the doc

amortemousque avatar Oct 04 '24 08:10 amortemousque

@amortemousque What about updating the view name dynamically? The current docs for setting the view name manually assume that you have a mapping of all your routes but that is not normally the case when using react-router since routes are defined in code without a central configuration file.

salazarm avatar Oct 04 '24 13:10 salazarm

To send customized view names, the best way is to use trackViewsManually: https://docs.datadoghq.com/real_user_monitoring/browser/advanced_configuration/?tab=npm#override-default-rum-view-names

BenoitZugmeyer avatar Oct 07 '24 15:10 BenoitZugmeyer