improve express regex middleware path parsing

[tlhunter]

What does this PR do?

• fixes fix(express): dont lose route if preceded by middleware with longer path #2065
  • this takes the commit from that PR and builds on it

Motivation

• we don't want to use the string regex representation of a middleware when determining the path

[github-actions]

Overall package size

Self size: 4.24 MB
Deduped: 58.44 MB
No deduping: 58.48 MB

Dependency sizes

name	version	self size	total size
@datadog/pprof	2.2.1	14.24 MB	15.12 MB
@datadog/native-iast-taint-tracking	1.4.1	14.85 MB	14.86 MB
@datadog/native-appsec	3.2.0	13.38 MB	13.39 MB
protobufjs	7.1.2	2.76 MB	6.55 MB
@datadog/native-iast-rewriter	2.0.1	2.09 MB	2.1 MB
@datadog/native-metrics	2.0.0	898.77 kB	1.3 MB
opentracing	0.14.7	194.81 kB	194.81 kB
semver	7.3.8	88.2 kB	118.6 kB
@datadog/sketches-js	2.1.0	109.9 kB	109.9 kB
lodash.sortby	4.7.0	75.76 kB	75.76 kB
lru-cache	7.14.0	74.95 kB	74.95 kB
ipaddr.js	2.0.1	59.52 kB	59.52 kB
ignore	5.2.0	48.87 kB	48.87 kB
import-in-the-middle	1.3.5	34.34 kB	38.81 kB
istanbul-lib-coverage	3.2.0	29.34 kB	29.34 kB
retry	0.10.1	27.44 kB	27.44 kB
lodash.uniq	4.5.0	25.01 kB	25.01 kB
limiter	1.1.5	23.17 kB	23.17 kB
lodash.kebabcase	4.1.1	17.75 kB	17.75 kB
lodash.pick	4.4.0	16.33 kB	16.33 kB
node-abort-controller	3.0.1	14.33 kB	14.33 kB
crypto-randomuuid	1.0.0	11.18 kB	11.18 kB
diagnostics_channel	1.1.0	7.07 kB	7.07 kB
path-to-regexp	0.1.7	6.78 kB	6.78 kB
koalas	1.0.2	6.47 kB	6.47 kB
methods	1.1.2	5.29 kB	5.29 kB
module-details-from-path	1.0.3	4.47 kB	4.47 kB

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

[pr-commenter]

Benchmarks

Comparing candidate commit 9e49b06 in PR branch domkck/express-router with baseline commit e4a2a27 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 444 metrics, 28 unstable metrics.

[tlhunter]

Looks like this code might not be compatible with Express v4:

  1) Plugin
       express
         with express >=4 (4.0.0)
           without configuration
             should not lose the current path when route handler preceeded by a longer middleware resource:
     Error: Router.use() requires callback functions but got a [object RegExp]
      at Function.proto.use (versions/express@4.0.0/node_modules/express/lib/router/index.js:327:11)
      at Function.use (packages/datadog-instrumentations/src/router.js:156:31)
      at Function.app.use (versions/express@4.0.0/node_modules/express/lib/application.js:195:16)
      at Context.<anonymous> (packages/datadog-plugin-express/test/index.spec.js:711:15)
      at processImmediate (node:internal/timers:466:21)
      at process.callbackTrampoline (node:internal/async_hooks:130:17)

[codecov]

Codecov Report

Merging #3203 (9e49b06) into master (e4a2a27) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3203      +/-   ##
==========================================
- Coverage   85.86%   85.82%   -0.05%     
==========================================
  Files         182      182              
  Lines        7223     7223              
  Branches       33       33              
==========================================
- Hits         6202     6199       -3     
- Misses       1021     1024       +3     

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[tlhunter]

Looks like Express v4.0 broke backwards compat with the router and didn't fix it until 4.6. I'll modify the tests to avoid it.

[tlhunter]

@rochdev I replaced the .startsWith and .endsWith with .includes. You were right that having a parent router with a path and a child handler being regex was sneaking through.

[rochdev]

I think for now this could be sufficient, although I wonder if maybe checking just endsWith would be enough? Then it would only match if the last nested route was a regex. Not sure whether that's actually better though 🤔

[tlhunter]

Interesting... What if a parent route is a regex and a child is a path?

[tlhunter]

I do agree that a single .endsWith would be a lot faster than two or even one .includes.

[rochdev]

I think for now this could be sufficient, although I wonder if maybe checking just endsWith would be enough? Then it would only match if the last nested route was a regex. Not sure whether that's actually better though 🤔