express instrumentation update breaking span names

[iamchathu]

What version of OpenTelemetry are you using?

All dependencies latest.

What version of Node are you using?

Node LTS (20.10)

What did you do?

Update the packages to latest

What did you expect to see?

Span name as HTTP_MEHTOD /route/user/calling

What did you see instead?

Span name is HTTP_METHOD /

Additional context

[JamieDanielson]

@iamchathu I am having trouble reproducing this. Can you provide more detail like your packages in package.json (and note what you upgraded from) and your setup code?

I have a simple app that produces telemetry from @opentelemetry/instrumentation-http and @opentelemetry/instrumentation-express, including the following

telemetry.sdk.version: 1.19.0
process.runtime.version: 20.10.0
process.runtime.name: nodejs
scope.name and version: @opentelemetry/instrumentation-express 0.34.1
scope.name and version: @opentelemetry/instrumentation-http 0.46.0
span.name: "GET /hello"

The instrumentations in use are from the @opentelemetry/auto-instrumentations-node package v0.40.3

  instrumentations: [
    getNodeAutoInstrumentations({
      '@opentelemetry/instrumentation-fs': {
        enabled: false,
      },
    }),
  ]

Below is a chunk of the output, only including relevant bits to reduce the size of the code block:

{
  "resourceSpans": [
    {
      "resource": {
        "attributes": [
          {
            "key": "telemetry.sdk.language",
            "value": {
              "stringValue": "nodejs"
            }
          },
          {
            "key": "telemetry.sdk.version",
            "value": {
              "stringValue": "1.19.0"
            }
          },
          {
            "key": "process.runtime.version",
            "value": {
              "stringValue": "20.10.0"
            }
          },
          {
            "key": "process.runtime.name",
            "value": {
              "stringValue": "nodejs"
            }
          }
        ]
      },
      "scopeSpans": [
        {
          "scope": {
            "name": "@opentelemetry/instrumentation-express",
            "version": "0.34.1"
          },
          "spans": [
            {
              "name": "request handler - /hello"
            }
          ]
        },
        {
          "scope": {
            "name": "@opentelemetry/instrumentation-http",
            "version": "0.46.0"
          },
          "spans": [
            {
              "name": "GET /hello"
            }
          ]
        }
      ]
    }
  ]
}

[iamchathu]

I think I found the edge case that makes my traces go like that. Can you try with the following route path setup?

  const app = express();

  const router = Router();
  router.get(['/sub/path', '/sub2/path'], (req, res) => {
    res.status(200).json({ sub: 'path' });
  });

  app.use('/', router);

Express support array of paths as an argument for single route handler. Where we heavily depends on our setup.

Seems this breaks the route handler name also it create route handler - undefined in the child span for that express layer. Maybe it get propagate to root and root goes with fallback '/'

[JohannesHuster]

I had a look at the code for another issue (#1947) and it looks like lists of paths are not supported by the express instrumentation, at the moment.

opentelemetry-js-contrib/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts

Line 128 in 038e0bf

typeof args[0] === 'string' ? args[0] : undefined

"/" is indeed a fallback in this case, but it's the fallback for not having route information at all (at a later point in the code).

Also, for a request to '/sub/path', the route information is available only as ['/sub/path', '/sub2/path']. That means, that without a lot more effort (or talking to the Express team, if they can introduce another property), this could only be expressed as one route (e.g. like (/sub/path|/sub2/path)). This might be the "right" thing to do anyways, if this is just one http.route as far as OpenTelemetry is concerned. The following does read like that to me (emphasis mine):

The matched route, that is, the path template in the format used by the respective server framework.

– HTTP Server semantic conventions

I'm note sure how other instrumentation is handling cases, where the server framework is providing multiple ways of specifying the same route (path template), though. router.get(['/sub/path', '/sub2/path'], ...) seems to be the equivalent of router.get('(/sub/path|/sub2/path)', ...)

The span names depend on the value of the attribute http.route, which is why you see "<METHOD> /" in your span names.

[JamieDanielson]

I believe this will be fixed with #2008 as it adds the ability to instrument with arrays as you've mentioned here in the repro scenario @iamchathu .

[JamieDanielson]

@iamchathu #2008 was merged with support for arrays, published in instrumentation-express-v0.39.0. Can you give that version a try and see if it resolves your problem?

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions]

This issue was closed because it has been stale for 14 days with no activity.