search

appsignal / appsignal-nodejs

🟩 AppSignal for Node.js
https://www.appsignal.com/nodejs/
MIT License
28 stars 9 forks source link

Research and fix issues with Next.js 14 #1014

Closed unflxw closed 4 months ago

unflxw commented 5 months ago

Intercom: https://app.intercom.com/a/inbox/yzor8gyw/inbox/admin/5246522/conversation/16410700308049?view=List

Customer is using Next.js 14.1.3. Performance samples are reported as UNKNOWN /some-path -- indicating that http.method is not provided in the span that is treated as the root span.

We should research what's going on and fix it.

To do

(this is updated with issues from the comments below)

unflxw commented 5 months ago

The customer reports that calls to Next.js fetch are no longer instrumented. Research if Next.js 14 no longer emits OpenTelemetry spans for its custom fetch implementation?

unflxw commented 5 months ago

The customer also ran into an issue when importing AppSignal's helpers from Next.js code, where Webpack fails to bundle AppSignal:

⨯ ./node_modules/@appsignal/nodejs/build/Release/extension.node
Module parse failed: Unexpected character '�' (1:0)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
(Source code omitted for this binary file)

See #114. The solution might be to ask customers to add us to the serverComponentsExternalPackages configuration option, or to add ourselves to the default bundled exclusion list, like other APM providers do.

unflxw commented 5 months ago

Another customer (Intercom) reported an "AppSignal not starting, no valid configuration found" error, which is not of the same nature as the above issues, but should also be looked into.

unflxw commented 5 months ago

Finally got around to start testing this. Ran into the Webpack bundling issue head-on, and I was able to fix it using the serverComponentsExternalPackages configuration option. Will reach out to the customer.

unflxw commented 5 months ago

Sent a PR to Vercel to add "@appsignal/nodejs" to the serverComponentsExternalPackages default list: https://github.com/vercel/next.js/pull/64503

Reproduced the issue with the UNKNOWN method and implemented a fix for it in the agent: https://github.com/appsignal/appsignal-agent/pull/1129

unflxw commented 5 months ago

I was not able to reproduce server-side calls performed with fetch not showing up. I will reach out to the customer.

unflxw commented 5 months ago

Released the fix in https://github.com/appsignal/appsignal-agent/pull/1129 as AppSignal for Node.js 3.3.3 -- this, however, resulted in incorrect behaviour when using the Pages Router. I have done a "soft rollback" by resetting the latest tag to 3.3.2, and I am looking into a full fix.

The serverComponentsExternalPackages workaround also does not work when using the Pages Router (which, on hindsight, makes sense, given the name of the configuration option) and customers experience issues when manually importing @appsignal/nodejs in their Next.js pages to use our helpers.

The fetch server-side calls not showing up is, for whatever reason, expected behaviour when using the Pages Router. I have amended the documentation to clarify this.

unflxw commented 5 months ago

I am not able to reproduce the issues that the customer using the Pages Router experienced. I have undone the "soft rollback" and latest is now 3.3.3.

A third customer (Intercom) has reported an issue where, when running Next.js (App Router) in production, errors emitted while rendering server components are intentionally obscured by Next.js itself.

unflxw commented 4 months ago

I was able to reproduce the issue with render or getServerSideProps appearing instead of the method, which was reported by the customer. AppSignal for Node.js 3.3.4 was released with a fix to the issue.

unflxw commented 4 months ago

A customer (Intercom link) has reported high cardinality action names after upgrading to 3.3.4. It seems that traces that should be ignored (because they do not have a proper root name) are not being properly ignored.

backlog-helper[bot] commented 4 months ago

This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

fabiolnm commented 4 months ago

Struggling to fix this error when trying to instrument a Next.js 14 application, app router based.

Versions

"@appsignal/nodejs": "3.4.2"
"next": "14.2.3",

Error

 â—‹ Compiling /instrumentation ...
 ⨯ ./node_modules/@appsignal/nodejs/build/Release/extension.node
Module parse failed: Unexpected character '�' (1:0)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
(Source code omitted for this binary file)

Import trace for requested module:
./node_modules/@appsignal/nodejs/build/Release/extension.node
./node_modules/@appsignal/nodejs/dist/extension_wrapper.js
./node_modules/@appsignal/nodejs/dist/extension.js
./node_modules/@appsignal/nodejs/dist/client.js
./node_modules/@appsignal/nodejs/dist/index.js
./appsignal.cjs

next.config.js

