fix(express): Ensure 404 routes don't attach route attribute

[s1gr1d]

Which problem is this PR solving?

This is an upstream fix for getsentry/sentry-javascript#16211

Instead of adding / to non-existing routes, it sets the attribute to undefined, which makes it clear it's a non-existing route and not the index route.

---

A tricky thing is that the layers look the same if a non-existing route is requested or the root is requested: ['/', '/'].

• /existing --> ['/', '/existing']
• /non-existing --> ['/', '/']
• / --> ['/', '/']

So in order to know if it was a 404 route, a utility function that does some additional checking was added.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: s1gr1d / name: Sigrid Huemer (214b126, 1df9019, 006539c, 726cdcf, 2cb552a)

[pichlermarc]

LGTM! thank you for your contribution @s1gr1d 🙂

cc @raphael-theriault-swi, @JamieDanielson, @pkanal (component owners)

[pichlermarc]

oh - looks like there are some lint errors, you can fix them by running npm run lint:fix and then pushing the resulting diff 🙂

[codecov]

Codecov Report

Attention: Patch coverage is 97.61905% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.73%. Comparing base (2ac08ff) to head (006539c).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...opentelemetry-instrumentation-express/src/utils.ts	97.36%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2843      +/-   ##
==========================================
+ Coverage   89.69%   89.73%   +0.03%     
==========================================
  Files         185      186       +1     
  Lines        9034     9085      +51     
  Branches     1852     1875      +23     
==========================================
+ Hits         8103     8152      +49     
- Misses        931      933       +2     

Files with missing lines	Coverage Δ
...try-instrumentation-express/src/instrumentation.ts	96.73% <100.00%> (-0.03%)	⬇️
...opentelemetry-instrumentation-express/src/utils.ts	97.19% <97.36%> (+0.09%)	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

failing tests

[s1gr1d]

I have to look into this again. I see another test is failing. It doesn't add http.route either when the route is / (but this should be added in this case).

[s1gr1d]

The problem was that the layers look the same if a non-existing route is requested or the root is requested: ['/', '/'].

So in order to know if it was a 404 route, I added a utility function that does some additional checking. I made sure that the existing tests still work and added some more tests for the utility function.

[pichlermarc]

Hi @s1gr1d, thanks for following up on this.

It seems that this PR is based on a commit that's a little over 2 months old. Would you mind rebasing it on top of main it so that we can see how this would fit in with the current state? 🙂

[s1gr1d]

This condition is for considering regex routes (without this, the regex tests would fail). This should cover most cases.

There is a previous conversation about this: #2008 (comment)

[s1gr1d]

@pichlermarc I rebased it and aligned the code so the "regex" tests don't fail anymore.