ref(node): Improve Express URL Parameterization

[Lms24]

This PR improves the URL parameterization for transaction names in our Express integration.

Previously, we would only obtain a parameterized version of the incoming request's URL after the registered handler(s) finished their job, right before we called transaction.finish(). This is definitely too late for DSC propagation, if the handlers made any outgoing request. In that case, the DSC (propagated via the baggage header) would not contain a transaction name.

As of this PR, we patch the Express.Router prototype, more precisely the process_params function. This function contains a reference to the incoming req object as well as to the layer that matched the route. We hook into the function to extract the matched and parameterized partial route of the layer and we attach it to the req object. This happens for every matched layer, until the entire route is resolved, in which stichtched together the entire parameterized route.

For the time being, we deliberately ignore route registrations with wildcards (e.g. app.get('/*', ...)) when stitching together the parameterized route. After we added a new part to the reconstructed route, we check if it has the same number of segments as the original (raw) URL. In case it has, we assume that the parameterized route is complete and we update the active transaction with the parameterized name. Additionally, we set the transaction source to route at this point. In case we never get to the point where we have an equal amount of URL segments, the transaction name is never updated, meaning its source stays url (and therefore, it's not added to the DSC). In that case, we continue to update the transaction name like before right before the end of the transaction.

After reading the Express source code, we confirmed that the process for resolving parts of a route and handling it is performed sequentially for each matched route segment. This means that each matched layer's handle function is invoked before the next part of the URL is matched. We therefore still have timing issues around DSC population if any of those intermediate handler functions make outgoing requests. A simple example:

The incoming request is /api/users/123

• We intercept the request and start the transaction with the name /api/users/123 and source url
• The first layer that matches is matches the route /*.
  • We start reconstructing the parameterized route with /
  • The handle function of this layer is invoked (e.g. to check authentication)
    • the handler makes an XHR request
    • at this point, we populate and freeze the DSC (without transaction name)
• The second handler matches the route /api/users/123
  • We obtain the parameterized route and our reconstructed route is now /api/users/:id
  • Now we have 3 segments in the original and in the reconstructed route
  • We update the transaction name with /api/users/:id and set its source to route
  • The handle function of this layer is invoked
    • every request that might happen here will still propagate the DSC from layer 1 because it can't be modified
• We finish the transaction

This example shows that we still have timing issues w.r.t DSC propagation. That is, assuming that the Node SDK is in this case the head of the trace and it didn't intercept incoming DSC from the incoming request.
However, with this PR we now at least have the chance to get the correct transaction name early enough.

ref: #5342

[lobsterkatie]

This looks good!

In your PR description, where you say "The second handler matches the route /api/users/123," do you mean that to be the parameterized route instead?

[lobsterkatie]

LGTM!

[brotherko]

Hi Team I'm wondering if this PR also solves express with a nested router?

[Lms24]

Hi @brotherko I think transaction-name-parameterization-wise nested routers should work. However, there are still missing spans on nested routers as outlined in #5510. If you want to try it out, feel free to do so and open an issue in case something is not working as expected. Thanks!

[brotherko]

Hi @Lms24 Thanks i'm not sure if we are referring to the same kind of parameterization here's the example

const route1 = Router();
const route2 = Router();

route1.use('/route2', route2)
route2.use('/:param1', (req, res) => res.send(`hello ${req.params.param1}`))

app.use('/route1', route1)

When I browse => route1/route2/someparameter
Sentry isn't able to capture the transaction as /route1/route2/:param1 but as route1/route2/someparameter

Is it the intended behavior? Or I'm just misusing express? thank you