fix(express): make rpcMetadata.route capture the last layer even when if the last layer is not REQUEST_HANDLER

[chigia001]

Which problem is this PR solving?

• fixes: Express instrumentation 0.33.0 breaks handling http_route metric attribute #1618
• The old implementation have 2 different condition for update rpcMetadata.span.name and rpcMetadata.route and we pick the stricter condition.

Short description of the changes

• Update the condition to match the previous implementation for rpcMetadata.route
• Move this block bellow all the ignore flow, which make it the same as old implementation.

[codecov]

Codecov Report

Merging #1620 (a5aec4a) into main (7d4b13e) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1620      +/-   ##
==========================================
- Coverage   91.79%   91.79%   -0.01%     
==========================================
  Files         139      139              
  Lines        7117     7115       -2     
  Branches     1431     1429       -2     
==========================================
- Hits         6533     6531       -2     
  Misses        584      584              

Files Changed	Coverage Δ
...try-instrumentation-express/src/instrumentation.ts	100.00% <100.00%> (ø)

[blumamir]

LGTM. Thanks again 🥇

Added a question regarding the test for this change

[blumamir]

This test should assert that the route is still captured when

type === ExpressLayerType.REQUEST_HANDLER

This type will be set when layer.name === 'bound dispatch'

  } else if (layer.name === 'bound dispatch') {
    return {
      attributes: {
        [AttributeNames.EXPRESS_NAME]: layerPath ?? 'request handler',
        [AttributeNames.EXPRESS_TYPE]: ExpressLayerType.REQUEST_HANDLER,
      },
      name: `request handler${layer.path ? ` - ${layerPath}` : ''}`,
    };

Is using the app.use triggers a layer with name 'bound dispatch'?

[chigia001]

  const router = express.Router();

  router.use("/router_use", () => {}); // #3
  router.get('/tada/, () => {}); // #2

  app.use('/toto', router); // #1
  app.use('/bare_use', () => {}); // #3

  app.get('/bare_get', () => {}); // #2

#1, the middleware that bound app.use/router.use with Router object is 'router' layer

#2 is bound dispatch, ie, route handler with HTTP method name. This is for both Express's App and Express's Router object. This is something our unit test cover most of the time. Express's route handle will match to the end of URL.

#3 is the last case, IE middleware layer, where app.use/router.use have generic handler. It can be with or without the route's parameter .use will only need to match the start of the url.

Most of our test with app.use/router.use is for common's middleware use case, that don't have layername at the begining.

This new test for specificly app.use with route layer.

[chigia001]

I add additional test for these 2 case, as I don't see we have specific test for them

  router.use("/router_use", () => {}); // #3
  app.get('/bare_get', () => {}); // #2

internally I think they are the same with case that we already cover (app.use === router.use and app.get === router.get) but just to make sure.