[instrumentation-express] http.route == "/" if every path matches

[JohannesHuster]

What version of OpenTelemetry are you using?

@opentelemetry/instrumentation-express: 0.35.0 (The issue is about this dependency)
@opentelemetry/instrumentation-http: 0.48.0
@opentelemetry/sdk-node: 0.48.0
@opentelemetry/sdk-trace-node: 1.21.0

What version of Node are you using?

20.11.0

What did you do?

app.use((req, res) => res.send('Hello')); // Or a more relevant example: app.use(cors());

Send any request to the server (or an OPTIONS request for the example in the comment).

Here is a repo with more and other (see Additional context) examples, with spans being printed to console: otel-express-routes

What did you expect to see?

For the root span something like:

{
  "attributes": {
    "http.route": "*" // or "/*"
  },
  "name": "OPTIONS *", // or "OPTIONS /*"
}

What did you see instead?

{
  "attributes": {
    "http.route": "/"
  },
  "name": "OPTIONS /",
}

Since name depends on http.route, this is really only about the latter.

Additional context

• The same behavior (http.route == "/") can be observed for root spans
  • when using "/*" as path with app.use or app.<METHOD>
  • and for default responses (404, 500).
• Middleware and router spans also include http.route == "/", when matching every path; e.g. app.use(router).
• Similarly, if a router is being added like app.use('/abc', router1); this leads to http.route == "/abc" for corresponding router spans, but "/abc" and "abc/*" routes are being matched. "/abcd" or similar is not matched.

What to do about it?

Looking at HTTP Server semantic conventions and Express 4 routing (plus testing a lot of paths), I think the following could be good alternatives for the cases mentioned above, because they match actual Express routing behavior, as expressed in Express 4 syntax.

• "*" or "/*" when every path is matched.
• "/some/prefix(/*)?" when "/some/prefix" and every path starting with "/some/prefix/" matches.

I don't have any prior experience with OTel semantic conventions, though. What do you think?

I have these changes (using "*" and "(/*)?") running locally and would be happy to provide a PR. I would just have to try and break it a bit more and have a look at extending the tests to cover these cases.

My current changes would also remove double slashes (#1547), but this option would work just as well well for that, I think.

[JohannesHuster]

I might have put a bit too much responsibility on http.route for non-request spans and described my current understanding in a related issue: #1993