feat: Allow defining page.route

[nordfjord]

Which problem is this PR solving?

When it comes to frontend it can be hard to know which "route" you're actually on. Currently the SDK will place window.location.pathname onto the page.route attribute. This is great for many cases, but in our application we have many routes such as /entity/:entity_id where it'd be nice to group them together.

Short description of the changes

• Allow passing a getRoute function to the SDK which will be called to fill in http.route

How to verify that this has the expected result

• View the test

Possibilities

It should be possible for users to use the getRoute function to interrogate their router for what route they're actually on, or in more complex cases apply some inference based on the pathname (e.g. replace segments matching uuids with a placeholder)

For someone using react-router they might use a function like this:

function createRouteGetter(router) {
  let route = window.location.pathname
  router.subscribe(state => {
    let newRoute  = ""
    // some shenanigans here since routes can be nested
    for (let i = state.matches.length - 1; i >= 0; --i) {
      const path = state.matches[i]?.route.path
      if (!path) continue
      if (!route.startsWith(path)) newRoute = path + (newRoute.startsWith("/") ? newRoute : "/" + newRoute)
    }
    route = newRoute
  });
  return () => route
}

new HoneycombSDK({
  // ...
  getRoute: createRouteGetter(router)
})

[pkanal]

I was thinking about this a little bit and I feel like this in itself would make a great span processor that we can export from the distro, it would be consumed by passing this processor to the spanProcessor config option itself. This way we keep the surface area of the API more consistent and get this ready for reuse / contribute it upstream to OTel.

We should extend our spanProcessor option to be spanProcessors that can take multiple span processors but we can address that in a separate PR.

[nordfjord]

That sounds very sensible to me, how would you like me to action in the context of this PR?

[pkanal]

Would you be up for reworking this PR by creating a new span processor for this and showing an example of how to use it in the example app?

I will open up an issue to accept multiple span processors and open up a separate PR myself for that.

[nordfjord]

Happy to spend some time on that, one question: should I remove the page.route attribute from this processor?

[nordfjord]

@pkanal, I started thinking that maybe a PageRouteSpanProcessor makes less sense than something more generic, so here is a DynamicAttributesSpanProcessor which can be exposed and the BrowserAttributesSpanProcessor can be implemented in terms of. This would avoid needing to introduce a separate processor for each little attribute that comes up over time.

[pkanal]

I like this approach! I'm wondering whether this should be something we package with the SDK or whether this should be documented for users since everyone's use case will be slightly different? 🤔

Thoughts @wolfgangcodes / @MustafaHaddara ?

[nordfjord]

I'm arriving at the same conclusion. I think adding dynamic attributes globally like this will be a common use case, but at the same time this is so exceedingly generic as to be almost worthless. I'm hoping that sometime soon the OTel client instrumentation SIG comes up with a proposal to allow dynamic resource attributes which might fit these use cases better. At that point the page.route thing is something you'd just set up in your render cycle with a trace.addResourceAttributes({ 'page.route': match.route }) and would also take care of things like changing the enduser.id or tenant.id when a user logs in during a session.

[pkanal]

Hmmmm yeah, I think that is what span processors are meant to do in general. So are we coming to the conclusion that the best thing to do here is make sure that this distro is able to support multiple span processors and that this should be expressed as a custom span processor in a consuming app?

[nordfjord]

Yes, agreed, I'll close this PR