module.exports = {
  reactStrictMode: true,
  experimental: {
    instrumentationHook: true,
    serverComponentsExternalPackages: [
      'docusign-esign',
      // https://docs.appsignal.com/nodejs/3.x/integrations/nextjs.html
      '@appsignal/nodejs'
    ],
  },
  webpack(config) {
    config.module.rules.push({
      test: /\.svg$/,
      use: ['@svgr/webpack'],
    })
    return config
  },
  async headers() {
    ...
  },
}
fabiolnm commented 4 months ago

It seems to be related to https://github.com/appsignal/appsignal-nodejs/issues/928

unflxw commented 4 months ago

Hi @fabiolnm,

Thank you for reporting this issue. Setting serverComponentsExternalPackages should fix this, but unfortunately I believe that serverComponentsExternalPackages is not honored when a custom webpack(config) is provided.

Could you check whether removing the custom webpack(config) fixes the issue? I know this is not a feasible fix, but it helps us understand what the problem is.

Could you also check whether adding node-loader to your Webpack configuration, as described in #928, fixes it?

fabiolnm commented 4 months ago

Same error with:

module.exports = {
  reactStrictMode: true,
  experimental: {
    instrumentationHook: true,
    serverComponentsExternalPackages: ['@appsignal/nodejs'],
  },
}

trace

 â—‹ Compiling /instrumentation ...
 ⨯ ./node_modules/@appsignal/nodejs/build/Release/extension.node
Module parse failed: Unexpected character '�' (1:0)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
(Source code omitted for this binary file)

Import trace for requested module:
./node_modules/@appsignal/nodejs/build/Release/extension.node
./node_modules/@appsignal/nodejs/dist/extension_wrapper.js
./node_modules/@appsignal/nodejs/dist/extension.js
./node_modules/@appsignal/nodejs/dist/client.js
./node_modules/@appsignal/nodejs/dist/index.js
./appsignal.cjs
fabiolnm commented 4 months ago

TLDR

Loader changes the nature of the error to other never-ending Can't resolve 'package_name' errors.

next.config.js

module.exports = {
  reactStrictMode: true,
  experimental: {
    instrumentationHook: true,
    serverComponentsExternalPackages: ['@appsignal/nodejs'],
  },
  webpack(config) {
    config.module.rules.push({
      test: /\.svg$/,
      use: ['@svgr/webpack'],
    })
    config.module.rules.push({
      test: /\.node$/i,
      loader: 'node-loader',
    })
    return config
  },
}

Can't resolve 'package_name' errors

 â—‹ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/call.js:21:1
Module not found: Can't resolve 'stream'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/client.js
./node_modules/@grpc/grpc-js/build/src/index.js
./node_modules/@opentelemetry/exporter-trace-otlp-grpc/build/src/OTLPTraceExporter.js
./node_modules/@opentelemetry/exporter-trace-otlp-grpc/build/src/index.js
./node_modules/@opentelemetry/sdk-node/build/src/TracerProviderWithEnvExporter.js
./node_modules/@opentelemetry/sdk-node/build/src/sdk.js
./node_modules/@opentelemetry/sdk-node/build/src/index.js
./node_modules/@appsignal/nodejs/dist/client.js
./node_modules/@appsignal/nodejs/dist/index.js
./appsignal.cjs
./src/instrumentation.ts

Trying to fix by installing packages

npm install stream && npm run dev
 â—‹ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/channel-credentials.js:20:1
Module not found: Can't resolve 'tls'
https://nextjs.org/docs/messages/module-not-found
Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/index.js

npm install tls && npm run dev
 â—‹ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/channelz.js:20:1
Module not found: Can't resolve 'net'
https://nextjs.org/docs/messages/module-not-found
Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/index.js

npm install net && npm run dev
â—‹ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/compression-filter.js:20:1
Module not found: Can't resolve 'zlib'
https://nextjs.org/docs/messages/module-not-found
Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/internal-channel.js

Zlib

npm install browserify-zlib

Also have to add a resolver alias to webpack config

config.resolve.alias.zlib = 'browserify-zlib'

Errors never end

npm run dev

â—‹ Compiling /instrumentation ...
 ⨯ ./node_modules/@grpc/grpc-js/build/src/http_proxy.js:23:1
Module not found: Can't resolve 'http'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/@grpc/grpc-js/build/src/internal-channel.js
unflxw commented 4 months ago

On @fabiolnm's issue above, in case someone else runs into it: make sure that, in instrumentation.ts, the call to require("./appsignal.cjs") is wrapped in if (process.env.NEXT_RUNTIME === "nodejs") -- this is important, as it tells Next not to attempt to bundle it for the browser.

Everything else is being looked at, so I am closing this issue for now. If you're encountering any issues with Next.js 14, please reach out to us at support@appsignal.com, or open a new issue.